[net-next 00/18] IEEE 802.15.4 passive scan support

[net-next 00/18] IEEE 802.15.4 passive scan support

[Miquel Raynal]

Hello,

Here is a series attempting to bring support for passive scans in the
IEEE 802.15.4 stack. A second series follows in order to align the
tooling with these changes, bringing support for a number of new
features such as:

* Passively sending (or stopping) beacons. So far only intervals ranging
  from 0 to 14 are valid. Bigger values would request the PAN
  coordinator to answer BEACONS_REQ (active scans), this is not
  supported yet.
  # iwpan dev wpan0 beacons send interval 2
  # iwpan dev wpan0 beacons stop

* Scanning all the channels or only a subset:
  # iwpan dev wpan1 scan type passive duration 3

* If a beacon is received during this operation the internal PAN list is
  updated and can be dumped or flushed with:
  # iwpan dev wpan1 pans dump
  PAN 0xffff (on wpan1)
      coordinator 0x2efefdd4cdbf9330
      page 0
      channel 13
      superframe spec. 0xcf22
      LQI 0
      seen 7156ms ago
  # iwpan dev wpan1 pans flush

* It is also possible to monitor the events with:
  # iwpan event

* As well as triggering a non blocking scan:
  # iwpan dev wpan1 scan trigger type passive duration 3
  # iwpan dev wpan1 scan done
  # iwpan dev wpan1 scan abort

The PAN list gets automatically updated by dropping the expired PANs
each time the user requests access to the list.

Internally, both requests (scan/beacons) are handled periodically by
delayed workqueues.

So far the only technical point that is missing in this series is the
possibility to grab a reference over the module driving the net device
in order to prevent module unloading during a scan or when the beacons
work is ongoing.

Finally, this series is a deep reshuffle of David Girault's original
work, hence the fact that he is almost systematically credited, either
by being the only author when I created the patches based on his changes
with almost no modification, or with a Co-developped-by tag whenever the
final code base is significantly different than his first proposal while
still being greatly inspired from it.

Cheers,
Miquèl

David Girault (5):
  net: ieee802154: Move IEEE 802.15.4 Kconfig main entry
  net: mac802154: Include the softMAC stack inside the IEEE 802.15.4
    menu
  net: ieee802154: Move the address structure earlier
  net: ieee802154: Add a kernel doc header to the ieee802154_addr
    structure
  net: ieee802154: Trace the registration of new PANs

Miquel Raynal (13):
  ieee802154: hwsim: Ensure proper channel selection at probe time
  ieee802154: hwsim: Provide a symbol duration
  net: ieee802154: Return meaningful error codes from the netlink
    helpers
  net: ieee802154: Add support for internal PAN management
  net: ieee802154: Define a beacon frame header
  net: ieee802154: Define frame types
  net: ieee802154: Add support for scanning requests
  net: mac802154: Handle scan requests
  net: mac802154: Inform device drivers about the scanning operation
  net: ieee802154: Full PAN management
  net: ieee802154: Add support for beacon requests
  net: mac802154: Handle beacons requests
  net: mac802154: Let drivers provide their own beacons implementation

 drivers/net/ieee802154/mac802154_hwsim.c |  77 +++-
 include/linux/ieee802154.h               |   6 +
 include/net/cfg802154.h                  | 107 +++++-
 include/net/ieee802154_netdev.h          |  71 ++++
 include/net/mac802154.h                  |  40 ++
 include/net/nl802154.h                   |  95 +++++
 net/Kconfig                              |   3 +-
 net/ieee802154/Kconfig                   |   1 +
 net/ieee802154/Makefile                  |   2 +-
 net/ieee802154/core.c                    |   2 +
 net/ieee802154/core.h                    |  23 ++
 net/ieee802154/header_ops.c              |  29 ++
 net/ieee802154/nl802154.c                | 466 ++++++++++++++++++++++-
 net/ieee802154/nl802154.h                |   4 +
 net/ieee802154/pan.c                     | 211 ++++++++++
 net/ieee802154/rdev-ops.h                |  52 +++
 net/ieee802154/trace.h                   |  86 +++++
 net/mac802154/Makefile                   |   2 +-
 net/mac802154/cfg.c                      |  76 ++++
 net/mac802154/driver-ops.h               |  66 ++++
 net/mac802154/ieee802154_i.h             |  28 ++
 net/mac802154/main.c                     |   4 +
 net/mac802154/rx.c                       |  10 +-
 net/mac802154/scan.c                     | 374 ++++++++++++++++++
 net/mac802154/trace.h                    |  49 +++
 net/mac802154/util.c                     |  26 ++
 26 files changed, 1889 insertions(+), 21 deletions(-)
 create mode 100644 net/ieee802154/pan.c
 create mode 100644 net/mac802154/scan.c

-- 
2.27.0

[net-next 01/18] ieee802154: hwsim: Ensure proper channel selection at probe time

[Miquel Raynal]

A default channel is selected by default (13), let's clarify that this
is page 0 channel 13. Call the right helper to ensure the necessary
configuration for this channel has been applied.

So far there is very little configuration done in this helper but we
will soon add more information (like the symbol duration which is
missing) and having this helper called at probe time will prevent us to
this type of initialization at two different locations.

So far there is very little configuration done in this helper but thanks
to this improvement, future enhancements in this area (like setting a
symbol duration, which is missing) will be reflected automatically in
the default probe state.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/mac802154_hwsim.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index 62ced7a30d92..b1a4ee7dceda 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -778,8 +778,6 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
 
 	ieee802154_random_extended_addr(&hw->phy->perm_extended_addr);
 
-	/* hwsim phy channel 13 as default */
-	hw->phy->current_channel = 13;
 	pib = kzalloc(sizeof(*pib), GFP_KERNEL);
 	if (!pib) {
 		err = -ENOMEM;
@@ -793,6 +791,11 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
 	hw->flags = IEEE802154_HW_PROMISCUOUS | IEEE802154_HW_RX_DROP_BAD_CKSUM;
 	hw->parent = dev;
 
+	/* Set page 0 / channel 13 as default */
+	hw->phy->current_page = 0;
+	hw->phy->current_channel = 13;
+	hwsim_hw_channel(hw, hw->phy->current_page, hw->phy->current_channel);
+
 	err = ieee802154_register_hw(hw);
 	if (err)
 		goto err_reg;
-- 
2.27.0

[net-next 02/18] ieee802154: hwsim: Provide a symbol duration

[Miquel Raynal]

Add support for the symbol duration in the softMAC hwsim driver. The symbol
durations are provided in micro-seconds and are extracted from the IEEE
802.15.4 specification thanks to the other parameters and comments from
this driver.

Some of these durations are hard to find/derive, so they are left to 1
in order to avoid null values (for the upcoming changes related to
scanning). In this case a comment is stating that the value in unknown.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/mac802154_hwsim.c | 70 ++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index b1a4ee7dceda..38be88db5f7a 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -92,6 +92,7 @@ static int hwsim_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 {
 	struct hwsim_phy *phy = hw->priv;
 	struct hwsim_pib *pib, *pib_old;
+	int ret = 0;
 
 	pib = kzalloc(sizeof(*pib), GFP_KERNEL);
 	if (!pib)
@@ -99,6 +100,75 @@ static int hwsim_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 
 	pib->page = page;
 	pib->channel = channel;
+	switch (page) {
+	case 0:
+		if (BIT(channel) & 0x1)
+			/* 868 MHz BPSK	802.15.4-2003: 20 ksym/s */
+			hw->phy->symbol_duration = 50;
+		else if (BIT(channel) & 0x7fe)
+			/* 915 MHz BPSK	802.15.4-2003: 40 ksym/s */
+			hw->phy->symbol_duration = 25;
+		else if (BIT(channel) & 0x7FFF800)
+			/* 2.4 GHz O-QPSK 802.15.4-2003: 62.5 ksym/s */
+			hw->phy->symbol_duration = 16;
+		else
+			ret = -EINVAL;
+		break;
+	case 1:
+		if (BIT(channel) & (0x1 | 0x7fe))
+			/* unknown rate */
+			hw->phy->symbol_duration = 1;
+		else
+			ret = -EINVAL;
+		break;
+	case 2:
+		if (BIT(channel) & 0x1)
+			/* 868 MHz O-QPSK 802.15.4-2006: 25 ksym/s */
+			hw->phy->symbol_duration = 40;
+		else if (BIT(channel) & 0x7fe)
+			/* 915 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
+			hw->phy->symbol_duration = 16;
+		else
+			ret = -EINVAL;
+		break;
+	case 3:
+		if (BIT(channel) & 0x3fff)
+			/* 2.4 GHz CSS 802.15.4a-2007: 1/6 Msym/s */
+			hw->phy->symbol_duration = 6;
+		else
+			ret = -EINVAL;
+		break;
+	case 4:
+		if (BIT(channel) & (0x1 | 0x1e | 0xffe0))
+			/* UWB 802.15.4a-2007: 993.6 or 1017.6 or 3974.4 us */
+			hw->phy->symbol_duration = 1;
+		else
+			ret = -EINVAL;
+		break;
+	case 5:
+		if (BIT(channel) & (0xf | 0xf0))
+			/* unknown rate */
+			hw->phy->symbol_duration = 1;
+		else
+			ret = -EINVAL;
+		break;
+	case 6:
+		if (BIT(channel) & (0x3ff | 0x3ffc00))
+			/* unknown rate */
+			hw->phy->symbol_duration = 1;
+		else
+			ret = -EINVAL;
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	if (ret) {
+		dev_err(hw->parent, "Invalid channel %d on page %d\n",
+			page, channel);
+		kfree(pib);
+		return ret;
+	}
 
 	pib_old = rtnl_dereference(phy->pib);
 	rcu_assign_pointer(phy->pib, pib);
-- 
2.27.0

[net-next 03/18] net: ieee802154: Move IEEE 802.15.4 Kconfig main entry

[Miquel Raynal]

From: David Girault <david.girault@qorvo.com>

It makes certainly more sense to have all the low-range wireless
protocols such as Bluetooth, IEEE 802.11 (WiFi) and IEEE 802.15.4
together, so let's move the main IEEE 802.15.4 stack Kconfig entry at a
better location.

Signed-off-by: David Girault <david.girault@qorvo.com>
[miquel.raynal@bootlin.com: Isolate this change from a bigger commit and
rewrite the commit message.]
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 net/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/Kconfig b/net/Kconfig
index 8a1f9d0287de..0da89d09ffa6 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -228,7 +228,6 @@ source "net/x25/Kconfig"
 source "net/lapb/Kconfig"
 source "net/phonet/Kconfig"
 source "net/6lowpan/Kconfig"
-source "net/ieee802154/Kconfig"
 source "net/mac802154/Kconfig"
 source "net/sched/Kconfig"
 source "net/dcb/Kconfig"
@@ -380,6 +379,7 @@ source "net/mac80211/Kconfig"
 
 endif # WIRELESS
 
+source "net/ieee802154/Kconfig"
 source "net/rfkill/Kconfig"
 source "net/9p/Kconfig"
 source "net/caif/Kconfig"
-- 
2.27.0

[net-next 04/18] net: mac802154: Include the softMAC stack inside the IEEE 802.15.4 menu

[Miquel Raynal]

From: David Girault <david.girault@qorvo.com>

The softMAC stack has no meaning outside of the IEEE 802.15.4 stack and
cannot be used without it.

Signed-off-by: David Girault <david.girault@qorvo.com>
[miquel.raynal@bootlin.com: Isolate this change from a bigger commit]
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 net/Kconfig            | 1 -
 net/ieee802154/Kconfig | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/Kconfig b/net/Kconfig
index 0da89d09ffa6..a5e31078fd14 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -228,7 +228,6 @@ source "net/x25/Kconfig"
 source "net/lapb/Kconfig"
 source "net/phonet/Kconfig"
 source "net/6lowpan/Kconfig"
-source "net/mac802154/Kconfig"
 source "net/sched/Kconfig"
 source "net/dcb/Kconfig"
 source "net/dns_resolver/Kconfig"
diff --git a/net/ieee802154/Kconfig b/net/ieee802154/Kconfig
index 31aed75fe62d..7e4b1d49d445 100644
--- a/net/ieee802154/Kconfig
+++ b/net/ieee802154/Kconfig
@@ -36,6 +36,7 @@ config IEEE802154_SOCKET
 	  for 802.15.4 dataframes. Also RAW socket interface to build MAC
 	  header from userspace.
 
+source "net/mac802154/Kconfig"
 source "net/ieee802154/6lowpan/Kconfig"
 
 endif
-- 
2.27.0

[net-next 05/18] net: ieee802154: Move the address structure earlier

[Miquel Raynal]

From: David Girault <david.girault@qorvo.com>

Move the address structure earlier in the cfg802154.h header in order to
use it in subsequent additions. There is no functional change here.

Signed-off-by: David Girault <david.girault@qorvo.com>
[miquel.raynal@bootlin.com: Isolate this change from a bigger commit]
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/cfg802154.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 6ed07844eb24..9f57bafeb3bb 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -29,6 +29,15 @@ struct ieee802154_llsec_key_id;
 struct ieee802154_llsec_key;
 #endif /* CONFIG_IEEE802154_NL802154_EXPERIMENTAL */
 
+struct ieee802154_addr {
+	u8 mode;
+	__le16 pan_id;
+	union {
+		__le16 short_addr;
+		__le64 extended_addr;
+	};
+};
+
 struct cfg802154_ops {
 	struct net_device * (*add_virtual_intf_deprecated)(struct wpan_phy *wpan_phy,
 							   const char *name,
@@ -227,15 +236,6 @@ static inline void wpan_phy_net_set(struct wpan_phy *wpan_phy, struct net *net)
 	write_pnet(&wpan_phy->_net, net);
 }
 
-struct ieee802154_addr {
-	u8 mode;
-	__le16 pan_id;
-	union {
-		__le16 short_addr;
-		__le64 extended_addr;
-	};
-};
-
 struct ieee802154_llsec_key_id {
 	u8 mode;
 	u8 id;
-- 
2.27.0

[net-next 06/18] net: ieee802154: Add a kernel doc header to the ieee802154_addr structure

[Miquel Raynal]

From: David Girault <david.girault@qorvo.com>

While not being absolutely needed, it at least explain the mode vs. enum
fields.

Signed-off-by: David Girault <david.girault@qorvo.com>
[miquel.raynal@bootlin.com: Isolate this change from a bigger commit and
                            reword the comment]
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/cfg802154.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 9f57bafeb3bb..4f36003bca98 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -29,6 +29,16 @@ struct ieee802154_llsec_key_id;
 struct ieee802154_llsec_key;
 #endif /* CONFIG_IEEE802154_NL802154_EXPERIMENTAL */
 
+/**
+ * struct ieee802154_addr - IEEE802.15.4 device address
+ * @mode: Address mode from frame header. Can be one of:
+ *        - @IEEE802154_ADDR_NONE
+ *        - @IEEE802154_ADDR_SHORT
+ *        - @IEEE802154_ADDR_LONG
+ * @pan_id: The PAN ID this address belongs to
+ * @short_addr: address if @mode is @IEEE802154_ADDR_SHORT
+ * @extended_addr: address if @mode is @IEEE802154_ADDR_LONG
+ */
 struct ieee802154_addr {
 	u8 mode;
 	__le16 pan_id;
-- 
2.27.0

[net-next 07/18] net: ieee802154: Return meaningful error codes from the netlink helpers

[Miquel Raynal]

Returning -1 does not indicate anything useful.

Use a standard and meaningful error code instead.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 net/ieee802154/nl802154.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 277124f206e0..e0b072aecf0f 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -1441,7 +1441,7 @@ static int nl802154_send_key(struct sk_buff *msg, u32 cmd, u32 portid,
 
 	hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
 	if (!hdr)
-		return -1;
+		return -ENOBUFS;
 
 	if (nla_put_u32(msg, NL802154_ATTR_IFINDEX, dev->ifindex))
 		goto nla_put_failure;
@@ -1634,7 +1634,7 @@ static int nl802154_send_device(struct sk_buff *msg, u32 cmd, u32 portid,
 
 	hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
 	if (!hdr)
-		return -1;
+		return -ENOBUFS;
 
 	if (nla_put_u32(msg, NL802154_ATTR_IFINDEX, dev->ifindex))
 		goto nla_put_failure;
@@ -1812,7 +1812,7 @@ static int nl802154_send_devkey(struct sk_buff *msg, u32 cmd, u32 portid,
 
 	hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
 	if (!hdr)
-		return -1;
+		return -ENOBUFS;
 
 	if (nla_put_u32(msg, NL802154_ATTR_IFINDEX, dev->ifindex))
 		goto nla_put_failure;
@@ -1988,7 +1988,7 @@ static int nl802154_send_seclevel(struct sk_buff *msg, u32 cmd, u32 portid,
 
 	hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
 	if (!hdr)
-		return -1;
+		return -ENOBUFS;
 
 	if (nla_put_u32(msg, NL802154_ATTR_IFINDEX, dev->ifindex))
 		goto nla_put_failure;
-- 
2.27.0

[net-next 08/18] net: ieee802154: Add support for internal PAN management

[Miquel Raynal]

Let's introduce the basics of PAN management:
- structures defining PANs
- helpers for PANs registration
- helpers discarding old PANs

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/net/cfg802154.h |  31 ++++++
 net/ieee802154/Makefile |   2 +-
 net/ieee802154/core.c   |   2 +
 net/ieee802154/core.h   |  20 ++++
 net/ieee802154/pan.c    | 208 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 262 insertions(+), 1 deletion(-)
 create mode 100644 net/ieee802154/pan.c

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 4f36003bca98..4402f93cda32 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -48,6 +48,24 @@ struct ieee802154_addr {
 	};
 };
 
+/**
+ * struct ieee802154_pan_desc - PAN descriptor information
+ * @coord: PAN ID and coordinator address
+ * @page: page this PAN is on
+ * @channel: channel this PAN is on
+ * @superframe_spec: SuperFrame specification as received
+ * @link_quality: link quality indicator at which the beacon was received
+ * @gts_permit: the PAN coordinator accepts GTS requests
+ */
+struct ieee802154_pan_desc {
+	struct ieee802154_addr *coord;
+	u8 page;
+	u8 channel;
+	u16 superframe_spec;
+	u8 link_quality;
+	bool gts_permit;
+};
+
 struct cfg802154_ops {
 	struct net_device * (*add_virtual_intf_deprecated)(struct wpan_phy *wpan_phy,
 							   const char *name,
@@ -415,4 +433,17 @@ static inline const char *wpan_phy_name(struct wpan_phy *phy)
 	return dev_name(&phy->dev);
 }
 
+/**
+ * cfg802154_record_pan - Advertize a new PAN following a beacon's reception
+ * @wpan_phy: PHY receiving the beacon
+ * @pan: PAN descriptor
+ *
+ * Tells the internal pan management layer to either register this PAN if it is
+ * new or at least update its entry if already discovered.
+ *
+ * Returns 0 on success, a negative error code otherwise.
+ */
+int cfg802154_record_pan(struct wpan_phy *wpan_phy,
+			 struct ieee802154_pan_desc *pan);
+
 #endif /* __NET_CFG802154_H */
diff --git a/net/ieee802154/Makefile b/net/ieee802154/Makefile
index f05b7bdae2aa..6b7c66de730d 100644
--- a/net/ieee802154/Makefile
+++ b/net/ieee802154/Makefile
@@ -4,7 +4,7 @@ obj-$(CONFIG_IEEE802154_SOCKET) += ieee802154_socket.o
 obj-y += 6lowpan/
 
 ieee802154-y := netlink.o nl-mac.o nl-phy.o nl_policy.o core.o \
-                header_ops.o sysfs.o nl802154.o trace.o
+                header_ops.o sysfs.o nl802154.o pan.o trace.o
 ieee802154_socket-y := socket.o
 
 CFLAGS_trace.o := -I$(src)
diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
index de259b5170ab..0f73e0571883 100644
--- a/net/ieee802154/core.c
+++ b/net/ieee802154/core.c
@@ -115,6 +115,8 @@ wpan_phy_new(const struct cfg802154_ops *ops, size_t priv_size)
 		kfree(rdev);
 		return NULL;
 	}
+	spin_lock_init(&rdev->pan_lock);
+	INIT_LIST_HEAD(&rdev->pan_list);
 
 	/* atomic_inc_return makes it start at 1, make it start at 0 */
 	rdev->wpan_phy_idx--;
diff --git a/net/ieee802154/core.h b/net/ieee802154/core.h
index 1c19f575d574..dea1d6f70489 100644
--- a/net/ieee802154/core.h
+++ b/net/ieee802154/core.h
@@ -22,6 +22,12 @@ struct cfg802154_registered_device {
 	struct list_head wpan_dev_list;
 	int devlist_generation, wpan_dev_id;
 
+	/* pan management */
+	spinlock_t pan_lock;
+	struct list_head pan_list;
+	int pan_entries;
+	int pan_generation;
+
 	/* must be last because of the way we do wpan_phy_priv(),
 	 * and it should at least be aligned to NETDEV_ALIGN
 	 */
@@ -39,6 +45,17 @@ wpan_phy_to_rdev(struct wpan_phy *wpan_phy)
 extern struct list_head cfg802154_rdev_list;
 extern int cfg802154_rdev_list_generation;
 
+struct cfg802154_internal_pan {
+	struct list_head list;
+	unsigned long discovery_ts;
+	struct ieee802154_pan_desc desc;
+};
+
+/* Always update the list by dropping the expired PANs before iterating */
+#define ieee802154_for_each_pan(pan, rdev)				\
+	cfg802154_expire_pans(rdev);					\
+	list_for_each_entry((pan), &(rdev)->pan_list, list)
+
 int cfg802154_switch_netns(struct cfg802154_registered_device *rdev,
 			   struct net *net);
 /* free object */
@@ -47,4 +64,7 @@ struct cfg802154_registered_device *
 cfg802154_rdev_by_wpan_phy_idx(int wpan_phy_idx);
 struct wpan_phy *wpan_phy_idx_to_wpan_phy(int wpan_phy_idx);
 
+void cfg802154_expire_pans(struct cfg802154_registered_device *rdev);
+void cfg802154_flush_pans(struct cfg802154_registered_device *rdev);
+
 #endif /* __IEEE802154_CORE_H */
diff --git a/net/ieee802154/pan.c b/net/ieee802154/pan.c
new file mode 100644
index 000000000000..c71a3664d5c3
--- /dev/null
+++ b/net/ieee802154/pan.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * IEEE 802.15.4 PAN management
+ *
+ * Copyright (C) Qorvo, 2021
+ * Authors:
+ *   - David Girault <david.girault@qorvo.com>
+ *   - Miquel Raynal <miquel.raynal@bootlin.com>
+ */
+
+#include <linux/slab.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+
+#include <net/cfg802154.h>
+#include <net/af_ieee802154.h>
+
+#include "ieee802154.h"
+#include "core.h"
+
+/* Maximum number of PAN entries to store */
+static int max_pan_entries = 100;
+module_param(max_pan_entries, uint, 0644);
+MODULE_PARM_DESC(max_pan_entries,
+		 "Maximum number of PANs to discover per scan (default is 100)");
+
+static int pan_expiration = 60;
+module_param(pan_expiration, uint, 0644);
+MODULE_PARM_DESC(pan_expiration,
+		 "Expiration of the scan validity in seconds (default is 60s)");
+
+static struct cfg802154_internal_pan *
+cfg802154_alloc_pan(struct ieee802154_pan_desc *desc)
+{
+	struct cfg802154_internal_pan *new;
+	struct ieee802154_addr *coord;
+
+	new = kzalloc(sizeof(*new), GFP_KERNEL);
+	if (!new)
+		return ERR_PTR(-ENOMEM);
+
+	coord = kzalloc(sizeof(*coord), GFP_KERNEL);
+	if (!coord) {
+		kfree(new);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	new->discovery_ts = jiffies;
+	new->desc = *desc;
+
+	*coord = *desc->coord;
+	new->desc.coord = coord;
+
+	return new;
+}
+
+static void cfg802154_free_pan(struct cfg802154_internal_pan *pan)
+{
+	kfree(pan->desc.coord);
+	kfree(pan);
+}
+
+static void cfg802154_unlink_pan(struct cfg802154_registered_device *rdev,
+				 struct cfg802154_internal_pan *pan)
+{
+	lockdep_assert_held(&rdev->pan_lock);
+
+	list_del(&pan->list);
+	cfg802154_free_pan(pan);
+	rdev->pan_entries--;
+	rdev->pan_generation++;
+}
+
+static void cfg802154_link_pan(struct cfg802154_registered_device *rdev,
+			       struct cfg802154_internal_pan *pan)
+{
+	lockdep_assert_held(&rdev->pan_lock);
+
+	list_add_tail(&pan->list, &rdev->pan_list);
+	rdev->pan_entries++;
+	rdev->pan_generation++;
+}
+
+void cfg802154_expire_pans(struct cfg802154_registered_device *rdev)
+{
+	unsigned long expiration_time = jiffies - (pan_expiration * HZ);
+	struct cfg802154_internal_pan *pan, *tmp;
+
+	lockdep_assert_held(&rdev->pan_lock);
+
+	list_for_each_entry_safe(pan, tmp, &rdev->pan_list, list) {
+		if (!time_after(expiration_time, pan->discovery_ts))
+			continue;
+
+		cfg802154_unlink_pan(rdev, pan);
+	}
+}
+EXPORT_SYMBOL(cfg802154_expire_pans);
+
+static void cfg802154_expire_oldest_pan(struct cfg802154_registered_device *rdev)
+{
+	struct cfg802154_internal_pan *pan, *oldest;
+
+	lockdep_assert_held(&rdev->pan_lock);
+
+	if (WARN_ON(!max_pan_entries || list_empty(&rdev->pan_list)))
+		return;
+
+	oldest = list_first_entry(&rdev->pan_list,
+				  struct cfg802154_internal_pan, list);
+
+	list_for_each_entry(pan, &rdev->pan_list, list) {
+		if (!time_before(oldest->discovery_ts, pan->discovery_ts))
+			oldest = pan;
+	}
+
+	cfg802154_unlink_pan(rdev, oldest);
+}
+
+void cfg802154_flush_pans(struct cfg802154_registered_device *rdev)
+{
+	struct cfg802154_internal_pan *pan, *tmp;
+
+	lockdep_assert_held(&rdev->pan_lock);
+
+	list_for_each_entry_safe(pan, tmp, &rdev->pan_list, list)
+		cfg802154_unlink_pan(rdev, pan);
+}
+EXPORT_SYMBOL(cfg802154_flush_pans);
+
+static bool cfg802154_same_pan(struct ieee802154_pan_desc *a,
+			       struct ieee802154_pan_desc *b)
+{
+	int ret;
+
+	if (a->page != b->page)
+		return false;
+
+	if (a->channel != b->channel)
+		return false;
+
+	ret = memcmp(&a->coord->pan_id, &b->coord->pan_id,
+		     sizeof(a->coord->pan_id));
+	if (ret)
+		return false;
+
+	if (a->coord->mode != b->coord->mode)
+		return false;
+
+	if (a->coord->mode == IEEE802154_ADDR_SHORT)
+		ret = memcmp(&a->coord->short_addr, &b->coord->short_addr,
+			     IEEE802154_SHORT_ADDR_LEN);
+	else
+		ret = memcmp(&a->coord->extended_addr, &b->coord->extended_addr,
+			     IEEE802154_EXTENDED_ADDR_LEN);
+
+	return true;
+}
+
+static struct cfg802154_internal_pan *
+cfg802154_find_matching_pan(struct cfg802154_registered_device *rdev,
+			    struct cfg802154_internal_pan *tmp)
+{
+	struct cfg802154_internal_pan *pan;
+
+	list_for_each_entry(pan, &rdev->pan_list, list) {
+		if (cfg802154_same_pan(&pan->desc, &tmp->desc))
+			return pan;
+	}
+
+	return NULL;
+}
+
+static void cfg802154_pan_update(struct cfg802154_registered_device *rdev,
+				 struct cfg802154_internal_pan *new)
+{
+	struct cfg802154_internal_pan *found;
+
+	spin_lock_bh(&rdev->pan_lock);
+
+	found = cfg802154_find_matching_pan(rdev, new);
+	if (found)
+		cfg802154_unlink_pan(rdev, found);
+
+	if (unlikely(rdev->pan_entries >= max_pan_entries))
+		cfg802154_expire_oldest_pan(rdev);
+
+	cfg802154_link_pan(rdev, new);
+
+	spin_unlock_bh(&rdev->pan_lock);
+}
+
+int cfg802154_record_pan(struct wpan_phy *wpan_phy,
+			 struct ieee802154_pan_desc *desc)
+{
+	struct cfg802154_registered_device *rdev = wpan_phy_to_rdev(wpan_phy);
+	struct cfg802154_internal_pan *new;
+
+	new = cfg802154_alloc_pan(desc);
+	if (IS_ERR(new))
+		return (PTR_ERR(new));
+
+	cfg802154_pan_update(rdev, new);
+
+	return 0;
+}
+EXPORT_SYMBOL(cfg802154_record_pan);
-- 
2.27.0

[net-next 09/18] net: ieee802154: Define a beacon frame header

[Miquel Raynal]

This definition will be used when adding support for scanning and defines
the content of a beacon frame header as in the 802.15.4 specification.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/ieee802154_netdev.h | 36 +++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h
index d0d188c3294b..a92999817dc0 100644
--- a/include/net/ieee802154_netdev.h
+++ b/include/net/ieee802154_netdev.h
@@ -22,6 +22,42 @@
 
 #include <net/cfg802154.h>
 
+struct ieee802154_beaconhdr {
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+	u16 beacon_order:4,
+	    superframe_order:4,
+	    final_cap_slot:4,
+	    battery_life_ext:1,
+	    reserved0:1,
+	    pan_coordinator:1,
+	    assoc_permit:1;
+	u8  gts_count:3,
+	    gts_reserved:4,
+	    gts_permit:1;
+	u8  pend_short_addr_count:3,
+	    reserved1:1,
+	    pend_ext_addr_count:3,
+	    reserved2:1;
+#elif defined(__BIG_ENDIAN_BITFIELD)
+	u16 assoc_permit:1,
+	    pan_coordinator:1,
+	    reserved0:1,
+	    battery_life_ext:1,
+	    final_cap_slot:4,
+	    superframe_order:4,
+	    beacon_order:4;
+	u8  gts_permit:1,
+	    gts_reserved:4,
+	    gts_count:3;
+	u8  reserved2:1,
+	    pend_ext_addr_count:3,
+	    reserved1:1,
+	    pend_short_addr_count:3;
+#else
+#error	"Please fix <asm/byteorder.h>"
+#endif
+} __packed;
+
 struct ieee802154_sechdr {
 #if defined(__LITTLE_ENDIAN_BITFIELD)
 	u8 level:3,
-- 
2.27.0

[net-next 10/18] net: ieee802154: Define frame types

[Miquel Raynal]

A 802.15.4 frame can be of different types, here is a definition
matching the specification. This enumeration will be soon be used when
adding scanning support.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/ieee802154_netdev.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h
index a92999817dc0..04738ae3b25e 100644
--- a/include/net/ieee802154_netdev.h
+++ b/include/net/ieee802154_netdev.h
@@ -105,6 +105,17 @@ struct ieee802154_hdr_fc {
 #endif
 };
 
+enum ieee802154_frame_type {
+	IEEE802154_BEACON_FRAME,
+	IEEE802154_DATA_FRAME,
+	IEEE802154_ACKNOWLEDGEMENT_FRAME,
+	IEEE802154_MAC_COMMAND_FRAME,
+	IEEE802154_RESERVED_FRAME,
+	IEEE802154_MULTIPURPOSE_FRAME,
+	IEEE802154_FRAGMENT_FRAME,
+	IEEE802154_EXTENDED_FRAME,
+};
+
 struct ieee802154_hdr {
 	struct ieee802154_hdr_fc fc;
 	u8 seq;
-- 
2.27.0

[net-next 11/18] net: ieee802154: Add support for scanning requests

[Miquel Raynal]

This involves processing triggering scan requests, abort scan requests,
as well as providing information about if the last scan was finished or
not.

A scan request structure is created to list the requirements.

A netlink multicast scan group is also created for the occasion so that
listeners can only focus on scan activity.

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 |   2 +
 include/net/cfg802154.h    |  26 +++++
 include/net/nl802154.h     |  49 ++++++++
 net/ieee802154/core.h      |   3 +
 net/ieee802154/nl802154.c  | 234 +++++++++++++++++++++++++++++++++++++
 net/ieee802154/nl802154.h  |   4 +
 net/ieee802154/rdev-ops.h  |  28 +++++
 net/ieee802154/trace.h     |  40 +++++++
 8 files changed, 386 insertions(+)

diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
index 95c831162212..a9b09a9b8f70 100644
--- a/include/linux/ieee802154.h
+++ b/include/linux/ieee802154.h
@@ -44,6 +44,8 @@
 #define IEEE802154_SHORT_ADDR_LEN	2
 #define IEEE802154_PAN_ID_LEN		2
 
+/* Duration in superframe order */
+#define IEEE802154_MAX_SCAN_DURATION	14
 #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 4402f93cda32..292eaf280f01 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -66,6 +66,28 @@ struct ieee802154_pan_desc {
 	bool gts_permit;
 };
 
+/**
+ * struct cfg802154_scan_req - Scan request
+ *
+ * @type: type of scan to be performed
+ * @flags: flags bitfield controlling the operation
+ * @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;
+	u32 flags;
+	u8 page;
+	u32 channels;
+	u8 duration;
+	struct wpan_dev *wpan_dev;
+	struct wpan_phy *wpan_phy;
+};
+
 struct cfg802154_ops {
 	struct net_device * (*add_virtual_intf_deprecated)(struct wpan_phy *wpan_phy,
 							   const char *name,
@@ -104,6 +126,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,
diff --git a/include/net/nl802154.h b/include/net/nl802154.h
index 145acb8f2509..51eca3a2b14e 100644
--- a/include/net/nl802154.h
+++ b/include/net/nl802154.h
@@ -58,6 +58,10 @@ enum nl802154_commands {
 
 	NL802154_CMD_SET_WPAN_PHY_NETNS,
 
+	NL802154_CMD_TRIGGER_SCAN,
+	NL802154_CMD_ABORT_SCAN,
+	NL802154_CMD_SCAN_DONE,
+
 	/* add new commands above here */
 
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
@@ -133,6 +137,11 @@ enum nl802154_attrs {
 	NL802154_ATTR_PID,
 	NL802154_ATTR_NETNS_FD,
 
+	NL802154_ATTR_SCAN_TYPE,
+	NL802154_ATTR_SCAN_FLAGS,
+	NL802154_ATTR_SCAN_CHANNELS,
+	NL802154_ATTR_SCAN_DURATION,
+
 	/* add attributes here, update the policy in nl802154.c */
 
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
@@ -218,6 +227,46 @@ enum nl802154_wpan_phy_capability_attr {
 	NL802154_CAP_ATTR_MAX = __NL802154_CAP_ATTR_AFTER_LAST - 1
 };
 
+/**
+ * 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/core.h b/net/ieee802154/core.h
index dea1d6f70489..190513ee4b51 100644
--- a/net/ieee802154/core.h
+++ b/net/ieee802154/core.h
@@ -28,6 +28,9 @@ struct cfg802154_registered_device {
 	int pan_entries;
 	int pan_generation;
 
+	/* scanning */
+	struct cfg802154_scan_request *scan_req;
+
 	/* must be last because of the way we do wpan_phy_priv(),
 	 * and it should at least be aligned to NETDEV_ALIGN
 	 */
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index e0b072aecf0f..d59de9812721 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -26,10 +26,12 @@ static struct genl_family nl802154_fam;
 /* multicast groups */
 enum nl802154_multicast_groups {
 	NL802154_MCGRP_CONFIG,
+	NL802154_MCGRP_SCAN,
 };
 
 static const struct genl_multicast_group nl802154_mcgrps[] = {
 	[NL802154_MCGRP_CONFIG] = { .name = "config", },
+	[NL802154_MCGRP_SCAN] = { .name = "scan", },
 };
 
 /* returns ERR_PTR values */
@@ -216,6 +218,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
 
 	[NL802154_ATTR_PID] = { .type = NLA_U32 },
 	[NL802154_ATTR_NETNS_FD] = { .type = NLA_U32 },
+
+	[NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8, },
+	[NL802154_ATTR_SCAN_FLAGS] = { .type = NLA_U32, },
+	[NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32, },
+	[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, },
@@ -1281,6 +1289,216 @@ static int nl802154_wpan_phy_netns(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
+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;
+
+	/* Test iftype and avoid scanning if monitor type. */
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+		return -EOPNOTSUPP;
+
+	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_ACTIVE:
+	case NL802154_SCAN_PASSIVE:
+		request->type = type;
+		break;
+	default:
+		pr_err("Invalid scan type: %d\n", type);
+		err = -EINVAL;
+		goto free_request;
+	}
+
+	if (info->attrs[NL802154_ATTR_SCAN_FLAGS])
+		request->flags = nla_get_u32(info->attrs[NL802154_ATTR_SCAN_FLAGS]);
+
+	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 = rdev->wpan_phy.supported.channels[request->page];
+	}
+
+	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("Invalid duration %d > %d\n",
+			       request->duration, IEEE802154_MAX_SCAN_DURATION);
+			err = -EINVAL;
+			goto free_request;
+		}
+	} else {
+		/* Use maximum duration order by default */
+		request->duration = IEEE802154_MAX_SCAN_DURATION;
+	}
+
+	err = rdev_trigger_scan(rdev, request);
+	if (err) {
+		pr_err("Failure starting scanning (%d)\n", err);
+		goto free_request;
+	}
+
+	rdev->scan_req = request;
+
+	if (wpan_dev->netdev)
+		dev_hold(wpan_dev->netdev);
+
+	return 0;
+
+free_request:
+	kfree(request);
+
+	return err;
+}
+
+static int nl802154_add_scan_req(struct sk_buff *msg,
+				 struct cfg802154_scan_request *req)
+{
+	if (req->type &&
+	    nla_put_u8(msg, NL802154_ATTR_SCAN_TYPE, req->type))
+		goto nla_put_failure;
+
+	if (req->flags &&
+	    nla_put_u32(msg, NL802154_ATTR_SCAN_FLAGS, req->flags))
+		goto nla_put_failure;
+
+	if (req->page &&
+	    nla_put_u8(msg, NL802154_ATTR_PAGE, req->page))
+		goto nla_put_failure;
+
+	if (req->channels &&
+	    nla_put_u32(msg, NL802154_ATTR_SCAN_CHANNELS, req->channels))
+		goto nla_put_failure;
+
+	return 0;
+
+nla_put_failure:
+	return -ENOBUFS;
+}
+
+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;
+
+	if (nl802154_add_scan_req(msg, rdev->scan_req))
+		goto nla_put_failure;
+
+	genlmsg_end(msg, hdr);
+
+	return 0;
+
+ nla_put_failure:
+	genlmsg_cancel(msg, hdr);
+
+	return -EMSGSIZE;
+}
+
+static int nl802154_send_scan_done_msg(struct cfg802154_registered_device *rdev,
+				       struct wpan_dev *wpan_dev)
+{
+	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,
+				     NL802154_CMD_SCAN_DONE);
+	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_send_scan_done(struct cfg802154_registered_device *rdev,
+			    struct wpan_dev *wpan_dev)
+{
+	int err;
+
+	err = nl802154_send_scan_done_msg(rdev, wpan_dev);
+
+	/* Ignore errors when there are no listeners */
+	if (err == -ESRCH)
+		err = 0;
+
+	if (wpan_dev->netdev)
+		dev_put(wpan_dev->netdev);
+
+	kfree(rdev->scan_req);
+	rdev->scan_req = NULL;
+
+	return err;
+}
+EXPORT_SYMBOL(nl802154_send_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 will be released in the notification helper above when we
+	 * are sure all actions have ended.
+	 */
+	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 },
@@ -2369,6 +2587,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 8c4b6d08954c..84567cd4ea83 100644
--- a/net/ieee802154/nl802154.h
+++ b/net/ieee802154/nl802154.h
@@ -2,7 +2,11 @@
 #ifndef __IEEE802154_NL802154_H
 #define __IEEE802154_NL802154_H
 
+#include "core.h"
+
 int nl802154_init(void);
 void nl802154_exit(void);
+int nl802154_send_scan_done(struct cfg802154_registered_device *rdev,
+			    struct wpan_dev *wpan_dev);
 
 #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),
-- 
2.27.0

[net-next 12/18] net: mac802154: Handle scan requests

[Miquel Raynal]

Implement the core hooks in order to provide the softMAC layer support
for scan requests and aborts.

Changing the channels is prohibited during the scan.

The implementation uses a workqueue triggered at a certain interval
depending on the symbol duration for the current channel and the
duration order provided.

Received beacons during a passive scanning procedure are processed and
either registered in the list of known PANs or the existing entry gets
updated accordingly.

Active scanning is not supported yet.

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   |   4 +
 include/net/mac802154.h      |  14 ++
 net/mac802154/Makefile       |   2 +-
 net/mac802154/cfg.c          |  39 ++++++
 net/mac802154/ieee802154_i.h |  15 +++
 net/mac802154/main.c         |   2 +
 net/mac802154/rx.c           |  10 +-
 net/mac802154/scan.c         | 248 +++++++++++++++++++++++++++++++++++
 net/mac802154/util.c         |  26 ++++
 9 files changed, 357 insertions(+), 3 deletions(-)
 create mode 100644 net/mac802154/scan.c

diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
index a9b09a9b8f70..60b09ff65d3d 100644
--- a/include/linux/ieee802154.h
+++ b/include/linux/ieee802154.h
@@ -46,6 +46,10 @@
 
 /* Duration in superframe order */
 #define IEEE802154_MAX_SCAN_DURATION	14
+/* Superframe duration in slots */
+#define IEEE802154_SUPERFRAME_PERIOD	16
+/* Various periods expressed in symbols */
+#define IEEE802154_SLOT_PERIOD		60
 #define IEEE802154_LIFS_PERIOD		40
 #define IEEE802154_SIFS_PERIOD		12
 #define IEEE802154_MAX_SIFS_FRAME_SIZE	18
diff --git a/include/net/mac802154.h b/include/net/mac802154.h
index d524ffb9eb25..19bfbf591ea1 100644
--- a/include/net/mac802154.h
+++ b/include/net/mac802154.h
@@ -486,4 +486,18 @@ void ieee802154_stop_queue(struct ieee802154_hw *hw);
 void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb,
 			      bool ifs_handling);
 
+/**
+ * ieee802154_queue_delayed_work - add work onto the mac802154 workqueue
+ *
+ * Drivers and mac802154 use this to queue delayed work onto the mac802154
+ * workqueue.
+ *
+ * @hw: the hardware struct for the interface we are adding work for
+ * @dwork: delayable work to queue onto the mac802154 workqueue
+ * @delay: number of jiffies to wait before queueing
+ */
+void ieee802154_queue_delayed_work(struct ieee802154_hw *hw,
+				   struct delayed_work *dwork,
+				   unsigned long delay);
+
 #endif /* NET_MAC802154_H */
diff --git a/net/mac802154/Makefile b/net/mac802154/Makefile
index 4059295fdbf8..43d1347b37ee 100644
--- a/net/mac802154/Makefile
+++ b/net/mac802154/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_MAC802154)	+= mac802154.o
 mac802154-objs		:= main.o rx.o tx.o mac_cmd.o mib.o \
-			   iface.o llsec.o util.o cfg.o trace.o
+			   iface.o llsec.o util.o cfg.o scan.o trace.o
 
 CFLAGS_trace.o := -I$(src)
diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c
index fbeebe3bc31d..5c19d6f8e3eb 100644
--- a/net/mac802154/cfg.c
+++ b/net/mac802154/cfg.c
@@ -114,6 +114,10 @@ ieee802154_set_channel(struct wpan_phy *wpan_phy, u8 page, u8 channel)
 	    wpan_phy->current_channel == channel)
 		return 0;
 
+	/* Refuse to change channels during a scanning operation */
+	if (local->scanning)
+		return -EBUSY;
+
 	ret = drv_set_channel(local, page, channel);
 	if (!ret) {
 		wpan_phy->current_page = page;
@@ -260,6 +264,39 @@ ieee802154_set_ackreq_default(struct wpan_phy *wpan_phy,
 	return 0;
 }
 
+static int mac802154_trigger_scan(struct wpan_phy *wpan_phy,
+				  struct cfg802154_scan_request *req)
+{
+	struct ieee802154_local *local = wpan_phy_priv(wpan_phy);
+	struct ieee802154_sub_if_data *sdata;
+	int ret;
+
+	sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(req->wpan_dev);
+
+	ASSERT_RTNL();
+
+	mutex_lock(&local->scan_lock);
+	ret = mac802154_trigger_scan_locked(sdata, req);
+	mutex_unlock(&local->scan_lock);
+
+	return ret;
+}
+
+static int mac802154_abort_scan(struct wpan_phy *wpan_phy,
+				struct wpan_dev *wpan_dev)
+{
+	struct ieee802154_local *local = wpan_phy_priv(wpan_phy);
+	int ret;
+
+	ASSERT_RTNL();
+
+	mutex_lock(&local->scan_lock);
+	ret = mac802154_abort_scan_locked(local);
+	mutex_unlock(&local->scan_lock);
+
+	return ret;
+}
+
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 static void
 ieee802154_get_llsec_table(struct wpan_phy *wpan_phy,
@@ -467,6 +504,8 @@ const struct cfg802154_ops mac802154_config_ops = {
 	.set_max_frame_retries = ieee802154_set_max_frame_retries,
 	.set_lbt_mode = ieee802154_set_lbt_mode,
 	.set_ackreq_default = ieee802154_set_ackreq_default,
+	.trigger_scan = mac802154_trigger_scan,
+	.abort_scan = mac802154_abort_scan,
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	.get_llsec_table = ieee802154_get_llsec_table,
 	.lock_llsec_table = ieee802154_lock_llsec_table,
diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h
index 702560acc8ce..4945edf5c2ce 100644
--- a/net/mac802154/ieee802154_i.h
+++ b/net/mac802154/ieee802154_i.h
@@ -48,6 +48,15 @@ struct ieee802154_local {
 
 	struct hrtimer ifs_timer;
 
+	/* Scanning */
+	struct mutex scan_lock;
+	unsigned long scanning;
+	__le64 scan_addr;
+	int scan_channel_idx;
+	struct cfg802154_scan_request __rcu *scan_req;
+	struct ieee802154_sub_if_data __rcu *scan_sdata;
+	struct delayed_work scan_work;
+
 	bool started;
 	bool suspended;
 
@@ -166,6 +175,12 @@ void mac802154_unlock_table(struct net_device *dev);
 
 int mac802154_wpan_update_llsec(struct net_device *dev);
 
+/* scanning handling */
+void mac802154_scan_work(struct work_struct *work);
+int mac802154_trigger_scan_locked(struct ieee802154_sub_if_data *sdata,
+				  struct cfg802154_scan_request *request);
+int mac802154_abort_scan_locked(struct ieee802154_local *local);
+int mac802154_scan_rx(struct ieee802154_local *local, struct sk_buff *skb);
 /* interface handling */
 int ieee802154_iface_init(void);
 void ieee802154_iface_exit(void);
diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index 520cedc594e1..568991734610 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -90,12 +90,14 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
 
 	INIT_LIST_HEAD(&local->interfaces);
 	mutex_init(&local->iflist_mtx);
+	mutex_init(&local->scan_lock);
 
 	tasklet_setup(&local->tasklet, ieee802154_tasklet_handler);
 
 	skb_queue_head_init(&local->skb_queue);
 
 	INIT_WORK(&local->tx_work, ieee802154_xmit_worker);
+	INIT_DELAYED_WORK(&local->scan_work, mac802154_scan_work);
 
 	/* init supported flags with 802.15.4 default ranges */
 	phy->supported.max_minbe = 8;
diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
index b8ce84618a55..acbce67caedc 100644
--- a/net/mac802154/rx.c
+++ b/net/mac802154/rx.c
@@ -198,8 +198,13 @@ __ieee802154_rx_handle_packet(struct ieee802154_local *local,
 	ret = ieee802154_parse_frame_start(skb, &hdr);
 	if (ret) {
 		pr_debug("got invalid frame\n");
-		kfree_skb(skb);
-		return;
+		goto free_skb;
+	}
+
+	if (unlikely(local->scanning)) {
+		if (mac_cb(skb)->type == IEEE802154_FC_TYPE_BEACON)
+			mac802154_scan_rx(local, skb);
+		goto free_skb;
 	}
 
 	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
@@ -214,6 +219,7 @@ __ieee802154_rx_handle_packet(struct ieee802154_local *local,
 		break;
 	}
 
+free_skb:
 	kfree_skb(skb);
 }
 
diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
new file mode 100644
index 000000000000..c5b85eaec319
--- /dev/null
+++ b/net/mac802154/scan.c
@@ -0,0 +1,248 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * IEEE 802.15.4 scanning management
+ *
+ * Copyright (C) Qorvo, 2021
+ * Authors:
+ *   - David Girault <david.girault@qorvo.com>
+ *   - Miquel Raynal <miquel.raynal@bootlin.com>
+ */
+
+#include <linux/module.h>
+#include <linux/random.h>
+#include <linux/rtnetlink.h>
+#include <net/mac802154.h>
+
+#include "ieee802154_i.h"
+#include "driver-ops.h"
+#include "../ieee802154/nl802154.h"
+
+static bool mac802154_check_promiscuous(struct ieee802154_local *local)
+{
+	struct ieee802154_sub_if_data *sdata;
+	bool promiscuous_on = false;
+
+	/* Check if one subif is already in promiscuous mode. Since the list is
+	 * protected by its own mutex, take it here to ensure no modification
+	 * occurs during the check.
+	 */
+	mutex_lock(&local->iflist_mtx);
+	list_for_each_entry(sdata, &local->interfaces, list) {
+		if (ieee802154_sdata_running(sdata) &&
+		    sdata->wpan_dev.promiscuous_mode) {
+			/* At least one is in promiscuous mode */
+			promiscuous_on = true;
+			break;
+		}
+	}
+	mutex_unlock(&local->iflist_mtx);
+	return promiscuous_on;
+}
+
+static int mac802154_set_promiscuous_mode(struct ieee802154_local *local,
+					  bool state)
+{
+	bool promiscuous_on = mac802154_check_promiscuous(local);
+	int ret;
+
+	if ((state && promiscuous_on) || (!state && !promiscuous_on))
+		return 0;
+
+	ret = drv_set_promiscuous_mode(local, state);
+	if (ret)
+		pr_err("Failed to %s promiscuous mode for SW scanning",
+		       state ? "set" : "reset");
+
+	return ret;
+}
+
+static int mac802154_send_scan_done(struct ieee802154_local *local)
+{
+	struct cfg802154_registered_device *rdev;
+	struct cfg802154_scan_request *scan_req;
+	struct wpan_dev *wpan_dev;
+
+	scan_req = rcu_dereference_protected(local->scan_req,
+					     lockdep_is_held(&local->scan_lock));
+	rdev = wpan_phy_to_rdev(scan_req->wpan_phy);
+	wpan_dev = scan_req->wpan_dev;
+
+	return nl802154_send_scan_done(rdev, wpan_dev);
+}
+
+static int mac802154_end_of_scan(struct ieee802154_local *local)
+{
+	drv_set_channel(local, local->phy->current_page,
+			local->phy->current_channel);
+	local->scanning = false;
+	mac802154_set_promiscuous_mode(local, false);
+
+	return mac802154_send_scan_done(local);
+}
+
+int mac802154_abort_scan_locked(struct ieee802154_local *local)
+{
+	lockdep_assert_held(&local->scan_lock);
+
+	if (!local->scanning)
+		return -ESRCH;
+
+	cancel_delayed_work(&local->scan_work);
+
+	return mac802154_end_of_scan(local);
+}
+
+static unsigned int mac802154_scan_get_channel_time(u8 duration_order,
+						    u8 symbol_duration)
+{
+	u64 base_super_frame_duration = (u64)symbol_duration *
+		IEEE802154_SUPERFRAME_PERIOD * IEEE802154_SLOT_PERIOD;
+
+	return usecs_to_jiffies(base_super_frame_duration *
+				(BIT(duration_order) + 1));
+}
+
+void mac802154_scan_work(struct work_struct *work)
+{
+	struct ieee802154_local *local =
+		container_of(work, struct ieee802154_local, scan_work.work);
+	struct cfg802154_scan_request *scan_req;
+	struct ieee802154_sub_if_data *sdata;
+	unsigned int scan_duration;
+	bool end_of_scan = false;
+	unsigned long chan;
+	int ret;
+
+	mutex_lock(&local->scan_lock);
+
+	if (!local->scanning)
+		goto unlock_mutex;
+
+	sdata = rcu_dereference_protected(local->scan_sdata,
+					  lockdep_is_held(&local->scan_lock));
+	scan_req = rcu_dereference_protected(local->scan_req,
+					     lockdep_is_held(&local->scan_lock));
+
+	if (local->suspended || !ieee802154_sdata_running(sdata))
+		goto queue_work;
+
+	do {
+		chan = find_next_bit((const unsigned long *)&scan_req->channels,
+				     IEEE802154_MAX_CHANNEL + 1,
+				     local->scan_channel_idx + 1);
+
+		/* If there are no more channels left, complete the scan */
+		if (chan > IEEE802154_MAX_CHANNEL) {
+			end_of_scan = true;
+			goto unlock_mutex;
+		}
+
+		/* Channel switch cannot be made atomic so hide the chan number
+		 * in order to prevent beacon processing during this timeframe.
+		 */
+		local->scan_channel_idx = -1;
+		/* Bypass the stack on purpose */
+		ret = drv_set_channel(local, scan_req->page, chan);
+		local->scan_channel_idx = chan;
+	} while (ret);
+
+queue_work:
+	scan_duration = mac802154_scan_get_channel_time(scan_req->duration,
+							local->phy->symbol_duration);
+	pr_debug("Scan channel %lu of page %u for %ums\n",
+		 chan, scan_req->page, jiffies_to_msecs(scan_duration));
+	ieee802154_queue_delayed_work(&local->hw, &local->scan_work,
+				      scan_duration);
+
+unlock_mutex:
+	if (end_of_scan)
+		mac802154_end_of_scan(local);
+
+	mutex_unlock(&local->scan_lock);
+}
+
+int mac802154_trigger_scan_locked(struct ieee802154_sub_if_data *sdata,
+				  struct cfg802154_scan_request *request)
+{
+	struct ieee802154_local *local = sdata->local;
+	int ret;
+
+	lockdep_assert_held(&local->scan_lock);
+
+	if (local->scanning)
+		return -EBUSY;
+
+	/* TODO: support other scanning type */
+	if (request->type != NL802154_SCAN_PASSIVE)
+		return -EOPNOTSUPP;
+
+	/* Store scanning parameters */
+	rcu_assign_pointer(local->scan_req, request);
+	rcu_assign_pointer(local->scan_sdata, sdata);
+
+	/* Configure scan_addr to use net_device addr or random */
+	if (request->flags & NL802154_SCAN_FLAG_RANDOM_ADDR)
+		get_random_bytes(&local->scan_addr, sizeof(local->scan_addr));
+	else
+		local->scan_addr = cpu_to_le64(get_unaligned_be64(sdata->dev->dev_addr));
+
+	local->scan_channel_idx = -1;
+	local->scanning = true;
+
+	/* Software scanning requires to set promiscuous mode */
+	ret = mac802154_set_promiscuous_mode(local, true);
+	if (ret)
+		return mac802154_end_of_scan(local);
+
+	ieee802154_queue_delayed_work(&local->hw, &local->scan_work, 0);
+
+	return 0;
+}
+
+int mac802154_scan_rx(struct ieee802154_local *local, struct sk_buff *skb)
+{
+	struct ieee802154_beaconhdr *bh = (void *)skb->data;
+	struct ieee802154_addr *src = &mac_cb(skb)->source;
+	struct cfg802154_scan_request *scan_req;
+	struct ieee802154_pan_desc desc = {};
+	int ret;
+
+	/* Check the validity of the frame length */
+	if (skb->len < sizeof(*bh))
+		return -EINVAL;
+
+	if (unlikely(src->mode == IEEE802154_ADDR_NONE))
+		return -EINVAL;
+
+	if (unlikely(!bh->pan_coordinator))
+		return -ENODEV;
+
+	scan_req = rcu_dereference(local->scan_req);
+	if (unlikely(!scan_req))
+		return -EINVAL;
+
+	if (unlikely(local->scan_channel_idx < 0)) {
+		pr_info("Dropping beacon received during channel change\n");
+		return 0;
+	}
+
+	pr_debug("Beacon received on channel %d of page %d\n",
+		 local->scan_channel_idx, scan_req->page);
+
+	/* Parse beacon and create PAN information */
+	desc.coord = src;
+	desc.page = scan_req->page;
+	desc.channel = local->scan_channel_idx;
+	desc.link_quality = mac_cb(skb)->lqi;
+	desc.superframe_spec = get_unaligned_le16(skb->data);
+	desc.gts_permit = bh->gts_permit;
+
+	/* Create or update the PAN entry in the management layer */
+	ret = cfg802154_record_pan(local->phy, &desc);
+	if (ret) {
+		pr_err("Failed to save PAN descriptor\n");
+		return ret;
+	}
+
+	return 0;
+}
diff --git a/net/mac802154/util.c b/net/mac802154/util.c
index f2078238718b..5ee65cb1dbcd 100644
--- a/net/mac802154/util.c
+++ b/net/mac802154/util.c
@@ -94,3 +94,29 @@ void ieee802154_stop_device(struct ieee802154_local *local)
 	hrtimer_cancel(&local->ifs_timer);
 	drv_stop(local);
 }
+
+/* Nothing should have been stuffed into the workqueue during
+ * the suspend->resume cycle.
+ */
+static bool ieee802154_can_queue_work(struct ieee802154_local *local)
+{
+	if (local->suspended) {
+		pr_warn("queueing ieee802154 work while suspended\n");
+		return false;
+	}
+
+	return true;
+}
+
+void ieee802154_queue_delayed_work(struct ieee802154_hw *hw,
+				   struct delayed_work *dwork,
+				   unsigned long delay)
+{
+	struct ieee802154_local *local = hw_to_local(hw);
+
+	if (!ieee802154_can_queue_work(local))
+		return;
+
+	queue_delayed_work(local->workqueue, dwork, delay);
+}
+EXPORT_SYMBOL(ieee802154_queue_delayed_work);
-- 
2.27.0

[net-next 13/18] net: mac802154: Inform device drivers about the scanning operation

[Miquel Raynal]

Let's create a couple of driver hooks in order to tell the device
drivers that a scan is ongoing, if they need to apply a particular
configuration. These hooks are optional.

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/net/mac802154.h    | 13 +++++++++++++
 net/mac802154/driver-ops.h | 33 +++++++++++++++++++++++++++++++++
 net/mac802154/scan.c       |  7 +++++++
 net/mac802154/trace.h      | 28 ++++++++++++++++++++++++++++
 4 files changed, 81 insertions(+)

diff --git a/include/net/mac802154.h b/include/net/mac802154.h
index 19bfbf591ea1..97aefba7bf96 100644
--- a/include/net/mac802154.h
+++ b/include/net/mac802154.h
@@ -204,6 +204,16 @@ enum ieee802154_hw_flags {
  *
  * set_promiscuous_mode
  *	  Enables or disable promiscuous mode.
+ *
+ * enter_scan_mode
+ *	  Enters the scan mode, may then refuse certain operations.
+ *	  Can be NULL, if the driver has no internal configuration to do.
+ *	  Returns either zero, or negative errno.
+ *
+ * exit_scan_mode
+ *	  Exits the scan mode and returns to a fully functioning state.
+ *	  Should only be provided if ->enter_scan_mode() is populated.
+ *	  Returns either zero, or negative errno.
  */
 struct ieee802154_ops {
 	struct module	*owner;
@@ -230,6 +240,9 @@ struct ieee802154_ops {
 					     s8 retries);
 	int             (*set_promiscuous_mode)(struct ieee802154_hw *hw,
 						const bool on);
+	int		(*enter_scan_mode)(struct ieee802154_hw *hw,
+					   struct cfg802154_scan_request *request);
+	int		(*exit_scan_mode)(struct ieee802154_hw *hw);
 };
 
 /**
diff --git a/net/mac802154/driver-ops.h b/net/mac802154/driver-ops.h
index d23f0db98015..2f5650f7bf91 100644
--- a/net/mac802154/driver-ops.h
+++ b/net/mac802154/driver-ops.h
@@ -282,4 +282,37 @@ drv_set_promiscuous_mode(struct ieee802154_local *local, bool on)
 	return ret;
 }
 
+static inline int drv_enter_scan_mode(struct ieee802154_local *local,
+				      struct cfg802154_scan_request *request)
+{
+	int ret;
+
+	might_sleep();
+
+	if (!local->ops->enter_scan_mode || !local->ops->exit_scan_mode)
+		return 0;
+
+	trace_802154_drv_enter_scan_mode(local, request);
+	ret = local->ops->enter_scan_mode(&local->hw, request);
+	trace_802154_drv_return_int(local, ret);
+
+	return ret;
+}
+
+static inline int drv_exit_scan_mode(struct ieee802154_local *local)
+{
+	int ret;
+
+	might_sleep();
+
+	if (!local->ops->exit_scan_mode)
+		return 0;
+
+	trace_802154_drv_exit_scan_mode(local);
+	ret = local->ops->exit_scan_mode(&local->hw);
+	trace_802154_drv_return_int(local, ret);
+
+	return ret;
+}
+
 #endif /* __MAC802154_DRIVER_OPS */
diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
index c5b85eaec319..1382489d4e58 100644
--- a/net/mac802154/scan.c
+++ b/net/mac802154/scan.c
@@ -87,6 +87,8 @@ int mac802154_abort_scan_locked(struct ieee802154_local *local)
 	if (!local->scanning)
 		return -ESRCH;
 
+	drv_exit_scan_mode(local);
+
 	cancel_delayed_work(&local->scan_work);
 
 	return mac802154_end_of_scan(local);
@@ -186,6 +188,11 @@ int mac802154_trigger_scan_locked(struct ieee802154_sub_if_data *sdata,
 	else
 		local->scan_addr = cpu_to_le64(get_unaligned_be64(sdata->dev->dev_addr));
 
+	/* Inform the hardware about the scanning operation starting */
+	ret = drv_enter_scan_mode(local, request);
+	if (ret)
+		return ret;
+
 	local->scan_channel_idx = -1;
 	local->scanning = true;
 
diff --git a/net/mac802154/trace.h b/net/mac802154/trace.h
index df855c33daf2..9c0a4f07ced1 100644
--- a/net/mac802154/trace.h
+++ b/net/mac802154/trace.h
@@ -264,6 +264,34 @@ TRACE_EVENT(802154_drv_set_promiscuous_mode,
 		  BOOL_TO_STR(__entry->on))
 );
 
+TRACE_EVENT(802154_drv_enter_scan_mode,
+	TP_PROTO(struct ieee802154_local *local,
+		 struct cfg802154_scan_request *request),
+	TP_ARGS(local, request),
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+		__field(u8, page)
+		__field(u32, channels)
+		__field(u8, duration)
+		__field(u64, addr)
+	),
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+		__entry->page = request->page;
+		__entry->channels = request->channels;
+		__entry->duration = request->duration;
+		__entry->addr = local->scan_addr;
+	),
+	TP_printk(LOCAL_PR_FMT ", scan, page: %d, channels: %x, duration %d, addr: 0x%llx",
+		  LOCAL_PR_ARG, __entry->page, __entry->channels,
+		  __entry->duration, __entry->addr)
+);
+
+DEFINE_EVENT(local_only_evt4, 802154_drv_exit_scan_mode,
+	TP_PROTO(struct ieee802154_local *local),
+	TP_ARGS(local)
+);
+
 #endif /* !__MAC802154_DRIVER_TRACE || TRACE_HEADER_MULTI_READ */
 
 #undef TRACE_INCLUDE_PATH
-- 
2.27.0

[net-next 14/18] net: ieee802154: Full PAN management

[Miquel Raynal]

Now that scanning is supported and PANs properly registered, give
certain rights to the user, such as listing asynchronously the listed
PANs as well as flushing the list. By default, the list gets
automatically updated every time users request the list of pans. Too old
PANs are automatically dropped from the list. The expiration time is
configurable with a module parameter.

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/net/nl802154.h    |  43 ++++++++++++
 net/ieee802154/nl802154.c | 143 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 186 insertions(+)

diff --git a/include/net/nl802154.h b/include/net/nl802154.h
index 51eca3a2b14e..e185e92d29d6 100644
--- a/include/net/nl802154.h
+++ b/include/net/nl802154.h
@@ -61,6 +61,8 @@ enum nl802154_commands {
 	NL802154_CMD_TRIGGER_SCAN,
 	NL802154_CMD_ABORT_SCAN,
 	NL802154_CMD_SCAN_DONE,
+	NL802154_CMD_DUMP_PANS,
+	NL802154_CMD_FLUSH_PANS,
 
 	/* add new commands above here */
 
@@ -141,6 +143,7 @@ enum nl802154_attrs {
 	NL802154_ATTR_SCAN_FLAGS,
 	NL802154_ATTR_SCAN_CHANNELS,
 	NL802154_ATTR_SCAN_DURATION,
+	NL802154_ATTR_PAN,
 
 	/* add attributes here, update the policy in nl802154.c */
 
@@ -267,6 +270,46 @@ enum nl802154_scan_flags {
 	NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0),
 };
 
+/**
+ * enum nl802154_pan - Netlink attributes for a PAN
+ *
+ * @__NL802154_PAN_INVALID: invalid
+ * @NL802154_PAN_PANID: PANID of the PAN (2 bytes)
+ * @NL802154_PAN_COORD_ADDR: Coordinator address, (8 bytes or 2 bytes)
+ * @NL802154_PAN_CHANNEL: channel number, related to @NL802154_PAN_PAGE (u8)
+ * @NL802154_PAN_PAGE: channel page, related to @NL802154_PAN_CHANNEL (u8)
+ * @NL802154_PAN_PREAMBLE_CODE: Preamble code while the beacon was received,
+ *	this is PHY dependent and optional (4 bytes)
+ * @NL802154_PAN_SUPERFRAME_SPEC: superframe specification of the PAN (u16)
+ * @NL802154_PAN_LINK_QUALITY: signal quality of beacon in unspecified units,
+ *	scaled to 0..255 (u8)
+ * @NL802154_PAN_GTS_PERMIT: set to true if GTS is permitted on this PAN
+ * @NL802154_PAN_PAYLOAD_DATA: binary data containing the raw data from the
+ *	frame payload, (only if beacon or probe response had data)
+ * @NL802154_PAN_STATUS: status, if this PAN is "used"
+ * @NL802154_PAN_SEEN_MS_AGO: age of this PAN entry in ms
+ * @NL802154_PAN_PAD: attribute used for padding for 64-bit alignment
+ * @NL802154_PAN_MAX: highest PAN attribute
+ */
+enum nl802154_pan {
+	__NL802154_PAN_INVALID,
+	NL802154_PAN_PANID,
+	NL802154_PAN_COORD_ADDR,
+	NL802154_PAN_CHANNEL,
+	NL802154_PAN_PAGE,
+	NL802154_PAN_PREAMBLE_CODE,
+	NL802154_PAN_SUPERFRAME_SPEC,
+	NL802154_PAN_LINK_QUALITY,
+	NL802154_PAN_GTS_PERMIT,
+	NL802154_PAN_PAYLOAD_DATA,
+	NL802154_PAN_STATUS,
+	NL802154_PAN_SEEN_MS_AGO,
+	NL802154_PAN_PAD,
+
+	/* keep last */
+	NL802154_PAN_MAX,
+};
+
 /**
  * enum nl802154_cca_modes - cca modes
  *
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index d59de9812721..8de8f7c31bfe 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -223,6 +223,7 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
 	[NL802154_ATTR_SCAN_FLAGS] = { .type = NLA_U32, },
 	[NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32, },
 	[NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8, },
+	[NL802154_ATTR_PAN] = { .type = NLA_NESTED },
 
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	[NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
@@ -1499,6 +1500,136 @@ static int nl802154_abort_scan(struct sk_buff *skb, struct genl_info *info)
 	return rdev_abort_scan(rdev, wpan_dev);
 }
 
+static int nl802154_send_pan_info(struct sk_buff *msg,
+				  struct netlink_callback *cb,
+				  u32 seq, int flags,
+				  struct cfg802154_registered_device *rdev,
+				  struct wpan_dev *wpan_dev,
+				  struct cfg802154_internal_pan *intpan)
+{
+	struct ieee802154_pan_desc *pan = &intpan->desc;
+	struct nlattr *nla;
+	void *hdr;
+
+	ASSERT_RTNL();
+
+	hdr = nl802154hdr_put(msg, NETLINK_CB(cb->skb).portid, seq, flags,
+			      NL802154_CMD_SCAN_DONE);
+	if (!hdr)
+		return -ENOBUFS;
+
+	genl_dump_check_consistent(cb, hdr);
+
+	if (nla_put_u32(msg, NL802154_ATTR_GENERATION, rdev->pan_generation))
+		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;
+
+	nla = nla_nest_start_noflag(msg, NL802154_ATTR_PAN);
+	if (!nla)
+		goto nla_put_failure;
+
+	if (nla_put(msg, NL802154_PAN_PANID, IEEE802154_PAN_ID_LEN,
+		    &pan->coord->pan_id))
+		goto nla_put_failure;
+
+	if (pan->coord->mode == IEEE802154_ADDR_SHORT) {
+		if (nla_put(msg, NL802154_PAN_COORD_ADDR,
+			    IEEE802154_SHORT_ADDR_LEN,
+			    &pan->coord->short_addr))
+			goto nla_put_failure;
+	} else {
+		if (nla_put(msg, NL802154_PAN_COORD_ADDR,
+			    IEEE802154_EXTENDED_ADDR_LEN,
+			    &pan->coord->extended_addr))
+			goto nla_put_failure;
+	}
+
+	if (nla_put_u8(msg, NL802154_PAN_CHANNEL, pan->channel))
+		goto nla_put_failure;
+
+	if (nla_put_u8(msg, NL802154_PAN_PAGE, pan->page))
+		goto nla_put_failure;
+
+	if (nla_put_u16(msg, NL802154_PAN_SUPERFRAME_SPEC,
+			pan->superframe_spec))
+		goto nla_put_failure;
+
+	if (nla_put_u8(msg, NL802154_PAN_LINK_QUALITY, pan->link_quality))
+		goto nla_put_failure;
+
+	if (nla_put_u32(msg, NL802154_PAN_SEEN_MS_AGO,
+			jiffies_to_msecs(jiffies - intpan->discovery_ts)))
+		goto nla_put_failure;
+
+	if (pan->gts_permit && nla_put_flag(msg, NL802154_PAN_GTS_PERMIT))
+		goto nla_put_failure;
+
+	/* TODO: NL802154_PAN_PAYLOAD_DATA if any */
+
+	nla_nest_end(msg, nla);
+	genlmsg_end(msg, hdr);
+
+	return 0;
+
+ nla_put_failure:
+	genlmsg_cancel(msg, hdr);
+	return -EMSGSIZE;
+}
+
+static int nl802154_dump_pans(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	struct cfg802154_registered_device *rdev;
+	struct cfg802154_internal_pan *pan;
+	struct wpan_dev *wpan_dev;
+	int err;
+
+	err = nl802154_prepare_wpan_dev_dump(skb, cb, &rdev, &wpan_dev);
+	if (err)
+		return err;
+
+	spin_lock_bh(&rdev->pan_lock);
+
+	if (cb->args[2])
+		goto out;
+
+	cb->seq = rdev->pan_generation;
+
+	ieee802154_for_each_pan(pan, rdev) {
+		err = nl802154_send_pan_info(skb, cb, cb->nlh->nlmsg_seq,
+					     NLM_F_MULTI, rdev, wpan_dev, pan);
+		if (err < 0)
+			goto out_err;
+	}
+
+	cb->args[2] = 1;
+out:
+	err = skb->len;
+out_err:
+	spin_unlock_bh(&rdev->pan_lock);
+
+	nl802154_finish_wpan_dev_dump(rdev);
+
+	return err;
+}
+
+static int nl802154_flush_pans(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg802154_registered_device *rdev = info->user_ptr[0];
+
+	spin_lock_bh(&rdev->pan_lock);
+	cfg802154_flush_pans(rdev);
+	spin_unlock_bh(&rdev->pan_lock);
+
+	return 0;
+}
+
 #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 },
@@ -2603,6 +2734,18 @@ static const struct genl_ops nl802154_ops[] = {
 				  NL802154_FLAG_CHECK_NETDEV_UP |
 				  NL802154_FLAG_NEED_RTNL,
 	},
+	{
+		.cmd = NL802154_CMD_DUMP_PANS,
+		.dumpit = nl802154_dump_pans,
+		/* can be retrieved by unprivileged users */
+	},
+	{
+		.cmd = NL802154_CMD_FLUSH_PANS,
+		.doit = nl802154_flush_pans,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL802154_FLAG_NEED_NETDEV |
+				  NL802154_FLAG_NEED_RTNL,
+	},
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	{
 		.cmd = NL802154_CMD_SET_SEC_PARAMS,
-- 
2.27.0

[net-next 15/18] net: ieee802154: Add support for beacon requests

[Miquel Raynal]

This involves processing beacons requests: starting or stopping
the flow of beacons sent passively on a specific interface. The page and
channel must be changed beforehands if needed. Interval orders above 14
are reserved to tell a device it must answer BEACON_REQ coming from an
active scan procedure (this is not supported yet).

A beacons request structure is created to list the requirements.

Mac layers may now implement the ->send_beacons() and
->stop_beacons() 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/net/cfg802154.h   | 22 +++++++++++
 include/net/nl802154.h    |  3 ++
 net/ieee802154/nl802154.c | 81 +++++++++++++++++++++++++++++++++++++++
 net/ieee802154/rdev-ops.h | 24 ++++++++++++
 net/ieee802154/trace.h    | 21 ++++++++++
 5 files changed, 151 insertions(+)

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 292eaf280f01..e4132bd2b636 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -88,6 +88,24 @@ struct cfg802154_scan_request {
 	struct wpan_phy *wpan_phy;
 };
 
+/**
+ * struct cfg802154_beacons_request - beacons request descriptor
+ *
+ * @interval: interval n between sendings, in multiple order of the super frame
+ *            duration: aBaseSuperframeDuration * (2^n) unless the interval
+ *            order is greater or equal to 15, in this case beacons won't be
+ *            passively sent out at a fixed rate but instead inform the device
+ *            that it should answer beacon requests as part of active scan
+ *            procedures
+ * @wpan_dev: the concerned wpan device
+ * @wpan_phy: the wpan phy this was for
+ */
+struct cfg802154_beacons_request {
+	u8 interval;
+	struct wpan_dev *wpan_dev;
+	struct wpan_phy *wpan_phy;
+};
+
 struct cfg802154_ops {
 	struct net_device * (*add_virtual_intf_deprecated)(struct wpan_phy *wpan_phy,
 							   const char *name,
@@ -130,6 +148,10 @@ struct cfg802154_ops {
 				struct cfg802154_scan_request *request);
 	int	(*abort_scan)(struct wpan_phy *wpan_phy,
 			      struct wpan_dev *wpan_dev);
+	int	(*send_beacons)(struct wpan_phy *wpan_phy,
+				struct cfg802154_beacons_request *request);
+	int	(*stop_beacons)(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,
diff --git a/include/net/nl802154.h b/include/net/nl802154.h
index e185e92d29d6..a20d19b6d0d4 100644
--- a/include/net/nl802154.h
+++ b/include/net/nl802154.h
@@ -63,6 +63,8 @@ enum nl802154_commands {
 	NL802154_CMD_SCAN_DONE,
 	NL802154_CMD_DUMP_PANS,
 	NL802154_CMD_FLUSH_PANS,
+	NL802154_CMD_SEND_BEACONS,
+	NL802154_CMD_STOP_BEACONS,
 
 	/* add new commands above here */
 
@@ -144,6 +146,7 @@ enum nl802154_attrs {
 	NL802154_ATTR_SCAN_CHANNELS,
 	NL802154_ATTR_SCAN_DURATION,
 	NL802154_ATTR_PAN,
+	NL802154_ATTR_BEACON_INTERVAL,
 
 	/* add attributes here, update the policy in nl802154.c */
 
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 8de8f7c31bfe..11112b515a82 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -224,6 +224,7 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
 	[NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32, },
 	[NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8, },
 	[NL802154_ATTR_PAN] = { .type = NLA_NESTED },
+	[NL802154_ATTR_BEACON_INTERVAL] = { .type = NLA_U8, },
 
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	[NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
@@ -1630,6 +1631,70 @@ static int nl802154_flush_pans(struct sk_buff *skb, struct genl_info *info)
 	return 0;
 }
 
+static int
+nl802154_send_beacons(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_beacons_request *request;
+	int err;
+
+	/* Avoid sending beacons on monitor interfaces */
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+		return -EOPNOTSUPP;
+
+	request = kzalloc(sizeof(*request), GFP_KERNEL);
+	if (!request)
+		return -ENOMEM;
+
+	request->wpan_dev = wpan_dev;
+	request->wpan_phy = wpan_phy;
+
+	if (info->attrs[NL802154_ATTR_BEACON_INTERVAL]) {
+		request->interval = nla_get_u8(info->attrs[NL802154_ATTR_BEACON_INTERVAL]);
+		if (request->interval > IEEE802154_MAX_SCAN_DURATION) {
+			pr_err("Answering active scan requests is not supported yet\n");
+			err = -EINVAL;
+			goto free_request;
+		}
+	} else {
+		/* Use maximum duration order by default */
+		request->interval = IEEE802154_MAX_SCAN_DURATION;
+	}
+
+	err = rdev_send_beacons(rdev, request);
+	if (err) {
+		pr_err("Failure starting sending beacons (%d)\n", err);
+		goto free_request;
+	}
+
+	if (wpan_dev->netdev)
+		dev_hold(wpan_dev->netdev);
+
+free_request:
+	kfree(request);
+
+	return err;
+}
+
+static int
+nl802154_stop_beacons(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;
+	int err;
+
+	err = rdev_stop_beacons(rdev, wpan_dev);
+
+	if (err != -ESRCH && wpan_dev->netdev)
+		dev_put(wpan_dev->netdev);
+
+	return err;
+}
+
 #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 },
@@ -2746,6 +2811,22 @@ static const struct genl_ops nl802154_ops[] = {
 		.internal_flags = NL802154_FLAG_NEED_NETDEV |
 				  NL802154_FLAG_NEED_RTNL,
 	},
+	{
+		.cmd = NL802154_CMD_SEND_BEACONS,
+		.doit = nl802154_send_beacons,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL802154_FLAG_NEED_NETDEV |
+				  NL802154_FLAG_CHECK_NETDEV_UP |
+				  NL802154_FLAG_NEED_RTNL,
+	},
+	{
+		.cmd = NL802154_CMD_STOP_BEACONS,
+		.doit = nl802154_stop_beacons,
+		.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/rdev-ops.h b/net/ieee802154/rdev-ops.h
index e171d74c3251..fa85efeaa150 100644
--- a/net/ieee802154/rdev-ops.h
+++ b/net/ieee802154/rdev-ops.h
@@ -237,6 +237,30 @@ static inline int rdev_abort_scan(struct cfg802154_registered_device *rdev,
 	return ret;
 }
 
+static inline int rdev_send_beacons(struct cfg802154_registered_device *rdev,
+				    struct cfg802154_beacons_request *request)
+{
+	int ret;
+
+	/* TODO: check if this is an FFD? */
+
+	trace_802154_rdev_send_beacons(&rdev->wpan_phy, request);
+	ret = rdev->ops->send_beacons(&rdev->wpan_phy, request);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
+}
+
+static inline int rdev_stop_beacons(struct cfg802154_registered_device *rdev,
+				    struct wpan_dev *wpan_dev)
+{
+	int ret;
+
+	trace_802154_rdev_stop_beacons(&rdev->wpan_phy, wpan_dev);
+	ret = rdev->ops->stop_beacons(&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 e5405f737ded..353ba799244f 100644
--- a/net/ieee802154/trace.h
+++ b/net/ieee802154/trace.h
@@ -315,6 +315,22 @@ TRACE_EVENT(802154_rdev_trigger_scan,
 		  WPAN_PHY_PR_ARG, __entry->page, __entry->channels, __entry->duration)
 );
 
+TRACE_EVENT(802154_rdev_send_beacons,
+	TP_PROTO(struct wpan_phy *wpan_phy,
+		 struct cfg802154_beacons_request *request),
+	TP_ARGS(wpan_phy, request),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		__field(u8, interval)
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		__entry->interval = request->interval;
+	),
+	TP_printk(WPAN_PHY_PR_FMT ", sending beacons (interval order: %d)",
+		  WPAN_PHY_PR_ARG, __entry->interval)
+);
+
 DECLARE_EVENT_CLASS(802154_wdev_template,
 	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev),
 	TP_ARGS(wpan_phy, wpan_dev),
@@ -335,6 +351,11 @@ DEFINE_EVENT(802154_wdev_template, 802154_rdev_abort_scan,
 	TP_ARGS(wpan_phy, wpan_dev)
 );
 
+DEFINE_EVENT(802154_wdev_template, 802154_rdev_stop_beacons,
+	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),
-- 
2.27.0

[net-next 16/18] net: mac802154: Handle beacons requests

[Miquel Raynal]

Implement the core hooks in order to provide the softMAC layer support
for sending beacons. Besides being able to test the full passive
scanning procedure, this will also be used when defining PAN coordinator
status, in order to give a device the right to answer received
BEACON_REQ.

Changing the channels is prohibited while a beacon operation is
ongoing.

The implementation uses a workqueue triggered at a certain interval
depending on the symbol duration for the current channel and the
interval order provided.

Sending beacons in response to an active scan request is not
yet supported.

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/net/ieee802154_netdev.h |  24 ++++++++
 net/ieee802154/header_ops.c     |  29 +++++++++
 net/mac802154/cfg.c             |  41 ++++++++++++-
 net/mac802154/ieee802154_i.h    |  13 ++++
 net/mac802154/main.c            |   2 +
 net/mac802154/scan.c            | 102 ++++++++++++++++++++++++++++++++
 6 files changed, 209 insertions(+), 2 deletions(-)

diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h
index 04738ae3b25e..042f0196cced 100644
--- a/include/net/ieee802154_netdev.h
+++ b/include/net/ieee802154_netdev.h
@@ -116,6 +116,21 @@ enum ieee802154_frame_type {
 	IEEE802154_EXTENDED_FRAME,
 };
 
+enum ieee802154_frame_version {
+	IEEE802154_2003_STD,
+	IEEE802154_2006_STD,
+	IEEE802154_STD,
+	IEEE802154_RESERVED_STD,
+	IEEE802154_MULTIPURPOSE_STD = IEEE802154_2003_STD,
+};
+
+enum ieee802154_addressing_mode {
+	IEEE802154_NO_ADDRESSING,
+	IEEE802154_RESERVED,
+	IEEE802154_SHORT_ADDRESSING,
+	IEEE802154_EXTENDED_ADDRESSING,
+};
+
 struct ieee802154_hdr {
 	struct ieee802154_hdr_fc fc;
 	u8 seq;
@@ -124,6 +139,11 @@ struct ieee802154_hdr {
 	struct ieee802154_sechdr sec;
 };
 
+struct ieee802154_beacon_frame {
+	struct ieee802154_hdr mhr;
+	struct ieee802154_beaconhdr mac_pl;
+};
+
 /* pushes hdr onto the skb. fields of hdr->fc that can be calculated from
  * the contents of hdr will be, and the actual value of those bits in
  * hdr->fc will be ignored. this includes the INTRA_PAN bit and the frame
@@ -149,6 +169,10 @@ int ieee802154_hdr_peek_addrs(const struct sk_buff *skb,
  */
 int ieee802154_hdr_peek(const struct sk_buff *skb, struct ieee802154_hdr *hdr);
 
+/* pushes a beacon frame into an skb */
+int ieee802154_beacon_push(struct sk_buff *skb,
+			   struct ieee802154_beacon_frame *beacon);
+
 int ieee802154_max_payload(const struct ieee802154_hdr *hdr);
 
 static inline int
diff --git a/net/ieee802154/header_ops.c b/net/ieee802154/header_ops.c
index af337cf62764..5b8d67169312 100644
--- a/net/ieee802154/header_ops.c
+++ b/net/ieee802154/header_ops.c
@@ -6,6 +6,7 @@
  * Phoebe Buckheister <phoebe.buckheister@itwm.fraunhofer.de>
  */
 
+#include <linux/crc-ccitt.h>
 #include <linux/ieee802154.h>
 
 #include <net/mac802154.h>
@@ -120,6 +121,34 @@ ieee802154_hdr_push(struct sk_buff *skb, struct ieee802154_hdr *hdr)
 }
 EXPORT_SYMBOL_GPL(ieee802154_hdr_push);
 
+int ieee802154_beacon_push(struct sk_buff *skb,
+			   struct ieee802154_beacon_frame *beacon)
+{
+	struct ieee802154_beaconhdr *mac_pl = &beacon->mac_pl;
+	struct ieee802154_hdr *mhr = &beacon->mhr;
+	u16 crc;
+	int ret;
+
+	skb_reserve(skb, sizeof(*mhr));
+	ret = ieee802154_hdr_push(skb, mhr);
+	if (ret < 0)
+		return ret;
+
+	skb_reset_mac_header(skb);
+	skb->mac_len = ret;
+
+	skb_put_data(skb, mac_pl, sizeof(*mac_pl));
+
+	if (mac_pl->pend_short_addr_count || mac_pl->pend_ext_addr_count)
+		return -EOPNOTSUPP;
+
+	crc = crc_ccitt(0, skb->data, skb->len);
+	put_unaligned_le16(crc, skb_put(skb, 2));
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ieee802154_beacon_push);
+
 static int
 ieee802154_hdr_get_addr(const u8 *buf, int mode, bool omit_pan,
 			struct ieee802154_addr *addr)
diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c
index 5c19d6f8e3eb..87be6969ca65 100644
--- a/net/mac802154/cfg.c
+++ b/net/mac802154/cfg.c
@@ -114,8 +114,10 @@ ieee802154_set_channel(struct wpan_phy *wpan_phy, u8 page, u8 channel)
 	    wpan_phy->current_channel == channel)
 		return 0;
 
-	/* Refuse to change channels during a scanning operation */
-	if (local->scanning)
+	/* Refuse to change channels during a scanning operation or when a
+	 * beacons request is ongoing.
+	 */
+	if (local->scanning || local->ongoing_beacons_request)
 		return -EBUSY;
 
 	ret = drv_set_channel(local, page, channel);
@@ -297,6 +299,39 @@ static int mac802154_abort_scan(struct wpan_phy *wpan_phy,
 	return ret;
 }
 
+static int mac802154_send_beacons(struct wpan_phy *wpan_phy,
+				  struct cfg802154_beacons_request *request)
+{
+	struct ieee802154_local *local = wpan_phy_priv(wpan_phy);
+	struct ieee802154_sub_if_data *sdata;
+	int ret;
+
+	sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(request->wpan_dev);
+
+	ASSERT_RTNL();
+
+	mutex_lock(&local->beacons_lock);
+	ret = mac802154_send_beacons_locked(sdata, request);
+	mutex_unlock(&local->beacons_lock);
+
+	return ret;
+}
+
+static int mac802154_stop_beacons(struct wpan_phy *wpan_phy,
+				  struct wpan_dev *wpan_dev)
+{
+	struct ieee802154_local *local = wpan_phy_priv(wpan_phy);
+	int ret;
+
+	ASSERT_RTNL();
+
+	mutex_lock(&local->beacons_lock);
+	ret = mac802154_stop_beacons_locked(local);
+	mutex_unlock(&local->beacons_lock);
+
+	return ret;
+}
+
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 static void
 ieee802154_get_llsec_table(struct wpan_phy *wpan_phy,
@@ -506,6 +541,8 @@ const struct cfg802154_ops mac802154_config_ops = {
 	.set_ackreq_default = ieee802154_set_ackreq_default,
 	.trigger_scan = mac802154_trigger_scan,
 	.abort_scan = mac802154_abort_scan,
+	.send_beacons = mac802154_send_beacons,
+	.stop_beacons = mac802154_stop_beacons,
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	.get_llsec_table = ieee802154_get_llsec_table,
 	.lock_llsec_table = ieee802154_lock_llsec_table,
diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h
index 4945edf5c2ce..9d7bd0694ec7 100644
--- a/net/mac802154/ieee802154_i.h
+++ b/net/mac802154/ieee802154_i.h
@@ -57,6 +57,14 @@ struct ieee802154_local {
 	struct ieee802154_sub_if_data __rcu *scan_sdata;
 	struct delayed_work scan_work;
 
+	/* Beacons handling */
+	bool ongoing_beacons_request;
+	struct mutex beacons_lock;
+	unsigned int beacons_interval;
+	struct delayed_work beacons_work;
+	struct ieee802154_sub_if_data __rcu *beacons_sdata;
+	struct ieee802154_beacon_frame beacon;
+
 	bool started;
 	bool suspended;
 
@@ -181,6 +189,11 @@ int mac802154_trigger_scan_locked(struct ieee802154_sub_if_data *sdata,
 				  struct cfg802154_scan_request *request);
 int mac802154_abort_scan_locked(struct ieee802154_local *local);
 int mac802154_scan_rx(struct ieee802154_local *local, struct sk_buff *skb);
+void mac802154_beacons_work(struct work_struct *work);
+int mac802154_send_beacons_locked(struct ieee802154_sub_if_data *sdata,
+				  struct cfg802154_beacons_request *request);
+int mac802154_stop_beacons_locked(struct ieee802154_local *local);
+
 /* interface handling */
 int ieee802154_iface_init(void);
 void ieee802154_iface_exit(void);
diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index 568991734610..f831a1c8d885 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -91,6 +91,7 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
 	INIT_LIST_HEAD(&local->interfaces);
 	mutex_init(&local->iflist_mtx);
 	mutex_init(&local->scan_lock);
+	mutex_init(&local->beacons_lock);
 
 	tasklet_setup(&local->tasklet, ieee802154_tasklet_handler);
 
@@ -98,6 +99,7 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
 
 	INIT_WORK(&local->tx_work, ieee802154_xmit_worker);
 	INIT_DELAYED_WORK(&local->scan_work, mac802154_scan_work);
+	INIT_DELAYED_WORK(&local->beacons_work, mac802154_beacons_work);
 
 	/* init supported flags with 802.15.4 default ranges */
 	phy->supported.max_minbe = 8;
diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
index 1382489d4e58..b334fa856c00 100644
--- a/net/mac802154/scan.c
+++ b/net/mac802154/scan.c
@@ -104,6 +104,64 @@ static unsigned int mac802154_scan_get_channel_time(u8 duration_order,
 				(BIT(duration_order) + 1));
 }
 
+int mac802154_send_beacons_locked(struct ieee802154_sub_if_data *sdata,
+				  struct cfg802154_beacons_request *request)
+{
+	struct ieee802154_local *local = sdata->local;
+	unsigned int interval;
+
+	lockdep_assert_held(&local->beacons_lock);
+
+	if (local->ongoing_beacons_request)
+		return -EBUSY;
+
+	local->ongoing_beacons_request = true;
+
+	interval = mac802154_scan_get_channel_time(request->interval,
+						   request->wpan_phy->symbol_duration);
+
+	memset(&local->beacon, 0, sizeof(local->beacon));
+	local->beacon.mhr.fc.type = IEEE802154_BEACON_FRAME;
+	local->beacon.mhr.fc.security_enabled = 0;
+	local->beacon.mhr.fc.frame_pending = 0;
+	local->beacon.mhr.fc.ack_request = 0;
+	local->beacon.mhr.fc.intra_pan = 0;
+	local->beacon.mhr.fc.dest_addr_mode = IEEE802154_NO_ADDRESSING;
+	local->beacon.mhr.fc.version = IEEE802154_2003_STD;
+	local->beacon.mhr.fc.source_addr_mode = IEEE802154_EXTENDED_ADDRESSING;
+	atomic_set(&request->wpan_dev->bsn, -1);
+	local->beacon.mhr.source.mode = IEEE802154_ADDR_LONG;
+	local->beacon.mhr.source.pan_id = request->wpan_dev->pan_id;
+	local->beacon.mhr.source.extended_addr = request->wpan_dev->extended_addr;
+	local->beacon.mac_pl.beacon_order = request->interval;
+	local->beacon.mac_pl.superframe_order = request->interval;
+	local->beacon.mac_pl.final_cap_slot = 0xf;
+	local->beacon.mac_pl.battery_life_ext = 0;
+	local->beacon.mac_pl.pan_coordinator = 1;
+	local->beacon.mac_pl.assoc_permit = 1;
+
+	rcu_assign_pointer(local->beacons_sdata, sdata);
+	local->beacons_interval = interval;
+
+	/* Start the beacon work */
+	ieee802154_queue_delayed_work(&local->hw, &local->beacons_work, 0);
+
+	return 0;
+}
+
+int mac802154_stop_beacons_locked(struct ieee802154_local *local)
+{
+	lockdep_assert_held(&local->beacons_lock);
+
+	if (!local->ongoing_beacons_request)
+		return -ESRCH;
+
+	local->ongoing_beacons_request = false;
+	cancel_delayed_work(&local->beacons_work);
+
+	return 0;
+}
+
 void mac802154_scan_work(struct work_struct *work)
 {
 	struct ieee802154_local *local =
@@ -206,6 +264,50 @@ int mac802154_trigger_scan_locked(struct ieee802154_sub_if_data *sdata,
 	return 0;
 }
 
+void mac802154_beacons_work(struct work_struct *work)
+{
+	struct ieee802154_local *local =
+		container_of(work, struct ieee802154_local, beacons_work.work);
+	struct ieee802154_sub_if_data *sdata;
+	struct wpan_dev *wpan_dev;
+	struct sk_buff *skb;
+	int ret;
+
+	mutex_lock(&local->beacons_lock);
+
+	if (!local->ongoing_beacons_request)
+		goto unlock_mutex;
+
+	if (local->suspended)
+		goto queue_work;
+
+	sdata = rcu_dereference_protected(local->beacons_sdata,
+					  lockdep_is_held(&local->beacons_lock));
+	wpan_dev = &sdata->wpan_dev;
+
+	/* Update the sequence number */
+	local->beacon.mhr.seq = atomic_inc_return(&wpan_dev->bsn);
+
+	skb = alloc_skb(17 + 2, GFP_KERNEL);
+	if (!skb)
+		goto queue_work;
+
+	ret = ieee802154_beacon_push(skb, &local->beacon);
+	if (ret)
+		goto queue_work;
+
+	ret = drv_xmit_async(local, skb);
+	if (ret)
+		pr_err("Error when transmitting beacon (%d)\n", ret);
+
+queue_work:
+	ieee802154_queue_delayed_work(&local->hw, &local->beacons_work,
+				      local->beacons_interval);
+
+unlock_mutex:
+	mutex_unlock(&local->beacons_lock);
+}
+
 int mac802154_scan_rx(struct ieee802154_local *local, struct sk_buff *skb)
 {
 	struct ieee802154_beaconhdr *bh = (void *)skb->data;
-- 
2.27.0

[net-next 17/18] net: mac802154: Let drivers provide their own beacons implementation

[Miquel Raynal]

So far only a pure software procedure for sending beacons was possible.
Let's create a couple of driver's hooks in order to allow the device
drivers to provide their own implementation. If not provided, fallback
to the pure software logic.

It is possible for device drivers to only support a specific type of
request and return -EOPNOTSUPP otherwise, this will have the same effect
as not providing any hooks for these specific cases.

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/net/mac802154.h    | 13 +++++++++++++
 net/mac802154/driver-ops.h | 33 +++++++++++++++++++++++++++++++++
 net/mac802154/scan.c       | 17 +++++++++++++++++
 net/mac802154/trace.h      | 21 +++++++++++++++++++++
 4 files changed, 84 insertions(+)

diff --git a/include/net/mac802154.h b/include/net/mac802154.h
index 97aefba7bf96..72978fb72a3a 100644
--- a/include/net/mac802154.h
+++ b/include/net/mac802154.h
@@ -214,6 +214,16 @@ enum ieee802154_hw_flags {
  *	  Exits the scan mode and returns to a fully functioning state.
  *	  Should only be provided if ->enter_scan_mode() is populated.
  *	  Returns either zero, or negative errno.
+ *
+ * send_beacons
+ *	  Send beacons at a fixed rate over the current channel.
+ *	  Can be NULL, if the driver doesn't support sending beacons by itself.
+ *	  Returns either zero, or negative errno.
+ *
+ * stop_beacons
+ *	  Stops sending beacons.
+ *	  Should only be provided if ->send_beacons() is populated.
+ *	  Returns either zero, or negative errno.
  */
 struct ieee802154_ops {
 	struct module	*owner;
@@ -243,6 +253,9 @@ struct ieee802154_ops {
 	int		(*enter_scan_mode)(struct ieee802154_hw *hw,
 					   struct cfg802154_scan_request *request);
 	int		(*exit_scan_mode)(struct ieee802154_hw *hw);
+	int		(*send_beacons)(struct ieee802154_hw *hw,
+					struct cfg802154_beacons_request *request);
+	int		(*stop_beacons)(struct ieee802154_hw *hw);
 };
 
 /**
diff --git a/net/mac802154/driver-ops.h b/net/mac802154/driver-ops.h
index 2f5650f7bf91..003e6edee049 100644
--- a/net/mac802154/driver-ops.h
+++ b/net/mac802154/driver-ops.h
@@ -315,4 +315,37 @@ static inline int drv_exit_scan_mode(struct ieee802154_local *local)
 	return ret;
 }
 
+static inline int drv_send_beacons(struct ieee802154_local *local,
+				   struct cfg802154_beacons_request *request)
+{
+	int ret;
+
+	might_sleep();
+
+	if (!local->ops->send_beacons || !local->ops->stop_beacons)
+		return -EOPNOTSUPP;
+
+	trace_802154_drv_send_beacons(local, request);
+	ret = local->ops->send_beacons(&local->hw, request);
+	trace_802154_drv_return_int(local, ret);
+
+	return ret;
+}
+
+static inline int drv_stop_beacons(struct ieee802154_local *local)
+{
+	int ret;
+
+	might_sleep();
+
+	if (!local->ops->send_beacons || !local->ops->stop_beacons)
+		return -EOPNOTSUPP;
+
+	trace_802154_drv_stop_beacons(local);
+	ret = local->ops->stop_beacons(&local->hw);
+	trace_802154_drv_return_int(local, ret);
+
+	return ret;
+}
+
 #endif /* __MAC802154_DRIVER_OPS */
diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
index b334fa856c00..55af9d16744a 100644
--- a/net/mac802154/scan.c
+++ b/net/mac802154/scan.c
@@ -109,6 +109,7 @@ int mac802154_send_beacons_locked(struct ieee802154_sub_if_data *sdata,
 {
 	struct ieee802154_local *local = sdata->local;
 	unsigned int interval;
+	int ret;
 
 	lockdep_assert_held(&local->beacons_lock);
 
@@ -117,6 +118,14 @@ int mac802154_send_beacons_locked(struct ieee802154_sub_if_data *sdata,
 
 	local->ongoing_beacons_request = true;
 
+	/* Either let the hardware handle the beacons or handle them manually */
+	ret = drv_send_beacons(local, request);
+	if (ret != -EOPNOTSUPP) {
+		if (ret)
+			local->ongoing_beacons_request = false;
+		return ret;
+	}
+
 	interval = mac802154_scan_get_channel_time(request->interval,
 						   request->wpan_phy->symbol_duration);
 
@@ -151,6 +160,8 @@ int mac802154_send_beacons_locked(struct ieee802154_sub_if_data *sdata,
 
 int mac802154_stop_beacons_locked(struct ieee802154_local *local)
 {
+	int ret;
+
 	lockdep_assert_held(&local->beacons_lock);
 
 	if (!local->ongoing_beacons_request)
@@ -159,6 +170,12 @@ int mac802154_stop_beacons_locked(struct ieee802154_local *local)
 	local->ongoing_beacons_request = false;
 	cancel_delayed_work(&local->beacons_work);
 
+	ret = drv_stop_beacons(local);
+	if (ret != -EOPNOTSUPP)
+		return ret;
+
+	cancel_delayed_work(&local->beacons_work);
+
 	return 0;
 }
 
diff --git a/net/mac802154/trace.h b/net/mac802154/trace.h
index 9c0a4f07ced1..b487523f83c3 100644
--- a/net/mac802154/trace.h
+++ b/net/mac802154/trace.h
@@ -292,6 +292,27 @@ DEFINE_EVENT(local_only_evt4, 802154_drv_exit_scan_mode,
 	TP_ARGS(local)
 );
 
+TRACE_EVENT(802154_drv_send_beacons,
+	TP_PROTO(struct ieee802154_local *local,
+		 struct cfg802154_beacons_request *request),
+	TP_ARGS(local, request),
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+		__field(u8, interval)
+	),
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+		__entry->interval = request->interval;
+	),
+	TP_printk(LOCAL_PR_FMT ", send beacons at interval: %d",
+		  LOCAL_PR_ARG, __entry->interval)
+);
+
+DEFINE_EVENT(local_only_evt4, 802154_drv_stop_beacons,
+	TP_PROTO(struct ieee802154_local *local),
+	TP_ARGS(local)
+);
+
 #endif /* !__MAC802154_DRIVER_TRACE || TRACE_HEADER_MULTI_READ */
 
 #undef TRACE_INCLUDE_PATH
-- 
2.27.0

[net-next 18/18] net: ieee802154: Trace the registration of new PANs

[Miquel Raynal]

From: David Girault <david.girault@qorvo.com>

Add an internal trace when new PANs get discovered.

Signed-off-by: David Girault <david.girault@qorvo.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 net/ieee802154/pan.c   |  3 +++
 net/ieee802154/trace.h | 25 +++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/net/ieee802154/pan.c b/net/ieee802154/pan.c
index c71a3664d5c3..451576d9ab41 100644
--- a/net/ieee802154/pan.c
+++ b/net/ieee802154/pan.c
@@ -18,6 +18,7 @@
 
 #include "ieee802154.h"
 #include "core.h"
+#include "trace.h"
 
 /* Maximum number of PAN entries to store */
 static int max_pan_entries = 100;
@@ -182,6 +183,8 @@ static void cfg802154_pan_update(struct cfg802154_registered_device *rdev,
 	found = cfg802154_find_matching_pan(rdev, new);
 	if (found)
 		cfg802154_unlink_pan(rdev, found);
+	else
+		trace_802154_new_pan(&new->desc);
 
 	if (unlikely(rdev->pan_entries >= max_pan_entries))
 		cfg802154_expire_oldest_pan(rdev);
diff --git a/net/ieee802154/trace.h b/net/ieee802154/trace.h
index 353ba799244f..506fe4930440 100644
--- a/net/ieee802154/trace.h
+++ b/net/ieee802154/trace.h
@@ -356,6 +356,31 @@ DEFINE_EVENT(802154_wdev_template, 802154_rdev_stop_beacons,
 	TP_ARGS(wpan_phy, wpan_dev)
 );
 
+DECLARE_EVENT_CLASS(802154_pan_evt,
+	TP_PROTO(struct ieee802154_pan_desc *desc),
+	TP_ARGS(desc),
+	TP_STRUCT__entry(
+		__field(u16, pan_id)
+		__field(__le64, coord_addr)
+		__field(u8, channel)
+		__field(u8, page)
+	),
+	TP_fast_assign(
+		__entry->page = desc->page;
+		__entry->channel = desc->channel;
+		memcpy(&__entry->pan_id, &desc->coord->pan_id, 2);
+		memcpy(&__entry->coord_addr, &desc->coord->extended_addr, 8);
+	),
+	TP_printk("panid: %u, coord_addr: 0x%llx, page: %u, channel: %u",
+		  __entry->pan_id, __le64_to_cpu(__entry->coord_addr),
+		  __entry->page, __entry->channel)
+);
+
+DEFINE_EVENT(802154_pan_evt, 802154_new_pan,
+	TP_PROTO(struct ieee802154_pan_desc *desc),
+	TP_ARGS(desc)
+);
+
 TRACE_EVENT(802154_rdev_return_int,
 	TP_PROTO(struct wpan_phy *wpan_phy, int ret),
 	TP_ARGS(wpan_phy, ret),
-- 
2.27.0

Re: [net-next 08/18] net: ieee802154: Add support for internal PAN management

[Jakub Kicinski]

On Wed, 22 Dec 2021 16:57:33 +0100 Miquel Raynal wrote:
> +/* Maximum number of PAN entries to store */
> +static int max_pan_entries = 100;
> +module_param(max_pan_entries, uint, 0644);
> +MODULE_PARM_DESC(max_pan_entries,
> +		 "Maximum number of PANs to discover per scan (default is 100)");
> +
> +static int pan_expiration = 60;
> +module_param(pan_expiration, uint, 0644);
> +MODULE_PARM_DESC(pan_expiration,
> +		 "Expiration of the scan validity in seconds (default is 60s)");

Can these be per-device control knobs? Module params are rarely the
best answer.

Re: [net-next 01/18] ieee802154: hwsim: Ensure proper channel selection at probe time

[Alexander Aring]

Hi,

On Wed, 22 Dec 2021 at 10:57, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> A default channel is selected by default (13), let's clarify that this
> is page 0 channel 13. Call the right helper to ensure the necessary
> configuration for this channel has been applied.
>
> So far there is very little configuration done in this helper but we
> will soon add more information (like the symbol duration which is
> missing) and having this helper called at probe time will prevent us to
> this type of initialization at two different locations.
>

I see why this patch is necessary because in later patches the symbol
duration is set at ".set_channel()" callback like the at86rf230 driver
is doing it.
However there is an old TODO [0]. I think we should combine it and
implement it in ieee802154_set_channel() of "net/mac802154/cfg.c".
Also do the symbol duration setting according to the channel/page when
we call ieee802154_register_hw(), so we have it for the default
settings.

> So far there is very little configuration done in this helper but thanks
> to this improvement, future enhancements in this area (like setting a
> symbol duration, which is missing) will be reflected automatically in
> the default probe state.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/net/ieee802154/mac802154_hwsim.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
> index 62ced7a30d92..b1a4ee7dceda 100644
> --- a/drivers/net/ieee802154/mac802154_hwsim.c
> +++ b/drivers/net/ieee802154/mac802154_hwsim.c
> @@ -778,8 +778,6 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
>
>         ieee802154_random_extended_addr(&hw->phy->perm_extended_addr);
>
> -       /* hwsim phy channel 13 as default */
> -       hw->phy->current_channel = 13;
>         pib = kzalloc(sizeof(*pib), GFP_KERNEL);
>         if (!pib) {
>                 err = -ENOMEM;
> @@ -793,6 +791,11 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
>         hw->flags = IEEE802154_HW_PROMISCUOUS | IEEE802154_HW_RX_DROP_BAD_CKSUM;

sadly this patch doesn't apply on current net-next/master because
IEEE802154_HW_RX_DROP_BAD_CKSUM is not set.
I agree that it should be set, so we need a patch for it.

- Alex

[0] https://elixir.bootlin.com/linux/v5.16-rc7/source/drivers/net/ieee802154/at86rf230.c#L1059

Re: [net-next 08/18] net: ieee802154: Add support for internal PAN management

[Alexander Aring]

Hi,

On Wed, 22 Dec 2021 at 10:57, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Let's introduce the basics of PAN management:
> - structures defining PANs
> - helpers for PANs registration
> - helpers discarding old PANs
>

I think there exists a little misunderstanding about how the
architecture is between the structures wpan_phy, wpan_dev and
cfg802154.

 - wpan_phy: represents the PHY layer of IEEE 802154 and is a
registered device class.
 - wpan_dev: represents the MAC layer of IEEE 802154 and is a netdev interface.

You can have multiple wpan_dev operate on one wpan_phy. To my best
knowledge it's like having multiple access points running on one phy
(wireless) or macvlan on ethernet. You can actually do that with the
mac802154_hwsim driver. However as there exists currently no (as my
knowledge) hardware which supports e.g. multiple address filters we
wanted to be prepared for to support such handling. Although, there
exists some transceivers which support something like a "pan bridge"
which goes into such a direction.

What is a cfg802154 registered device? Well, at first it offers an
interface between SoftMAC and HardMAC from nl802154, that's the
cfg802154_ops structure. In theory a HardMAC transceiver would bypass
the SoftMAC stack by implementing "cfg802154_ops" on the driver layer
and try to do everything there as much as possible to support it. It
is not a registered device class but the instance is tight to a
wpan_phy. There can be multiple wpan_dev's (MAC layer instances on a
phy/cfg802154 registered device). We currently don't support a HardMAC
transceiver and I think because this misunderstanding came up.

That means as far I see you should move the most of those attributes
to per wpan_dev instead of per cfg802154.

- Alex

Re: [net-next 17/18] net: mac802154: Let drivers provide their own beacons implementation

[Alexander Aring]

Hi,

On Wed, 22 Dec 2021 at 10:58, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> So far only a pure software procedure for sending beacons was possible.
> Let's create a couple of driver's hooks in order to allow the device
> drivers to provide their own implementation. If not provided, fallback
> to the pure software logic.
>

Can you name a SoftMAC transceiver which provides such an "offload" feature?

- Alex

Re: [net-next 08/18] net: ieee802154: Add support for internal PAN management

[Alexander Aring]

Hi,

On Tue, 28 Dec 2021 at 17:22, Alexander Aring <alex.aring@gmail.com> wrote:
...
> That means as far I see you should move the most of those attributes
> to per wpan_dev instead of per cfg802154.

Sorry that's wrong.

I see now, that the result for a scan on every possible wpan_dev for a
specific wpan_phy should return the same result, that's why it belongs
to cfg802154 and this is correct (as a cfg802154 has a 1:1 mapping to
wpan_phy).
Same as in wireless...

- Alex

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Alexander Aring]

Hi,

On Wed, 22 Dec 2021 at 10:58, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
...
> +{
> +       bool promiscuous_on = mac802154_check_promiscuous(local);
> +       int ret;
> +
> +       if ((state && promiscuous_on) || (!state && !promiscuous_on))
> +               return 0;
> +
> +       ret = drv_set_promiscuous_mode(local, state);
> +       if (ret)
> +               pr_err("Failed to %s promiscuous mode for SW scanning",
> +                      state ? "set" : "reset");
> +

The semantic of promiscuous mode on the driver layer is to turn off
ack response, address filtering and crc checking. Some transceivers
don't allow a more fine tuning on what to enable/disable. I think we
should at least do the checksum checking per software then?

Sure there is a possible tune up for more "powerful" transceivers then...

- Alex

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Nicolas Schodet]

Hi,

* Alexander Aring <alex.aring@gmail.com> [2021-12-29 09:30]:
> Hi,
> On Wed, 22 Dec 2021 at 10:58, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> ...
> > +{
> > +       bool promiscuous_on = mac802154_check_promiscuous(local);
> > +       int ret;
> > +
> > +       if ((state && promiscuous_on) || (!state && !promiscuous_on))
> > +               return 0;
> > +
> > +       ret = drv_set_promiscuous_mode(local, state);
> > +       if (ret)
> > +               pr_err("Failed to %s promiscuous mode for SW scanning",
> > +                      state ? "set" : "reset");
> The semantic of promiscuous mode on the driver layer is to turn off
> ack response, address filtering and crc checking. Some transceivers
> don't allow a more fine tuning on what to enable/disable. I think we
> should at least do the checksum checking per software then?
> Sure there is a possible tune up for more "powerful" transceivers then...

In this case, the driver could change the (flags &
IEEE802154_HW_RX_DROP_BAD_CKSUM) bit dynamically to signal it does not
check the checksum anymore. Would it work?

Nicolas.

RE: [net-next 17/18] net: mac802154: Let drivers provide their own beacons implementation

[David Girault]

Hi Alexander,

At Qorvo, we have developped a SoftMAC driver for our DW3000 chip that will benefit such API.

To be short, beacon sending is controller by our driver to be synchronized chip clock or delayed until 
other operation in progress (a ranging for example).

Regards,
David Girault

________________________________________
De : Alexander Aring <alex.aring@gmail.com>
Envoyé : mardi 28 décembre 2021 23:25
À : Miquel Raynal
Cc : David S. Miller; Jakub Kicinski; open list:NETWORKING [GENERAL]; Stefan Schmidt; linux-wpan - ML; David Girault; Romuald Despres; Frederic Blain; Thomas Petazzoni; kernel list
Objet : Re: [net-next 17/18] net: mac802154: Let drivers provide their own beacons implementation


**This email has been sent from an EXTERNAL source**


Hi,

On Wed, 22 Dec 2021 at 10:58, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> So far only a pure software procedure for sending beacons was possible.
> Let's create a couple of driver's hooks in order to allow the device
> drivers to provide their own implementation. If not provided, fallback
> to the pure software logic.
>

Can you name a SoftMAC transceiver which provides such an "offload" feature?

- Alex

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Alexander Aring]

Hi,

On Wed, 29 Dec 2021 at 09:45, Nicolas Schodet <nico@ni.fr.eu.org> wrote:
>
> Hi,
>
> * Alexander Aring <alex.aring@gmail.com> [2021-12-29 09:30]:
> > Hi,
> > On Wed, 22 Dec 2021 at 10:58, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > ...
> > > +{
> > > +       bool promiscuous_on = mac802154_check_promiscuous(local);
> > > +       int ret;
> > > +
> > > +       if ((state && promiscuous_on) || (!state && !promiscuous_on))
> > > +               return 0;
> > > +
> > > +       ret = drv_set_promiscuous_mode(local, state);
> > > +       if (ret)
> > > +               pr_err("Failed to %s promiscuous mode for SW scanning",
> > > +                      state ? "set" : "reset");
> > The semantic of promiscuous mode on the driver layer is to turn off
> > ack response, address filtering and crc checking. Some transceivers
> > don't allow a more fine tuning on what to enable/disable. I think we
> > should at least do the checksum checking per software then?
> > Sure there is a possible tune up for more "powerful" transceivers then...
>
> In this case, the driver could change the (flags &
> IEEE802154_HW_RX_DROP_BAD_CKSUM) bit dynamically to signal it does not
> check the checksum anymore. Would it work?

I think that would work, although the intention of the hw->flags is to
define what the hardware is supposed to support as not changing those
values dynamically during runtime so mac will care about it. However
we don't expose those flags to the userspace, so far I know. We can
still introduce two separated flags if necessary in future.

Why do we need promiscuous mode at all? Why is it necessary for a
scan? What of "ack response, address filtering and crc checking" you
want to disable and why?

I see that beacons are sent out with
"local->beacon.mhr.fc.dest_addr_mode = IEEE802154_NO_ADDRESSING;"
shouldn't that be a broadcast destination?

- Alex

Re: [net-next 17/18] net: mac802154: Let drivers provide their own beacons implementation

[Alexander Aring]

Hi,

On Thu, 30 Dec 2021 at 12:00, David Girault <David.Girault@qorvo.com> wrote:
>
> Hi Alexander,
>
> At Qorvo, we have developped a SoftMAC driver for our DW3000 chip that will benefit such API.
>
Do you want to bring this driver upstream as well? Currently those
callbacks will be introduced but no user is there.

> To be short, beacon sending is controller by our driver to be synchronized chip clock or delayed until
> other operation in progress (a ranging for example).
>

ok.

- Alex

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Alexander Aring]

Hi,

On Thu, 30 Dec 2021 at 14:47, Alexander Aring <alex.aring@gmail.com> wrote:
>
> Hi,
>
> On Wed, 29 Dec 2021 at 09:45, Nicolas Schodet <nico@ni.fr.eu.org> wrote:
> >
> > Hi,
> >
> > * Alexander Aring <alex.aring@gmail.com> [2021-12-29 09:30]:
> > > Hi,
> > > On Wed, 22 Dec 2021 at 10:58, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > ...
> > > > +{
> > > > +       bool promiscuous_on = mac802154_check_promiscuous(local);
> > > > +       int ret;
> > > > +
> > > > +       if ((state && promiscuous_on) || (!state && !promiscuous_on))
> > > > +               return 0;
> > > > +
> > > > +       ret = drv_set_promiscuous_mode(local, state);
> > > > +       if (ret)
> > > > +               pr_err("Failed to %s promiscuous mode for SW scanning",
> > > > +                      state ? "set" : "reset");
> > > The semantic of promiscuous mode on the driver layer is to turn off
> > > ack response, address filtering and crc checking. Some transceivers
> > > don't allow a more fine tuning on what to enable/disable. I think we
> > > should at least do the checksum checking per software then?
> > > Sure there is a possible tune up for more "powerful" transceivers then...
> >
> > In this case, the driver could change the (flags &
> > IEEE802154_HW_RX_DROP_BAD_CKSUM) bit dynamically to signal it does not
> > check the checksum anymore. Would it work?
>
> I think that would work, although the intention of the hw->flags is to
> define what the hardware is supposed to support as not changing those
> values dynamically during runtime so mac will care about it. However
> we don't expose those flags to the userspace, so far I know. We can
> still introduce two separated flags if necessary in future.
>
> Why do we need promiscuous mode at all? Why is it necessary for a
> scan? What of "ack response, address filtering and crc checking" you
> want to disable and why?
>

I see now why promiscuous mode is necessary here. The actual
promiscuous mode setting for the driver is not the same as promiscuous
mode in 802.15.4 spec. For until now it was there for running a
sniffer device only.
As the 802.15.4 spec defines some "filtering levels" I came up with a
draft so we can define which filtering level should be done on the
hardware.

diff --git a/include/net/mac802154.h b/include/net/mac802154.h
index 72978fb72a3a..3839ed3f8f0d 100644
--- a/include/net/mac802154.h
+++ b/include/net/mac802154.h
@@ -130,6 +130,48 @@ enum ieee802154_hw_flags {
 #define IEEE802154_HW_OMIT_CKSUM       (IEEE802154_HW_TX_OMIT_CKSUM | \
                                         IEEE802154_HW_RX_OMIT_CKSUM)

+/**
+ * enum ieee802154_filter_mode - hardware filter mode that a driver
will pass to
+ *                              pass to mac802154.
+ *
+ * @IEEE802154_FILTER_MODE_0: No MFR filtering at all.
+ *
+ * @IEEE802154_FILTER_MODE_1: IEEE802154_FILTER_MODE_1 with a bad FCS filter.
+ *
+ * @IEEE802154_FILTER_MODE_2: Same as IEEE802154_FILTER_MODE_1, known as
+ *                           802.15.4 promiscuous mode, sets
+ *                           mib.PromiscuousMode.
+ *
+ * @IEEE802154_FILTER_MODE_3_SCAN: Same as IEEE802154_FILTER_MODE_2 without
+ *                                set mib.PromiscuousMode.
+ *
+ * @IEEE802154_FILTER_MODE_3_NO_SCAN:
+ *     IEEE802154_FILTER_MODE_3_SCAN with MFR additional filter on:
+ *
+ *     - No reserved value in frame type
+ *     - No reserved value in frame version
+ *     - Match mib.PanId or broadcast
+ *     - Destination address field:
+ *       - Match mib.ShortAddress or broadcast
+ *       - Match mib.ExtendedAddress or GroupRxMode is true
+ *       - ImplicitBroadcast is true and destination address field/destination
+ *         panid is not included.
+ *       - Device is coordinator only source address present in data
+ *         frame/command frame and source panid matches mib.PanId
+ *       - Device is coordinator only source address present in multipurpose
+ *         frame and destination panid matches macPanId
+ *     - Beacon frames source panid matches mib.PanId. If mib.PanId is
+ *       broadcast it should always be accepted.
+ *
+ */
+enum ieee802154_filter_mode {
+       IEEE802154_FILTER_MODE_0,
+       IEEE802154_FILTER_MODE_1,
+       IEEE802154_FILTER_MODE_2,
+       IEEE802154_FILTER_MODE_3_SCAN,
+       IEEE802154_FILTER_MODE_3_NO_SCAN,
+};
+
 /* struct ieee802154_ops - callbacks from mac802154 to the driver
  *
  * This structure contains various callbacks that the driver may
@@ -249,7 +291,7 @@ struct ieee802154_ops {
        int             (*set_frame_retries)(struct ieee802154_hw *hw,
                                             s8 retries);
        int             (*set_promiscuous_mode)(struct ieee802154_hw *hw,
-                                               const bool on);
+                                               enum
ieee802154_filter_mode mode);
        int             (*enter_scan_mode)(struct ieee802154_hw *hw,
                                           struct
cfg802154_scan_request *request);
        int             (*exit_scan_mode)(struct ieee802154_hw *hw);

---

In your case it will be IEEE802154_FILTER_MODE_3_SCAN mode, for a
sniffer we probably want as default IEEE802154_FILTER_MODE_0, as
"promiscuous mode" currently is.

- Alex

Re: [net-next 08/18] net: ieee802154: Add support for internal PAN management

[Miquel Raynal]

Hi Jakub,

kuba@kernel.org wrote on Wed, 22 Dec 2021 12:55:55 -0800:

> On Wed, 22 Dec 2021 16:57:33 +0100 Miquel Raynal wrote:
> > +/* Maximum number of PAN entries to store */
> > +static int max_pan_entries = 100;
> > +module_param(max_pan_entries, uint, 0644);
> > +MODULE_PARM_DESC(max_pan_entries,
> > +		 "Maximum number of PANs to discover per scan (default is 100)");
> > +
> > +static int pan_expiration = 60;
> > +module_param(pan_expiration, uint, 0644);
> > +MODULE_PARM_DESC(pan_expiration,
> > +		 "Expiration of the scan validity in seconds (default is 60s)");  
> 
> Can these be per-device control knobs? Module params are rarely the
> best answer.

I believe we can do that on a per FFD device basis (for now it will be
on a per-device basis, but later when we will have the necessary
information we might do something more fine grained). Would a couple of
sysfs entries work?

Thanks,
Miquèl

Re: [net-next 08/18] net: ieee802154: Add support for internal PAN management

[Jakub Kicinski]

On Tue, 4 Jan 2022 15:41:51 +0100 Miquel Raynal wrote:
> > On Wed, 22 Dec 2021 16:57:33 +0100 Miquel Raynal wrote:  
> > > +/* Maximum number of PAN entries to store */
> > > +static int max_pan_entries = 100;
> > > +module_param(max_pan_entries, uint, 0644);
> > > +MODULE_PARM_DESC(max_pan_entries,
> > > +		 "Maximum number of PANs to discover per scan (default is 100)");
> > > +
> > > +static int pan_expiration = 60;
> > > +module_param(pan_expiration, uint, 0644);
> > > +MODULE_PARM_DESC(pan_expiration,
> > > +		 "Expiration of the scan validity in seconds (default is 60s)");    
> > 
> > Can these be per-device control knobs? Module params are rarely the
> > best answer.  
> 
> I believe we can do that on a per FFD device basis (for now it will be
> on a per-device basis, but later when we will have the necessary
> information we might do something more fine grained). Would a couple of
> sysfs entries work?

Is there no netlink object where this would fit? Sorry, I'm not at all
familiar with WPAN. If it's orthogonal to current cfg802154 objects
sysfs is fine, I guess.

Re: [net-next 08/18] net: ieee802154: Add support for internal PAN management

[Miquel Raynal]

Hi Alexander,

alex.aring@gmail.com wrote on Tue, 28 Dec 2021 17:22:38 -0500:

> Hi,
> 
> On Wed, 22 Dec 2021 at 10:57, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Let's introduce the basics of PAN management:
> > - structures defining PANs
> > - helpers for PANs registration
> > - helpers discarding old PANs
> >  
> 
> I think there exists a little misunderstanding about how the
> architecture is between the structures wpan_phy, wpan_dev and
> cfg802154.
> 
>  - wpan_phy: represents the PHY layer of IEEE 802154 and is a
> registered device class.
>  - wpan_dev: represents the MAC layer of IEEE 802154 and is a netdev interface.
> 
> You can have multiple wpan_dev operate on one wpan_phy. To my best
> knowledge it's like having multiple access points running on one phy
> (wireless) or macvlan on ethernet. You can actually do that with the
> mac802154_hwsim driver. However as there exists currently no (as my
> knowledge) hardware which supports e.g. multiple address filters we
> wanted to be prepared for to support such handling. Although, there
> exists some transceivers which support something like a "pan bridge"
> which goes into such a direction.
> 
> What is a cfg802154 registered device? Well, at first it offers an
> interface between SoftMAC and HardMAC from nl802154, that's the
> cfg802154_ops structure. In theory a HardMAC transceiver would bypass
> the SoftMAC stack by implementing "cfg802154_ops" on the driver layer
> and try to do everything there as much as possible to support it. It
> is not a registered device class but the instance is tight to a
> wpan_phy. There can be multiple wpan_dev's (MAC layer instances on a
> phy/cfg802154 registered device). We currently don't support a HardMAC
> transceiver and I think because this misunderstanding came up.

Thanks for the explanation, I think it helps because the relationship
between wpan_dev and wpan_phy was not yet fully clear to me.

In order to clarify further your explanation and be sure that I
understand it the correct way, I tried to picture the above explanation
into a figure. Would you mind looking at it and tell me if something
does not fit?

https://bootlin.com/~miquel/ieee802154.pdf

Thanks,
Miquèl

Re: [net-next 08/18] net: ieee802154: Add support for internal PAN management

[Miquel Raynal]

Hi Jakub,

kuba@kernel.org wrote on Tue, 4 Jan 2022 07:01:27 -0800:

> On Tue, 4 Jan 2022 15:41:51 +0100 Miquel Raynal wrote:
> > > On Wed, 22 Dec 2021 16:57:33 +0100 Miquel Raynal wrote:    
> > > > +/* Maximum number of PAN entries to store */
> > > > +static int max_pan_entries = 100;
> > > > +module_param(max_pan_entries, uint, 0644);
> > > > +MODULE_PARM_DESC(max_pan_entries,
> > > > +		 "Maximum number of PANs to discover per scan (default is 100)");
> > > > +
> > > > +static int pan_expiration = 60;
> > > > +module_param(pan_expiration, uint, 0644);
> > > > +MODULE_PARM_DESC(pan_expiration,
> > > > +		 "Expiration of the scan validity in seconds (default is 60s)");      
> > > 
> > > Can these be per-device control knobs? Module params are rarely the
> > > best answer.    
> > 
> > I believe we can do that on a per FFD device basis (for now it will be
> > on a per-device basis, but later when we will have the necessary
> > information we might do something more fine grained). Would a couple of
> > sysfs entries work?  
> 
> Is there no netlink object where this would fit? Sorry, I'm not at all
> familiar with WPAN. If it's orthogonal to current cfg802154 objects
> sysfs is fine, I guess.

Yes it's definitely possible to add a netlink arg for these two
parameters as well.

Thanks,
Miquèl

Re: [net-next 01/18] ieee802154: hwsim: Ensure proper channel selection at probe time

[Miquel Raynal]

Hi Alexander,

alex.aring@gmail.com wrote on Tue, 28 Dec 2021 16:05:43 -0500:

> Hi,
> 
> On Wed, 22 Dec 2021 at 10:57, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > A default channel is selected by default (13), let's clarify that this
> > is page 0 channel 13. Call the right helper to ensure the necessary
> > configuration for this channel has been applied.
> >
> > So far there is very little configuration done in this helper but we
> > will soon add more information (like the symbol duration which is
> > missing) and having this helper called at probe time will prevent us to
> > this type of initialization at two different locations.
> >  
> 
> I see why this patch is necessary because in later patches the symbol
> duration is set at ".set_channel()" callback like the at86rf230 driver
> is doing it.
> However there is an old TODO [0]. I think we should combine it and
> implement it in ieee802154_set_channel() of "net/mac802154/cfg.c".
> Also do the symbol duration setting according to the channel/page when
> we call ieee802154_register_hw(), so we have it for the default
> settings.

While I totally agree on the background idea, I don't really see how
this is possible. Every driver internally knows what it supports but
AFAIU the core itself has no easy and standard access to it?

Another question that I have: is the protocol and center frequency
enough to always derive the symbol rate? I am not sure this is correct,
but I thought not all symbol rates could be derived, like for example
certain UWB PHY protocols which can use different PRF on a single
channel which has an effect on the symbol duration?

> > So far there is very little configuration done in this helper but thanks
> > to this improvement, future enhancements in this area (like setting a
> > symbol duration, which is missing) will be reflected automatically in
> > the default probe state.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/net/ieee802154/mac802154_hwsim.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
> > index 62ced7a30d92..b1a4ee7dceda 100644
> > --- a/drivers/net/ieee802154/mac802154_hwsim.c
> > +++ b/drivers/net/ieee802154/mac802154_hwsim.c
> > @@ -778,8 +778,6 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
> >
> >         ieee802154_random_extended_addr(&hw->phy->perm_extended_addr);
> >
> > -       /* hwsim phy channel 13 as default */
> > -       hw->phy->current_channel = 13;
> >         pib = kzalloc(sizeof(*pib), GFP_KERNEL);
> >         if (!pib) {
> >                 err = -ENOMEM;
> > @@ -793,6 +791,11 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
> >         hw->flags = IEEE802154_HW_PROMISCUOUS | IEEE802154_HW_RX_DROP_BAD_CKSUM;  
> 
> sadly this patch doesn't apply on current net-next/master because
> IEEE802154_HW_RX_DROP_BAD_CKSUM is not set.
> I agree that it should be set, so we need a patch for it.

Right, I just have a patch aside setting this to enforce beacons
checksum were good. I can certainly set this flag officially.

> 
> - Alex
> 
> [0] https://elixir.bootlin.com/linux/v5.16-rc7/source/drivers/net/ieee802154/at86rf230.c#L1059


Thanks,
Miquèl

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Miquel Raynal]

Hi Alexander,

> I see that beacons are sent out with
> "local->beacon.mhr.fc.dest_addr_mode = IEEE802154_NO_ADDRESSING;"
> shouldn't that be a broadcast destination?

In the case of a beacon, 7.3.1.2 Beacon frame MHR field indicate:

	When the Frame Version field is 0b00–0b01:
		— The Destination Addressing mode field shall be set to
		indicated that the Destination Address and Destination
		PAN ID fields are not present.

So I think the NO_ADDRESSING choice here is legit. The destination
field however is useful in the Enhanced beacon frame case, but that's
not yet supported.

Thanks,
Miquèl

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Miquel Raynal]

Hi Alexander,

alex.aring@gmail.com wrote on Fri, 31 Dec 2021 14:27:12 -0500:

> Hi,
> 
> On Thu, 30 Dec 2021 at 14:47, Alexander Aring <alex.aring@gmail.com> wrote:
> >
> > Hi,
> >
> > On Wed, 29 Dec 2021 at 09:45, Nicolas Schodet <nico@ni.fr.eu.org> wrote:  
> > >
> > > Hi,
> > >
> > > * Alexander Aring <alex.aring@gmail.com> [2021-12-29 09:30]:  
> > > > Hi,
> > > > On Wed, 22 Dec 2021 at 10:58, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > ...  
> > > > > +{
> > > > > +       bool promiscuous_on = mac802154_check_promiscuous(local);
> > > > > +       int ret;
> > > > > +
> > > > > +       if ((state && promiscuous_on) || (!state && !promiscuous_on))
> > > > > +               return 0;
> > > > > +
> > > > > +       ret = drv_set_promiscuous_mode(local, state);
> > > > > +       if (ret)
> > > > > +               pr_err("Failed to %s promiscuous mode for SW scanning",
> > > > > +                      state ? "set" : "reset");  
> > > > The semantic of promiscuous mode on the driver layer is to turn off
> > > > ack response, address filtering and crc checking. Some transceivers
> > > > don't allow a more fine tuning on what to enable/disable. I think we
> > > > should at least do the checksum checking per software then?
> > > > Sure there is a possible tune up for more "powerful" transceivers then...  
> > >
> > > In this case, the driver could change the (flags &
> > > IEEE802154_HW_RX_DROP_BAD_CKSUM) bit dynamically to signal it does not
> > > check the checksum anymore. Would it work?  
> >
> > I think that would work, although the intention of the hw->flags is to
> > define what the hardware is supposed to support as not changing those
> > values dynamically during runtime so mac will care about it. However
> > we don't expose those flags to the userspace, so far I know. We can
> > still introduce two separated flags if necessary in future.
> >
> > Why do we need promiscuous mode at all? Why is it necessary for a
> > scan? What of "ack response, address filtering and crc checking" you
> > want to disable and why?
> >  
> 
> I see now why promiscuous mode is necessary here. The actual
> promiscuous mode setting for the driver is not the same as promiscuous
> mode in 802.15.4 spec. For until now it was there for running a
> sniffer device only.
> As the 802.15.4 spec defines some "filtering levels" I came up with a
> draft so we can define which filtering level should be done on the
> hardware.

I like the idea but I'm not sure on what side you want to tackle the
problem first. Is it the phy drivers which should advertise the mac
about the promiscuous mode they support (which matches the description
below but does not fit the purpose of an enum very well)? Or is it the
MAC that requests a particular filtering mode? In this case what a phy
driver should do if:
- the requested mode is more constrained than its usual promiscuous
  capabilities?
- the requested mode is less constrained than its usual promiscuous
  capabilities?

> 
> diff --git a/include/net/mac802154.h b/include/net/mac802154.h
> index 72978fb72a3a..3839ed3f8f0d 100644
> --- a/include/net/mac802154.h
> +++ b/include/net/mac802154.h
> @@ -130,6 +130,48 @@ enum ieee802154_hw_flags {
>  #define IEEE802154_HW_OMIT_CKSUM       (IEEE802154_HW_TX_OMIT_CKSUM | \
>                                          IEEE802154_HW_RX_OMIT_CKSUM)
> 
> +/**
> + * enum ieee802154_filter_mode - hardware filter mode that a driver
> will pass to
> + *                              pass to mac802154.

Isn't it the opposite: The filtering level the mac is requesting? Here
it looks like we are describing driver capabilities (ie what drivers
advertise supporting).

> + *
> + * @IEEE802154_FILTER_MODE_0: No MFR filtering at all.

I suppose this would be for a sniffer accepting all frames, including
the bad ones.

> + *
> + * @IEEE802154_FILTER_MODE_1: IEEE802154_FILTER_MODE_1 with a bad FCS filter.

This means that the driver should only discard bad frames and propagate
all the remaining frames, right? So this typically is a regular sniffer
mode.

> + *
> + * @IEEE802154_FILTER_MODE_2: Same as IEEE802154_FILTER_MODE_1, known as
> + *                           802.15.4 promiscuous mode, sets
> + *                           mib.PromiscuousMode.

I believe what you call mib.PromiscuousMode is the mode that is
referred in the spec, ie. being in the official promiscuous mode? So
that is the mode that should be used "by default" when really asking
for a 802154 promiscuous mode.

Is there really a need for a different mode than mode_1 ?

> + *
> + * @IEEE802154_FILTER_MODE_3_SCAN: Same as IEEE802154_FILTER_MODE_2 without
> + *                                set mib.PromiscuousMode.

And here what is the difference between MODE_1 and MODE_3 ?

I suppose here we should as well drop all non-beacon frames?

> + *
> + * @IEEE802154_FILTER_MODE_3_NO_SCAN:
> + *     IEEE802154_FILTER_MODE_3_SCAN with MFR additional filter on:
> + *
> + *     - No reserved value in frame type
> + *     - No reserved value in frame version
> + *     - Match mib.PanId or broadcast
> + *     - Destination address field:
> + *       - Match mib.ShortAddress or broadcast
> + *       - Match mib.ExtendedAddress or GroupRxMode is true
> + *       - ImplicitBroadcast is true and destination address field/destination
> + *         panid is not included.
> + *       - Device is coordinator only source address present in data
> + *         frame/command frame and source panid matches mib.PanId
> + *       - Device is coordinator only source address present in multipurpose
> + *         frame and destination panid matches macPanId
> + *     - Beacon frames source panid matches mib.PanId. If mib.PanId is
> + *       broadcast it should always be accepted.

This is a bit counter intuitive, but do we agree on the fact that the
higher level of filtering should refer to promiscuous = false?

> + *
> + */
> +enum ieee802154_filter_mode {
> +       IEEE802154_FILTER_MODE_0,
> +       IEEE802154_FILTER_MODE_1,
> +       IEEE802154_FILTER_MODE_2,
> +       IEEE802154_FILTER_MODE_3_SCAN,
> +       IEEE802154_FILTER_MODE_3_NO_SCAN,
> +};
> +
>  /* struct ieee802154_ops - callbacks from mac802154 to the driver
>   *
>   * This structure contains various callbacks that the driver may
> @@ -249,7 +291,7 @@ struct ieee802154_ops {
>         int             (*set_frame_retries)(struct ieee802154_hw *hw,
>                                              s8 retries);
>         int             (*set_promiscuous_mode)(struct ieee802154_hw *hw,
> -                                               const bool on);
> +                                               enum
> ieee802154_filter_mode mode);
>         int             (*enter_scan_mode)(struct ieee802154_hw *hw,
>                                            struct
> cfg802154_scan_request *request);
>         int             (*exit_scan_mode)(struct ieee802154_hw *hw);
> 
> ---
> 
> In your case it will be IEEE802154_FILTER_MODE_3_SCAN mode, for a
> sniffer we probably want as default IEEE802154_FILTER_MODE_0, as
> "promiscuous mode" currently is.
> 
> - Alex


Thanks,
Miquèl

Re: [net-next 08/18] net: ieee802154: Add support for internal PAN management

[Alexander Aring]

Hi,

On Tue, 4 Jan 2022 at 10:05, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> alex.aring@gmail.com wrote on Tue, 28 Dec 2021 17:22:38 -0500:
>
> > Hi,
> >
> > On Wed, 22 Dec 2021 at 10:57, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Let's introduce the basics of PAN management:
> > > - structures defining PANs
> > > - helpers for PANs registration
> > > - helpers discarding old PANs
> > >
> >
> > I think there exists a little misunderstanding about how the
> > architecture is between the structures wpan_phy, wpan_dev and
> > cfg802154.
> >
> >  - wpan_phy: represents the PHY layer of IEEE 802154 and is a
> > registered device class.
> >  - wpan_dev: represents the MAC layer of IEEE 802154 and is a netdev interface.
> >
> > You can have multiple wpan_dev operate on one wpan_phy. To my best
> > knowledge it's like having multiple access points running on one phy
> > (wireless) or macvlan on ethernet. You can actually do that with the
> > mac802154_hwsim driver. However as there exists currently no (as my
> > knowledge) hardware which supports e.g. multiple address filters we
> > wanted to be prepared for to support such handling. Although, there
> > exists some transceivers which support something like a "pan bridge"
> > which goes into such a direction.
> >
> > What is a cfg802154 registered device? Well, at first it offers an
> > interface between SoftMAC and HardMAC from nl802154, that's the
> > cfg802154_ops structure. In theory a HardMAC transceiver would bypass
> > the SoftMAC stack by implementing "cfg802154_ops" on the driver layer
> > and try to do everything there as much as possible to support it. It
> > is not a registered device class but the instance is tight to a
> > wpan_phy. There can be multiple wpan_dev's (MAC layer instances on a
> > phy/cfg802154 registered device). We currently don't support a HardMAC
> > transceiver and I think because this misunderstanding came up.
>
> Thanks for the explanation, I think it helps because the relationship
> between wpan_dev and wpan_phy was not yet fully clear to me.
>
> In order to clarify further your explanation and be sure that I
> understand it the correct way, I tried to picture the above explanation
> into a figure. Would you mind looking at it and tell me if something
> does not fit?
>
> https://bootlin.com/~miquel/ieee802154.pdf

I think so, yes... if a transceiver has e.g. two antennas/phy's it can
also register two phy's and so on... then phy's can also move into net
namespaces (like what we do for hwsim for routing testing [0]). Should
keep that in mind.

- Alex

[0] https://github.com/linux-wpan/rpld/blob/nonstoring_mode/test/ns_setup

Re: [net-next 01/18] ieee802154: hwsim: Ensure proper channel selection at probe time

[Alexander Aring]

Hi,

On Tue, 4 Jan 2022 at 10:44, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> alex.aring@gmail.com wrote on Tue, 28 Dec 2021 16:05:43 -0500:
>
> > Hi,
> >
> > On Wed, 22 Dec 2021 at 10:57, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > A default channel is selected by default (13), let's clarify that this
> > > is page 0 channel 13. Call the right helper to ensure the necessary
> > > configuration for this channel has been applied.
> > >
> > > So far there is very little configuration done in this helper but we
> > > will soon add more information (like the symbol duration which is
> > > missing) and having this helper called at probe time will prevent us to
> > > this type of initialization at two different locations.
> > >
> >
> > I see why this patch is necessary because in later patches the symbol
> > duration is set at ".set_channel()" callback like the at86rf230 driver
> > is doing it.
> > However there is an old TODO [0]. I think we should combine it and
> > implement it in ieee802154_set_channel() of "net/mac802154/cfg.c".
> > Also do the symbol duration setting according to the channel/page when
> > we call ieee802154_register_hw(), so we have it for the default
> > settings.
>
> While I totally agree on the background idea, I don't really see how
> this is possible. Every driver internally knows what it supports but
> AFAIU the core itself has no easy and standard access to it?
>

I am a little bit confused here, because a lot of timing related
things in the phy information rate points to "x times symbols". If
this value depends on the transceiver, how are they compatible then?

> Another question that I have: is the protocol and center frequency
> enough to always derive the symbol rate? I am not sure this is correct,
> but I thought not all symbol rates could be derived, like for example
> certain UWB PHY protocols which can use different PRF on a single
> channel which has an effect on the symbol duration?

Regarding UWB PHY I see that for values like LIFS/SIFS they reference
a "preambleSymbols" value which is defined.

I need to do more research regarding this.

- Alex

Re: [net-next 01/18] ieee802154: hwsim: Ensure proper channel selection at probe time

[Alexander Aring]

Hi,

On Tue, 4 Jan 2022 at 18:08, Alexander Aring <alex.aring@gmail.com> wrote:
>
> Hi,
>
> On Tue, 4 Jan 2022 at 10:44, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > alex.aring@gmail.com wrote on Tue, 28 Dec 2021 16:05:43 -0500:
> >
> > > Hi,
> > >
> > > On Wed, 22 Dec 2021 at 10:57, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >
> > > > A default channel is selected by default (13), let's clarify that this
> > > > is page 0 channel 13. Call the right helper to ensure the necessary
> > > > configuration for this channel has been applied.
> > > >
> > > > So far there is very little configuration done in this helper but we
> > > > will soon add more information (like the symbol duration which is
> > > > missing) and having this helper called at probe time will prevent us to
> > > > this type of initialization at two different locations.
> > > >
> > >
> > > I see why this patch is necessary because in later patches the symbol
> > > duration is set at ".set_channel()" callback like the at86rf230 driver
> > > is doing it.
> > > However there is an old TODO [0]. I think we should combine it and
> > > implement it in ieee802154_set_channel() of "net/mac802154/cfg.c".
> > > Also do the symbol duration setting according to the channel/page when
> > > we call ieee802154_register_hw(), so we have it for the default
> > > settings.
> >
> > While I totally agree on the background idea, I don't really see how
> > this is possible. Every driver internally knows what it supports but
> > AFAIU the core itself has no easy and standard access to it?
> >
>
> I am a little bit confused here, because a lot of timing related
> things in the phy information rate points to "x times symbols". If

s/rate/base/

- Alex

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Alexander Aring]

Hi.

On Tue, 4 Jan 2022 at 13:18, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
...
> >
> > I see now why promiscuous mode is necessary here. The actual
> > promiscuous mode setting for the driver is not the same as promiscuous
> > mode in 802.15.4 spec. For until now it was there for running a
> > sniffer device only.
> > As the 802.15.4 spec defines some "filtering levels" I came up with a
> > draft so we can define which filtering level should be done on the
> > hardware.
>
> I like the idea but I'm not sure on what side you want to tackle the
> problem first. Is it the phy drivers which should advertise the mac
> about the promiscuous mode they support (which matches the description
> below but does not fit the purpose of an enum very well)? Or is it the
> MAC that requests a particular filtering mode? In this case what a phy
> driver should do if:
> - the requested mode is more constrained than its usual promiscuous
>   capabilities?

Then, the driver needs to go one level lower and tell mac802154 to
filter more out.

> - the requested mode is less constrained than its usual promiscuous
>   capabilities?
>

Then mac802154 needs to filter more out.

I am more worried at the point the transceiver will shut off automatic
acknowledge handling which we probably can't do in software in cases
where it's required. Some transceivers will shut that off if they turn
off address filtering and if they don't have a detailed setting for
that they will ack every frame what they see, which is... not so good.

Future work would be to warn about mismatch of seeing frames, what the
hardware would filter out vs what mac802154 sees. More further work
could be to use a monitor interface and raw sockets and verify
transceivers how they act to frames.

> >
> > diff --git a/include/net/mac802154.h b/include/net/mac802154.h
> > index 72978fb72a3a..3839ed3f8f0d 100644
> > --- a/include/net/mac802154.h
> > +++ b/include/net/mac802154.h
> > @@ -130,6 +130,48 @@ enum ieee802154_hw_flags {
> >  #define IEEE802154_HW_OMIT_CKSUM       (IEEE802154_HW_TX_OMIT_CKSUM | \
> >                                          IEEE802154_HW_RX_OMIT_CKSUM)
> >
> > +/**
> > + * enum ieee802154_filter_mode - hardware filter mode that a driver
> > will pass to
> > + *                              pass to mac802154.
>
> Isn't it the opposite: The filtering level the mac is requesting? Here
> it looks like we are describing driver capabilities (ie what drivers
> advertise supporting).
>

I am sorry. I meant what the transceiver "should" deliver or "level
less" to mac802154.

I think the filtering when not much resources are required can also be
done in a hardirq context. There exists a tasklet which is there to
switch to a softirq context [0], currently we do all parsing there.

> > + *
> > + * @IEEE802154_FILTER_MODE_0: No MFR filtering at all.
>
> I suppose this would be for a sniffer accepting all frames, including
> the bad ones.
>

yes.

> > + *
> > + * @IEEE802154_FILTER_MODE_1: IEEE802154_FILTER_MODE_1 with a bad FCS filter.
>
> This means that the driver should only discard bad frames and propagate
> all the remaining frames, right? So this typically is a regular sniffer
> mode.
>

I think this depends on what you want to filter out, so far I know in
wireless this is configurable. Wireshark always expects the FCS in
their payload for a linux 802.15.4 monitor interface and I think this
is because of some historical reason to support the first 802.15.4
sniffers in wireshark.
There is a difference between filter bad FCS and cutoff FCS. I need to
look it up but I think wireless would cut off the checksum if FCS is
filtered on hardware (may even some transceivers will not deliver FCS
to you if you enable filtering).

> > + *
> > + * @IEEE802154_FILTER_MODE_2: Same as IEEE802154_FILTER_MODE_1, known as
> > + *                           802.15.4 promiscuous mode, sets
> > + *                           mib.PromiscuousMode.
>
> I believe what you call mib.PromiscuousMode is the mode that is
> referred in the spec, ie. being in the official promiscuous mode? So
> that is the mode that should be used "by default" when really asking
> for a 802154 promiscuous mode.
>

then we don't call it in driver level promiscuous mode, we call it
"filtering level". And this is the filtering for cases when the
standard says set "mib.PromiscuousMode".

> Is there really a need for a different mode than mode_1 ?
>

I think so, I am not sure what they or will define if PromiscuousMode
is set or not and might the transceiver need to get notice about it?
It's not needed now, but we might keep it in mind then.

> > + *
> > + * @IEEE802154_FILTER_MODE_3_SCAN: Same as IEEE802154_FILTER_MODE_2 without
> > + *                                set mib.PromiscuousMode.
>
> And here what is the difference between MODE_1 and MODE_3 ?
>
> I suppose here we should as well drop all non-beacon frames?

Yes, additionally there could be a transceiver doing this filtering on
hardware and tell that it's in scan and this is the difference.

>
> > + *
> > + * @IEEE802154_FILTER_MODE_3_NO_SCAN:
> > + *     IEEE802154_FILTER_MODE_3_SCAN with MFR additional filter on:
> > + *

should be IEEE802154_FILTER_MODE_2. Maybe we can also get some better
names for that but the standard describes it with numbers as well.

> > + *     - No reserved value in frame type
> > + *     - No reserved value in frame version
> > + *     - Match mib.PanId or broadcast
> > + *     - Destination address field:
> > + *       - Match mib.ShortAddress or broadcast
> > + *       - Match mib.ExtendedAddress or GroupRxMode is true
> > + *       - ImplicitBroadcast is true and destination address field/destination
> > + *         panid is not included.
> > + *       - Device is coordinator only source address present in data
> > + *         frame/command frame and source panid matches mib.PanId
> > + *       - Device is coordinator only source address present in multipurpose
> > + *         frame and destination panid matches macPanId
> > + *     - Beacon frames source panid matches mib.PanId. If mib.PanId is
> > + *       broadcast it should always be accepted.
>
> This is a bit counter intuitive, but do we agree on the fact that the
> higher level of filtering should refer to promiscuous = false?
>

Yes, it's a lot of filter rules at this level.
Yes, promiscuous is false in this case. That is what currently what
wpan "node" interface should filter at mac802154 [1] (for cases device
coordinator is false).

I might mention a lot of future work here. I think we can live for now
to make a difference between those levels and be sure that we drop
everything else in the scan operation (inclusive check fcs in
software). Moving stuff that we can do in hardware to hardware and the
rest in software is a bigger task here...

- Alex

[0] https://elixir.bootlin.com/linux/v5.16-rc8/source/net/mac802154/rx.c#L294
[1] https://elixir.bootlin.com/linux/v5.16-rc8/source/net/mac802154/rx.c#L132

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Alexander Aring]

Hi,

On Tue, 4 Jan 2022 at 12:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> > I see that beacons are sent out with
> > "local->beacon.mhr.fc.dest_addr_mode = IEEE802154_NO_ADDRESSING;"
> > shouldn't that be a broadcast destination?
>
> In the case of a beacon, 7.3.1.2 Beacon frame MHR field indicate:
>
>         When the Frame Version field is 0b00–0b01:
>                 — The Destination Addressing mode field shall be set to
>                 indicated that the Destination Address and Destination
>                 PAN ID fields are not present.
>
> So I think the NO_ADDRESSING choice here is legit. The destination
> field however is useful in the Enhanced beacon frame case, but that's
> not yet supported.

ok, yes I agree.

Thanks.

- Alex

Re: [net-next 01/18] ieee802154: hwsim: Ensure proper channel selection at probe time

[Miquel Raynal]

Hi Alexander,

alex.aring@gmail.com wrote on Tue, 4 Jan 2022 18:10:44 -0500:

> Hi,
> 
> On Tue, 4 Jan 2022 at 18:08, Alexander Aring <alex.aring@gmail.com> wrote:
> >
> > Hi,
> >
> > On Tue, 4 Jan 2022 at 10:44, Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > >
> > > Hi Alexander,
> > >
> > > alex.aring@gmail.com wrote on Tue, 28 Dec 2021 16:05:43 -0500:
> > >  
> > > > Hi,
> > > >
> > > > On Wed, 22 Dec 2021 at 10:57, Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > > > >
> > > > > A default channel is selected by default (13), let's clarify that this
> > > > > is page 0 channel 13. Call the right helper to ensure the necessary
> > > > > configuration for this channel has been applied.
> > > > >
> > > > > So far there is very little configuration done in this helper but we
> > > > > will soon add more information (like the symbol duration which is
> > > > > missing) and having this helper called at probe time will prevent us to
> > > > > this type of initialization at two different locations.
> > > > >  
> > > >
> > > > I see why this patch is necessary because in later patches the symbol
> > > > duration is set at ".set_channel()" callback like the at86rf230 driver
> > > > is doing it.
> > > > However there is an old TODO [0]. I think we should combine it and
> > > > implement it in ieee802154_set_channel() of "net/mac802154/cfg.c".
> > > > Also do the symbol duration setting according to the channel/page when
> > > > we call ieee802154_register_hw(), so we have it for the default
> > > > settings.  
> > >
> > > While I totally agree on the background idea, I don't really see how
> > > this is possible. Every driver internally knows what it supports but
> > > AFAIU the core itself has no easy and standard access to it?
> > >  
> >
> > I am a little bit confused here, because a lot of timing related
> > things in the phy information rate points to "x times symbols". If  
> 
> s/rate/base/

Yes indeed, but I bet it works because the phy drivers set the symbol
duration by themselves. You can see that none of them does something
clever like:

switch (phy.protocol) {
	case XXXXX:
		symbol_duration = y;
		break;
	...

Instead, they all go through the page/channel list in a quite hardcoded
way because the phy driver knows internally that protocol X is used on
{page, channel}, but the protocol id, while not being totally absent
from drivers, is always provided as a comment.

So getting rid of the core TODO you mentioned earlier means:
- Listing properly the PHY protocols in the core (if not already done)
- For each PHY protocol knowing the possible base frequencies
- For each of these base frequencies knowing the symbol duration
- Having the possibility to add more information such as the PRF in
  order to let the core pick the right symbol duration when there is
  more than one possibility for a {protocol, base frequency} couple
- Convert the phy drivers (at least hwsim) to fill these new fields
  correctly and expect the core to set the symbol duration properly.

Two side notes as well:
- I was not able to find all the the corresponding protocol from the
  hwsim driver in the spec (these channels are marked "unknown")
- The symbol duration in a few specific UWB cases is below 1us while
  the core expects a value in us. Should we change the symbol duration
  to ns?

I believe all this is doable in a reasonable time frame provided that
I only focus on the few protocols supported by hwsim which I already
"addressed" and perhaps a couple of simple drivers. On the core side,
the logic might be: "is the driver providing information about the phy
protocols used? if yes, then set the symbol duration if we have enough
data, otherwise let the driver handle it by itself". Such logic would
allow a progressive shift towards the situation where drivers do not
have to bother with symbol duration by themselves.

As this looks like a project on its own and my first goal was to be
able to use hwsim to demonstrate the different scan features, maybe we
can continue to discuss this and consider tackling it as a separate
series whish would apply on top of the current one, what do you think?

Thanks,
Miquèl

Re: [net-next 17/18] net: mac802154: Let drivers provide their own beacons implementation

[Miquel Raynal]

Hi Alexander,

alex.aring@gmail.com wrote on Thu, 30 Dec 2021 14:48:41 -0500:

> Hi,
> 
> On Thu, 30 Dec 2021 at 12:00, David Girault <David.Girault@qorvo.com> wrote:
> >
> > Hi Alexander,
> >
> > At Qorvo, we have developped a SoftMAC driver for our DW3000 chip that will benefit such API.
> >  
> Do you want to bring this driver upstream as well? Currently those
> callbacks will be introduced but no user is there.

I think so far the upstream fate of the DW3000 driver has not been ruled
out so let's assume it won't be upstreamed (at least not fully), that's
also why we decided to begin with the hwsim driver.

However, when designing this series, it appeared quite clear that any
hardMAC driver would need this type of interface. The content of the
interface, I agree, could be further discussed and even edited, but the
main idea of giving the information to the phy driver about what is
happening regarding eg. scan operations or beacon frames, might make
sense regardless of the current users, no?

This being said, if other people decide to upstream a hardMAC driver
and need these hooks to behave a little bit differently, it's their
right to tweak them and that would also be part of the game.

Although we might not need these hooks in a near future at all if we
move to the filtering modes, because the promiscuous call with the
specific level might indicate to the device how it should configure
itself already.

> > To be short, beacon sending is controller by our driver to be synchronized chip clock or delayed until
> > other operation in progress (a ranging for example).
> >  
> 
> ok.
> 
> - Alex

Thanks,
Miquèl

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Miquel Raynal]

Hi Alexander,

alex.aring@gmail.com wrote on Tue, 4 Jan 2022 20:16:30 -0500:

> Hi.
> 
> On Tue, 4 Jan 2022 at 13:18, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >  
> ...
> > >
> > > I see now why promiscuous mode is necessary here. The actual
> > > promiscuous mode setting for the driver is not the same as promiscuous
> > > mode in 802.15.4 spec. For until now it was there for running a
> > > sniffer device only.
> > > As the 802.15.4 spec defines some "filtering levels" I came up with a
> > > draft so we can define which filtering level should be done on the
> > > hardware.  
> >
> > I like the idea but I'm not sure on what side you want to tackle the
> > problem first. Is it the phy drivers which should advertise the mac
> > about the promiscuous mode they support (which matches the description
> > below but does not fit the purpose of an enum very well)? Or is it the
> > MAC that requests a particular filtering mode? In this case what a phy
> > driver should do if:
> > - the requested mode is more constrained than its usual promiscuous
> >   capabilities?  
> 
> Then, the driver needs to go one level lower and tell mac802154 to
> filter more out.
> 
> > - the requested mode is less constrained than its usual promiscuous
> >   capabilities?
> >  
> 
> Then mac802154 needs to filter more out.
> 
> I am more worried at the point the transceiver will shut off automatic
> acknowledge handling which we probably can't do in software in cases
> where it's required. Some transceivers will shut that off if they turn
> off address filtering and if they don't have a detailed setting for
> that they will ack every frame what they see, which is... not so good.
> 
> Future work would be to warn about mismatch of seeing frames, what the
> hardware would filter out vs what mac802154 sees. More further work
> could be to use a monitor interface and raw sockets and verify
> transceivers how they act to frames.
> 
> > >
> > > diff --git a/include/net/mac802154.h b/include/net/mac802154.h
> > > index 72978fb72a3a..3839ed3f8f0d 100644
> > > --- a/include/net/mac802154.h
> > > +++ b/include/net/mac802154.h
> > > @@ -130,6 +130,48 @@ enum ieee802154_hw_flags {
> > >  #define IEEE802154_HW_OMIT_CKSUM       (IEEE802154_HW_TX_OMIT_CKSUM | \
> > >                                          IEEE802154_HW_RX_OMIT_CKSUM)
> > >
> > > +/**
> > > + * enum ieee802154_filter_mode - hardware filter mode that a driver
> > > will pass to
> > > + *                              pass to mac802154.  
> >
> > Isn't it the opposite: The filtering level the mac is requesting? Here
> > it looks like we are describing driver capabilities (ie what drivers
> > advertise supporting).
> >  
> 
> I am sorry. I meant what the transceiver "should" deliver or "level
> less" to mac802154.
> 
> I think the filtering when not much resources are required can also be
> done in a hardirq context. There exists a tasklet which is there to
> switch to a softirq context [0], currently we do all parsing there.
> 
> > > + *
> > > + * @IEEE802154_FILTER_MODE_0: No MFR filtering at all.  
> >
> > I suppose this would be for a sniffer accepting all frames, including
> > the bad ones.
> >  
> 
> yes.
> 
> > > + *
> > > + * @IEEE802154_FILTER_MODE_1: IEEE802154_FILTER_MODE_1 with a bad FCS filter.  
> >
> > This means that the driver should only discard bad frames and propagate
> > all the remaining frames, right? So this typically is a regular sniffer
> > mode.
> >  
> 
> I think this depends on what you want to filter out, so far I know in
> wireless this is configurable. Wireshark always expects the FCS in
> their payload for a linux 802.15.4 monitor interface and I think this
> is because of some historical reason to support the first 802.15.4
> sniffers in wireshark.
> There is a difference between filter bad FCS and cutoff FCS. I need to
> look it up but I think wireless would cut off the checksum if FCS is
> filtered on hardware (may even some transceivers will not deliver FCS
> to you if you enable filtering).
> 
> > > + *
> > > + * @IEEE802154_FILTER_MODE_2: Same as IEEE802154_FILTER_MODE_1, known as
> > > + *                           802.15.4 promiscuous mode, sets
> > > + *                           mib.PromiscuousMode.  
> >
> > I believe what you call mib.PromiscuousMode is the mode that is
> > referred in the spec, ie. being in the official promiscuous mode? So
> > that is the mode that should be used "by default" when really asking
> > for a 802154 promiscuous mode.
> >  
> 
> then we don't call it in driver level promiscuous mode, we call it
> "filtering level". And this is the filtering for cases when the
> standard says set "mib.PromiscuousMode".
> 
> > Is there really a need for a different mode than mode_1 ?
> >  
> 
> I think so, I am not sure what they or will define if PromiscuousMode
> is set or not and might the transceiver need to get notice about it?
> It's not needed now, but we might keep it in mind then.
> 
> > > + *
> > > + * @IEEE802154_FILTER_MODE_3_SCAN: Same as IEEE802154_FILTER_MODE_2 without
> > > + *                                set mib.PromiscuousMode.  
> >
> > And here what is the difference between MODE_1 and MODE_3 ?
> >
> > I suppose here we should as well drop all non-beacon frames?  
> 
> Yes, additionally there could be a transceiver doing this filtering on
> hardware and tell that it's in scan and this is the difference.
> 
> >  
> > > + *
> > > + * @IEEE802154_FILTER_MODE_3_NO_SCAN:
> > > + *     IEEE802154_FILTER_MODE_3_SCAN with MFR additional filter on:
> > > + *  
> 
> should be IEEE802154_FILTER_MODE_2. Maybe we can also get some better
> names for that but the standard describes it with numbers as well.
> 
> > > + *     - No reserved value in frame type
> > > + *     - No reserved value in frame version
> > > + *     - Match mib.PanId or broadcast
> > > + *     - Destination address field:
> > > + *       - Match mib.ShortAddress or broadcast
> > > + *       - Match mib.ExtendedAddress or GroupRxMode is true
> > > + *       - ImplicitBroadcast is true and destination address field/destination
> > > + *         panid is not included.
> > > + *       - Device is coordinator only source address present in data
> > > + *         frame/command frame and source panid matches mib.PanId
> > > + *       - Device is coordinator only source address present in multipurpose
> > > + *         frame and destination panid matches macPanId
> > > + *     - Beacon frames source panid matches mib.PanId. If mib.PanId is
> > > + *       broadcast it should always be accepted.  
> >
> > This is a bit counter intuitive, but do we agree on the fact that the
> > higher level of filtering should refer to promiscuous = false?
> >  
> 
> Yes, it's a lot of filter rules at this level.
> Yes, promiscuous is false in this case. That is what currently what
> wpan "node" interface should filter at mac802154 [1] (for cases device
> coordinator is false).
> 
> I might mention a lot of future work here. I think we can live for now
> to make a difference between those levels and be sure that we drop
> everything else in the scan operation (inclusive check fcs in
> software). Moving stuff that we can do in hardware to hardware and the
> rest in software is a bigger task here...

On the symbol duration side I feel I'm close to a working PoC.

So there is 'only' this item left in my mind. Could you please clarify
what you expect from me exactly in terms of support for the promiscuous
filters we discussed so far?

Also, just for the record,
- should I keep copying the netdev list for v2?
- should I monitor if net-next is open before sending or do you have
  your own set of rules?

> [0] https://elixir.bootlin.com/linux/v5.16-rc8/source/net/mac802154/rx.c#L294
> [1] https://elixir.bootlin.com/linux/v5.16-rc8/source/net/mac802154/rx.c#L132

Thanks,
Miquèl

Re: [net-next 01/18] ieee802154: hwsim: Ensure proper channel selection at probe time

[Alexander Aring]

Hi,

On Wed, 5 Jan 2022 at 03:14, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> alex.aring@gmail.com wrote on Tue, 4 Jan 2022 18:10:44 -0500:
>
> > Hi,
> >
> > On Tue, 4 Jan 2022 at 18:08, Alexander Aring <alex.aring@gmail.com> wrote:
> > >
> > > Hi,
> > >
> > > On Tue, 4 Jan 2022 at 10:44, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >
> > > > Hi Alexander,
> > > >
> > > > alex.aring@gmail.com wrote on Tue, 28 Dec 2021 16:05:43 -0500:
> > > >
> > > > > Hi,
> > > > >
> > > > > On Wed, 22 Dec 2021 at 10:57, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > > >
> > > > > > A default channel is selected by default (13), let's clarify that this
> > > > > > is page 0 channel 13. Call the right helper to ensure the necessary
> > > > > > configuration for this channel has been applied.
> > > > > >
> > > > > > So far there is very little configuration done in this helper but we
> > > > > > will soon add more information (like the symbol duration which is
> > > > > > missing) and having this helper called at probe time will prevent us to
> > > > > > this type of initialization at two different locations.
> > > > > >
> > > > >
> > > > > I see why this patch is necessary because in later patches the symbol
> > > > > duration is set at ".set_channel()" callback like the at86rf230 driver
> > > > > is doing it.
> > > > > However there is an old TODO [0]. I think we should combine it and
> > > > > implement it in ieee802154_set_channel() of "net/mac802154/cfg.c".
> > > > > Also do the symbol duration setting according to the channel/page when
> > > > > we call ieee802154_register_hw(), so we have it for the default
> > > > > settings.
> > > >
> > > > While I totally agree on the background idea, I don't really see how
> > > > this is possible. Every driver internally knows what it supports but
> > > > AFAIU the core itself has no easy and standard access to it?
> > > >
> > >
> > > I am a little bit confused here, because a lot of timing related
> > > things in the phy information rate points to "x times symbols". If
> >
> > s/rate/base/
>
> Yes indeed, but I bet it works because the phy drivers set the symbol
> duration by themselves. You can see that none of them does something
> clever like:
>
> switch (phy.protocol) {
>         case XXXXX:
>                 symbol_duration = y;
>                 break;
>         ...
>
> Instead, they all go through the page/channel list in a quite hardcoded
> way because the phy driver knows internally that protocol X is used on
> {page, channel}, but the protocol id, while not being totally absent
> from drivers, is always provided as a comment.
>
> So getting rid of the core TODO you mentioned earlier means:
> - Listing properly the PHY protocols in the core (if not already done)
> - For each PHY protocol knowing the possible base frequencies
> - For each of these base frequencies knowing the symbol duration
> - Having the possibility to add more information such as the PRF in
>   order to let the core pick the right symbol duration when there is
>   more than one possibility for a {protocol, base frequency} couple
> - Convert the phy drivers (at least hwsim) to fill these new fields
>   correctly and expect the core to set the symbol duration properly.
>

I think this becomes quite large to provide all that information and
somehow this reminds me about the other TODO to extend the wireless
regdb with 802.15.4 specs and somehow let the kernel get the
information from there.

> Two side notes as well:
> - I was not able to find all the the corresponding protocol from the
>   hwsim driver in the spec (these channels are marked "unknown")

I think those frequencies were taken from the fakelb driver, I think
currently that the static channel array has a limitation on its own to
provide all channel/page combinations which could be set.

> - The symbol duration in a few specific UWB cases is below 1us while
>   the core expects a value in us. Should we change the symbol duration
>   to ns?
>

At some point yes, I think we need to switch to ns.

> I believe all this is doable in a reasonable time frame provided that
> I only focus on the few protocols supported by hwsim which I already
> "addressed" and perhaps a couple of simple drivers. On the core side,
> the logic might be: "is the driver providing information about the phy
> protocols used? if yes, then set the symbol duration if we have enough
> data, otherwise let the driver handle it by itself". Such logic would
> allow a progressive shift towards the situation where drivers do not
> have to bother with symbol duration by themselves.
>
> As this looks like a project on its own and my first goal was to be
> able to use hwsim to demonstrate the different scan features, maybe we
> can continue to discuss this and consider tackling it as a separate
> series whish would apply on top of the current one, what do you think?

I think the current way that the driver sets is fine, we can still
find other possible ways... for doing it better (e.g. regdb) is a new
project, I agree with that.

- Alex

Re: [net-next 17/18] net: mac802154: Let drivers provide their own beacons implementation

[Alexander Aring]

Hi,

On Wed, 5 Jan 2022 at 03:48, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> alex.aring@gmail.com wrote on Thu, 30 Dec 2021 14:48:41 -0500:
>
> > Hi,
> >
> > On Thu, 30 Dec 2021 at 12:00, David Girault <David.Girault@qorvo.com> wrote:
> > >
> > > Hi Alexander,
> > >
> > > At Qorvo, we have developped a SoftMAC driver for our DW3000 chip that will benefit such API.
> > >
> > Do you want to bring this driver upstream as well? Currently those
> > callbacks will be introduced but no user is there.
>
> I think so far the upstream fate of the DW3000 driver has not been ruled
> out so let's assume it won't be upstreamed (at least not fully), that's
> also why we decided to begin with the hwsim driver.
>

ok.

> However, when designing this series, it appeared quite clear that any
> hardMAC driver would need this type of interface. The content of the
> interface, I agree, could be further discussed and even edited, but the
> main idea of giving the information to the phy driver about what is
> happening regarding eg. scan operations or beacon frames, might make
> sense regardless of the current users, no?
>

A HardMAC driver does not use this driver interface... but there
exists a SoftMAC driver for a HardMAC transceiver. This driver
currently works because we use dataframes only... It will not support
scanning currently and somehow we should make iit not available for
drivers like that and for drivers which don't set symbol duration.
They need to be fixed.

> This being said, if other people decide to upstream a hardMAC driver
> and need these hooks to behave a little bit differently, it's their
> right to tweak them and that would also be part of the game.
>
> Although we might not need these hooks in a near future at all if we
> move to the filtering modes, because the promiscuous call with the
> specific level might indicate to the device how it should configure
> itself already.
>

My concern is that somebody else might want to remove those callbacks
because they are not used.

- Alex

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Alexander Aring]

Hi,


On Wed, 5 Jan 2022 at 15:55, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
...
> > rest in software is a bigger task here...
>
> On the symbol duration side I feel I'm close to a working PoC.
>

oh, ok.

> So there is 'only' this item left in my mind. Could you please clarify
> what you expect from me exactly in terms of support for the promiscuous
> filters we discussed so far?
>

I think for now it's okay to set the device into promiscuous mode and
enable the flag which checks for bad FCS... we can still implement the
filter modes later (and I think it should work on all supported
transceivers (except that SoftMAC/HardMAC thing)).

One point to promiscuous mode, currently we have a checking for if a
phy is in promiscuous mode on ifup and it would forbid to ifup a node
interface if the phy is in promiscuous mode (because of the missing
automatic acknowledgement). I see there is a need to turn the phy into
promiscuous mode during runtime... so we need somehow make sure the
constraints are still valid here. Maybe we even forbid multiple devs
on a phy if the transceiver/driver/firmware is poor and this is
currently all transceivers (except hwsim? But that doesn't use any ack
handling anyway).

> Also, just for the record,
> - should I keep copying the netdev list for v2?

yes, why not.

> - should I monitor if net-next is open before sending or do you have
>   your own set of rules?
>

I need to admit, Stefan is the "Thanks, applied." hero here and he
should answer this question.

- Alex

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Stefan Schmidt]

Hello.

On 06.01.22 01:38, Alexander Aring wrote:
> Hi,
> 
> 
> On Wed, 5 Jan 2022 at 15:55, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> ...

>> Also, just for the record,
>> - should I keep copying the netdev list for v2?
> 
> yes, why not.

>> - should I monitor if net-next is open before sending or do you have
>>    your own set of rules?
>>
> 
> I need to admit, Stefan is the "Thanks, applied." hero here and he
> should answer this question.

No need to monitor if net-next is open for these patches (just don't add 
a net-next patch subject prefix as this would confuse Jakub and Dave. 
wpan-next would be more appropriate).

I am following this patchset and the review from Alex. I have not done a 
full in depth review myself yet, its on my list.

Basically keep working with Alex and use the wpan-next prefix and I will 
pick up the patches to my wpan-next tree and sent a pull to net-next 
when we are happy with it. Does that sound good to you?

regards
Stefan Schmidt

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Miquel Raynal]

Hi Stefan,

stefan@datenfreihafen.org wrote on Thu, 6 Jan 2022 09:44:50 +0100:

> Hello.
> 
> On 06.01.22 01:38, Alexander Aring wrote:
> > Hi,
> > 
> > 
> > On Wed, 5 Jan 2022 at 15:55, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > ...  
> 
> >> Also, just for the record,
> >> - should I keep copying the netdev list for v2?  
> > 
> > yes, why not.  
> 
> >> - should I monitor if net-next is open before sending or do you have
> >>    your own set of rules?
> >>  
> > 
> > I need to admit, Stefan is the "Thanks, applied." hero here and he
> > should answer this question.  
> 
> No need to monitor if net-next is open for these patches (just don't add a net-next patch subject prefix as this would confuse Jakub and Dave. wpan-next would be more appropriate).

Sure! It might be worth updating [1] to tell people about this prefix?
(only the userspace tools prefix is mentioned).

[1] https://linux-wpan.org/contributing.html 

> I am following this patchset and the review from Alex. I have not done a full in depth review myself yet, its on my list.

Yeah sure, no problem.

> Basically keep working with Alex and use the wpan-next prefix and I will pick up the patches to my wpan-next tree and sent a pull to net-next when we are happy with it. Does that sound good to you?

Yes of course, that's ideal.

Thanks,
Miquèl

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Miquel Raynal]

Hi Alexander,

alex.aring@gmail.com wrote on Wed, 5 Jan 2022 19:38:12 -0500:

> Hi,
> 
> 
> On Wed, 5 Jan 2022 at 15:55, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> ...
> > > rest in software is a bigger task here...  
> >
> > On the symbol duration side I feel I'm close to a working PoC.
> >  
> 
> oh, ok.

I think it's ready, I'll soon send two series:
- the symbol duration update
- v2 for this series, which will not apply without the symbol duration
  update.

> > So there is 'only' this item left in my mind. Could you please clarify
> > what you expect from me exactly in terms of support for the promiscuous
> > filters we discussed so far?
> >  
> 
> I think for now it's okay to set the device into promiscuous mode and
> enable the flag which checks for bad FCS... we can still implement the
> filter modes later (and I think it should work on all supported
> transceivers (except that SoftMAC/HardMAC thing)).

I considered the following options in order to do that:
1- Hack all ->set_promiscuous() driver implementations to set
   IEEE802154_HW_RX_DROP_BAD_CKSUM as long as it was not already set
   initially.
2- Set the above flag at scan level, ie. in
   scan.c:mac802154_set_promiscuous_mode(). But this would be a bit
   ugly and I'd need to add a persistent field somewhere in the
   wpan_dev structure to remember how the flags settings where before
   the scan code hacked it.
3- Add more code in hwsim to handle checksum manually instead of
   by default setting the above flag to request the core to do the
   job. This way no driver would actually set this flag. We can then
   consider it "volatile" and would not need to track its state.
4- We know that we are in a scan thanks to a mac802154 internal
   variable, we can just assume that all drivers are in promiscuous
   mode and that none of them actually checks the FCS. This is
   certainly the simplest yet effective solution. In the worst case, we
   are just doing the check twice, which I believe does not hurt as
   long as the checksum is not cut off. If the checksum is cut, then
   the core is buggy because it always remove the two last bytes.

I picked 4 for now, but if you think this is unreliable, please
tell me what do you prefer otherwise.

> One point to promiscuous mode, currently we have a checking for if a
> phy is in promiscuous mode on ifup and it would forbid to ifup a node
> interface if the phy is in promiscuous mode (because of the missing
> automatic acknowledgement). I see there is a need to turn the phy into
> promiscuous mode during runtime... so we need somehow make sure the
> constraints are still valid here.

Yes, the code (rx.c) currently drops everything that is not a beacon
during a scan.

> Maybe we even forbid multiple devs
> on a phy if the transceiver/driver/firmware is poor and this is
> currently all transceivers (except hwsim? But that doesn't use any ack
> handling anyway).

Thanks,
Miquèl

Re: [net-next 17/18] net: mac802154: Let drivers provide their own beacons implementation

[Miquel Raynal]

Hi Alexander,

alex.aring@gmail.com wrote on Wed, 5 Jan 2022 19:23:04 -0500:

> Hi,
> 
> On Wed, 5 Jan 2022 at 03:48, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > alex.aring@gmail.com wrote on Thu, 30 Dec 2021 14:48:41 -0500:
> >  
> > > Hi,
> > >
> > > On Thu, 30 Dec 2021 at 12:00, David Girault <David.Girault@qorvo.com> wrote:  
> > > >
> > > > Hi Alexander,
> > > >
> > > > At Qorvo, we have developped a SoftMAC driver for our DW3000 chip that will benefit such API.
> > > >  
> > > Do you want to bring this driver upstream as well? Currently those
> > > callbacks will be introduced but no user is there.  
> >
> > I think so far the upstream fate of the DW3000 driver has not been ruled
> > out so let's assume it won't be upstreamed (at least not fully), that's
> > also why we decided to begin with the hwsim driver.
> >  
> 
> ok.
> 
> > However, when designing this series, it appeared quite clear that any
> > hardMAC driver would need this type of interface. The content of the
> > interface, I agree, could be further discussed and even edited, but the
> > main idea of giving the information to the phy driver about what is
> > happening regarding eg. scan operations or beacon frames, might make
> > sense regardless of the current users, no?
> >  
> 
> A HardMAC driver does not use this driver interface... but there
> exists a SoftMAC driver for a HardMAC transceiver. This driver
> currently works because we use dataframes only... It will not support
> scanning currently and somehow we should make iit not available for
> drivers like that and for drivers which don't set symbol duration.
> They need to be fixed.

My bad. I did not look at it correctly. I made a mistake when talking
about a hardMAC.

Instead, it is a "custom" low level MAC layer. I believe we can compare
the current mac802154 layer mostly to the MLME that is mentioned in the
spec. Well here the additional layer that needs these hooks would be
the MCPS. I don't know if this will be upstreamed or not, but the need
for these hooks is real if such an intermediate low level MAC layer
gets introduced.

In v2 I will get rid of the two patches adding "driver access" to scans
and beacons in order to facilitate the merge of the big part. Then we
will have plenty of time to discuss how we can create such an interface.
Perhaps I'll be able to propose more code as well to make use of these
hooks, we will see.

> > This being said, if other people decide to upstream a hardMAC driver
> > and need these hooks to behave a little bit differently, it's their
> > right to tweak them and that would also be part of the game.
> >
> > Although we might not need these hooks in a near future at all if we
> > move to the filtering modes, because the promiscuous call with the
> > specific level might indicate to the device how it should configure
> > itself already.
> >  
> 
> My concern is that somebody else might want to remove those callbacks
> because they are not used.

Yes, this is likely to happen quickly because of robots :)

Thanks,
Miquèl

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Jakub Kicinski]

On Wed, 5 Jan 2022 19:38:12 -0500 Alexander Aring wrote:
> > Also, just for the record,
> > - should I keep copying the netdev list for v2?  
> 
> yes, why not.

On the question of lists copied it may make sense to CC linux-wireless@
in case they have some precedent to share, and drop linux-kernel@.

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Alexander Aring]

Hi,

On Thu, 6 Jan 2022 at 14:15, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> alex.aring@gmail.com wrote on Wed, 5 Jan 2022 19:38:12 -0500:
>
> > Hi,
> >
> >
> > On Wed, 5 Jan 2022 at 15:55, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > ...
> > > > rest in software is a bigger task here...
> > >
> > > On the symbol duration side I feel I'm close to a working PoC.
> > >
> >
> > oh, ok.
>
> I think it's ready, I'll soon send two series:
> - the symbol duration update
> - v2 for this series, which will not apply without the symbol duration
>   update.
>

ok. Thanks.

> > > So there is 'only' this item left in my mind. Could you please clarify
> > > what you expect from me exactly in terms of support for the promiscuous
> > > filters we discussed so far?
> > >
> >
> > I think for now it's okay to set the device into promiscuous mode and
> > enable the flag which checks for bad FCS... we can still implement the
> > filter modes later (and I think it should work on all supported
> > transceivers (except that SoftMAC/HardMAC thing)).
>
> I considered the following options in order to do that:
> 1- Hack all ->set_promiscuous() driver implementations to set
>    IEEE802154_HW_RX_DROP_BAD_CKSUM as long as it was not already set
>    initially.
> 2- Set the above flag at scan level, ie. in
>    scan.c:mac802154_set_promiscuous_mode(). But this would be a bit
>    ugly and I'd need to add a persistent field somewhere in the
>    wpan_dev structure to remember how the flags settings where before
>    the scan code hacked it.

I think there exists two layers of "promiscuous mode": there exists a
phy level and a mac level. I am not sure at some points what's meant
now. Whereas phy is regarding the filtering mode whatever will be
delivered to mac802154, the wpan (mac) level is what 802.15.4 mac says
it is. The mac promiscuous mode requires the phy promiscuous mode (so
far I understand).

> 3- Add more code in hwsim to handle checksum manually instead of
>    by default setting the above flag to request the core to do the
>    job. This way no driver would actually set this flag. We can then
>    consider it "volatile" and would not need to track its state.
> 4- We know that we are in a scan thanks to a mac802154 internal
>    variable, we can just assume that all drivers are in promiscuous
>    mode and that none of them actually checks the FCS. This is
>    certainly the simplest yet effective solution. In the worst case, we
>    are just doing the check twice, which I believe does not hurt as
>    long as the checksum is not cut off. If the checksum is cut, then
>    the core is buggy because it always remove the two last bytes.
>
> I picked 4 for now, but if you think this is unreliable, please
> tell me what do you prefer otherwise.
>

I think we have some flag to add a calculated checksum
"IEEE802154_HW_RX_OMIT_CKSUM" which is currently not used by any
driver. I think your case that the checksum is cut off does not exist
in 4.? So far I understand we can still move the FCS check to the
hardware by not breaking anything if the hardware supports it and the
behavior should be the same.
So do the 4.?

> > One point to promiscuous mode, currently we have a checking for if a
> > phy is in promiscuous mode on ifup and it would forbid to ifup a node
> > interface if the phy is in promiscuous mode (because of the missing
> > automatic acknowledgement). I see there is a need to turn the phy into
> > promiscuous mode during runtime... so we need somehow make sure the
> > constraints are still valid here.
>
> Yes, the code (rx.c) currently drops everything that is not a beacon
> during a scan.
>

Okay, I will look at this code closely regarding whenever multiple
wpan_devs are running.

You should also check for possible stop of all possible wpan dev
transmit queues, if it's not already done. I suppose a scan can take a
long time and we should not send some data frames out. I am thinking
about the long time scan operation... if we stop the queue for a long
time I think we will drop a lot, however the scan can only be
triggered by the right permissions and the user should be aware of the
side effects. Proper reliable upper layer protocols will care or non
reliable will not care about this.

There still exists the driver "ca8210" which is the mentioned HardMAC
transceiver in SoftMAC. There should somehow be a flag that it cannot
do a scan and the operation should not be allowed as the xmit callback
allows dataframes only.

- Alex

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Alexander Aring]

Hi,

On Thu, 6 Jan 2022 at 20:00, Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 5 Jan 2022 19:38:12 -0500 Alexander Aring wrote:
> > > Also, just for the record,
> > > - should I keep copying the netdev list for v2?
> >
> > yes, why not.
>
> On the question of lists copied it may make sense to CC linux-wireless@
> in case they have some precedent to share, and drop linux-kernel@.

Yes, that makes sense.

- Alex

Re: [net-next 17/18] net: mac802154: Let drivers provide their own beacons implementation

[Alexander Aring]

Hi,

On Thu, 6 Jan 2022 at 14:15, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> alex.aring@gmail.com wrote on Wed, 5 Jan 2022 19:23:04 -0500:
>
...
> >
> > A HardMAC driver does not use this driver interface... but there
> > exists a SoftMAC driver for a HardMAC transceiver. This driver
> > currently works because we use dataframes only... It will not support
> > scanning currently and somehow we should make iit not available for
> > drivers like that and for drivers which don't set symbol duration.
> > They need to be fixed.
>
> My bad. I did not look at it correctly. I made a mistake when talking
> about a hardMAC.
>
> Instead, it is a "custom" low level MAC layer. I believe we can compare
> the current mac802154 layer mostly to the MLME that is mentioned in the
> spec. Well here the additional layer that needs these hooks would be
> the MCPS. I don't know if this will be upstreamed or not, but the need
> for these hooks is real if such an intermediate low level MAC layer
> gets introduced.
>
> In v2 I will get rid of the two patches adding "driver access" to scans
> and beacons in order to facilitate the merge of the big part. Then we
> will have plenty of time to discuss how we can create such an interface.
> Perhaps I'll be able to propose more code as well to make use of these
> hooks, we will see.
>

That the we have a standardised interface between Ieee802154 and
(HardMAC or SoftMAC(mac802154)) (see cfg802154_ops) which is defined
according the spec would make it more "stable" that it will work with
different HardMAC transceivers (which follows that interface) and
mac802154 stack (which also follows that interface). If I understood
you correctly.
I think this is one reason why we are not having any HardMAC
transceivers driver supported in a proper way yet.

I can also imagine about a hwsim HardMAC transceiver which redirects
cfg802154 to mac802154 SoftMAC instance again (something like that),
to have a virtual HardMAC transceiver for testing purpose, etc. In
theory that should work...

- Alex

Re: [net-next 17/18] net: mac802154: Let drivers provide their own beacons implementation

[Miquel Raynal]

Hi Alexander,

alex.aring@gmail.com wrote on Thu, 6 Jan 2022 23:21:45 -0500:

> Hi,
> 
> On Thu, 6 Jan 2022 at 14:15, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > alex.aring@gmail.com wrote on Wed, 5 Jan 2022 19:23:04 -0500:
> >  
> ...
> > >
> > > A HardMAC driver does not use this driver interface... but there
> > > exists a SoftMAC driver for a HardMAC transceiver. This driver
> > > currently works because we use dataframes only... It will not support
> > > scanning currently and somehow we should make iit not available for
> > > drivers like that and for drivers which don't set symbol duration.
> > > They need to be fixed.  
> >
> > My bad. I did not look at it correctly. I made a mistake when talking
> > about a hardMAC.
> >
> > Instead, it is a "custom" low level MAC layer. I believe we can compare
> > the current mac802154 layer mostly to the MLME that is mentioned in the
> > spec. Well here the additional layer that needs these hooks would be
> > the MCPS. I don't know if this will be upstreamed or not, but the need
> > for these hooks is real if such an intermediate low level MAC layer
> > gets introduced.
> >
> > In v2 I will get rid of the two patches adding "driver access" to scans
> > and beacons in order to facilitate the merge of the big part. Then we
> > will have plenty of time to discuss how we can create such an interface.
> > Perhaps I'll be able to propose more code as well to make use of these
> > hooks, we will see.
> >  
> 
> That the we have a standardised interface between Ieee802154 and
> (HardMAC or SoftMAC(mac802154)) (see cfg802154_ops) which is defined
> according the spec would make it more "stable" that it will work with
> different HardMAC transceivers (which follows that interface) and
> mac802154 stack (which also follows that interface). If I understood
> you correctly.


I am not sure. I am really talking about a softMAC. I am not sure
where to put that layer "vertically" but according to the spec the MCPS
(MAC Common Part Sublayer) is the layer that contains the data
primitives, while MLME has been designed for management and
configuration.

> I think this is one reason why we are not having any HardMAC
> transceivers driver supported in a proper way yet.
> 
> I can also imagine about a hwsim HardMAC transceiver which redirects
> cfg802154 to mac802154 SoftMAC instance again (something like that),
> to have a virtual HardMAC transceiver for testing purpose, etc. In
> theory that should work...

Yeah I see what you mean, but IMHO that's basically duplicating the
softMAC layer, we already have hwsim wired to cfg802154 through
mac802154. In a certain way we could argue that this is a hardMAC =)

Thanks,
Miquèl

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Miquel Raynal]

Hi Alexander,

alex.aring@gmail.com wrote on Thu, 6 Jan 2022 20:07:24 -0500:

> Hi,
> 
> On Thu, 6 Jan 2022 at 14:15, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > alex.aring@gmail.com wrote on Wed, 5 Jan 2022 19:38:12 -0500:
> >  
> > > Hi,
> > >
> > >
> > > On Wed, 5 Jan 2022 at 15:55, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > ...  
> > > > > rest in software is a bigger task here...  
> > > >
> > > > On the symbol duration side I feel I'm close to a working PoC.
> > > >  
> > >
> > > oh, ok.  
> >
> > I think it's ready, I'll soon send two series:
> > - the symbol duration update
> > - v2 for this series, which will not apply without the symbol duration
> >   update.
> >  
> 
> ok. Thanks.
> 
> > > > So there is 'only' this item left in my mind. Could you please clarify
> > > > what you expect from me exactly in terms of support for the promiscuous
> > > > filters we discussed so far?
> > > >  
> > >
> > > I think for now it's okay to set the device into promiscuous mode and
> > > enable the flag which checks for bad FCS... we can still implement the
> > > filter modes later (and I think it should work on all supported
> > > transceivers (except that SoftMAC/HardMAC thing)).  
> >
> > I considered the following options in order to do that:
> > 1- Hack all ->set_promiscuous() driver implementations to set
> >    IEEE802154_HW_RX_DROP_BAD_CKSUM as long as it was not already set
> >    initially.
> > 2- Set the above flag at scan level, ie. in
> >    scan.c:mac802154_set_promiscuous_mode(). But this would be a bit
> >    ugly and I'd need to add a persistent field somewhere in the
> >    wpan_dev structure to remember how the flags settings where before
> >    the scan code hacked it.  
> 
> I think there exists two layers of "promiscuous mode": there exists a
> phy level and a mac level. I am not sure at some points what's meant
> now. Whereas phy is regarding the filtering mode whatever will be
> delivered to mac802154, the wpan (mac) level is what 802.15.4 mac says
> it is. The mac promiscuous mode requires the phy promiscuous mode (so
> far I understand).
> 
> > 3- Add more code in hwsim to handle checksum manually instead of
> >    by default setting the above flag to request the core to do the
> >    job. This way no driver would actually set this flag. We can then
> >    consider it "volatile" and would not need to track its state.
> > 4- We know that we are in a scan thanks to a mac802154 internal
> >    variable, we can just assume that all drivers are in promiscuous
> >    mode and that none of them actually checks the FCS. This is
> >    certainly the simplest yet effective solution. In the worst case, we
> >    are just doing the check twice, which I believe does not hurt as
> >    long as the checksum is not cut off. If the checksum is cut, then
> >    the core is buggy because it always remove the two last bytes.
> >
> > I picked 4 for now, but if you think this is unreliable, please
> > tell me what do you prefer otherwise.
> >  
> 
> I think we have some flag to add a calculated checksum
> "IEEE802154_HW_RX_OMIT_CKSUM" which is currently not used by any
> driver. I think your case that the checksum is cut off does not exist
> in 4.? So far I understand we can still move the FCS check to the
> hardware by not breaking anything if the hardware supports it and the
> behavior should be the same.

That is correct.

> So do the 4.?

Done, thanks!

> > > One point to promiscuous mode, currently we have a checking for if a
> > > phy is in promiscuous mode on ifup and it would forbid to ifup a node
> > > interface if the phy is in promiscuous mode (because of the missing
> > > automatic acknowledgement). I see there is a need to turn the phy into
> > > promiscuous mode during runtime... so we need somehow make sure the
> > > constraints are still valid here.  
> >
> > Yes, the code (rx.c) currently drops everything that is not a beacon
> > during a scan.
> >  
> 
> Okay, I will look at this code closely regarding whenever multiple
> wpan_devs are running.

The "scanning" boolean is stored as a wpan_phy member (IIRC) so we
should be good on this regard (now that I have a clearer picture of the
dependencies).

> You should also check for possible stop of all possible wpan dev
> transmit queues, if it's not already done.

I forgot about this path. Indeed I'll add a check in the transmit path
as well, of course.

> I suppose a scan can take a
> long time and we should not send some data frames out. I am thinking
> about the long time scan operation... if we stop the queue for a long
> time I think we will drop a lot, however the scan can only be
> triggered by the right permissions and the user should be aware of the
> side effects. Proper reliable upper layer protocols will care or non
> reliable will not care about this.
> 
> There still exists the driver "ca8210" which is the mentioned HardMAC
> transceiver in SoftMAC. There should somehow be a flag that it cannot
> do a scan and the operation should not be allowed as the xmit callback
> allows dataframes only.

So it cannot do an active scan, but a passive scan would be allowed
(there is no transmission, and the beacons are regular valid frames,
I suppose they should not be filtered out by the hardware).

So we actually need these hooks back :-) Because the right thing to do
here is to use the "FYI here is the scan op that is starting" message
from the mac to the drivers and this driver should return "nope,
-ENOTSUPP". The mac would react in this case by canceling the
operation and returning an error to the caller. Same when sending
beacons if we consider beacons as !dataframes.

Thanks,
Miquèl

Re: [net-next 12/18] net: mac802154: Handle scan requests

[Alexander Aring]

Hi,

On Fri, 7 Jan 2022 at 06:02, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
...
> > >
> >
> > Okay, I will look at this code closely regarding whenever multiple
> > wpan_devs are running.
>
> The "scanning" boolean is stored as a wpan_phy member (IIRC) so we
> should be good on this regard (now that I have a clearer picture of the
> dependencies).
>

ok.

> > You should also check for possible stop of all possible wpan dev
> > transmit queues, if it's not already done.
>
> I forgot about this path. Indeed I'll add a check in the transmit path
> as well, of course.
>

What I mean is look into the functions "ieee802154_stop_queue()" and
"ieee802154_wake_queue()".

> > I suppose a scan can take a
> > long time and we should not send some data frames out. I am thinking
> > about the long time scan operation... if we stop the queue for a long
> > time I think we will drop a lot, however the scan can only be
> > triggered by the right permissions and the user should be aware of the
> > side effects. Proper reliable upper layer protocols will care or non
> > reliable will not care about this.
> >
> > There still exists the driver "ca8210" which is the mentioned HardMAC
> > transceiver in SoftMAC. There should somehow be a flag that it cannot
> > do a scan and the operation should not be allowed as the xmit callback
> > allows dataframes only.
>
> So it cannot do an active scan, but a passive scan would be allowed
> (there is no transmission, and the beacons are regular valid frames,
> I suppose they should not be filtered out by the hardware).
>
> So we actually need these hooks back :-) Because the right thing to do
> here is to use the "FYI here is the scan op that is starting" message
> from the mac to the drivers and this driver should return "nope,
> -ENOTSUPP". The mac would react in this case by canceling the
> operation and returning an error to the caller. Same when sending
> beacons if we consider beacons as !dataframes.
>

I believe that a HardMAC transceiver will handle a lot of the scan
process on the transceiver side and is only capable of dumping what
it's stored and start/stop scan? This is one reason why the
scan/dump/etc cfg802154 interface  should be close to the standard
(MLME). At the end it should from userspace make no difference if at
the end is a HardMAC or SoftMAC. Although there will always be a
limitation and a SoftMAC is probably always "more" powerful than a
HardMAC, but at some point there is a "minimum" of capable
functionality... and SoftMAC transceivers might always have "some"
things which they might offload to the hardware.

- Alex

Re: [net-next 17/18] net: mac802154: Let drivers provide their own beacons implementation

[Alexander Aring]

Hi,

On Fri, 7 Jan 2022 at 02:40, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> alex.aring@gmail.com wrote on Thu, 6 Jan 2022 23:21:45 -0500:
>
> > Hi,
> >
> > On Thu, 6 Jan 2022 at 14:15, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Alexander,
> > >
> > > alex.aring@gmail.com wrote on Wed, 5 Jan 2022 19:23:04 -0500:
> > >
> > ...
> > > >
> > > > A HardMAC driver does not use this driver interface... but there
> > > > exists a SoftMAC driver for a HardMAC transceiver. This driver
> > > > currently works because we use dataframes only... It will not support
> > > > scanning currently and somehow we should make iit not available for
> > > > drivers like that and for drivers which don't set symbol duration.
> > > > They need to be fixed.
> > >
> > > My bad. I did not look at it correctly. I made a mistake when talking
> > > about a hardMAC.
> > >
> > > Instead, it is a "custom" low level MAC layer. I believe we can compare
> > > the current mac802154 layer mostly to the MLME that is mentioned in the
> > > spec. Well here the additional layer that needs these hooks would be
> > > the MCPS. I don't know if this will be upstreamed or not, but the need
> > > for these hooks is real if such an intermediate low level MAC layer
> > > gets introduced.
> > >
> > > In v2 I will get rid of the two patches adding "driver access" to scans
> > > and beacons in order to facilitate the merge of the big part. Then we
> > > will have plenty of time to discuss how we can create such an interface.
> > > Perhaps I'll be able to propose more code as well to make use of these
> > > hooks, we will see.
> > >
> >
> > That the we have a standardised interface between Ieee802154 and
> > (HardMAC or SoftMAC(mac802154)) (see cfg802154_ops) which is defined
> > according the spec would make it more "stable" that it will work with
> > different HardMAC transceivers (which follows that interface) and
> > mac802154 stack (which also follows that interface). If I understood
> > you correctly.
>
>
> I am not sure. I am really talking about a softMAC. I am not sure
> where to put that layer "vertically" but according to the spec the MCPS
> (MAC Common Part Sublayer) is the layer that contains the data
> primitives, while MLME has been designed for management and
> configuration.
>

ok.

> > I think this is one reason why we are not having any HardMAC
> > transceivers driver supported in a proper way yet.
> >
> > I can also imagine about a hwsim HardMAC transceiver which redirects
> > cfg802154 to mac802154 SoftMAC instance again (something like that),
> > to have a virtual HardMAC transceiver for testing purpose, etc. In
> > theory that should work...
>
> Yeah I see what you mean, but IMHO that's basically duplicating the
> softMAC layer, we already have hwsim wired to cfg802154 through
> mac802154. In a certain way we could argue that this is a hardMAC =)

Would be good to show people "here is how to write a HardMAC
driver..." if this is even possible without any change yet.

- Alex