| Message ID | 20221129160046.538864-2-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | IEEE 802.15.4 passive scan support | expand |
Hi, On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal <miquel.raynal@bootlin.com> 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 <david.girault@qorvo.com> > Signed-off-by: David Girault <david.girault@qorvo.com> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > 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? - Alex
Hi Alexander, aahringo@redhat.com wrote on Sun, 4 Dec 2022 17:44:24 -0500: > Hi, > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal > <miquel.raynal@bootlin.com> 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 <david.girault@qorvo.com> > > Signed-off-by: David Girault <david.girault@qorvo.com> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > 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
Hi, On Mon, Dec 5, 2022 at 4:58 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > aahringo@redhat.com wrote on Sun, 4 Dec 2022 17:44:24 -0500: > > > Hi, > > > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal > > <miquel.raynal@bootlin.com> 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 <david.girault@qorvo.com> > > > Signed-off-by: David Girault <david.girault@qorvo.com> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > --- > > > 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: > I think it was also to implement a way to signal all listeners that there is such an operation ongoing? Do we have something like that? > * 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? Sure this works in some way, I would put a reason parameter on DONE, but however... - Alex
Hi Alexander, aahringo@redhat.com wrote on Wed, 7 Dec 2022 08:27:35 -0500: > Hi, > > On Mon, Dec 5, 2022 at 4:58 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Hi Alexander, > > > > aahringo@redhat.com wrote on Sun, 4 Dec 2022 17:44:24 -0500: > > > > > Hi, > > > > > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal > > > <miquel.raynal@bootlin.com> 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 <david.girault@qorvo.com> > > > > Signed-off-by: David Girault <david.girault@qorvo.com> > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > --- > > > > 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: > > > > I think it was also to implement a way to signal all listeners that > there is such an operation ongoing? Do we have something like that? When the kernel receives a CMD_TRIGGER_SCAN it starts the scan and if everything is on track sends to the userspace listeners a CMD_TRIGGER_SCAN in return, meaning "a scan has started". So any listener would see: CMD_TRIGGER_SCAN SCAN_EVENT (+ content of the beacon) ... SCAN_EVENT (+ content of the beacon) CMD_SCAN_DONE (+ reason, see below) > > * 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? > > Sure this works in some way, I would put a reason parameter on DONE, > but however... Of course, I can do that. So NL802154_CMD_SCAN_DONE plus an u8 parameter: NL802154_SCAN_DONE_REASON_FINISHED NL802154_SCAN_DONE_REASON_ABORTED ? Thanks, Miquèl
Hi, On Wed, Dec 7, 2022 at 8:45 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > aahringo@redhat.com wrote on Wed, 7 Dec 2022 08:27:35 -0500: > > > Hi, > > > > On Mon, Dec 5, 2022 at 4:58 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > Hi Alexander, > > > > > > aahringo@redhat.com wrote on Sun, 4 Dec 2022 17:44:24 -0500: > > > > > > > Hi, > > > > > > > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal > > > > <miquel.raynal@bootlin.com> 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 <david.girault@qorvo.com> > > > > > Signed-off-by: David Girault <david.girault@qorvo.com> > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > > --- > > > > > 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: > > > > > > > I think it was also to implement a way to signal all listeners that > > there is such an operation ongoing? Do we have something like that? > > When the kernel receives a CMD_TRIGGER_SCAN it starts the scan and if > everything is on track sends to the userspace listeners a > CMD_TRIGGER_SCAN in return, meaning "a scan has started". > > So any listener would see: > CMD_TRIGGER_SCAN > SCAN_EVENT (+ content of the beacon) > ... > SCAN_EVENT (+ content of the beacon) > CMD_SCAN_DONE (+ reason, see below) > okay. > > > * 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? > > > > Sure this works in some way, I would put a reason parameter on DONE, > > but however... > > Of course, I can do that. So NL802154_CMD_SCAN_DONE plus an u8 > parameter: > NL802154_SCAN_DONE_REASON_FINISHED > NL802154_SCAN_DONE_REASON_ABORTED > ? yes, please. - Alex
On Tue, 29 Nov 2022 17:00:41 +0100 Miquel Raynal wrote: > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > +{ > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > + struct net_device *dev = info->user_ptr[1]; > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > + struct cfg802154_scan_request *request; > + u8 type; > + int err; > + > + /* Monitors are not allowed to perform scans */ > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) extack ? > + return -EPERM; > + > + request = kzalloc(sizeof(*request), GFP_KERNEL); > + if (!request) > + return -ENOMEM; > + > + request->wpan_dev = wpan_dev; > + request->wpan_phy = wpan_phy; > + > + type = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_TYPE]); what checks info->attrs[NL802154_ATTR_SCAN_TYPE] is not NULL? > + switch (type) { > + case NL802154_SCAN_PASSIVE: > + request->type = type; > + break; > + default: > + pr_err("Unsupported scan type: %d\n", type); > + err = -EINVAL; extack (printfs are now supported) > + goto free_request; > + } > + > + if (info->attrs[NL802154_ATTR_PAGE]) { > + request->page = nla_get_u8(info->attrs[NL802154_ATTR_PAGE]); > + if (request->page > IEEE802154_MAX_PAGE) { bound check should be part of the policy NLA_POLICY_MAX() > + pr_err("Invalid page %d > %d\n", > + request->page, IEEE802154_MAX_PAGE); > + err = -EINVAL; extack > + goto free_request; > + } > + } else { > + /* Use current page by default */ > + request->page = wpan_phy->current_page; > + } > + > + if (info->attrs[NL802154_ATTR_SCAN_CHANNELS]) { > + request->channels = nla_get_u32(info->attrs[NL802154_ATTR_SCAN_CHANNELS]); > + if (request->channels >= BIT(IEEE802154_MAX_CHANNEL + 1)) { policy as well > + pr_err("Invalid channels bitfield %x ≥ %lx\n", > + request->channels, > + BIT(IEEE802154_MAX_CHANNEL + 1)); > + err = -EINVAL; > + goto free_request; > + } > + } else { > + /* Scan all supported channels by default */ > + request->channels = wpan_phy->supported.channels[request->page]; > + } > + > + if (info->attrs[NL802154_ATTR_SCAN_PREAMBLE_CODES] || > + info->attrs[NL802154_ATTR_SCAN_MEAN_PRF]) { > + pr_err("Preamble codes and mean PRF not supported yet\n"); NLA_REJECT also in policy > + err = -EINVAL; > + goto free_request; > + } > + > + if (info->attrs[NL802154_ATTR_SCAN_DURATION]) { > + request->duration = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_DURATION]); > + if (request->duration > IEEE802154_MAX_SCAN_DURATION) { > + pr_err("Duration is out of range\n"); > + err = -EINVAL; > + goto free_request; > + } > + } else { > + /* Use maximum duration order by default */ > + request->duration = IEEE802154_MAX_SCAN_DURATION; > + } > + > + if (wpan_dev->netdev) > + dev_hold(wpan_dev->netdev); Can we put a tracker in the request and use netdev_hold() ? > + > + err = rdev_trigger_scan(rdev, request); > + if (err) { > + pr_err("Failure starting scanning (%d)\n", err); > + goto free_device; > + }
Hi, On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal <miquel.raynal@bootlin.com> 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 <david.girault@qorvo.com> > Signed-off-by: David Girault <david.girault@qorvo.com> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > 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, > > /* add new commands above here */ > > @@ -134,6 +137,12 @@ enum nl802154_attrs { > NL802154_ATTR_NETNS_FD, > > NL802154_ATTR_COORDINATOR, > + NL802154_ATTR_SCAN_TYPE, > + NL802154_ATTR_SCAN_FLAGS, > + NL802154_ATTR_SCAN_CHANNELS, > + NL802154_ATTR_SCAN_PREAMBLE_CODES, > + NL802154_ATTR_SCAN_MEAN_PRF, > + NL802154_ATTR_SCAN_DURATION, > > /* add attributes here, update the policy in nl802154.c */ > > @@ -259,6 +268,46 @@ enum nl802154_coord { > NL802154_COORD_MAX, > }; > > +/** > + * enum nl802154_scan_types - Scan types > + * > + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved > + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak > + * energy in each requested channel > + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using > + * a Beacon Request command > + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames > + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation > + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request > + * command instead of Beacon Request command > + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames > + * instead of Beacon frames > + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number > + */ > +enum nl802154_scan_types { > + __NL802154_SCAN_INVALID, > + NL802154_SCAN_ED, > + NL802154_SCAN_ACTIVE, > + NL802154_SCAN_PASSIVE, > + NL802154_SCAN_ORPHAN, > + NL802154_SCAN_ENHANCED_ACTIVE, > + NL802154_SCAN_RIT_PASSIVE, > + > + /* keep last */ > + NL802154_SCAN_ATTR_MAX, > +}; > + > +/** > + * enum nl802154_scan_flags - Scan request control flags > + * > + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie. > + * a different one for every scan iteration). When the flag is set, full > + * randomisation is assumed. > + */ > +enum nl802154_scan_flags { > + NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0), > +}; > + > /** > * enum nl802154_cca_modes - cca modes > * > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c > index 80dc73182785..c497ffd8e897 100644 > --- a/net/ieee802154/nl802154.c > +++ b/net/ieee802154/nl802154.c > @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = { > > [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED }, > > + [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 }, > + [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 }, > + [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 }, > + [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 }, > + [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 }, > + > #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL > [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, }, > [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, }, > @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev, > } > EXPORT_SYMBOL_GPL(nl802154_scan_event); > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > +{ > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > + struct net_device *dev = info->user_ptr[1]; > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > + struct cfg802154_scan_request *request; > + u8 type; > + int err; > + > + /* Monitors are not allowed to perform scans */ > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > + return -EPERM; btw: why are monitors not allowed? - Alex
Hi Alexander, aahringo@redhat.com wrote on Sun, 5 Feb 2023 20:39:32 -0500: > Hi, > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal > <miquel.raynal@bootlin.com> 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 <david.girault@qorvo.com> > > Signed-off-by: David Girault <david.girault@qorvo.com> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > 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, > > > > /* add new commands above here */ > > > > @@ -134,6 +137,12 @@ enum nl802154_attrs { > > NL802154_ATTR_NETNS_FD, > > > > NL802154_ATTR_COORDINATOR, > > + NL802154_ATTR_SCAN_TYPE, > > + NL802154_ATTR_SCAN_FLAGS, > > + NL802154_ATTR_SCAN_CHANNELS, > > + NL802154_ATTR_SCAN_PREAMBLE_CODES, > > + NL802154_ATTR_SCAN_MEAN_PRF, > > + NL802154_ATTR_SCAN_DURATION, > > > > /* add attributes here, update the policy in nl802154.c */ > > > > @@ -259,6 +268,46 @@ enum nl802154_coord { > > NL802154_COORD_MAX, > > }; > > > > +/** > > + * enum nl802154_scan_types - Scan types > > + * > > + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved > > + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak > > + * energy in each requested channel > > + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using > > + * a Beacon Request command > > + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames > > + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation > > + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request > > + * command instead of Beacon Request command > > + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames > > + * instead of Beacon frames > > + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number > > + */ > > +enum nl802154_scan_types { > > + __NL802154_SCAN_INVALID, > > + NL802154_SCAN_ED, > > + NL802154_SCAN_ACTIVE, > > + NL802154_SCAN_PASSIVE, > > + NL802154_SCAN_ORPHAN, > > + NL802154_SCAN_ENHANCED_ACTIVE, > > + NL802154_SCAN_RIT_PASSIVE, > > + > > + /* keep last */ > > + NL802154_SCAN_ATTR_MAX, > > +}; > > + > > +/** > > + * enum nl802154_scan_flags - Scan request control flags > > + * > > + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie. > > + * a different one for every scan iteration). When the flag is set, full > > + * randomisation is assumed. > > + */ > > +enum nl802154_scan_flags { > > + NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0), > > +}; > > + > > /** > > * enum nl802154_cca_modes - cca modes > > * > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c > > index 80dc73182785..c497ffd8e897 100644 > > --- a/net/ieee802154/nl802154.c > > +++ b/net/ieee802154/nl802154.c > > @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = { > > > > [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED }, > > > > + [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 }, > > + [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 }, > > + [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 }, > > + [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 }, > > + [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 }, > > + > > #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL > > [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, }, > > [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, }, > > @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev, > > } > > EXPORT_SYMBOL_GPL(nl802154_scan_event); > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > +{ > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > + struct net_device *dev = info->user_ptr[1]; > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > + struct cfg802154_scan_request *request; > > + u8 type; > > + int err; > > + > > + /* Monitors are not allowed to perform scans */ > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > + return -EPERM; > > btw: why are monitors not allowed? I guess I had the "active scan" use case in mind which of course does not work with monitors. Maybe I can relax this a little bit indeed, right now I don't remember why I strongly refused scans on monitors. Thanks, Miquèl
Hi, On Mon, Feb 6, 2023 at 4:13 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > aahringo@redhat.com wrote on Sun, 5 Feb 2023 20:39:32 -0500: > > > Hi, > > > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal > > <miquel.raynal@bootlin.com> 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 <david.girault@qorvo.com> > > > Signed-off-by: David Girault <david.girault@qorvo.com> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > --- > > > 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, > > > > > > /* add new commands above here */ > > > > > > @@ -134,6 +137,12 @@ enum nl802154_attrs { > > > NL802154_ATTR_NETNS_FD, > > > > > > NL802154_ATTR_COORDINATOR, > > > + NL802154_ATTR_SCAN_TYPE, > > > + NL802154_ATTR_SCAN_FLAGS, > > > + NL802154_ATTR_SCAN_CHANNELS, > > > + NL802154_ATTR_SCAN_PREAMBLE_CODES, > > > + NL802154_ATTR_SCAN_MEAN_PRF, > > > + NL802154_ATTR_SCAN_DURATION, > > > > > > /* add attributes here, update the policy in nl802154.c */ > > > > > > @@ -259,6 +268,46 @@ enum nl802154_coord { > > > NL802154_COORD_MAX, > > > }; > > > > > > +/** > > > + * enum nl802154_scan_types - Scan types > > > + * > > > + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved > > > + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak > > > + * energy in each requested channel > > > + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using > > > + * a Beacon Request command > > > + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames > > > + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation > > > + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request > > > + * command instead of Beacon Request command > > > + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames > > > + * instead of Beacon frames > > > + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number > > > + */ > > > +enum nl802154_scan_types { > > > + __NL802154_SCAN_INVALID, > > > + NL802154_SCAN_ED, > > > + NL802154_SCAN_ACTIVE, > > > + NL802154_SCAN_PASSIVE, > > > + NL802154_SCAN_ORPHAN, > > > + NL802154_SCAN_ENHANCED_ACTIVE, > > > + NL802154_SCAN_RIT_PASSIVE, > > > + > > > + /* keep last */ > > > + NL802154_SCAN_ATTR_MAX, > > > +}; > > > + > > > +/** > > > + * enum nl802154_scan_flags - Scan request control flags > > > + * > > > + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie. > > > + * a different one for every scan iteration). When the flag is set, full > > > + * randomisation is assumed. > > > + */ > > > +enum nl802154_scan_flags { > > > + NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0), > > > +}; > > > + > > > /** > > > * enum nl802154_cca_modes - cca modes > > > * > > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c > > > index 80dc73182785..c497ffd8e897 100644 > > > --- a/net/ieee802154/nl802154.c > > > +++ b/net/ieee802154/nl802154.c > > > @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = { > > > > > > [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED }, > > > > > > + [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 }, > > > + [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 }, > > > + [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 }, > > > + [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 }, > > > + [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 }, > > > + > > > #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL > > > [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, }, > > > [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, }, > > > @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev, > > > } > > > EXPORT_SYMBOL_GPL(nl802154_scan_event); > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > +{ > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > + struct net_device *dev = info->user_ptr[1]; > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > + struct cfg802154_scan_request *request; > > > + u8 type; > > > + int err; > > > + > > > + /* Monitors are not allowed to perform scans */ > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > + return -EPERM; > > > > btw: why are monitors not allowed? > > I guess I had the "active scan" use case in mind which of course does > not work with monitors. Maybe I can relax this a little bit indeed, > right now I don't remember why I strongly refused scans on monitors. Isn't it that scans really work close to phy level? Means in this case we disable mostly everything of MAC filtering on the transceiver side. Then I don't see any reasons why even monitors can't do anything, they also can send something. But they really don't have any specific source address set, so long addresses are none for source addresses, I don't see any problem here. They also don't have AACK handling, but it's not required for scan anyway... If this gets too complicated right now, then I am also fine with returning an error here, we can enable it later but would it be better to use ENOTSUPP or something like that in this case? EPERM sounds like you can do that, but you don't have the permissions. - Alex
Hi, On Mon, Feb 6, 2023 at 7:33 PM Alexander Aring <aahringo@redhat.com> wrote: > > Hi, > > On Mon, Feb 6, 2023 at 4:13 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Hi Alexander, > > > > aahringo@redhat.com wrote on Sun, 5 Feb 2023 20:39:32 -0500: > > > > > Hi, > > > > > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal > > > <miquel.raynal@bootlin.com> 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 <david.girault@qorvo.com> > > > > Signed-off-by: David Girault <david.girault@qorvo.com> > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > --- > > > > 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, > > > > > > > > /* add new commands above here */ > > > > > > > > @@ -134,6 +137,12 @@ enum nl802154_attrs { > > > > NL802154_ATTR_NETNS_FD, > > > > > > > > NL802154_ATTR_COORDINATOR, > > > > + NL802154_ATTR_SCAN_TYPE, > > > > + NL802154_ATTR_SCAN_FLAGS, > > > > + NL802154_ATTR_SCAN_CHANNELS, > > > > + NL802154_ATTR_SCAN_PREAMBLE_CODES, > > > > + NL802154_ATTR_SCAN_MEAN_PRF, > > > > + NL802154_ATTR_SCAN_DURATION, > > > > > > > > /* add attributes here, update the policy in nl802154.c */ > > > > > > > > @@ -259,6 +268,46 @@ enum nl802154_coord { > > > > NL802154_COORD_MAX, > > > > }; > > > > > > > > +/** > > > > + * enum nl802154_scan_types - Scan types > > > > + * > > > > + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved > > > > + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak > > > > + * energy in each requested channel > > > > + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using > > > > + * a Beacon Request command > > > > + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames > > > > + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation > > > > + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request > > > > + * command instead of Beacon Request command > > > > + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames > > > > + * instead of Beacon frames > > > > + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number > > > > + */ > > > > +enum nl802154_scan_types { > > > > + __NL802154_SCAN_INVALID, > > > > + NL802154_SCAN_ED, > > > > + NL802154_SCAN_ACTIVE, > > > > + NL802154_SCAN_PASSIVE, > > > > + NL802154_SCAN_ORPHAN, > > > > + NL802154_SCAN_ENHANCED_ACTIVE, > > > > + NL802154_SCAN_RIT_PASSIVE, > > > > + > > > > + /* keep last */ > > > > + NL802154_SCAN_ATTR_MAX, > > > > +}; > > > > + > > > > +/** > > > > + * enum nl802154_scan_flags - Scan request control flags > > > > + * > > > > + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie. > > > > + * a different one for every scan iteration). When the flag is set, full > > > > + * randomisation is assumed. > > > > + */ > > > > +enum nl802154_scan_flags { > > > > + NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0), > > > > +}; > > > > + > > > > /** > > > > * enum nl802154_cca_modes - cca modes > > > > * > > > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c > > > > index 80dc73182785..c497ffd8e897 100644 > > > > --- a/net/ieee802154/nl802154.c > > > > +++ b/net/ieee802154/nl802154.c > > > > @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = { > > > > > > > > [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED }, > > > > > > > > + [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 }, > > > > + [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 }, > > > > + [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 }, > > > > + [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 }, > > > > + [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 }, > > > > + > > > > #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL > > > > [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, }, > > > > [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, }, > > > > @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev, > > > > } > > > > EXPORT_SYMBOL_GPL(nl802154_scan_event); > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > +{ > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > + struct net_device *dev = info->user_ptr[1]; > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > + struct cfg802154_scan_request *request; > > > > + u8 type; > > > > + int err; > > > > + > > > > + /* Monitors are not allowed to perform scans */ > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > + return -EPERM; > > > > > > btw: why are monitors not allowed? > > > > I guess I had the "active scan" use case in mind which of course does > > not work with monitors. Maybe I can relax this a little bit indeed, > > right now I don't remember why I strongly refused scans on monitors. > > Isn't it that scans really work close to phy level? Means in this case > we disable mostly everything of MAC filtering on the transceiver side. > Then I don't see any reasons why even monitors can't do anything, they > also can send something. But they really don't have any specific > source address set, so long addresses are none for source addresses, I > don't see any problem here. They also don't have AACK handling, but > it's not required for scan anyway... > > If this gets too complicated right now, then I am also fine with > returning an error here, we can enable it later but would it be better > to use ENOTSUPP or something like that in this case? EPERM sounds like > you can do that, but you don't have the permissions. > For me a scan should also be possible from iwpan phy $PHY scan (or whatever the scan command is, or just enable beacon)... to go over the dev is just a shortcut for "I mean whatever the phy is under this dev" ? - Alex
Hi, On Tue, Feb 7, 2023 at 7:55 AM Alexander Aring <aahringo@redhat.com> wrote: > > Hi, > > On Mon, Feb 6, 2023 at 7:33 PM Alexander Aring <aahringo@redhat.com> wrote: > > > > Hi, > > > > On Mon, Feb 6, 2023 at 4:13 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > Hi Alexander, > > > > > > aahringo@redhat.com wrote on Sun, 5 Feb 2023 20:39:32 -0500: > > > > > > > Hi, > > > > > > > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal > > > > <miquel.raynal@bootlin.com> 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 <david.girault@qorvo.com> > > > > > Signed-off-by: David Girault <david.girault@qorvo.com> > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > > --- > > > > > 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, > > > > > > > > > > /* add new commands above here */ > > > > > > > > > > @@ -134,6 +137,12 @@ enum nl802154_attrs { > > > > > NL802154_ATTR_NETNS_FD, > > > > > > > > > > NL802154_ATTR_COORDINATOR, > > > > > + NL802154_ATTR_SCAN_TYPE, > > > > > + NL802154_ATTR_SCAN_FLAGS, > > > > > + NL802154_ATTR_SCAN_CHANNELS, > > > > > + NL802154_ATTR_SCAN_PREAMBLE_CODES, > > > > > + NL802154_ATTR_SCAN_MEAN_PRF, > > > > > + NL802154_ATTR_SCAN_DURATION, > > > > > > > > > > /* add attributes here, update the policy in nl802154.c */ > > > > > > > > > > @@ -259,6 +268,46 @@ enum nl802154_coord { > > > > > NL802154_COORD_MAX, > > > > > }; > > > > > > > > > > +/** > > > > > + * enum nl802154_scan_types - Scan types > > > > > + * > > > > > + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved > > > > > + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak > > > > > + * energy in each requested channel > > > > > + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using > > > > > + * a Beacon Request command > > > > > + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames > > > > > + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation > > > > > + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request > > > > > + * command instead of Beacon Request command > > > > > + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames > > > > > + * instead of Beacon frames > > > > > + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number > > > > > + */ > > > > > +enum nl802154_scan_types { > > > > > + __NL802154_SCAN_INVALID, > > > > > + NL802154_SCAN_ED, > > > > > + NL802154_SCAN_ACTIVE, > > > > > + NL802154_SCAN_PASSIVE, > > > > > + NL802154_SCAN_ORPHAN, > > > > > + NL802154_SCAN_ENHANCED_ACTIVE, > > > > > + NL802154_SCAN_RIT_PASSIVE, > > > > > + > > > > > + /* keep last */ > > > > > + NL802154_SCAN_ATTR_MAX, > > > > > +}; > > > > > + > > > > > +/** > > > > > + * enum nl802154_scan_flags - Scan request control flags > > > > > + * > > > > > + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie. > > > > > + * a different one for every scan iteration). When the flag is set, full > > > > > + * randomisation is assumed. > > > > > + */ > > > > > +enum nl802154_scan_flags { > > > > > + NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0), > > > > > +}; > > > > > + > > > > > /** > > > > > * enum nl802154_cca_modes - cca modes > > > > > * > > > > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c > > > > > index 80dc73182785..c497ffd8e897 100644 > > > > > --- a/net/ieee802154/nl802154.c > > > > > +++ b/net/ieee802154/nl802154.c > > > > > @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = { > > > > > > > > > > [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED }, > > > > > > > > > > + [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 }, > > > > > + [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 }, > > > > > + [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 }, > > > > > + [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 }, > > > > > + [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 }, > > > > > + > > > > > #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL > > > > > [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, }, > > > > > [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, }, > > > > > @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev, > > > > > } > > > > > EXPORT_SYMBOL_GPL(nl802154_scan_event); > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > +{ > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > + struct cfg802154_scan_request *request; > > > > > + u8 type; > > > > > + int err; > > > > > + > > > > > + /* Monitors are not allowed to perform scans */ > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > + return -EPERM; > > > > > > > > btw: why are monitors not allowed? > > > > > > I guess I had the "active scan" use case in mind which of course does > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > right now I don't remember why I strongly refused scans on monitors. > > > > Isn't it that scans really work close to phy level? Means in this case > > we disable mostly everything of MAC filtering on the transceiver side. > > Then I don't see any reasons why even monitors can't do anything, they > > also can send something. But they really don't have any specific > > source address set, so long addresses are none for source addresses, I > > don't see any problem here. They also don't have AACK handling, but > > it's not required for scan anyway... > > > > If this gets too complicated right now, then I am also fine with > > returning an error here, we can enable it later but would it be better > > to use ENOTSUPP or something like that in this case? EPERM sounds like > > you can do that, but you don't have the permissions. > > > > For me a scan should also be possible from iwpan phy $PHY scan (or > whatever the scan command is, or just enable beacon)... to go over the not enable beacon, this is for broadcasting valid pans around... as said a monitor has no address. the user "could" do that to experiment around via AF_PACKET raw. - Alex
Hi Stefan, Jakub, kuba@kernel.org wrote on Fri, 3 Feb 2023 20:19:23 -0800: > On Tue, 29 Nov 2022 17:00:41 +0100 Miquel Raynal wrote: > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > +{ > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > + struct net_device *dev = info->user_ptr[1]; > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > + struct cfg802154_scan_request *request; > > + u8 type; > > + int err; > > + > > + /* Monitors are not allowed to perform scans */ > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > extack ? Thanks for pointing at it, I just did know about it. I did convert most of the printk's into extack strings. Shall I keep both or is fine to just keep the extack thing? For now I've dropped the printk's, please tell me if this is wrong. > > > + return -EPERM; Stefan, do you prefer a series of patches applying on top of your current next or should I re-roll the entire series (scan + beacons)? I am preparing a series applying on top of the current list of applied patches. This means next PR to net maintainers will include this patch as it is today + fixes on top. If this is fine for both parties, I will send these (including the other changes discussed with Alexander). Just let me know. Sorry btw for the delay, I really had to finish other activities before switching back. > > + > > + request = kzalloc(sizeof(*request), GFP_KERNEL); > > + if (!request) > > + return -ENOMEM; > > + > > + request->wpan_dev = wpan_dev; > > + request->wpan_phy = wpan_phy; > > + > > + type = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_TYPE]); > > what checks info->attrs[NL802154_ATTR_SCAN_TYPE] is not NULL? > > > + switch (type) { > > + case NL802154_SCAN_PASSIVE: > > + request->type = type; > > + break; > > + default: > > + pr_err("Unsupported scan type: %d\n", type); > > + err = -EINVAL; > > extack (printfs are now supported) > > > + goto free_request; > > + } > > + > > + if (info->attrs[NL802154_ATTR_PAGE]) { > > + request->page = nla_get_u8(info->attrs[NL802154_ATTR_PAGE]); > > + if (request->page > IEEE802154_MAX_PAGE) { > > bound check should be part of the policy NLA_POLICY_MAX() I just improved the policies to make these checks useless and simplify a lot the code there, thanks as well for pointing at it. > > + pr_err("Invalid page %d > %d\n", > > + request->page, IEEE802154_MAX_PAGE); > > + err = -EINVAL; > > extack > > > + goto free_request; > > + } > > + } else { > > + /* Use current page by default */ > > + request->page = wpan_phy->current_page; > > + } > > + > > + if (info->attrs[NL802154_ATTR_SCAN_CHANNELS]) { > > + request->channels = nla_get_u32(info->attrs[NL802154_ATTR_SCAN_CHANNELS]); > > + if (request->channels >= BIT(IEEE802154_MAX_CHANNEL + 1)) { > > policy as well > > > + pr_err("Invalid channels bitfield %x ≥ %lx\n", > > + request->channels, > > + BIT(IEEE802154_MAX_CHANNEL + 1)); > > + err = -EINVAL; > > + goto free_request; > > + } > > + } else { > > + /* Scan all supported channels by default */ > > + request->channels = wpan_phy->supported.channels[request->page]; > > + } > > + > > + if (info->attrs[NL802154_ATTR_SCAN_PREAMBLE_CODES] || > > + info->attrs[NL802154_ATTR_SCAN_MEAN_PRF]) { > > + pr_err("Preamble codes and mean PRF not supported yet\n"); > > NLA_REJECT also in policy > > > + err = -EINVAL; > > + goto free_request; > > + } > > + > > + if (info->attrs[NL802154_ATTR_SCAN_DURATION]) { > > + request->duration = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_DURATION]); > > + if (request->duration > IEEE802154_MAX_SCAN_DURATION) { > > + pr_err("Duration is out of range\n"); > > + err = -EINVAL; > > + goto free_request; > > + } > > + } else { > > + /* Use maximum duration order by default */ > > + request->duration = IEEE802154_MAX_SCAN_DURATION; > > + } > > + > > + if (wpan_dev->netdev) > > + dev_hold(wpan_dev->netdev); > > Can we put a tracker in the request and use netdev_hold() ? I'll look into it. Thanks, Miquèl
Hello. On 10.02.23 11:18, Miquel Raynal wrote: > Hi Stefan, Jakub, > > kuba@kernel.org wrote on Fri, 3 Feb 2023 20:19:23 -0800: > >> On Tue, 29 Nov 2022 17:00:41 +0100 Miquel Raynal wrote: >>> +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) >>> +{ >>> + struct cfg802154_registered_device *rdev = info->user_ptr[0]; >>> + struct net_device *dev = info->user_ptr[1]; >>> + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; >>> + struct wpan_phy *wpan_phy = &rdev->wpan_phy; >>> + struct cfg802154_scan_request *request; >>> + u8 type; >>> + int err; >>> + >>> + /* Monitors are not allowed to perform scans */ >>> + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) >> >> extack ? > > Thanks for pointing at it, I just did know about it. I did convert > most of the printk's into extack strings. Shall I keep both or is fine > to just keep the extack thing? > > For now I've dropped the printk's, please tell me if this is wrong. > >> >>> + return -EPERM; > > Stefan, do you prefer a series of patches applying on top of your > current next or should I re-roll the entire series (scan + beacons)? > > I am preparing a series applying on top of the current list of applied > patches. This means next PR to net maintainers will include this patch > as it is today + fixes on top. If this is fine for both parties, I will > send these (including the other changes discussed with Alexander). Just > let me know. On top please. The other patches are already sitting in a published git tree and I want to avoid doing a rebase on the published tree. Once your new patches are in and Jakub is happy I will send an updated pull request with them included. regards Stefan Schmidt
Hi Stefan, stefan@datenfreihafen.org wrote on Fri, 10 Feb 2023 11:26:45 +0100: > Hello. > > On 10.02.23 11:18, Miquel Raynal wrote: > > Hi Stefan, Jakub, > > > > kuba@kernel.org wrote on Fri, 3 Feb 2023 20:19:23 -0800: > > > >> On Tue, 29 Nov 2022 17:00:41 +0100 Miquel Raynal wrote: > >>> +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > >>> +{ > >>> + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > >>> + struct net_device *dev = info->user_ptr[1]; > >>> + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > >>> + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > >>> + struct cfg802154_scan_request *request; > >>> + u8 type; > >>> + int err; > >>> + > >>> + /* Monitors are not allowed to perform scans */ > >>> + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > >> > >> extack ? > > > > Thanks for pointing at it, I just did know about it. I did convert > > most of the printk's into extack strings. Shall I keep both or is fine > > to just keep the extack thing? > > > > For now I've dropped the printk's, please tell me if this is wrong. > > > >> > >>> + return -EPERM; > > > > Stefan, do you prefer a series of patches applying on top of your > > current next or should I re-roll the entire series (scan + beacons)? > > > > I am preparing a series applying on top of the current list of applied > > patches. This means next PR to net maintainers will include this patch > > as it is today + fixes on top. If this is fine for both parties, I will > > send these (including the other changes discussed with Alexander). Just > > let me know. > > On top please. The other patches are already sitting in a published git tree and I want to avoid doing a rebase on the published tree. > > Once your new patches are in and Jakub is happy I will send an updated pull request with them included. Thanks a lot for the quick answer! Thanks, Miquèl
Hi Alexander, aahringo@redhat.com wrote on Mon, 6 Feb 2023 19:33:24 -0500: > Hi, > > On Mon, Feb 6, 2023 at 4:13 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Hi Alexander, > > > > aahringo@redhat.com wrote on Sun, 5 Feb 2023 20:39:32 -0500: > > > > > Hi, > > > > > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal > > > <miquel.raynal@bootlin.com> 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 <david.girault@qorvo.com> > > > > Signed-off-by: David Girault <david.girault@qorvo.com> > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > --- > > > > 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, > > > > > > > > /* add new commands above here */ > > > > > > > > @@ -134,6 +137,12 @@ enum nl802154_attrs { > > > > NL802154_ATTR_NETNS_FD, > > > > > > > > NL802154_ATTR_COORDINATOR, > > > > + NL802154_ATTR_SCAN_TYPE, > > > > + NL802154_ATTR_SCAN_FLAGS, > > > > + NL802154_ATTR_SCAN_CHANNELS, > > > > + NL802154_ATTR_SCAN_PREAMBLE_CODES, > > > > + NL802154_ATTR_SCAN_MEAN_PRF, > > > > + NL802154_ATTR_SCAN_DURATION, > > > > > > > > /* add attributes here, update the policy in nl802154.c */ > > > > > > > > @@ -259,6 +268,46 @@ enum nl802154_coord { > > > > NL802154_COORD_MAX, > > > > }; > > > > > > > > +/** > > > > + * enum nl802154_scan_types - Scan types > > > > + * > > > > + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved > > > > + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak > > > > + * energy in each requested channel > > > > + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using > > > > + * a Beacon Request command > > > > + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames > > > > + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation > > > > + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request > > > > + * command instead of Beacon Request command > > > > + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames > > > > + * instead of Beacon frames > > > > + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number > > > > + */ > > > > +enum nl802154_scan_types { > > > > + __NL802154_SCAN_INVALID, > > > > + NL802154_SCAN_ED, > > > > + NL802154_SCAN_ACTIVE, > > > > + NL802154_SCAN_PASSIVE, > > > > + NL802154_SCAN_ORPHAN, > > > > + NL802154_SCAN_ENHANCED_ACTIVE, > > > > + NL802154_SCAN_RIT_PASSIVE, > > > > + > > > > + /* keep last */ > > > > + NL802154_SCAN_ATTR_MAX, > > > > +}; > > > > + > > > > +/** > > > > + * enum nl802154_scan_flags - Scan request control flags > > > > + * > > > > + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie. > > > > + * a different one for every scan iteration). When the flag is set, full > > > > + * randomisation is assumed. > > > > + */ > > > > +enum nl802154_scan_flags { > > > > + NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0), > > > > +}; > > > > + > > > > /** > > > > * enum nl802154_cca_modes - cca modes > > > > * > > > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c > > > > index 80dc73182785..c497ffd8e897 100644 > > > > --- a/net/ieee802154/nl802154.c > > > > +++ b/net/ieee802154/nl802154.c > > > > @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = { > > > > > > > > [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED }, > > > > > > > > + [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 }, > > > > + [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 }, > > > > + [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 }, > > > > + [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 }, > > > > + [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 }, > > > > + > > > > #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL > > > > [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, }, > > > > [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, }, > > > > @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev, > > > > } > > > > EXPORT_SYMBOL_GPL(nl802154_scan_event); > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > +{ > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > + struct net_device *dev = info->user_ptr[1]; > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > + struct cfg802154_scan_request *request; > > > > + u8 type; > > > > + int err; > > > > + > > > > + /* Monitors are not allowed to perform scans */ > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > + return -EPERM; > > > > > > btw: why are monitors not allowed? > > > > I guess I had the "active scan" use case in mind which of course does > > not work with monitors. Maybe I can relax this a little bit indeed, > > right now I don't remember why I strongly refused scans on monitors. > > Isn't it that scans really work close to phy level? Means in this case > we disable mostly everything of MAC filtering on the transceiver side. > Then I don't see any reasons why even monitors can't do anything, they > also can send something. But they really don't have any specific > source address set, so long addresses are none for source addresses, I > don't see any problem here. They also don't have AACK handling, but > it's not required for scan anyway... I think I remember why I did not want to enable scans on monitors: we actually change the filtering level to "scan", which is very different to what a monitor is supposed to receive, which means in scan mode a monitor would no longer receive all what it is supposed to receive. Nothing that cannot be workaround'ed by software, probably, but I believe it is safer right now to avoid introducing potential regressions. So I will just change the error code and still refuse scans on monitor interfaces for now, until we figure out if it's actually safe or not (and if we really want to allow it). > If this gets too complicated right now, then I am also fine with > returning an error here, we can enable it later but would it be better > to use ENOTSUPP or something like that in this case? EPERM sounds like > you can do that, but you don't have the permissions. Got it. Thanks, Miquèl
On Fri, 10 Feb 2023 11:18:43 +0100 Miquel Raynal wrote: > > > + /* Monitors are not allowed to perform scans */ > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > extack ? > > Thanks for pointing at it, I just did know about it. I did convert > most of the printk's into extack strings. Shall I keep both or is fine > to just keep the extack thing? > > For now I've dropped the printk's, please tell me if this is wrong. That's right - just the extack, we don't want duplicated prints.
Hi Jakub, kuba@kernel.org wrote on Fri, 10 Feb 2023 10:59:25 -0800: > On Fri, 10 Feb 2023 11:18:43 +0100 Miquel Raynal wrote: > > > > + /* Monitors are not allowed to perform scans */ > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > extack ? > > > > Thanks for pointing at it, I just did know about it. I did convert > > most of the printk's into extack strings. Shall I keep both or is fine > > to just keep the extack thing? > > > > For now I've dropped the printk's, please tell me if this is wrong. > > That's right - just the extack, we don't want duplicated prints. Thanks for the confirmation. Series ready, I need to test it further just to verify there is no unwanted regression, I'll send it early next week. Thanks, Miquèl
Hi, On Fri, Feb 10, 2023 at 12:21 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > aahringo@redhat.com wrote on Mon, 6 Feb 2023 19:33:24 -0500: > > > Hi, > > > > On Mon, Feb 6, 2023 at 4:13 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > Hi Alexander, > > > > > > aahringo@redhat.com wrote on Sun, 5 Feb 2023 20:39:32 -0500: > > > > > > > Hi, > > > > > > > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal > > > > <miquel.raynal@bootlin.com> 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 <david.girault@qorvo.com> > > > > > Signed-off-by: David Girault <david.girault@qorvo.com> > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > > --- > > > > > 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, > > > > > > > > > > /* add new commands above here */ > > > > > > > > > > @@ -134,6 +137,12 @@ enum nl802154_attrs { > > > > > NL802154_ATTR_NETNS_FD, > > > > > > > > > > NL802154_ATTR_COORDINATOR, > > > > > + NL802154_ATTR_SCAN_TYPE, > > > > > + NL802154_ATTR_SCAN_FLAGS, > > > > > + NL802154_ATTR_SCAN_CHANNELS, > > > > > + NL802154_ATTR_SCAN_PREAMBLE_CODES, > > > > > + NL802154_ATTR_SCAN_MEAN_PRF, > > > > > + NL802154_ATTR_SCAN_DURATION, > > > > > > > > > > /* add attributes here, update the policy in nl802154.c */ > > > > > > > > > > @@ -259,6 +268,46 @@ enum nl802154_coord { > > > > > NL802154_COORD_MAX, > > > > > }; > > > > > > > > > > +/** > > > > > + * enum nl802154_scan_types - Scan types > > > > > + * > > > > > + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved > > > > > + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak > > > > > + * energy in each requested channel > > > > > + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using > > > > > + * a Beacon Request command > > > > > + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames > > > > > + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation > > > > > + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request > > > > > + * command instead of Beacon Request command > > > > > + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames > > > > > + * instead of Beacon frames > > > > > + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number > > > > > + */ > > > > > +enum nl802154_scan_types { > > > > > + __NL802154_SCAN_INVALID, > > > > > + NL802154_SCAN_ED, > > > > > + NL802154_SCAN_ACTIVE, > > > > > + NL802154_SCAN_PASSIVE, > > > > > + NL802154_SCAN_ORPHAN, > > > > > + NL802154_SCAN_ENHANCED_ACTIVE, > > > > > + NL802154_SCAN_RIT_PASSIVE, > > > > > + > > > > > + /* keep last */ > > > > > + NL802154_SCAN_ATTR_MAX, > > > > > +}; > > > > > + > > > > > +/** > > > > > + * enum nl802154_scan_flags - Scan request control flags > > > > > + * > > > > > + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie. > > > > > + * a different one for every scan iteration). When the flag is set, full > > > > > + * randomisation is assumed. > > > > > + */ > > > > > +enum nl802154_scan_flags { > > > > > + NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0), > > > > > +}; > > > > > + > > > > > /** > > > > > * enum nl802154_cca_modes - cca modes > > > > > * > > > > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c > > > > > index 80dc73182785..c497ffd8e897 100644 > > > > > --- a/net/ieee802154/nl802154.c > > > > > +++ b/net/ieee802154/nl802154.c > > > > > @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = { > > > > > > > > > > [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED }, > > > > > > > > > > + [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 }, > > > > > + [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 }, > > > > > + [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 }, > > > > > + [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 }, > > > > > + [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 }, > > > > > + > > > > > #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL > > > > > [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, }, > > > > > [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, }, > > > > > @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev, > > > > > } > > > > > EXPORT_SYMBOL_GPL(nl802154_scan_event); > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > +{ > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > + struct cfg802154_scan_request *request; > > > > > + u8 type; > > > > > + int err; > > > > > + > > > > > + /* Monitors are not allowed to perform scans */ > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > + return -EPERM; > > > > > > > > btw: why are monitors not allowed? > > > > > > I guess I had the "active scan" use case in mind which of course does > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > right now I don't remember why I strongly refused scans on monitors. > > > > Isn't it that scans really work close to phy level? Means in this case > > we disable mostly everything of MAC filtering on the transceiver side. > > Then I don't see any reasons why even monitors can't do anything, they > > also can send something. But they really don't have any specific > > source address set, so long addresses are none for source addresses, I > > don't see any problem here. They also don't have AACK handling, but > > it's not required for scan anyway... > > I think I remember why I did not want to enable scans on monitors: we > actually change the filtering level to "scan", which is very > different to what a monitor is supposed to receive, which means in scan > mode a monitor would no longer receive all what it is supposed to > receive. Nothing that cannot be workaround'ed by software, probably, > but I believe it is safer right now to avoid introducing potential > regressions. So I will just change the error code and still refuse > scans on monitor interfaces for now, until we figure out if it's > actually safe or not (and if we really want to allow it). > Okay, for scan yes we tell them to be in scan mode and then the transceiver can filter whatever it delivers to the next level which is necessary for filtering scan mac frames only. AACK handling is disabled for scan mode for all types != MONITORS. For monitors we mostly allow everything and AACK is _always_ disabled. The transceiver filter is completely disabled for at least what looks like a 802.15.4 MAC header (even malformed). There are some frame length checks which are necessary for specific hardware because otherwise they would read out the frame buffer. For me it can still feed the mac802154 stack for scanning (with filtering level as what the monitor sets to, but currently our scan filter is equal to the monitor filter mode anyway (which probably can be changed in future?)). So in my opinion the monitor can do both -> feed the scan mac802154 deliver path and the packet layer. And I also think that on a normal interface type the packet layer should be feeded by those frames as well and do not hit the mac802154 layer scan path only. However this can be done in future and I think, indeed there might be other problems to tackle to enable such functionality. - Alex
Hi Alexander, > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > +{ > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > + struct cfg802154_scan_request *request; > > > > > > + u8 type; > > > > > > + int err; > > > > > > + > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > + return -EPERM; > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > we disable mostly everything of MAC filtering on the transceiver side. > > > Then I don't see any reasons why even monitors can't do anything, they > > > also can send something. But they really don't have any specific > > > source address set, so long addresses are none for source addresses, I > > > don't see any problem here. They also don't have AACK handling, but > > > it's not required for scan anyway... > > > > I think I remember why I did not want to enable scans on monitors: we > > actually change the filtering level to "scan", which is very > > different to what a monitor is supposed to receive, which means in scan > > mode a monitor would no longer receive all what it is supposed to > > receive. Nothing that cannot be workaround'ed by software, probably, > > but I believe it is safer right now to avoid introducing potential > > regressions. So I will just change the error code and still refuse > > scans on monitor interfaces for now, until we figure out if it's > > actually safe or not (and if we really want to allow it). > > > > Okay, for scan yes we tell them to be in scan mode and then the > transceiver can filter whatever it delivers to the next level which is > necessary for filtering scan mac frames only. AACK handling is > disabled for scan mode for all types != MONITORS. > > For monitors we mostly allow everything and AACK is _always_ disabled. > The transceiver filter is completely disabled for at least what looks > like a 802.15.4 MAC header (even malformed). There are some frame > length checks which are necessary for specific hardware because > otherwise they would read out the frame buffer. For me it can still > feed the mac802154 stack for scanning (with filtering level as what > the monitor sets to, but currently our scan filter is equal to the > monitor filter mode anyway (which probably can be changed in > future?)). So in my opinion the monitor can do both -> feed the scan > mac802154 deliver path and the packet layer. And I also think that on > a normal interface type the packet layer should be feeded by those > frames as well and do not hit the mac802154 layer scan path only. Actually that would be an out-of-spec situation, here is a quote of chapter "6.3.1.3 Active and passive channel scan" During an active or passive scan, the MAC sublayer shall discard all frames received over the PHY data service that are not Beacon frames. I don't think this is possible to do anyway on devices with a single hardware filter setting? > However this can be done in future and I think, indeed there might be > other problems to tackle to enable such functionality. Indeed. Thanks, Miquèl
Hi Alexander, > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > +{ > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > + struct cfg802154_scan_request *request; > > > > > + u8 type; > > > > > + int err; > > > > > + > > > > > + /* Monitors are not allowed to perform scans */ > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > + return -EPERM; > > > > > > > > btw: why are monitors not allowed? > > > > > > I guess I had the "active scan" use case in mind which of course does > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > right now I don't remember why I strongly refused scans on monitors. > > > > Isn't it that scans really work close to phy level? Means in this case > > we disable mostly everything of MAC filtering on the transceiver side. > > Then I don't see any reasons why even monitors can't do anything, they > > also can send something. But they really don't have any specific > > source address set, so long addresses are none for source addresses, I > > don't see any problem here. They also don't have AACK handling, but > > it's not required for scan anyway... > > > > If this gets too complicated right now, then I am also fine with > > returning an error here, we can enable it later but would it be better > > to use ENOTSUPP or something like that in this case? EPERM sounds like > > you can do that, but you don't have the permissions. > > > > For me a scan should also be possible from iwpan phy $PHY scan (or > whatever the scan command is, or just enable beacon)... to go over the > dev is just a shortcut for "I mean whatever the phy is under this dev" > ? Actually only coordinators (in a specific state) should be able to send beacons, so I am kind of against allowing that shortcut, because there are usually two dev interfaces on top of the phy's, a regular "NODE" and a "COORD", so I don't think we should go that way. For scans however it makes sense, I've added the necessary changes in wpan-tools. The TOP_LEVEL(scan) macro however does not support using the same command name twice because it creates a macro, so this one only supports a device name (the interface command has kind of the same situation and uses a HIDDEN() macro which cannot be used here). So in summary here is what is supported: - dev <dev> beacon - dev <dev> scan trigger|abort - phy <phy> scan trigger|abort - dev <dev> scan (the blocking one, which triggers, listens and returns) Do you agree? Thanks, Miquèl
Hi, On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > +{ > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > + struct cfg802154_scan_request *request; > > > > > > + u8 type; > > > > > > + int err; > > > > > > + > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > + return -EPERM; > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > we disable mostly everything of MAC filtering on the transceiver side. > > > Then I don't see any reasons why even monitors can't do anything, they > > > also can send something. But they really don't have any specific > > > source address set, so long addresses are none for source addresses, I > > > don't see any problem here. They also don't have AACK handling, but > > > it's not required for scan anyway... > > > > > > If this gets too complicated right now, then I am also fine with > > > returning an error here, we can enable it later but would it be better > > > to use ENOTSUPP or something like that in this case? EPERM sounds like > > > you can do that, but you don't have the permissions. > > > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or > > whatever the scan command is, or just enable beacon)... to go over the > > dev is just a shortcut for "I mean whatever the phy is under this dev" > > ? > > Actually only coordinators (in a specific state) should be able to send > beacons, so I am kind of against allowing that shortcut, because there > are usually two dev interfaces on top of the phy's, a regular "NODE" > and a "COORD", so I don't think we should go that way. > > For scans however it makes sense, I've added the necessary changes in > wpan-tools. The TOP_LEVEL(scan) macro however does not support using > the same command name twice because it creates a macro, so this one > only supports a device name (the interface command has kind of the same > situation and uses a HIDDEN() macro which cannot be used here). > Yes, I was thinking about scanning only. > So in summary here is what is supported: > - dev <dev> beacon > - dev <dev> scan trigger|abort > - phy <phy> scan trigger|abort > - dev <dev> scan (the blocking one, which triggers, listens and returns) > > Do you agree? > Okay, yes. I trust you. - Alex
Hi, On Mon, Feb 13, 2023 at 5:16 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > > +{ > > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > > + struct cfg802154_scan_request *request; > > > > > > > + u8 type; > > > > > > > + int err; > > > > > > > + > > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > > + return -EPERM; > > > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > > we disable mostly everything of MAC filtering on the transceiver side. > > > > Then I don't see any reasons why even monitors can't do anything, they > > > > also can send something. But they really don't have any specific > > > > source address set, so long addresses are none for source addresses, I > > > > don't see any problem here. They also don't have AACK handling, but > > > > it's not required for scan anyway... > > > > > > I think I remember why I did not want to enable scans on monitors: we > > > actually change the filtering level to "scan", which is very > > > different to what a monitor is supposed to receive, which means in scan > > > mode a monitor would no longer receive all what it is supposed to > > > receive. Nothing that cannot be workaround'ed by software, probably, > > > but I believe it is safer right now to avoid introducing potential > > > regressions. So I will just change the error code and still refuse > > > scans on monitor interfaces for now, until we figure out if it's > > > actually safe or not (and if we really want to allow it). > > > > > > > Okay, for scan yes we tell them to be in scan mode and then the > > transceiver can filter whatever it delivers to the next level which is > > necessary for filtering scan mac frames only. AACK handling is > > disabled for scan mode for all types != MONITORS. > > > > For monitors we mostly allow everything and AACK is _always_ disabled. > > The transceiver filter is completely disabled for at least what looks > > like a 802.15.4 MAC header (even malformed). There are some frame > > length checks which are necessary for specific hardware because > > otherwise they would read out the frame buffer. For me it can still > > feed the mac802154 stack for scanning (with filtering level as what > > the monitor sets to, but currently our scan filter is equal to the > > monitor filter mode anyway (which probably can be changed in > > future?)). So in my opinion the monitor can do both -> feed the scan > > mac802154 deliver path and the packet layer. And I also think that on > > a normal interface type the packet layer should be feeded by those > > frames as well and do not hit the mac802154 layer scan path only. > > Actually that would be an out-of-spec situation, here is a quote of > chapter "6.3.1.3 Active and passive channel scan" > > During an active or passive scan, the MAC sublayer shall > discard all frames received over the PHY data service that are > not Beacon frames. > Monitor interfaces are not anything that the spec describes, it is some interface type which offers the user (mostly over AF_PACKET raw socket) full phy level access with the _default_ options. I already run user space stacks (for hacking/development only) on monitor interfaces to connect with Linux 802.15.4 interfaces, e.g. see [0] (newer came upstream, somewhere I have also a 2 years old updated version, use hwsim not fakelb). In other words, by default it should bypass 802.15.4 MAC and it still conforms with your spec, just that it is in user space. However, there exists problems to do that, but it kind of works for the most use cases. I said here by default because some people have different use cases of what they want to do in the kernel. e.g. encryption (so users only get encrypted frames, etc.) We don't support that but we can, same for doing a scan. It probably requires just more mac802154 layer filtering. There are enough examples in wireless that they do "crazy" things and you can do that only with SoftMAC transceivers because it uses more software parts like mac80211 and HardMAC transceivers only do what the spec says and delivers it to the next layer. Some of them have more functionality I guess, but then it depends on driver implementation and a lot of other things. Monitors also act as a sniffer device, but you _could_ also send frames out and then the fun part begins. > I don't think this is possible to do anyway on devices with a single > hardware filter setting? > On SoftMAC it need to support a filtering level where we can disable all filtering and get 802.15.4 MAC frames like it's on air (even without non valid checksum, but we simply don't care if the driver/transceiver does not support it we will always confirm it is correct again until somebody comes around and say "oh we can do FCS level then mac802154 does not need to check on this because it is always correct")... This is currently the NONE filtering level I think? For HardMAC it is more complicated; they don't do that, they do the "scan" operation on their transceiver and you can dump the result and probably never forward any beacon related frames? (I had this question once when Michael Richardson replied). - Alex [0] https://github.com/RIOT-OS/RIOT/pull/5582
Hi, On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote: > > Hi, > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal > <miquel.raynal@bootlin.com> wrote: > > > > Hi Alexander, > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > > +{ > > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > > + struct cfg802154_scan_request *request; > > > > > > > + u8 type; > > > > > > > + int err; > > > > > > > + > > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > > + return -EPERM; > > > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > > we disable mostly everything of MAC filtering on the transceiver side. > > > > Then I don't see any reasons why even monitors can't do anything, they > > > > also can send something. But they really don't have any specific > > > > source address set, so long addresses are none for source addresses, I > > > > don't see any problem here. They also don't have AACK handling, but > > > > it's not required for scan anyway... > > > > > > > > If this gets too complicated right now, then I am also fine with > > > > returning an error here, we can enable it later but would it be better > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like > > > > you can do that, but you don't have the permissions. > > > > > > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or > > > whatever the scan command is, or just enable beacon)... to go over the > > > dev is just a shortcut for "I mean whatever the phy is under this dev" > > > ? > > > > Actually only coordinators (in a specific state) should be able to send > > beacons, so I am kind of against allowing that shortcut, because there > > are usually two dev interfaces on top of the phy's, a regular "NODE" > > and a "COORD", so I don't think we should go that way. > > > > For scans however it makes sense, I've added the necessary changes in > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using > > the same command name twice because it creates a macro, so this one > > only supports a device name (the interface command has kind of the same > > situation and uses a HIDDEN() macro which cannot be used here). > > > > Yes, I was thinking about scanning only. > > > So in summary here is what is supported: > > - dev <dev> beacon > > - dev <dev> scan trigger|abort > > - phy <phy> scan trigger|abort > > - dev <dev> scan (the blocking one, which triggers, listens and returns) > > > > Do you agree? > > > > Okay, yes. I trust you. btw: at the point when a scan requires a source address... it cannot be done because then a scan is related to a MAC instance -> an wpan interface and we need to bind to it. But I think it doesn't? - Alex
Hi Alexander, aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500: > Hi, > > On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote: > > > > Hi, > > > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal > > <miquel.raynal@bootlin.com> wrote: > > > > > > Hi Alexander, > > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > > > +{ > > > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > > > + struct cfg802154_scan_request *request; > > > > > > > > + u8 type; > > > > > > > > + int err; > > > > > > > > + > > > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > > > + return -EPERM; > > > > > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > > > we disable mostly everything of MAC filtering on the transceiver side. > > > > > Then I don't see any reasons why even monitors can't do anything, they > > > > > also can send something. But they really don't have any specific > > > > > source address set, so long addresses are none for source addresses, I > > > > > don't see any problem here. They also don't have AACK handling, but > > > > > it's not required for scan anyway... > > > > > > > > > > If this gets too complicated right now, then I am also fine with > > > > > returning an error here, we can enable it later but would it be better > > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like > > > > > you can do that, but you don't have the permissions. > > > > > > > > > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or > > > > whatever the scan command is, or just enable beacon)... to go over the > > > > dev is just a shortcut for "I mean whatever the phy is under this dev" > > > > ? > > > > > > Actually only coordinators (in a specific state) should be able to send > > > beacons, so I am kind of against allowing that shortcut, because there > > > are usually two dev interfaces on top of the phy's, a regular "NODE" > > > and a "COORD", so I don't think we should go that way. > > > > > > For scans however it makes sense, I've added the necessary changes in > > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using > > > the same command name twice because it creates a macro, so this one > > > only supports a device name (the interface command has kind of the same > > > situation and uses a HIDDEN() macro which cannot be used here). > > > > > > > Yes, I was thinking about scanning only. > > > > > So in summary here is what is supported: > > > - dev <dev> beacon > > > - dev <dev> scan trigger|abort > > > - phy <phy> scan trigger|abort > > > - dev <dev> scan (the blocking one, which triggers, listens and returns) > > > > > > Do you agree? > > > > > > > Okay, yes. I trust you. > > btw: at the point when a scan requires a source address... it cannot > be done because then a scan is related to a MAC instance -> an wpan > interface and we need to bind to it. But I think it doesn't? I'm not sure I follow you here. You mean in case of active scan? The operation is always tight to a device in the end, even if you provide a phy in userspace. So I guess it's not a problem. Or maybe I didn't get the question right? Thanks, Miquèl
Hi Alexander, aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:51:12 -0500: > Hi, > > On Mon, Feb 13, 2023 at 5:16 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Hi Alexander, > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > > > +{ > > > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > > > + struct cfg802154_scan_request *request; > > > > > > > > + u8 type; > > > > > > > > + int err; > > > > > > > > + > > > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > > > + return -EPERM; > > > > > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > > > we disable mostly everything of MAC filtering on the transceiver side. > > > > > Then I don't see any reasons why even monitors can't do anything, they > > > > > also can send something. But they really don't have any specific > > > > > source address set, so long addresses are none for source addresses, I > > > > > don't see any problem here. They also don't have AACK handling, but > > > > > it's not required for scan anyway... > > > > > > > > I think I remember why I did not want to enable scans on monitors: we > > > > actually change the filtering level to "scan", which is very > > > > different to what a monitor is supposed to receive, which means in scan > > > > mode a monitor would no longer receive all what it is supposed to > > > > receive. Nothing that cannot be workaround'ed by software, probably, > > > > but I believe it is safer right now to avoid introducing potential > > > > regressions. So I will just change the error code and still refuse > > > > scans on monitor interfaces for now, until we figure out if it's > > > > actually safe or not (and if we really want to allow it). > > > > > > > > > > Okay, for scan yes we tell them to be in scan mode and then the > > > transceiver can filter whatever it delivers to the next level which is > > > necessary for filtering scan mac frames only. AACK handling is > > > disabled for scan mode for all types != MONITORS. > > > > > > For monitors we mostly allow everything and AACK is _always_ disabled. > > > The transceiver filter is completely disabled for at least what looks > > > like a 802.15.4 MAC header (even malformed). There are some frame > > > length checks which are necessary for specific hardware because > > > otherwise they would read out the frame buffer. For me it can still > > > feed the mac802154 stack for scanning (with filtering level as what > > > the monitor sets to, but currently our scan filter is equal to the > > > monitor filter mode anyway (which probably can be changed in > > > future?)). So in my opinion the monitor can do both -> feed the scan > > > mac802154 deliver path and the packet layer. And I also think that on > > > a normal interface type the packet layer should be feeded by those > > > frames as well and do not hit the mac802154 layer scan path only. > > > > Actually that would be an out-of-spec situation, here is a quote of > > chapter "6.3.1.3 Active and passive channel scan" > > > > During an active or passive scan, the MAC sublayer shall > > discard all frames received over the PHY data service that are > > not Beacon frames. > > > > Monitor interfaces are not anything that the spec describes, it is > some interface type which offers the user (mostly over AF_PACKET raw > socket) full phy level access with the _default_ options. I already > run user space stacks (for hacking/development only) on monitor > interfaces to connect with Linux 802.15.4 interfaces, e.g. see [0] > (newer came upstream, somewhere I have also a 2 years old updated > version, use hwsim not fakelb). :-) > > In other words, by default it should bypass 802.15.4 MAC and it still > conforms with your spec, just that it is in user space. However, there > exists problems to do that, but it kind of works for the most use > cases. I said here by default because some people have different use > cases of what they want to do in the kernel. e.g. encryption (so users > only get encrypted frames, etc.) We don't support that but we can, > same for doing a scan. It probably requires just more mac802154 layer > filtering. > > There are enough examples in wireless that they do "crazy" things and > you can do that only with SoftMAC transceivers because it uses more > software parts like mac80211 and HardMAC transceivers only do what the > spec says and delivers it to the next layer. Some of them have more > functionality I guess, but then it depends on driver implementation > and a lot of other things. > > Monitors also act as a sniffer device, but you _could_ also send > frames out and then the fun part begins. Yes, you're right, it's up to us to allow monitor actions. > > I don't think this is possible to do anyway on devices with a single > > hardware filter setting? > > > > On SoftMAC it need to support a filtering level where we can disable > all filtering and get 802.15.4 MAC frames like it's on air (even > without non valid checksum, but we simply don't care if the > driver/transceiver does not support it we will always confirm it is > correct again until somebody comes around and say "oh we can do FCS > level then mac802154 does not need to check on this because it is > always correct")... This is currently the NONE filtering level I > think? But right now we ask for the "scan" filtering, which kind of discards most frames. Would you like a special config for monitors, like receiving everything on each channel you jump on? Or shall we stick to only transmitting beacon frames during a scan on a monitor interface? I guess it's rather easy to handle in each case. Just let me know what you prefer. I think I have a small preference for the scan filtering level, but I'm open. > For HardMAC it is more complicated; they don't do that, they do the > "scan" operation on their transceiver and you can dump the result and > probably never forward any beacon related frames? (I had this question > once when Michael Richardson replied). Yes, in this case we'll have to figure out something else... > > - Alex > > [0] https://github.com/RIOT-OS/RIOT/pull/5582 > Thanks, Miquèl
miquel.raynal@bootlin.com wrote on Tue, 14 Feb 2023 15:06:00 +0100: > Hi Alexander, > > aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500: > > > Hi, > > > > On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote: > > > > > > Hi, > > > > > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal > > > <miquel.raynal@bootlin.com> wrote: > > > > > > > > Hi Alexander, > > > > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > > > > +{ > > > > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > > > > + struct cfg802154_scan_request *request; > > > > > > > > > + u8 type; > > > > > > > > > + int err; > > > > > > > > > + > > > > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > > > > + return -EPERM; > > > > > > > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > > > > we disable mostly everything of MAC filtering on the transceiver side. > > > > > > Then I don't see any reasons why even monitors can't do anything, they > > > > > > also can send something. But they really don't have any specific > > > > > > source address set, so long addresses are none for source addresses, I > > > > > > don't see any problem here. They also don't have AACK handling, but > > > > > > it's not required for scan anyway... > > > > > > > > > > > > If this gets too complicated right now, then I am also fine with > > > > > > returning an error here, we can enable it later but would it be better > > > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like > > > > > > you can do that, but you don't have the permissions. > > > > > > > > > > > > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or > > > > > whatever the scan command is, or just enable beacon)... to go over the > > > > > dev is just a shortcut for "I mean whatever the phy is under this dev" > > > > > ? > > > > > > > > Actually only coordinators (in a specific state) should be able to send > > > > beacons, so I am kind of against allowing that shortcut, because there > > > > are usually two dev interfaces on top of the phy's, a regular "NODE" > > > > and a "COORD", so I don't think we should go that way. > > > > > > > > For scans however it makes sense, I've added the necessary changes in > > > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using > > > > the same command name twice because it creates a macro, so this one > > > > only supports a device name (the interface command has kind of the same > > > > situation and uses a HIDDEN() macro which cannot be used here). > > > > > > > > > > Yes, I was thinking about scanning only. > > > > > > > So in summary here is what is supported: > > > > - dev <dev> beacon > > > > - dev <dev> scan trigger|abort > > > > - phy <phy> scan trigger|abort > > > > - dev <dev> scan (the blocking one, which triggers, listens and returns) > > > > > > > > Do you agree? > > > > > > > > > > Okay, yes. I trust you. > > > > btw: at the point when a scan requires a source address... it cannot > > be done because then a scan is related to a MAC instance -> an wpan > > interface and we need to bind to it. But I think it doesn't? > > I'm not sure I follow you here. You mean in case of active scan? Actually a beacon requests do not require any kind of source identifier, so what do you mean by source address? A regular beacon, however, does. Which means sending beacons would include being able to set an address into a monitor interface. So if we want to play with beacons on monitor interfaces, we should also relax the pan_id/addressing rules. > The operation is always tight to a device in the end, even if you > provide a phy in userspace. So I guess it's not a problem. Or maybe I > didn't get the question right? > > Thanks, > Miquèl Thanks, Miquèl
Hi, On Tue, Feb 14, 2023 at 9:07 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500: > > > Hi, > > > > On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote: > > > > > > Hi, > > > > > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal > > > <miquel.raynal@bootlin.com> wrote: > > > > > > > > Hi Alexander, > > > > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > > > > +{ > > > > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > > > > + struct cfg802154_scan_request *request; > > > > > > > > > + u8 type; > > > > > > > > > + int err; > > > > > > > > > + > > > > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > > > > + return -EPERM; > > > > > > > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > > > > we disable mostly everything of MAC filtering on the transceiver side. > > > > > > Then I don't see any reasons why even monitors can't do anything, they > > > > > > also can send something. But they really don't have any specific > > > > > > source address set, so long addresses are none for source addresses, I > > > > > > don't see any problem here. They also don't have AACK handling, but > > > > > > it's not required for scan anyway... > > > > > > > > > > > > If this gets too complicated right now, then I am also fine with > > > > > > returning an error here, we can enable it later but would it be better > > > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like > > > > > > you can do that, but you don't have the permissions. > > > > > > > > > > > > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or > > > > > whatever the scan command is, or just enable beacon)... to go over the > > > > > dev is just a shortcut for "I mean whatever the phy is under this dev" > > > > > ? > > > > > > > > Actually only coordinators (in a specific state) should be able to send > > > > beacons, so I am kind of against allowing that shortcut, because there > > > > are usually two dev interfaces on top of the phy's, a regular "NODE" > > > > and a "COORD", so I don't think we should go that way. > > > > > > > > For scans however it makes sense, I've added the necessary changes in > > > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using > > > > the same command name twice because it creates a macro, so this one > > > > only supports a device name (the interface command has kind of the same > > > > situation and uses a HIDDEN() macro which cannot be used here). > > > > > > > > > > Yes, I was thinking about scanning only. > > > > > > > So in summary here is what is supported: > > > > - dev <dev> beacon > > > > - dev <dev> scan trigger|abort > > > > - phy <phy> scan trigger|abort > > > > - dev <dev> scan (the blocking one, which triggers, listens and returns) > > > > > > > > Do you agree? > > > > > > > > > > Okay, yes. I trust you. > > > > btw: at the point when a scan requires a source address... it cannot > > be done because then a scan is related to a MAC instance -> an wpan > > interface and we need to bind to it. But I think it doesn't? > > I'm not sure I follow you here. You mean in case of active scan? The > operation is always tight to a device in the end, even if you provide a > phy in userspace. So I guess it's not a problem. Or maybe I didn't get > the question right? As soon scan requires to put somewhere mib values inside e.g. address information (which need to compared to source address settings (mib)?) then it's no longer a phy operation -> wpan_phy, it is binded to a wpan_dev (mac instance on a phy). But the addresses are set to NONE address type? I am not sure where all that data is stored right now for a scan operation, if it's operating on a phy it should be stored on wpan_phy. Note: there are also differences between wpan_phy and ieee802154_local, also wpan_dev and ieee802154_sub_if_data structures. It has something to do with visibility and SoftMAC vs HardMAC, however the last one we don't really have an infrastructure for and we probably need to move something around there. In short wpan_phy/wpan_dev should be only visible by HardMAC (I think) and the others are only additional data for the same instances used by mac802154... - Alex
Hi, On Tue, Feb 14, 2023 at 9:46 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > miquel.raynal@bootlin.com wrote on Tue, 14 Feb 2023 15:06:00 +0100: > > > Hi Alexander, > > > > aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500: > > > > > Hi, > > > > > > On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote: > > > > > > > > Hi, > > > > > > > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal > > > > <miquel.raynal@bootlin.com> wrote: > > > > > > > > > > Hi Alexander, > > > > > > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > > > > > +{ > > > > > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > > > > > + struct cfg802154_scan_request *request; > > > > > > > > > > + u8 type; > > > > > > > > > > + int err; > > > > > > > > > > + > > > > > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > > > > > + return -EPERM; > > > > > > > > > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > > > > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > > > > > we disable mostly everything of MAC filtering on the transceiver side. > > > > > > > Then I don't see any reasons why even monitors can't do anything, they > > > > > > > also can send something. But they really don't have any specific > > > > > > > source address set, so long addresses are none for source addresses, I > > > > > > > don't see any problem here. They also don't have AACK handling, but > > > > > > > it's not required for scan anyway... > > > > > > > > > > > > > > If this gets too complicated right now, then I am also fine with > > > > > > > returning an error here, we can enable it later but would it be better > > > > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like > > > > > > > you can do that, but you don't have the permissions. > > > > > > > > > > > > > > > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or > > > > > > whatever the scan command is, or just enable beacon)... to go over the > > > > > > dev is just a shortcut for "I mean whatever the phy is under this dev" > > > > > > ? > > > > > > > > > > Actually only coordinators (in a specific state) should be able to send > > > > > beacons, so I am kind of against allowing that shortcut, because there > > > > > are usually two dev interfaces on top of the phy's, a regular "NODE" > > > > > and a "COORD", so I don't think we should go that way. > > > > > > > > > > For scans however it makes sense, I've added the necessary changes in > > > > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using > > > > > the same command name twice because it creates a macro, so this one > > > > > only supports a device name (the interface command has kind of the same > > > > > situation and uses a HIDDEN() macro which cannot be used here). > > > > > > > > > > > > > Yes, I was thinking about scanning only. > > > > > > > > > So in summary here is what is supported: > > > > > - dev <dev> beacon > > > > > - dev <dev> scan trigger|abort > > > > > - phy <phy> scan trigger|abort > > > > > - dev <dev> scan (the blocking one, which triggers, listens and returns) > > > > > > > > > > Do you agree? > > > > > > > > > > > > > Okay, yes. I trust you. > > > > > > btw: at the point when a scan requires a source address... it cannot > > > be done because then a scan is related to a MAC instance -> an wpan > > > interface and we need to bind to it. But I think it doesn't? > > > > I'm not sure I follow you here. You mean in case of active scan? > > Actually a beacon requests do not require any kind of source identifier, > so what do you mean by source address? > Is this more clear now? > A regular beacon, however, does. Which means sending beacons would > include being able to set an address into a monitor interface. So if we > want to play with beacons on monitor interfaces, we should also relax > the pan_id/addressing rules. mhhh, this is required for active scans? Then a scan operation cannot be run on giving a phy only as a parameter... - Alex
Hi, On Tue, Feb 14, 2023 at 9:28 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:51:12 -0500: > > > Hi, > > > > On Mon, Feb 13, 2023 at 5:16 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > Hi Alexander, > > > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > > > > +{ > > > > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > > > > + struct cfg802154_scan_request *request; > > > > > > > > > + u8 type; > > > > > > > > > + int err; > > > > > > > > > + > > > > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > > > > + return -EPERM; > > > > > > > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > > > > we disable mostly everything of MAC filtering on the transceiver side. > > > > > > Then I don't see any reasons why even monitors can't do anything, they > > > > > > also can send something. But they really don't have any specific > > > > > > source address set, so long addresses are none for source addresses, I > > > > > > don't see any problem here. They also don't have AACK handling, but > > > > > > it's not required for scan anyway... > > > > > > > > > > I think I remember why I did not want to enable scans on monitors: we > > > > > actually change the filtering level to "scan", which is very > > > > > different to what a monitor is supposed to receive, which means in scan > > > > > mode a monitor would no longer receive all what it is supposed to > > > > > receive. Nothing that cannot be workaround'ed by software, probably, > > > > > but I believe it is safer right now to avoid introducing potential > > > > > regressions. So I will just change the error code and still refuse > > > > > scans on monitor interfaces for now, until we figure out if it's > > > > > actually safe or not (and if we really want to allow it). > > > > > > > > > > > > > Okay, for scan yes we tell them to be in scan mode and then the > > > > transceiver can filter whatever it delivers to the next level which is > > > > necessary for filtering scan mac frames only. AACK handling is > > > > disabled for scan mode for all types != MONITORS. > > > > > > > > For monitors we mostly allow everything and AACK is _always_ disabled. > > > > The transceiver filter is completely disabled for at least what looks > > > > like a 802.15.4 MAC header (even malformed). There are some frame > > > > length checks which are necessary for specific hardware because > > > > otherwise they would read out the frame buffer. For me it can still > > > > feed the mac802154 stack for scanning (with filtering level as what > > > > the monitor sets to, but currently our scan filter is equal to the > > > > monitor filter mode anyway (which probably can be changed in > > > > future?)). So in my opinion the monitor can do both -> feed the scan > > > > mac802154 deliver path and the packet layer. And I also think that on > > > > a normal interface type the packet layer should be feeded by those > > > > frames as well and do not hit the mac802154 layer scan path only. > > > > > > Actually that would be an out-of-spec situation, here is a quote of > > > chapter "6.3.1.3 Active and passive channel scan" > > > > > > During an active or passive scan, the MAC sublayer shall > > > discard all frames received over the PHY data service that are > > > not Beacon frames. > > > > > > > Monitor interfaces are not anything that the spec describes, it is > > some interface type which offers the user (mostly over AF_PACKET raw > > socket) full phy level access with the _default_ options. I already > > run user space stacks (for hacking/development only) on monitor > > interfaces to connect with Linux 802.15.4 interfaces, e.g. see [0] > > (newer came upstream, somewhere I have also a 2 years old updated > > version, use hwsim not fakelb). > > :-) > > > > > In other words, by default it should bypass 802.15.4 MAC and it still > > conforms with your spec, just that it is in user space. However, there > > exists problems to do that, but it kind of works for the most use > > cases. I said here by default because some people have different use > > cases of what they want to do in the kernel. e.g. encryption (so users > > only get encrypted frames, etc.) We don't support that but we can, > > same for doing a scan. It probably requires just more mac802154 layer > > filtering. > > > > There are enough examples in wireless that they do "crazy" things and > > you can do that only with SoftMAC transceivers because it uses more > > software parts like mac80211 and HardMAC transceivers only do what the > > spec says and delivers it to the next layer. Some of them have more > > functionality I guess, but then it depends on driver implementation > > and a lot of other things. > > > > Monitors also act as a sniffer device, but you _could_ also send > > frames out and then the fun part begins. > > Yes, you're right, it's up to us to allow monitor actions. > > > > I don't think this is possible to do anyway on devices with a single > > > hardware filter setting? > > > > > > > On SoftMAC it need to support a filtering level where we can disable > > all filtering and get 802.15.4 MAC frames like it's on air (even > > without non valid checksum, but we simply don't care if the > > driver/transceiver does not support it we will always confirm it is > > correct again until somebody comes around and say "oh we can do FCS > > level then mac802154 does not need to check on this because it is > > always correct")... This is currently the NONE filtering level I > > think? > > But right now we ask for the "scan" filtering, which kind of discards > most frames. Would you like a special config for monitors, like > receiving everything on each channel you jump on? Or shall we stick to > only transmitting beacon frames during a scan on a monitor interface? > good question... > I guess it's rather easy to handle in each case. Just let me know what > you prefer. I think I have a small preference for the scan filtering > level, but I'm open. > I would capture and deliver everything to the user.. if the user does a scan while doing whatever the user is doing with the monitor interface at this time, the user need to live with the consequences and you need to be root (okay probably every wireless manager will give the normal user access to it, but still you need to know what you are doing) > > For HardMAC it is more complicated; they don't do that, they do the > > "scan" operation on their transceiver and you can dump the result and > > probably never forward any beacon related frames? (I had this question > > once when Michael Richardson replied). > > Yes, in this case we'll have to figure out something else... > ok, I am curious. Probably it is very driver/device specific but yea, HardMAC needs to at least support what 802.15.4 says, the rest is optional and result in -ENOTSUPP? - Alex
Hi Alexander, aahringo@redhat.com wrote on Thu, 16 Feb 2023 23:37:01 -0500: > Hi, > > On Tue, Feb 14, 2023 at 9:46 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > miquel.raynal@bootlin.com wrote on Tue, 14 Feb 2023 15:06:00 +0100: > > > > > Hi Alexander, > > > > > > aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500: > > > > > > > Hi, > > > > > > > > On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote: > > > > > > > > > > Hi, > > > > > > > > > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal > > > > > <miquel.raynal@bootlin.com> wrote: > > > > > > > > > > > > Hi Alexander, > > > > > > > > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > > > > > > +{ > > > > > > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > > > > > > + struct cfg802154_scan_request *request; > > > > > > > > > > > + u8 type; > > > > > > > > > > > + int err; > > > > > > > > > > > + > > > > > > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > > > > > > + return -EPERM; > > > > > > > > > > > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > > > > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > > > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > > > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > > > > > > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > > > > > > we disable mostly everything of MAC filtering on the transceiver side. > > > > > > > > Then I don't see any reasons why even monitors can't do anything, they > > > > > > > > also can send something. But they really don't have any specific > > > > > > > > source address set, so long addresses are none for source addresses, I > > > > > > > > don't see any problem here. They also don't have AACK handling, but > > > > > > > > it's not required for scan anyway... > > > > > > > > > > > > > > > > If this gets too complicated right now, then I am also fine with > > > > > > > > returning an error here, we can enable it later but would it be better > > > > > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like > > > > > > > > you can do that, but you don't have the permissions. > > > > > > > > > > > > > > > > > > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or > > > > > > > whatever the scan command is, or just enable beacon)... to go over the > > > > > > > dev is just a shortcut for "I mean whatever the phy is under this dev" > > > > > > > ? > > > > > > > > > > > > Actually only coordinators (in a specific state) should be able to send > > > > > > beacons, so I am kind of against allowing that shortcut, because there > > > > > > are usually two dev interfaces on top of the phy's, a regular "NODE" > > > > > > and a "COORD", so I don't think we should go that way. > > > > > > > > > > > > For scans however it makes sense, I've added the necessary changes in > > > > > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using > > > > > > the same command name twice because it creates a macro, so this one > > > > > > only supports a device name (the interface command has kind of the same > > > > > > situation and uses a HIDDEN() macro which cannot be used here). > > > > > > > > > > > > > > > > Yes, I was thinking about scanning only. > > > > > > > > > > > So in summary here is what is supported: > > > > > > - dev <dev> beacon > > > > > > - dev <dev> scan trigger|abort > > > > > > - phy <phy> scan trigger|abort > > > > > > - dev <dev> scan (the blocking one, which triggers, listens and returns) > > > > > > > > > > > > Do you agree? > > > > > > > > > > > > > > > > Okay, yes. I trust you. > > > > > > > > btw: at the point when a scan requires a source address... it cannot > > > > be done because then a scan is related to a MAC instance -> an wpan > > > > interface and we need to bind to it. But I think it doesn't? > > > > > > I'm not sure I follow you here. You mean in case of active scan? > > > > Actually a beacon requests do not require any kind of source identifier, > > so what do you mean by source address? > > > > Is this more clear now? Yes, thanks! > > A regular beacon, however, does. Which means sending beacons would > > include being able to set an address into a monitor interface. So if we > > want to play with beacons on monitor interfaces, we should also relax > > the pan_id/addressing rules. > > mhhh, this is required for active scans? Then a scan operation cannot > be run on giving a phy only as a parameter... No, this is not required for active scans. Scans do not require anything else than phy parameters, AFAICS. This is required for sending beacons though. So we cannot send beacons from monitors if we don't relax the pan_id/addressing rules on these interfaces. Do you think we should? Thanks, Miquèl
Hi Alexander, aahringo@redhat.com wrote on Thu, 16 Feb 2023 23:46:42 -0500: > Hi, > > On Tue, Feb 14, 2023 at 9:28 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Hi Alexander, > > > > aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:51:12 -0500: > > > > > Hi, > > > > > > On Mon, Feb 13, 2023 at 5:16 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > > Hi Alexander, > > > > > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > > > > > +{ > > > > > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > > > > > + struct cfg802154_scan_request *request; > > > > > > > > > > + u8 type; > > > > > > > > > > + int err; > > > > > > > > > > + > > > > > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > > > > > + return -EPERM; > > > > > > > > > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > > > > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > > > > > we disable mostly everything of MAC filtering on the transceiver side. > > > > > > > Then I don't see any reasons why even monitors can't do anything, they > > > > > > > also can send something. But they really don't have any specific > > > > > > > source address set, so long addresses are none for source addresses, I > > > > > > > don't see any problem here. They also don't have AACK handling, but > > > > > > > it's not required for scan anyway... > > > > > > > > > > > > I think I remember why I did not want to enable scans on monitors: we > > > > > > actually change the filtering level to "scan", which is very > > > > > > different to what a monitor is supposed to receive, which means in scan > > > > > > mode a monitor would no longer receive all what it is supposed to > > > > > > receive. Nothing that cannot be workaround'ed by software, probably, > > > > > > but I believe it is safer right now to avoid introducing potential > > > > > > regressions. So I will just change the error code and still refuse > > > > > > scans on monitor interfaces for now, until we figure out if it's > > > > > > actually safe or not (and if we really want to allow it). > > > > > > > > > > > > > > > > Okay, for scan yes we tell them to be in scan mode and then the > > > > > transceiver can filter whatever it delivers to the next level which is > > > > > necessary for filtering scan mac frames only. AACK handling is > > > > > disabled for scan mode for all types != MONITORS. > > > > > > > > > > For monitors we mostly allow everything and AACK is _always_ disabled. > > > > > The transceiver filter is completely disabled for at least what looks > > > > > like a 802.15.4 MAC header (even malformed). There are some frame > > > > > length checks which are necessary for specific hardware because > > > > > otherwise they would read out the frame buffer. For me it can still > > > > > feed the mac802154 stack for scanning (with filtering level as what > > > > > the monitor sets to, but currently our scan filter is equal to the > > > > > monitor filter mode anyway (which probably can be changed in > > > > > future?)). So in my opinion the monitor can do both -> feed the scan > > > > > mac802154 deliver path and the packet layer. And I also think that on > > > > > a normal interface type the packet layer should be feeded by those > > > > > frames as well and do not hit the mac802154 layer scan path only. > > > > > > > > Actually that would be an out-of-spec situation, here is a quote of > > > > chapter "6.3.1.3 Active and passive channel scan" > > > > > > > > During an active or passive scan, the MAC sublayer shall > > > > discard all frames received over the PHY data service that are > > > > not Beacon frames. > > > > > > > > > > Monitor interfaces are not anything that the spec describes, it is > > > some interface type which offers the user (mostly over AF_PACKET raw > > > socket) full phy level access with the _default_ options. I already > > > run user space stacks (for hacking/development only) on monitor > > > interfaces to connect with Linux 802.15.4 interfaces, e.g. see [0] > > > (newer came upstream, somewhere I have also a 2 years old updated > > > version, use hwsim not fakelb). > > > > :-) > > > > > > > > In other words, by default it should bypass 802.15.4 MAC and it still > > > conforms with your spec, just that it is in user space. However, there > > > exists problems to do that, but it kind of works for the most use > > > cases. I said here by default because some people have different use > > > cases of what they want to do in the kernel. e.g. encryption (so users > > > only get encrypted frames, etc.) We don't support that but we can, > > > same for doing a scan. It probably requires just more mac802154 layer > > > filtering. > > > > > > There are enough examples in wireless that they do "crazy" things and > > > you can do that only with SoftMAC transceivers because it uses more > > > software parts like mac80211 and HardMAC transceivers only do what the > > > spec says and delivers it to the next layer. Some of them have more > > > functionality I guess, but then it depends on driver implementation > > > and a lot of other things. > > > > > > Monitors also act as a sniffer device, but you _could_ also send > > > frames out and then the fun part begins. > > > > Yes, you're right, it's up to us to allow monitor actions. > > > > > > I don't think this is possible to do anyway on devices with a single > > > > hardware filter setting? > > > > > > > > > > On SoftMAC it need to support a filtering level where we can disable > > > all filtering and get 802.15.4 MAC frames like it's on air (even > > > without non valid checksum, but we simply don't care if the > > > driver/transceiver does not support it we will always confirm it is > > > correct again until somebody comes around and say "oh we can do FCS > > > level then mac802154 does not need to check on this because it is > > > always correct")... This is currently the NONE filtering level I > > > think? > > > > But right now we ask for the "scan" filtering, which kind of discards > > most frames. Would you like a special config for monitors, like > > receiving everything on each channel you jump on? Or shall we stick to > > only transmitting beacon frames during a scan on a monitor interface? > > > > good question... > > > I guess it's rather easy to handle in each case. Just let me know what > > you prefer. I think I have a small preference for the scan filtering > > level, but I'm open. > > > > I would capture and deliver everything to the user.. if the user does > a scan while doing whatever the user is doing with the monitor > interface at this time, the user need to live with the consequences > and you need to be root (okay probably every wireless manager will > give the normal user access to it, but still you need to know what you > are doing) Fair enough. > > > For HardMAC it is more complicated; they don't do that, they do the > > > "scan" operation on their transceiver and you can dump the result and > > > probably never forward any beacon related frames? (I had this question > > > once when Michael Richardson replied). > > > > Yes, in this case we'll have to figure out something else... > > > > ok, I am curious. Probably it is very driver/device specific but yea, > HardMAC needs to at least support what 802.15.4 says, the rest is > optional and result in -ENOTSUPP? TBH this is still a gray area in my mental model. I'm not sure what these devices will really offer in terms of interfaces. Thanks, Miquèl
Hi Alexander, aahringo@redhat.com wrote on Thu, 16 Feb 2023 23:34:30 -0500: > Hi, > > On Tue, Feb 14, 2023 at 9:07 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Hi Alexander, > > > > aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500: > > > > > Hi, > > > > > > On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote: > > > > > > > > Hi, > > > > > > > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal > > > > <miquel.raynal@bootlin.com> wrote: > > > > > > > > > > Hi Alexander, > > > > > > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > > > > > +{ > > > > > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > > > > > + struct cfg802154_scan_request *request; > > > > > > > > > > + u8 type; > > > > > > > > > > + int err; > > > > > > > > > > + > > > > > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > > > > > + return -EPERM; > > > > > > > > > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > > > > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > > > > > we disable mostly everything of MAC filtering on the transceiver side. > > > > > > > Then I don't see any reasons why even monitors can't do anything, they > > > > > > > also can send something. But they really don't have any specific > > > > > > > source address set, so long addresses are none for source addresses, I > > > > > > > don't see any problem here. They also don't have AACK handling, but > > > > > > > it's not required for scan anyway... > > > > > > > > > > > > > > If this gets too complicated right now, then I am also fine with > > > > > > > returning an error here, we can enable it later but would it be better > > > > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like > > > > > > > you can do that, but you don't have the permissions. > > > > > > > > > > > > > > > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or > > > > > > whatever the scan command is, or just enable beacon)... to go over the > > > > > > dev is just a shortcut for "I mean whatever the phy is under this dev" > > > > > > ? > > > > > > > > > > Actually only coordinators (in a specific state) should be able to send > > > > > beacons, so I am kind of against allowing that shortcut, because there > > > > > are usually two dev interfaces on top of the phy's, a regular "NODE" > > > > > and a "COORD", so I don't think we should go that way. > > > > > > > > > > For scans however it makes sense, I've added the necessary changes in > > > > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using > > > > > the same command name twice because it creates a macro, so this one > > > > > only supports a device name (the interface command has kind of the same > > > > > situation and uses a HIDDEN() macro which cannot be used here). > > > > > > > > > > > > > Yes, I was thinking about scanning only. > > > > > > > > > So in summary here is what is supported: > > > > > - dev <dev> beacon > > > > > - dev <dev> scan trigger|abort > > > > > - phy <phy> scan trigger|abort > > > > > - dev <dev> scan (the blocking one, which triggers, listens and returns) > > > > > > > > > > Do you agree? > > > > > > > > > > > > > Okay, yes. I trust you. > > > > > > btw: at the point when a scan requires a source address... it cannot > > > be done because then a scan is related to a MAC instance -> an wpan > > > interface and we need to bind to it. But I think it doesn't? > > > > I'm not sure I follow you here. You mean in case of active scan? The > > operation is always tight to a device in the end, even if you provide a > > phy in userspace. So I guess it's not a problem. Or maybe I didn't get > > the question right? > > As soon scan requires to put somewhere mib values inside e.g. address > information (which need to compared to source address settings (mib)?) > then it's no longer a phy operation -> wpan_phy, it is binded to a > wpan_dev (mac instance on a phy). But the addresses are set to NONE > address type? > I am not sure where all that data is stored right now for a scan > operation, if it's operating on a phy it should be stored on wpan_phy. > > Note: there are also differences between wpan_phy and > ieee802154_local, also wpan_dev and ieee802154_sub_if_data structures. > It has something to do with visibility and SoftMAC vs HardMAC, however > the last one we don't really have an infrastructure for and we > probably need to move something around there. In short > wpan_phy/wpan_dev should be only visible by HardMAC (I think) and the > others are only additional data for the same instances used by > mac802154... Ok, I got what you meant. So to be clear, I assume active and passive scans are phy activities, they only involve phy parameters. Beaconing however need access to mac parameters. For now the structure defining user requests in terms of scanning and beaconing is stored into ieee802154_local, but we can move it away if needed at some point? For now I have no real example of hardMAC device so it's a bit hard to anticipate all their needs, but do you want me to move it to wpan_dev? (I would like to keep both request descriptors aside from each other). Thanks, Miquèl
Hi, On Fri, Feb 17, 2023 at 4:02 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > aahringo@redhat.com wrote on Thu, 16 Feb 2023 23:34:30 -0500: > > > Hi, > > > > On Tue, Feb 14, 2023 at 9:07 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > Hi Alexander, > > > > > > aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500: > > > > > > > Hi, > > > > > > > > On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote: > > > > > > > > > > Hi, > > > > > > > > > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal > > > > > <miquel.raynal@bootlin.com> wrote: > > > > > > > > > > > > Hi Alexander, > > > > > > > > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) > > > > > > > > > > > +{ > > > > > > > > > > > + struct cfg802154_registered_device *rdev = info->user_ptr[0]; > > > > > > > > > > > + struct net_device *dev = info->user_ptr[1]; > > > > > > > > > > > + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; > > > > > > > > > > > + struct wpan_phy *wpan_phy = &rdev->wpan_phy; > > > > > > > > > > > + struct cfg802154_scan_request *request; > > > > > > > > > > > + u8 type; > > > > > > > > > > > + int err; > > > > > > > > > > > + > > > > > > > > > > > + /* Monitors are not allowed to perform scans */ > > > > > > > > > > > + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) > > > > > > > > > > > + return -EPERM; > > > > > > > > > > > > > > > > > > > > btw: why are monitors not allowed? > > > > > > > > > > > > > > > > > > I guess I had the "active scan" use case in mind which of course does > > > > > > > > > not work with monitors. Maybe I can relax this a little bit indeed, > > > > > > > > > right now I don't remember why I strongly refused scans on monitors. > > > > > > > > > > > > > > > > Isn't it that scans really work close to phy level? Means in this case > > > > > > > > we disable mostly everything of MAC filtering on the transceiver side. > > > > > > > > Then I don't see any reasons why even monitors can't do anything, they > > > > > > > > also can send something. But they really don't have any specific > > > > > > > > source address set, so long addresses are none for source addresses, I > > > > > > > > don't see any problem here. They also don't have AACK handling, but > > > > > > > > it's not required for scan anyway... > > > > > > > > > > > > > > > > If this gets too complicated right now, then I am also fine with > > > > > > > > returning an error here, we can enable it later but would it be better > > > > > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like > > > > > > > > you can do that, but you don't have the permissions. > > > > > > > > > > > > > > > > > > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or > > > > > > > whatever the scan command is, or just enable beacon)... to go over the > > > > > > > dev is just a shortcut for "I mean whatever the phy is under this dev" > > > > > > > ? > > > > > > > > > > > > Actually only coordinators (in a specific state) should be able to send > > > > > > beacons, so I am kind of against allowing that shortcut, because there > > > > > > are usually two dev interfaces on top of the phy's, a regular "NODE" > > > > > > and a "COORD", so I don't think we should go that way. > > > > > > > > > > > > For scans however it makes sense, I've added the necessary changes in > > > > > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using > > > > > > the same command name twice because it creates a macro, so this one > > > > > > only supports a device name (the interface command has kind of the same > > > > > > situation and uses a HIDDEN() macro which cannot be used here). > > > > > > > > > > > > > > > > Yes, I was thinking about scanning only. > > > > > > > > > > > So in summary here is what is supported: > > > > > > - dev <dev> beacon > > > > > > - dev <dev> scan trigger|abort > > > > > > - phy <phy> scan trigger|abort > > > > > > - dev <dev> scan (the blocking one, which triggers, listens and returns) > > > > > > > > > > > > Do you agree? > > > > > > > > > > > > > > > > Okay, yes. I trust you. > > > > > > > > btw: at the point when a scan requires a source address... it cannot > > > > be done because then a scan is related to a MAC instance -> an wpan > > > > interface and we need to bind to it. But I think it doesn't? > > > > > > I'm not sure I follow you here. You mean in case of active scan? The > > > operation is always tight to a device in the end, even if you provide a > > > phy in userspace. So I guess it's not a problem. Or maybe I didn't get > > > the question right? > > > > As soon scan requires to put somewhere mib values inside e.g. address > > information (which need to compared to source address settings (mib)?) > > then it's no longer a phy operation -> wpan_phy, it is binded to a > > wpan_dev (mac instance on a phy). But the addresses are set to NONE > > address type? > > I am not sure where all that data is stored right now for a scan > > operation, if it's operating on a phy it should be stored on wpan_phy. > > > > Note: there are also differences between wpan_phy and > > ieee802154_local, also wpan_dev and ieee802154_sub_if_data structures. > > It has something to do with visibility and SoftMAC vs HardMAC, however > > the last one we don't really have an infrastructure for and we > > probably need to move something around there. In short > > wpan_phy/wpan_dev should be only visible by HardMAC (I think) and the > > others are only additional data for the same instances used by > > mac802154... > > Ok, I got what you meant. > > So to be clear, I assume active and passive scans are phy activities, > they only involve phy parameters. Beaconing however need access to mac > parameters. > ok. > For now the structure defining user requests in terms of scanning and > beaconing is stored into ieee802154_local, but we can move it > away if needed at some point? For now I have no real example of > hardMAC device so it's a bit hard to anticipate all their > needs, but do you want me to move it to wpan_dev? (I would like to keep > both request descriptors aside from each other). yes, forget about moving things to the right structure... somehow I think it's good to have them to not get in too much pain when trying to introduce a hardmac driver... - Alex
Hi, On Fri, Feb 17, 2023 at 3:53 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: ... > > > > ok, I am curious. Probably it is very driver/device specific but yea, > > HardMAC needs to at least support what 802.15.4 says, the rest is > > optional and result in -ENOTSUPP? > > TBH this is still a gray area in my mental model. I'm not sure what > these devices will really offer in terms of interfaces. ca8210 is one. They use those SAP-commands (MCPS-SAP and MLME-SAP) which are described by 802.15.4 spec... there is this cfg802154_ops structure which will redirect netlink to either SoftMAC or HardMAC it should somehow conform to this... However I think it should be the minimum functionality inside of this, there might be a lot of optional things which only SoftMAC supports. Also nl802154 should be oriented to this. Are you agreeing here? - Alex
Hi, On Mon, Feb 20, 2023 at 9:54 PM Alexander Aring <aahringo@redhat.com> wrote: > > Hi, > > On Fri, Feb 17, 2023 at 3:53 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > ... > > > > > > ok, I am curious. Probably it is very driver/device specific but yea, > > > HardMAC needs to at least support what 802.15.4 says, the rest is > > > optional and result in -ENOTSUPP? > > > > TBH this is still a gray area in my mental model. I'm not sure what > > these devices will really offer in terms of interfaces. > > ca8210 is one. They use those SAP-commands (MCPS-SAP and MLME-SAP) > which are described by 802.15.4 spec... there is this cfg802154_ops > structure which will redirect netlink to either SoftMAC or HardMAC it > should somehow conform to this... > However I think it should be the minimum functionality inside of this, > there might be a lot of optional things which only SoftMAC supports. > Also nl802154 should be oriented to this. > > Are you agreeing here? it's just not nl802154/cfg802154 also sending specific kind of frames out added to cfg802154 which we don't have yet inside of this "callback structure to interfacing SoftMAC or HardMAC". - Alex
Hi Alexander, aahringo@redhat.com wrote on Mon, 20 Feb 2023 21:54:41 -0500: > Hi, > > On Fri, Feb 17, 2023 at 3:53 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > ... > > > > > > ok, I am curious. Probably it is very driver/device specific but yea, > > > HardMAC needs to at least support what 802.15.4 says, the rest is > > > optional and result in -ENOTSUPP? > > > > TBH this is still a gray area in my mental model. I'm not sure what > > these devices will really offer in terms of interfaces. > > ca8210 is one. They use those SAP-commands (MCPS-SAP and MLME-SAP) > which are described by 802.15.4 spec... there is this cfg802154_ops > structure which will redirect netlink to either SoftMAC or HardMAC it > should somehow conform to this... Absolutely. > However I think it should be the minimum functionality inside of this, > there might be a lot of optional things which only SoftMAC supports. > Also nl802154 should be oriented to this. > > Are you agreeing here? Yes. That support can also be improved if we ever have to support advanced functionalities with "new" and compatible HardMAC devices. Thanks, Miquèl
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, /* add new commands above here */ @@ -134,6 +137,12 @@ enum nl802154_attrs { NL802154_ATTR_NETNS_FD, NL802154_ATTR_COORDINATOR, + NL802154_ATTR_SCAN_TYPE, + NL802154_ATTR_SCAN_FLAGS, + NL802154_ATTR_SCAN_CHANNELS, + NL802154_ATTR_SCAN_PREAMBLE_CODES, + NL802154_ATTR_SCAN_MEAN_PRF, + NL802154_ATTR_SCAN_DURATION, /* add attributes here, update the policy in nl802154.c */ @@ -259,6 +268,46 @@ enum nl802154_coord { NL802154_COORD_MAX, }; +/** + * enum nl802154_scan_types - Scan types + * + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak + * energy in each requested channel + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using + * a Beacon Request command + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request + * command instead of Beacon Request command + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames + * instead of Beacon frames + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number + */ +enum nl802154_scan_types { + __NL802154_SCAN_INVALID, + NL802154_SCAN_ED, + NL802154_SCAN_ACTIVE, + NL802154_SCAN_PASSIVE, + NL802154_SCAN_ORPHAN, + NL802154_SCAN_ENHANCED_ACTIVE, + NL802154_SCAN_RIT_PASSIVE, + + /* keep last */ + NL802154_SCAN_ATTR_MAX, +}; + +/** + * enum nl802154_scan_flags - Scan request control flags + * + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie. + * a different one for every scan iteration). When the flag is set, full + * randomisation is assumed. + */ +enum nl802154_scan_flags { + NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0), +}; + /** * enum nl802154_cca_modes - cca modes * diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c index 80dc73182785..c497ffd8e897 100644 --- a/net/ieee802154/nl802154.c +++ b/net/ieee802154/nl802154.c @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = { [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED }, + [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 }, + [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 }, + [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 }, + [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 }, + [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 }, + #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, }, [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, }, @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev, } EXPORT_SYMBOL_GPL(nl802154_scan_event); +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info) +{ + struct cfg802154_registered_device *rdev = info->user_ptr[0]; + struct net_device *dev = info->user_ptr[1]; + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; + struct wpan_phy *wpan_phy = &rdev->wpan_phy; + struct cfg802154_scan_request *request; + u8 type; + int err; + + /* Monitors are not allowed to perform scans */ + if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) + return -EPERM; + + request = kzalloc(sizeof(*request), GFP_KERNEL); + if (!request) + return -ENOMEM; + + request->wpan_dev = wpan_dev; + request->wpan_phy = wpan_phy; + + type = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_TYPE]); + switch (type) { + case NL802154_SCAN_PASSIVE: + request->type = type; + break; + default: + pr_err("Unsupported scan type: %d\n", type); + err = -EINVAL; + goto free_request; + } + + if (info->attrs[NL802154_ATTR_PAGE]) { + request->page = nla_get_u8(info->attrs[NL802154_ATTR_PAGE]); + if (request->page > IEEE802154_MAX_PAGE) { + pr_err("Invalid page %d > %d\n", + request->page, IEEE802154_MAX_PAGE); + err = -EINVAL; + goto free_request; + } + } else { + /* Use current page by default */ + request->page = wpan_phy->current_page; + } + + if (info->attrs[NL802154_ATTR_SCAN_CHANNELS]) { + request->channels = nla_get_u32(info->attrs[NL802154_ATTR_SCAN_CHANNELS]); + if (request->channels >= BIT(IEEE802154_MAX_CHANNEL + 1)) { + pr_err("Invalid channels bitfield %x ≥ %lx\n", + request->channels, + BIT(IEEE802154_MAX_CHANNEL + 1)); + err = -EINVAL; + goto free_request; + } + } else { + /* Scan all supported channels by default */ + request->channels = wpan_phy->supported.channels[request->page]; + } + + if (info->attrs[NL802154_ATTR_SCAN_PREAMBLE_CODES] || + info->attrs[NL802154_ATTR_SCAN_MEAN_PRF]) { + pr_err("Preamble codes and mean PRF not supported yet\n"); + err = -EINVAL; + goto free_request; + } + + if (info->attrs[NL802154_ATTR_SCAN_DURATION]) { + request->duration = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_DURATION]); + if (request->duration > IEEE802154_MAX_SCAN_DURATION) { + pr_err("Duration is out of range\n"); + err = -EINVAL; + goto free_request; + } + } else { + /* Use maximum duration order by default */ + request->duration = IEEE802154_MAX_SCAN_DURATION; + } + + if (wpan_dev->netdev) + dev_hold(wpan_dev->netdev); + + err = rdev_trigger_scan(rdev, request); + if (err) { + pr_err("Failure starting scanning (%d)\n", err); + goto free_device; + } + + return 0; + +free_device: + if (wpan_dev->netdev) + dev_put(wpan_dev->netdev); +free_request: + kfree(request); + + return err; +} + +static int nl802154_prep_scan_msg(struct sk_buff *msg, + struct cfg802154_registered_device *rdev, + struct wpan_dev *wpan_dev, + u32 portid, u32 seq, int flags, u8 cmd) +{ + void *hdr; + + hdr = nl802154hdr_put(msg, portid, seq, flags, cmd); + if (!hdr) + return -ENOBUFS; + + if (nla_put_u32(msg, NL802154_ATTR_WPAN_PHY, rdev->wpan_phy_idx)) + goto nla_put_failure; + + if (wpan_dev->netdev && + nla_put_u32(msg, NL802154_ATTR_IFINDEX, wpan_dev->netdev->ifindex)) + goto nla_put_failure; + + if (nla_put_u64_64bit(msg, NL802154_ATTR_WPAN_DEV, + wpan_dev_id(wpan_dev), NL802154_ATTR_PAD)) + goto nla_put_failure; + + genlmsg_end(msg, hdr); + + return 0; + +nla_put_failure: + genlmsg_cancel(msg, hdr); + + return -EMSGSIZE; +} + +static int nl802154_send_scan_msg(struct cfg802154_registered_device *rdev, + struct wpan_dev *wpan_dev, u8 cmd) +{ + struct sk_buff *msg; + int ret; + + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); + if (!msg) + return -ENOMEM; + + ret = nl802154_prep_scan_msg(msg, rdev, wpan_dev, 0, 0, 0, cmd); + if (ret < 0) { + nlmsg_free(msg); + return ret; + } + + return genlmsg_multicast_netns(&nl802154_fam, + wpan_phy_net(&rdev->wpan_phy), msg, 0, + NL802154_MCGRP_SCAN, GFP_KERNEL); +} + +int nl802154_scan_started(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev) +{ + struct cfg802154_registered_device *rdev = wpan_phy_to_rdev(wpan_phy); + int err; + + /* Ignore errors when there are no listeners */ + err = nl802154_send_scan_msg(rdev, wpan_dev, NL802154_CMD_TRIGGER_SCAN); + if (err == -ESRCH) + err = 0; + + return err; +} +EXPORT_SYMBOL_GPL(nl802154_scan_started); + +int nl802154_scan_done(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev, + u8 cmd) +{ + struct cfg802154_registered_device *rdev = wpan_phy_to_rdev(wpan_phy); + int err; + + /* Ignore errors when there are no listeners */ + err = nl802154_send_scan_msg(rdev, wpan_dev, cmd); + if (err == -ESRCH) + err = 0; + + if (wpan_dev->netdev) + dev_put(wpan_dev->netdev); + + return err; +} +EXPORT_SYMBOL_GPL(nl802154_scan_done); + +static int nl802154_abort_scan(struct sk_buff *skb, struct genl_info *info) +{ + struct cfg802154_registered_device *rdev = info->user_ptr[0]; + struct net_device *dev = info->user_ptr[1]; + struct wpan_dev *wpan_dev = dev->ieee802154_ptr; + + /* Resources are released in the notification helper above */ + return rdev_abort_scan(rdev, wpan_dev); +} + #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL static const struct nla_policy nl802154_dev_addr_policy[NL802154_DEV_ADDR_ATTR_MAX + 1] = { [NL802154_DEV_ADDR_ATTR_PAN_ID] = { .type = NLA_U16 }, @@ -2472,6 +2671,22 @@ static const struct genl_ops nl802154_ops[] = { .internal_flags = NL802154_FLAG_NEED_NETDEV | NL802154_FLAG_NEED_RTNL, }, + { + .cmd = NL802154_CMD_TRIGGER_SCAN, + .doit = nl802154_trigger_scan, + .flags = GENL_ADMIN_PERM, + .internal_flags = NL802154_FLAG_NEED_NETDEV | + NL802154_FLAG_CHECK_NETDEV_UP | + NL802154_FLAG_NEED_RTNL, + }, + { + .cmd = NL802154_CMD_ABORT_SCAN, + .doit = nl802154_abort_scan, + .flags = GENL_ADMIN_PERM, + .internal_flags = NL802154_FLAG_NEED_NETDEV | + NL802154_FLAG_CHECK_NETDEV_UP | + NL802154_FLAG_NEED_RTNL, + }, #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL { .cmd = NL802154_CMD_SET_SEC_PARAMS, diff --git a/net/ieee802154/nl802154.h b/net/ieee802154/nl802154.h index 89b805500032..88b827a42324 100644 --- a/net/ieee802154/nl802154.h +++ b/net/ieee802154/nl802154.h @@ -6,5 +6,8 @@ int nl802154_init(void); void nl802154_exit(void); int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev, struct ieee802154_coord_desc *desc); +int nl802154_scan_started(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev); +int nl802154_scan_done(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev, + u8 cmd); #endif /* __IEEE802154_NL802154_H */ diff --git a/net/ieee802154/rdev-ops.h b/net/ieee802154/rdev-ops.h index 598f5af49775..e171d74c3251 100644 --- a/net/ieee802154/rdev-ops.h +++ b/net/ieee802154/rdev-ops.h @@ -209,6 +209,34 @@ rdev_set_ackreq_default(struct cfg802154_registered_device *rdev, return ret; } +static inline int rdev_trigger_scan(struct cfg802154_registered_device *rdev, + struct cfg802154_scan_request *request) +{ + int ret; + + if (!rdev->ops->trigger_scan) + return -EOPNOTSUPP; + + trace_802154_rdev_trigger_scan(&rdev->wpan_phy, request); + ret = rdev->ops->trigger_scan(&rdev->wpan_phy, request); + trace_802154_rdev_return_int(&rdev->wpan_phy, ret); + return ret; +} + +static inline int rdev_abort_scan(struct cfg802154_registered_device *rdev, + struct wpan_dev *wpan_dev) +{ + int ret; + + if (!rdev->ops->abort_scan) + return -EOPNOTSUPP; + + trace_802154_rdev_abort_scan(&rdev->wpan_phy, wpan_dev); + ret = rdev->ops->abort_scan(&rdev->wpan_phy, wpan_dev); + trace_802154_rdev_return_int(&rdev->wpan_phy, ret); + return ret; +} + #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL /* TODO this is already a nl802154, so move into ieee802154 */ static inline void diff --git a/net/ieee802154/trace.h b/net/ieee802154/trace.h index 19c2e5d60e76..e5405f737ded 100644 --- a/net/ieee802154/trace.h +++ b/net/ieee802154/trace.h @@ -295,6 +295,46 @@ TRACE_EVENT(802154_rdev_set_ackreq_default, WPAN_DEV_PR_ARG, BOOL_TO_STR(__entry->ackreq)) ); +TRACE_EVENT(802154_rdev_trigger_scan, + TP_PROTO(struct wpan_phy *wpan_phy, + struct cfg802154_scan_request *request), + TP_ARGS(wpan_phy, request), + TP_STRUCT__entry( + WPAN_PHY_ENTRY + __field(u8, page) + __field(u32, channels) + __field(u8, duration) + ), + TP_fast_assign( + WPAN_PHY_ASSIGN; + __entry->page = request->page; + __entry->channels = request->channels; + __entry->duration = request->duration; + ), + TP_printk(WPAN_PHY_PR_FMT ", scan, page: %d, channels: %x, duration %d", + WPAN_PHY_PR_ARG, __entry->page, __entry->channels, __entry->duration) +); + +DECLARE_EVENT_CLASS(802154_wdev_template, + TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev), + TP_ARGS(wpan_phy, wpan_dev), + TP_STRUCT__entry( + WPAN_PHY_ENTRY + WPAN_DEV_ENTRY + ), + TP_fast_assign( + WPAN_PHY_ASSIGN; + WPAN_DEV_ASSIGN; + ), + TP_printk(WPAN_PHY_PR_FMT ", " WPAN_DEV_PR_FMT, + WPAN_PHY_PR_ARG, WPAN_DEV_PR_ARG) +); + +DEFINE_EVENT(802154_wdev_template, 802154_rdev_abort_scan, + TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev), + TP_ARGS(wpan_phy, wpan_dev) +); + TRACE_EVENT(802154_rdev_return_int, TP_PROTO(struct wpan_phy *wpan_phy, int ret), TP_ARGS(wpan_phy, ret),