Subject: Re: [PATCH wpan-next 1/6] ieee802154: Add support for user scanning requests

From: Miquel Raynal <[email-masked]>
Date: Mon, 5 Dec 2022 10:57:52 +0100

Hi Alexander,

[email-masked] wrote on Sun, 4 Dec 2022 17:44:24 -0500:

> Hi,
> 
> On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal
> <[email-masked]> wrote:
> >
> > The ieee802154 layer should be able to scan a set of channels in order
> > to look for beacons advertizing PANs. Supporting this involves adding
> > two user commands: triggering scans and aborting scans. The user should
> > also be notified when a new beacon is received and also upon scan
> > termination.
> >
> > A scan request structure is created to list the requirements and to be
> > accessed asynchronously when changing channels or receiving beacons.
> >
> > Mac layers may now implement the ->trigger_scan() and ->abort_scan()
> > hooks.
> >
> > Co-developed-by: David Girault <[email-masked]>
> > Signed-off-by: David Girault <[email-masked]>
> > Signed-off-by: Miquel Raynal <[email-masked]>
> > ---
> >  include/linux/ieee802154.h |   3 +
> >  include/net/cfg802154.h    |  25 +++++
> >  include/net/nl802154.h     |  49 +++++++++
> >  net/ieee802154/nl802154.c  | 215 +++++++++++++++++++++++++++++++++++++
> >  net/ieee802154/nl802154.h  |   3 +
> >  net/ieee802154/rdev-ops.h  |  28 +++++
> >  net/ieee802154/trace.h     |  40 +++++++
> >  7 files changed, 363 insertions(+)
> >
> > diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
> > index 0303eb84d596..b22e4147d334 100644
> > --- a/include/linux/ieee802154.h
> > +++ b/include/linux/ieee802154.h
> > @@ -44,6 +44,9 @@
> >  #define IEEE802154_SHORT_ADDR_LEN      2
> >  #define IEEE802154_PAN_ID_LEN          2
> >
> > +/* Duration in superframe order */
> > +#define IEEE802154_MAX_SCAN_DURATION   14
> > +#define IEEE802154_ACTIVE_SCAN_DURATION        15
> >  #define IEEE802154_LIFS_PERIOD         40
> >  #define IEEE802154_SIFS_PERIOD         12
> >  #define IEEE802154_MAX_SIFS_FRAME_SIZE 18
> > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > index d09c393d229f..76d4f95e9974 100644
> > --- a/include/net/cfg802154.h
> > +++ b/include/net/cfg802154.h
> > @@ -18,6 +18,7 @@
> >
> >  struct wpan_phy;
> >  struct wpan_phy_cca;
> > +struct cfg802154_scan_request;
> >
> >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> >  struct ieee802154_llsec_device_key;
> > @@ -67,6 +68,10 @@ struct cfg802154_ops {
> >                                 struct wpan_dev *wpan_dev, bool mode);
> >         int     (*set_ackreq_default)(struct wpan_phy *wpan_phy,
> >                                       struct wpan_dev *wpan_dev, bool ackreq);
> > +       int     (*trigger_scan)(struct wpan_phy *wpan_phy,
> > +                               struct cfg802154_scan_request *request);
> > +       int     (*abort_scan)(struct wpan_phy *wpan_phy,
> > +                             struct wpan_dev *wpan_dev);
> >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> >         void    (*get_llsec_table)(struct wpan_phy *wpan_phy,
> >                                    struct wpan_dev *wpan_dev,
> > @@ -278,6 +283,26 @@ struct ieee802154_coord_desc {
> >         bool gts_permit;
> >  };
> >
> > +/**
> > + * struct cfg802154_scan_request - Scan request
> > + *
> > + * @type: type of scan to be performed
> > + * @page: page on which to perform the scan
> > + * @channels: channels in te %page to be scanned
> > + * @duration: time spent on each channel, calculated with:
> > + *            aBaseSuperframeDuration * (2 ^ duration + 1)
> > + * @wpan_dev: the wpan device on which to perform the scan
> > + * @wpan_phy: the wpan phy on which to perform the scan
> > + */
> > +struct cfg802154_scan_request {
> > +       enum nl802154_scan_types type;
> > +       u8 page;
> > +       u32 channels;
> > +       u8 duration;
> > +       struct wpan_dev *wpan_dev;
> > +       struct wpan_phy *wpan_phy;
> > +};
> > +
> >  struct ieee802154_llsec_key_id {
> >         u8 mode;
> >         u8 id;
> > diff --git a/include/net/nl802154.h b/include/net/nl802154.h
> > index b79a89d5207c..79fbd820b25a 100644
> > --- a/include/net/nl802154.h
> > +++ b/include/net/nl802154.h
> > @@ -73,6 +73,9 @@ enum nl802154_commands {
> >         NL802154_CMD_DEL_SEC_LEVEL,
> >
> >         NL802154_CMD_SCAN_EVENT,
> > +       NL802154_CMD_TRIGGER_SCAN,
> > +       NL802154_CMD_ABORT_SCAN,
> > +       NL802154_CMD_SCAN_DONE,  
> 
> Is NL802154_CMD_SCAN_DONE reserved now? I don't see it implemented in
> this series and I think we had some discussion about the need of abort
> vs done. Is the event now?

To be very honest I went back and forth about the "abort" information
so I don't remember exactly what was supposed to be implemented. The
current implementation forwards to userspace the reason (whether the
scan was finished or was aborted for an external reason). So it is
implemented this way:

* Patch 6/6 adds in mac802154/scan.c:

+       cmd = aborted ? NL802154_CMD_ABORT_SCAN : NL802154_CMD_SCAN_DONE;
+       nl802154_scan_done(wpan_phy, wpan_dev, cmd);

* And in patch 1/6, in ieee802154/nl802154.c:

+static int nl802154_send_scan_msg(struct cfg802154_registered_device *rdev,
+                                 struct wpan_dev *wpan_dev, u8 cmd)
+{
[...]
+
+       ret = nl802154_prep_scan_msg(msg, rdev, wpan_dev, 0, 0, 0, cmd);

Is this working for you?

Thanks,
Miquèl