From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Alexander Aring <alex.aring@gmail.com>
Cc: Stefan Schmidt <stefan@datenfreihafen.org>,
linux-wpan - ML <linux-wpan@vger.kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
"open list:NETWORKING [GENERAL]" <netdev@vger.kernel.org>,
Michael Hennerich <michael.hennerich@analog.com>,
Harry Morris <h.morris@cascoda.com>,
Varka Bhadram <varkabhadram@gmail.com>,
Xue Liu <liuxuenetmail@gmail.com>, Alan Ott <alan@signal11.us>,
David Girault <david.girault@qorvo.com>,
Romuald Despres <romuald.despres@qorvo.com>,
Frederic Blain <frederic.blain@qorvo.com>,
Nicolas Schodet <nico@ni.fr.eu.org>,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
"linux-wireless@vger.kernel.org Wireless"
<linux-wireless@vger.kernel.org>
Subject: Re: [wpan-next v2 27/27] net: ieee802154: ca8210: Refuse most of the scan operations
Date: Thu, 13 Jan 2022 10:29:54 +0100 [thread overview]
Message-ID: <20220113102954.7a0e213e@xps13> (raw)
In-Reply-To: <CAB_54W5ojqi2obtNLihCMXsZkh+aN_cVbSTRptq3=PXkpknzJQ@mail.gmail.com>
Hi Alexander,
alex.aring@gmail.com wrote on Wed, 12 Jan 2022 17:48:59 -0500:
> Hi,
>
> On Wed, 12 Jan 2022 at 12:34, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > The Cascada 8210 hardware transceiver is kind of a hardMAC which
> > interfaces with the softMAC and in practice does not support sending
> > anything else than dataframes. This means we cannot send any BEACON_REQ
> > during active scans nor any BEACON in general. Refuse these operations
> > officially so that the user is aware of the limitation.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > drivers/net/ieee802154/ca8210.c | 25 ++++++++++++++++++++++++-
> > 1 file changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> > index d3a9e4fe05f4..49c274280e3c 100644
> > --- a/drivers/net/ieee802154/ca8210.c
> > +++ b/drivers/net/ieee802154/ca8210.c
> > @@ -2385,6 +2385,25 @@ static int ca8210_set_promiscuous_mode(struct ieee802154_hw *hw, const bool on)
> > return link_to_linux_err(status);
> > }
> >
> > +static int ca8210_enter_scan_mode(struct ieee802154_hw *hw,
> > + struct cfg802154_scan_request *request)
> > +{
> > + /* This xceiver can only send dataframes */
> > + if (request->type != NL802154_SCAN_PASSIVE)
> > + return -EOPNOTSUPP;
> > +
> > + return 0;
> > +}
> > +
> > +static int ca8210_enter_beacons_mode(struct ieee802154_hw *hw,
> > + struct cfg802154_beacons_request *request)
> > +{
> > + /* This xceiver can only send dataframes */
> > + return -EOPNOTSUPP;
> > +}
> > +
> > +static void ca8210_exit_scan_beacons_mode(struct ieee802154_hw *hw) { }
> > +
> > static const struct ieee802154_ops ca8210_phy_ops = {
> > .start = ca8210_start,
> > .stop = ca8210_stop,
> > @@ -2397,7 +2416,11 @@ static const struct ieee802154_ops ca8210_phy_ops = {
> > .set_cca_ed_level = ca8210_set_cca_ed_level,
> > .set_csma_params = ca8210_set_csma_params,
> > .set_frame_retries = ca8210_set_frame_retries,
> > - .set_promiscuous_mode = ca8210_set_promiscuous_mode
> > + .set_promiscuous_mode = ca8210_set_promiscuous_mode,
> > + .enter_scan_mode = ca8210_enter_scan_mode,
> > + .exit_scan_mode = ca8210_exit_scan_beacons_mode,
> > + .enter_beacons_mode = ca8210_enter_beacons_mode,
> > + .exit_beacons_mode = ca8210_exit_scan_beacons_mode,
> > };
>
> so there is no flag that this driver can't support scanning currently
> and it works now because the offload functionality will return
> -ENOTSUPP? This is misleading because I would assume if it's not
> supported we can do it by software which the driver can't do.
I believe there is a misunderstanding.
This is what I have understood from your previous comments in v1:
"This driver does not support transmitting anything else than
datagrams", which is what I assumed was a regular data packet. IOW,
sending a MAC_CMD such as a beacon request or sending a beacon was not
supported physically by the hardware. Hence, most of the scans
operations cannot be performed and must be rejected (all but a passive
scan, assuming that receiving beacons was okay).
Please mind the update in that hook which currently is just an FYI from
the mac to the drivers and not a "do it by yourself" injunction. So
answering -EOPNOTSUPP to the mac here does not mean:
"I cannot handle it by myself, the scan cannot happen"
but
"I cannot handle the forged frames, so let's just not try"
> ... I see that the offload functions now are getting used and have a
> reason to be upstream, but the use of it is wrong.
As a personal matter of taste, I don't like flags when it comes to
something complex like supporting a specific operation. Just in the
scanning procedure there are 4 different actions and a driver might
support only a subset of these, which is totally fine but hard to
properly describe by well-named flags. Here the driver hooks say to
the driver which are interested "here is what is going to happen" and
then they can:
- ignore the details by just not implementing the hooks, let the mac do
its job, they will then transmit the relevant frames forged by the
mac
- eventually enter a specific mode internally for this operation, but
basically do the same as above, ie. transmitting the frames forged
by the mac
- refuse the operation by returning an error code if something cannot
be done
I've experienced a number of situations in the MTD world and later with
IIO drivers where flags have been remodeled and reused in different
manners, until the flag description gets totally wrong and
undescriptive regarding what it actually does. Hence my main idea of
letting drivers refuse these operations instead of having the mac doing
it for them.
I can definitely use flags if you want, but in this case, what flags do
you want to see?
Thanks,
Miquèl
next prev parent reply other threads:[~2022-01-13 9:30 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-12 17:32 [wpan-next v2 00/27] IEEE 802.15.4 scan support Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 01/27] net: mac802154: Split the set channel hook implementation Miquel Raynal
2022-01-12 22:30 ` Alexander Aring
2022-01-12 22:53 ` Alexander Aring
2022-01-13 11:12 ` Miquel Raynal
2022-01-13 9:32 ` Miquel Raynal
2022-01-13 23:27 ` Alexander Aring
2022-01-12 17:32 ` [wpan-next v2 02/27] net: mac802154: Ensure proper channel selection at probe time Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 03/27] net: ieee802154: Improve the way supported channels are declared Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 04/27] net: ieee802154: Give more details to the core about the channel configurations Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 05/27] net: mac802154: Convert the symbol duration into nanoseconds Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 06/27] net: mac802154: Set the symbol duration automatically Miquel Raynal
2022-01-12 22:25 ` Alexander Aring
2022-01-13 9:52 ` Miquel Raynal
2022-01-13 23:36 ` Alexander Aring
2022-01-14 10:18 ` Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 07/27] net: ieee802154: hwsim: Ensure frame checksum are valid Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 08/27] net: ieee802154: Drop symbol duration settings when the core does it already Miquel Raynal
2022-01-12 22:26 ` Alexander Aring
2022-01-13 11:16 ` Miquel Raynal
2022-01-13 23:34 ` Alexander Aring
2022-01-14 10:21 ` Miquel Raynal
2022-01-16 23:21 ` Alexander Aring
2022-01-17 9:12 ` Miquel Raynal
2022-01-17 23:38 ` Alexander Aring
2022-01-18 10:38 ` Miquel Raynal
2022-01-18 22:43 ` Alexander Aring
2022-01-19 22:26 ` Miquel Raynal
2022-01-19 23:26 ` Alexander Aring
2022-01-12 17:32 ` [wpan-next v2 09/27] net: ieee802154: Move IEEE 802.15.4 Kconfig main entry Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 10/27] net: mac802154: Include the softMAC stack inside the IEEE 802.15.4 menu Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 11/27] net: ieee802154: Move the address structure earlier Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 12/27] net: ieee802154: Add a kernel doc header to the ieee802154_addr structure Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 13/27] net: ieee802154: Return meaningful error codes from the netlink helpers Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 14/27] net: ieee802154: Add support for internal PAN management Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 15/27] net: ieee802154: Define a beacon frame header Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 16/27] net: ieee802154: Define frame types Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 17/27] net: ieee802154: Add support for scanning requests Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 18/27] net: mac802154: Handle scan requests Miquel Raynal
2022-01-12 22:44 ` Alexander Aring
2022-01-13 17:07 ` Miquel Raynal
2022-01-14 0:01 ` Alexander Aring
2022-01-14 18:44 ` Miquel Raynal
2022-01-16 22:44 ` Alexander Aring
2022-01-16 22:50 ` Alexander Aring
2022-01-17 9:00 ` Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 19/27] net: ieee802154: Full PAN management Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 20/27] net: ieee802154: Add support for beacon requests Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 21/27] net: mac802154: Handle beacons requests Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 22/27] net: ieee802154: Trace the registration of new PANs Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 23/27] net: mac802154: Add support for active scans Miquel Raynal
2022-01-12 23:16 ` Alexander Aring
2022-01-13 17:14 ` Miquel Raynal
2022-01-14 0:30 ` Alexander Aring
2022-01-14 0:39 ` Alexander Aring
2022-01-12 17:33 ` [wpan-next v2 24/27] net: mac802154: Add support for processing beacon requests Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 25/27] net: mac802154: Inform device drivers about scans Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 26/27] net: mac802154: Inform device drivers about beacon operations Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 27/27] net: ieee802154: ca8210: Refuse most of the scan operations Miquel Raynal
2022-01-12 22:48 ` Alexander Aring
2022-01-13 9:29 ` Miquel Raynal [this message]
2022-01-14 1:00 ` Alexander Aring
2022-01-14 18:37 ` Miquel Raynal
2022-01-16 22:48 ` Alexander Aring
2022-01-17 9:04 ` Miquel Raynal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220113102954.7a0e213e@xps13 \
--to=miquel.raynal@bootlin.com \
--cc=alan@signal11.us \
--cc=alex.aring@gmail.com \
--cc=davem@davemloft.net \
--cc=david.girault@qorvo.com \
--cc=frederic.blain@qorvo.com \
--cc=h.morris@cascoda.com \
--cc=kuba@kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=linux-wpan@vger.kernel.org \
--cc=liuxuenetmail@gmail.com \
--cc=michael.hennerich@analog.com \
--cc=netdev@vger.kernel.org \
--cc=nico@ni.fr.eu.org \
--cc=romuald.despres@qorvo.com \
--cc=stefan@datenfreihafen.org \
--cc=thomas.petazzoni@bootlin.com \
--cc=varkabhadram@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).