[PATCH wpan-next 0/6] IEEE 802.15.4 passive scan support

[PATCH wpan-next 0/6] IEEE 802.15.4 passive scan support

[Miquel Raynal]

Hello,

We now have the infrastructure to report beacons/PANs, we also have the
capability to transmit MLME commands synchronously. It is time to use
these to implement a proper scan implementation.

There are a few side-changes which are necessary for the soft MAC scan
implementation to compile/work, but nothing big. The two main changes
are:
* The introduction of a user API for managing scans.
* The soft MAC implementation of a scan.

In all the past, current and future submissions, David and Romuald from
Qorvo are credited in various ways (main author, co-author,
suggested-by) depending of the amount of rework that was involved on
each patch, reflecting as much as possible the open-source guidelines we
follow in the kernel. All this effort is made possible thanks to Qorvo
Inc which is pushing towards a featureful upstream WPAN support.

Cheers,
Miquèl

Miquel Raynal (6):
  ieee802154: Add support for user scanning requests
  ieee802154: Define a beacon frame header
  ieee802154: Introduce a helper to validate a channel
  mac802154: Prepare forcing specific symbol duration
  mac802154: Add MLME Tx locked helpers
  mac802154: Handle passive scanning

 include/linux/ieee802154.h      |   7 +
 include/net/cfg802154.h         |  55 +++++-
 include/net/ieee802154_netdev.h |  36 ++++
 include/net/nl802154.h          |  49 ++++++
 net/ieee802154/nl802154.c       | 218 +++++++++++++++++++++++-
 net/ieee802154/nl802154.h       |   3 +
 net/ieee802154/rdev-ops.h       |  28 ++++
 net/ieee802154/trace.h          |  40 +++++
 net/mac802154/Makefile          |   2 +-
 net/mac802154/cfg.c             |  33 +++-
 net/mac802154/ieee802154_i.h    |  43 ++++-
 net/mac802154/iface.c           |   3 +
 net/mac802154/main.c            |  36 ++--
 net/mac802154/rx.c              |  36 +++-
 net/mac802154/scan.c            | 286 ++++++++++++++++++++++++++++++++
 net/mac802154/tx.c              |  42 +++--
 16 files changed, 885 insertions(+), 32 deletions(-)
 create mode 100644 net/mac802154/scan.c

-- 
2.34.1

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

[Miquel Raynal]

The ieee802154 layer should be able to scan a set of channels in order
to look for beacons advertizing PANs. Supporting this involves adding
two user commands: triggering scans and aborting scans. The user should
also be notified when a new beacon is received and also upon scan
termination.

A scan request structure is created to list the requirements and to be
accessed asynchronously when changing channels or receiving beacons.

Mac layers may now implement the ->trigger_scan() and ->abort_scan()
hooks.

Co-developed-by: David Girault <david.girault@qorvo.com>
Signed-off-by: David Girault <david.girault@qorvo.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/linux/ieee802154.h |   3 +
 include/net/cfg802154.h    |  25 +++++
 include/net/nl802154.h     |  49 +++++++++
 net/ieee802154/nl802154.c  | 215 +++++++++++++++++++++++++++++++++++++
 net/ieee802154/nl802154.h  |   3 +
 net/ieee802154/rdev-ops.h  |  28 +++++
 net/ieee802154/trace.h     |  40 +++++++
 7 files changed, 363 insertions(+)

diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
index 0303eb84d596..b22e4147d334 100644
--- a/include/linux/ieee802154.h
+++ b/include/linux/ieee802154.h
@@ -44,6 +44,9 @@
 #define IEEE802154_SHORT_ADDR_LEN	2
 #define IEEE802154_PAN_ID_LEN		2
 
+/* Duration in superframe order */
+#define IEEE802154_MAX_SCAN_DURATION	14
+#define IEEE802154_ACTIVE_SCAN_DURATION	15
 #define IEEE802154_LIFS_PERIOD		40
 #define IEEE802154_SIFS_PERIOD		12
 #define IEEE802154_MAX_SIFS_FRAME_SIZE	18
diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index d09c393d229f..76d4f95e9974 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -18,6 +18,7 @@
 
 struct wpan_phy;
 struct wpan_phy_cca;
+struct cfg802154_scan_request;
 
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 struct ieee802154_llsec_device_key;
@@ -67,6 +68,10 @@ struct cfg802154_ops {
 				struct wpan_dev *wpan_dev, bool mode);
 	int	(*set_ackreq_default)(struct wpan_phy *wpan_phy,
 				      struct wpan_dev *wpan_dev, bool ackreq);
+	int	(*trigger_scan)(struct wpan_phy *wpan_phy,
+				struct cfg802154_scan_request *request);
+	int	(*abort_scan)(struct wpan_phy *wpan_phy,
+			      struct wpan_dev *wpan_dev);
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	void	(*get_llsec_table)(struct wpan_phy *wpan_phy,
 				   struct wpan_dev *wpan_dev,
@@ -278,6 +283,26 @@ struct ieee802154_coord_desc {
 	bool gts_permit;
 };
 
+/**
+ * struct cfg802154_scan_request - Scan request
+ *
+ * @type: type of scan to be performed
+ * @page: page on which to perform the scan
+ * @channels: channels in te %page to be scanned
+ * @duration: time spent on each channel, calculated with:
+ *            aBaseSuperframeDuration * (2 ^ duration + 1)
+ * @wpan_dev: the wpan device on which to perform the scan
+ * @wpan_phy: the wpan phy on which to perform the scan
+ */
+struct cfg802154_scan_request {
+	enum nl802154_scan_types type;
+	u8 page;
+	u32 channels;
+	u8 duration;
+	struct wpan_dev *wpan_dev;
+	struct wpan_phy *wpan_phy;
+};
+
 struct ieee802154_llsec_key_id {
 	u8 mode;
 	u8 id;
diff --git a/include/net/nl802154.h b/include/net/nl802154.h
index b79a89d5207c..79fbd820b25a 100644
--- a/include/net/nl802154.h
+++ b/include/net/nl802154.h
@@ -73,6 +73,9 @@ enum nl802154_commands {
 	NL802154_CMD_DEL_SEC_LEVEL,
 
 	NL802154_CMD_SCAN_EVENT,
+	NL802154_CMD_TRIGGER_SCAN,
+	NL802154_CMD_ABORT_SCAN,
+	NL802154_CMD_SCAN_DONE,
 
 	/* add new commands above here */
 
@@ -134,6 +137,12 @@ enum nl802154_attrs {
 	NL802154_ATTR_NETNS_FD,
 
 	NL802154_ATTR_COORDINATOR,
+	NL802154_ATTR_SCAN_TYPE,
+	NL802154_ATTR_SCAN_FLAGS,
+	NL802154_ATTR_SCAN_CHANNELS,
+	NL802154_ATTR_SCAN_PREAMBLE_CODES,
+	NL802154_ATTR_SCAN_MEAN_PRF,
+	NL802154_ATTR_SCAN_DURATION,
 
 	/* add attributes here, update the policy in nl802154.c */
 
@@ -259,6 +268,46 @@ enum nl802154_coord {
 	NL802154_COORD_MAX,
 };
 
+/**
+ * enum nl802154_scan_types - Scan types
+ *
+ * @__NL802154_SCAN_INVALID: scan type number 0 is reserved
+ * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak
+ *	energy in each requested channel
+ * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using
+ *	a Beacon Request command
+ * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames
+ * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation
+ * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request
+ *	command instead of Beacon Request command
+ * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames
+ *	instead of Beacon frames
+ * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number
+ */
+enum nl802154_scan_types {
+	__NL802154_SCAN_INVALID,
+	NL802154_SCAN_ED,
+	NL802154_SCAN_ACTIVE,
+	NL802154_SCAN_PASSIVE,
+	NL802154_SCAN_ORPHAN,
+	NL802154_SCAN_ENHANCED_ACTIVE,
+	NL802154_SCAN_RIT_PASSIVE,
+
+	/* keep last */
+	NL802154_SCAN_ATTR_MAX,
+};
+
+/**
+ * enum nl802154_scan_flags - Scan request control flags
+ *
+ * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie.
+ *	a different one for every scan iteration). When the flag is set, full
+ *	randomisation is assumed.
+ */
+enum nl802154_scan_flags {
+	NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0),
+};
+
 /**
  * enum nl802154_cca_modes - cca modes
  *
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 80dc73182785..c497ffd8e897 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
 
 	[NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED },
 
+	[NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 },
+	[NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 },
+	[NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 },
+	[NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 },
+	[NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 },
+
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	[NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
 	[NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, },
@@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
 }
 EXPORT_SYMBOL_GPL(nl802154_scan_event);
 
+static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg802154_registered_device *rdev = info->user_ptr[0];
+	struct net_device *dev = info->user_ptr[1];
+	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
+	struct wpan_phy *wpan_phy = &rdev->wpan_phy;
+	struct cfg802154_scan_request *request;
+	u8 type;
+	int err;
+
+	/* Monitors are not allowed to perform scans */
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+		return -EPERM;
+
+	request = kzalloc(sizeof(*request), GFP_KERNEL);
+	if (!request)
+		return -ENOMEM;
+
+	request->wpan_dev = wpan_dev;
+	request->wpan_phy = wpan_phy;
+
+	type = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_TYPE]);
+	switch (type) {
+	case NL802154_SCAN_PASSIVE:
+		request->type = type;
+		break;
+	default:
+		pr_err("Unsupported scan type: %d\n", type);
+		err = -EINVAL;
+		goto free_request;
+	}
+
+	if (info->attrs[NL802154_ATTR_PAGE]) {
+		request->page = nla_get_u8(info->attrs[NL802154_ATTR_PAGE]);
+		if (request->page > IEEE802154_MAX_PAGE) {
+			pr_err("Invalid page %d > %d\n",
+			       request->page, IEEE802154_MAX_PAGE);
+			err = -EINVAL;
+			goto free_request;
+		}
+	} else {
+		/* Use current page by default */
+		request->page = wpan_phy->current_page;
+	}
+
+	if (info->attrs[NL802154_ATTR_SCAN_CHANNELS]) {
+		request->channels = nla_get_u32(info->attrs[NL802154_ATTR_SCAN_CHANNELS]);
+		if (request->channels >= BIT(IEEE802154_MAX_CHANNEL + 1)) {
+			pr_err("Invalid channels bitfield %x ≥ %lx\n",
+			       request->channels,
+			       BIT(IEEE802154_MAX_CHANNEL + 1));
+			err = -EINVAL;
+			goto free_request;
+		}
+	} else {
+		/* Scan all supported channels by default */
+		request->channels = wpan_phy->supported.channels[request->page];
+	}
+
+	if (info->attrs[NL802154_ATTR_SCAN_PREAMBLE_CODES] ||
+	    info->attrs[NL802154_ATTR_SCAN_MEAN_PRF]) {
+		pr_err("Preamble codes and mean PRF not supported yet\n");
+		err = -EINVAL;
+		goto free_request;
+	}
+
+	if (info->attrs[NL802154_ATTR_SCAN_DURATION]) {
+		request->duration = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_DURATION]);
+		if (request->duration > IEEE802154_MAX_SCAN_DURATION) {
+			pr_err("Duration is out of range\n");
+			err = -EINVAL;
+			goto free_request;
+		}
+	} else {
+		/* Use maximum duration order by default */
+		request->duration = IEEE802154_MAX_SCAN_DURATION;
+	}
+
+	if (wpan_dev->netdev)
+		dev_hold(wpan_dev->netdev);
+
+	err = rdev_trigger_scan(rdev, request);
+	if (err) {
+		pr_err("Failure starting scanning (%d)\n", err);
+		goto free_device;
+	}
+
+	return 0;
+
+free_device:
+	if (wpan_dev->netdev)
+		dev_put(wpan_dev->netdev);
+free_request:
+	kfree(request);
+
+	return err;
+}
+
+static int nl802154_prep_scan_msg(struct sk_buff *msg,
+				  struct cfg802154_registered_device *rdev,
+				  struct wpan_dev *wpan_dev,
+				  u32 portid, u32 seq, int flags, u8 cmd)
+{
+	void *hdr;
+
+	hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
+	if (!hdr)
+		return -ENOBUFS;
+
+	if (nla_put_u32(msg, NL802154_ATTR_WPAN_PHY, rdev->wpan_phy_idx))
+		goto nla_put_failure;
+
+	if (wpan_dev->netdev &&
+	    nla_put_u32(msg, NL802154_ATTR_IFINDEX, wpan_dev->netdev->ifindex))
+		goto nla_put_failure;
+
+	if (nla_put_u64_64bit(msg, NL802154_ATTR_WPAN_DEV,
+			      wpan_dev_id(wpan_dev), NL802154_ATTR_PAD))
+		goto nla_put_failure;
+
+	genlmsg_end(msg, hdr);
+
+	return 0;
+
+nla_put_failure:
+	genlmsg_cancel(msg, hdr);
+
+	return -EMSGSIZE;
+}
+
+static int nl802154_send_scan_msg(struct cfg802154_registered_device *rdev,
+				  struct wpan_dev *wpan_dev, u8 cmd)
+{
+	struct sk_buff *msg;
+	int ret;
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	ret = nl802154_prep_scan_msg(msg, rdev, wpan_dev, 0, 0, 0, cmd);
+	if (ret < 0) {
+		nlmsg_free(msg);
+		return ret;
+	}
+
+	return genlmsg_multicast_netns(&nl802154_fam,
+				       wpan_phy_net(&rdev->wpan_phy), msg, 0,
+				       NL802154_MCGRP_SCAN, GFP_KERNEL);
+}
+
+int nl802154_scan_started(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev)
+{
+	struct cfg802154_registered_device *rdev = wpan_phy_to_rdev(wpan_phy);
+	int err;
+
+	/* Ignore errors when there are no listeners */
+	err = nl802154_send_scan_msg(rdev, wpan_dev, NL802154_CMD_TRIGGER_SCAN);
+	if (err == -ESRCH)
+		err = 0;
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(nl802154_scan_started);
+
+int nl802154_scan_done(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
+		       u8 cmd)
+{
+	struct cfg802154_registered_device *rdev = wpan_phy_to_rdev(wpan_phy);
+	int err;
+
+	/* Ignore errors when there are no listeners */
+	err = nl802154_send_scan_msg(rdev, wpan_dev, cmd);
+	if (err == -ESRCH)
+		err = 0;
+
+	if (wpan_dev->netdev)
+		dev_put(wpan_dev->netdev);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(nl802154_scan_done);
+
+static int nl802154_abort_scan(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg802154_registered_device *rdev = info->user_ptr[0];
+	struct net_device *dev = info->user_ptr[1];
+	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
+
+	/* Resources are released in the notification helper above */
+	return rdev_abort_scan(rdev, wpan_dev);
+}
+
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 static const struct nla_policy nl802154_dev_addr_policy[NL802154_DEV_ADDR_ATTR_MAX + 1] = {
 	[NL802154_DEV_ADDR_ATTR_PAN_ID] = { .type = NLA_U16 },
@@ -2472,6 +2671,22 @@ static const struct genl_ops nl802154_ops[] = {
 		.internal_flags = NL802154_FLAG_NEED_NETDEV |
 				  NL802154_FLAG_NEED_RTNL,
 	},
+	{
+		.cmd = NL802154_CMD_TRIGGER_SCAN,
+		.doit = nl802154_trigger_scan,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL802154_FLAG_NEED_NETDEV |
+				  NL802154_FLAG_CHECK_NETDEV_UP |
+				  NL802154_FLAG_NEED_RTNL,
+	},
+	{
+		.cmd = NL802154_CMD_ABORT_SCAN,
+		.doit = nl802154_abort_scan,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL802154_FLAG_NEED_NETDEV |
+				  NL802154_FLAG_CHECK_NETDEV_UP |
+				  NL802154_FLAG_NEED_RTNL,
+	},
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	{
 		.cmd = NL802154_CMD_SET_SEC_PARAMS,
diff --git a/net/ieee802154/nl802154.h b/net/ieee802154/nl802154.h
index 89b805500032..88b827a42324 100644
--- a/net/ieee802154/nl802154.h
+++ b/net/ieee802154/nl802154.h
@@ -6,5 +6,8 @@ int nl802154_init(void);
 void nl802154_exit(void);
 int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
 			struct ieee802154_coord_desc *desc);
+int nl802154_scan_started(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev);
+int nl802154_scan_done(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
+		       u8 cmd);
 
 #endif /* __IEEE802154_NL802154_H */
diff --git a/net/ieee802154/rdev-ops.h b/net/ieee802154/rdev-ops.h
index 598f5af49775..e171d74c3251 100644
--- a/net/ieee802154/rdev-ops.h
+++ b/net/ieee802154/rdev-ops.h
@@ -209,6 +209,34 @@ rdev_set_ackreq_default(struct cfg802154_registered_device *rdev,
 	return ret;
 }
 
+static inline int rdev_trigger_scan(struct cfg802154_registered_device *rdev,
+				    struct cfg802154_scan_request *request)
+{
+	int ret;
+
+	if (!rdev->ops->trigger_scan)
+		return -EOPNOTSUPP;
+
+	trace_802154_rdev_trigger_scan(&rdev->wpan_phy, request);
+	ret = rdev->ops->trigger_scan(&rdev->wpan_phy, request);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
+}
+
+static inline int rdev_abort_scan(struct cfg802154_registered_device *rdev,
+				  struct wpan_dev *wpan_dev)
+{
+	int ret;
+
+	if (!rdev->ops->abort_scan)
+		return -EOPNOTSUPP;
+
+	trace_802154_rdev_abort_scan(&rdev->wpan_phy, wpan_dev);
+	ret = rdev->ops->abort_scan(&rdev->wpan_phy, wpan_dev);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
+}
+
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 /* TODO this is already a nl802154, so move into ieee802154 */
 static inline void
diff --git a/net/ieee802154/trace.h b/net/ieee802154/trace.h
index 19c2e5d60e76..e5405f737ded 100644
--- a/net/ieee802154/trace.h
+++ b/net/ieee802154/trace.h
@@ -295,6 +295,46 @@ TRACE_EVENT(802154_rdev_set_ackreq_default,
 		WPAN_DEV_PR_ARG, BOOL_TO_STR(__entry->ackreq))
 );
 
+TRACE_EVENT(802154_rdev_trigger_scan,
+	TP_PROTO(struct wpan_phy *wpan_phy,
+		 struct cfg802154_scan_request *request),
+	TP_ARGS(wpan_phy, request),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		__field(u8, page)
+		__field(u32, channels)
+		__field(u8, duration)
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		__entry->page = request->page;
+		__entry->channels = request->channels;
+		__entry->duration = request->duration;
+	),
+	TP_printk(WPAN_PHY_PR_FMT ", scan, page: %d, channels: %x, duration %d",
+		  WPAN_PHY_PR_ARG, __entry->page, __entry->channels, __entry->duration)
+);
+
+DECLARE_EVENT_CLASS(802154_wdev_template,
+	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev),
+	TP_ARGS(wpan_phy, wpan_dev),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		WPAN_DEV_ENTRY
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		WPAN_DEV_ASSIGN;
+	),
+	TP_printk(WPAN_PHY_PR_FMT ", " WPAN_DEV_PR_FMT,
+		  WPAN_PHY_PR_ARG, WPAN_DEV_PR_ARG)
+);
+
+DEFINE_EVENT(802154_wdev_template, 802154_rdev_abort_scan,
+	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev),
+	TP_ARGS(wpan_phy, wpan_dev)
+);
+
 TRACE_EVENT(802154_rdev_return_int,
 	TP_PROTO(struct wpan_phy *wpan_phy, int ret),
 	TP_ARGS(wpan_phy, ret),
-- 
2.34.1

[PATCH wpan-next 2/6] 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 4c33a20ea57f..2f2196049a86 100644
--- a/include/net/ieee802154_netdev.h
+++ b/include/net/ieee802154_netdev.h
@@ -38,6 +38,42 @@
 
 #include <net/cfg802154.h>
 
+struct ieee802154_beacon_hdr {
+#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.34.1

[PATCH wpan-next 3/6] ieee802154: Introduce a helper to validate a channel

[Miquel Raynal]

This helper for now only checks if the page member and channel member
are valid (in the specification range) and supported (by checking the
device capabilities). Soon two new parameters will be introduced and
having this helper will let us only modify its content rather than
modifying the logic everywhere else in the subsystem.

There is not functional change.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/cfg802154.h   | 11 +++++++++++
 net/ieee802154/nl802154.c |  3 +--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 76d4f95e9974..11bedfa96371 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -246,6 +246,17 @@ static inline void wpan_phy_net_set(struct wpan_phy *wpan_phy, struct net *net)
 	write_pnet(&wpan_phy->_net, net);
 }
 
+static inline bool ieee802154_chan_is_valid(struct wpan_phy *phy,
+                                            u8 page, u8 channel)
+{
+        if (page > IEEE802154_MAX_PAGE ||
+            channel > IEEE802154_MAX_CHANNEL ||
+            !(phy->supported.channels[page] & BIT(channel)))
+                return false;
+
+	return true;
+}
+
 /**
  * struct ieee802154_addr - IEEE802.15.4 device address
  * @mode: Address mode from frame header. Can be one of:
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index c497ffd8e897..d0c169e8b1b7 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -975,8 +975,7 @@ static int nl802154_set_channel(struct sk_buff *skb, struct genl_info *info)
 	channel = nla_get_u8(info->attrs[NL802154_ATTR_CHANNEL]);
 
 	/* check 802.15.4 constraints */
-	if (page > IEEE802154_MAX_PAGE || channel > IEEE802154_MAX_CHANNEL ||
-	    !(rdev->wpan_phy.supported.channels[page] & BIT(channel)))
+	if (!ieee802154_chan_is_valid(&rdev->wpan_phy, page, channel))
 		return -EINVAL;
 
 	return rdev_set_channel(rdev, page, channel);
-- 
2.34.1

[PATCH wpan-next 4/6] mac802154: Prepare forcing specific symbol duration

[Miquel Raynal]

The scan logic will bypass the whole ->set_channel() logic from the top
by calling the driver hook to just switch between channels when
required.

We can no longer rely on the "current" page/channel settings to set the
right symbol duration. Let's add these as new parameters to allow
providing the page/channel couple that we want.

There is no functional change.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/cfg802154.h |  3 ++-
 net/mac802154/cfg.c     |  2 +-
 net/mac802154/main.c    | 20 +++++++++++---------
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 11bedfa96371..5d4750d24f13 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -483,6 +483,7 @@ static inline const char *wpan_phy_name(struct wpan_phy *phy)
 	return dev_name(&phy->dev);
 }
 
-void ieee802154_configure_durations(struct wpan_phy *phy);
+void ieee802154_configure_durations(struct wpan_phy *phy,
+				    unsigned int page, unsigned int channel);
 
 #endif /* __NET_CFG802154_H */
diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c
index dc2d918fac68..469d6e8dd2dd 100644
--- a/net/mac802154/cfg.c
+++ b/net/mac802154/cfg.c
@@ -118,7 +118,7 @@ ieee802154_set_channel(struct wpan_phy *wpan_phy, u8 page, u8 channel)
 	if (!ret) {
 		wpan_phy->current_page = page;
 		wpan_phy->current_channel = channel;
-		ieee802154_configure_durations(wpan_phy);
+		ieee802154_configure_durations(wpan_phy, page, channel);
 	}
 
 	return ret;
diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index 3ed31daf7b9c..12a13a850fdf 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -113,32 +113,33 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
 }
 EXPORT_SYMBOL(ieee802154_alloc_hw);
 
-void ieee802154_configure_durations(struct wpan_phy *phy)
+void ieee802154_configure_durations(struct wpan_phy *phy,
+				    unsigned int page, unsigned int channel)
 {
 	u32 duration = 0;
 
-	switch (phy->current_page) {
+	switch (page) {
 	case 0:
-		if (BIT(phy->current_channel) & 0x1)
+		if (BIT(channel) & 0x1)
 			/* 868 MHz BPSK 802.15.4-2003: 20 ksym/s */
 			duration = 50 * NSEC_PER_USEC;
-		else if (BIT(phy->current_channel) & 0x7FE)
+		else if (BIT(channel) & 0x7FE)
 			/* 915 MHz BPSK	802.15.4-2003: 40 ksym/s */
 			duration = 25 * NSEC_PER_USEC;
-		else if (BIT(phy->current_channel) & 0x7FFF800)
+		else if (BIT(channel) & 0x7FFF800)
 			/* 2400 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
 			duration = 16 * NSEC_PER_USEC;
 		break;
 	case 2:
-		if (BIT(phy->current_channel) & 0x1)
+		if (BIT(channel) & 0x1)
 			/* 868 MHz O-QPSK 802.15.4-2006: 25 ksym/s */
 			duration = 40 * NSEC_PER_USEC;
-		else if (BIT(phy->current_channel) & 0x7FE)
+		else if (BIT(channel) & 0x7FE)
 			/* 915 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
 			duration = 16 * NSEC_PER_USEC;
 		break;
 	case 3:
-		if (BIT(phy->current_channel) & 0x3FFF)
+		if (BIT(channel) & 0x3FFF)
 			/* 2.4 GHz CSS 802.15.4a-2007: 1/6 Msym/s */
 			duration = 6 * NSEC_PER_USEC;
 		break;
@@ -201,7 +202,8 @@ int ieee802154_register_hw(struct ieee802154_hw *hw)
 
 	ieee802154_setup_wpan_phy_pib(local->phy);
 
-	ieee802154_configure_durations(local->phy);
+	ieee802154_configure_durations(local->phy, local->phy->current_page,
+				       local->phy->current_channel);
 
 	if (!(hw->flags & IEEE802154_HW_CSMA_PARAMS)) {
 		local->phy->supported.min_csma_backoffs = 4;
-- 
2.34.1

[PATCH wpan-next 5/6] mac802154: Add MLME Tx locked helpers

[Miquel Raynal]

These have the exact same behavior as before, except they expect the
rtnl to be already taken (and will complain otherwise). This allows
performing MLME transmissions from different contexts.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 net/mac802154/ieee802154_i.h |  6 ++++++
 net/mac802154/tx.c           | 42 +++++++++++++++++++++++++-----------
 2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h
index 509e0172fe82..aeadee543a9c 100644
--- a/net/mac802154/ieee802154_i.h
+++ b/net/mac802154/ieee802154_i.h
@@ -141,10 +141,16 @@ int ieee802154_mlme_op_pre(struct ieee802154_local *local);
 int ieee802154_mlme_tx(struct ieee802154_local *local,
 		       struct ieee802154_sub_if_data *sdata,
 		       struct sk_buff *skb);
+int ieee802154_mlme_tx_locked(struct ieee802154_local *local,
+			      struct ieee802154_sub_if_data *sdata,
+			      struct sk_buff *skb);
 void ieee802154_mlme_op_post(struct ieee802154_local *local);
 int ieee802154_mlme_tx_one(struct ieee802154_local *local,
 			   struct ieee802154_sub_if_data *sdata,
 			   struct sk_buff *skb);
+int ieee802154_mlme_tx_one_locked(struct ieee802154_local *local,
+				  struct ieee802154_sub_if_data *sdata,
+				  struct sk_buff *skb);
 netdev_tx_t
 ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev);
 netdev_tx_t
diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c
index 9d8d43cf1e64..2a6f1ed763c9 100644
--- a/net/mac802154/tx.c
+++ b/net/mac802154/tx.c
@@ -137,34 +137,37 @@ int ieee802154_mlme_op_pre(struct ieee802154_local *local)
 	return ieee802154_sync_and_hold_queue(local);
 }
 
-int ieee802154_mlme_tx(struct ieee802154_local *local,
-		       struct ieee802154_sub_if_data *sdata,
-		       struct sk_buff *skb)
+int ieee802154_mlme_tx_locked(struct ieee802154_local *local,
+			      struct ieee802154_sub_if_data *sdata,
+			      struct sk_buff *skb)
 {
-	int ret;
-
 	/* Avoid possible calls to ->ndo_stop() when we asynchronously perform
 	 * MLME transmissions.
 	 */
-	rtnl_lock();
+	ASSERT_RTNL();
 
 	/* Ensure the device was not stopped, otherwise error out */
-	if (!local->open_count) {
-		rtnl_unlock();
+	if (!local->open_count)
 		return -ENETDOWN;
-	}
 
 	/* Warn if the ieee802154 core thinks MLME frames can be sent while the
 	 * net interface expects this cannot happen.
 	 */
-	if (WARN_ON_ONCE(!netif_running(sdata->dev))) {
-		rtnl_unlock();
+	if (WARN_ON_ONCE(!netif_running(sdata->dev)))
 		return -ENETDOWN;
-	}
 
 	ieee802154_tx(local, skb);
-	ret = ieee802154_sync_queue(local);
+	return ieee802154_sync_queue(local);
+}
 
+int ieee802154_mlme_tx(struct ieee802154_local *local,
+		       struct ieee802154_sub_if_data *sdata,
+		       struct sk_buff *skb)
+{
+	int ret;
+
+	rtnl_lock();
+	ret = ieee802154_mlme_tx_locked(local, sdata, skb);
 	rtnl_unlock();
 
 	return ret;
@@ -188,6 +191,19 @@ int ieee802154_mlme_tx_one(struct ieee802154_local *local,
 	return ret;
 }
 
+int ieee802154_mlme_tx_one_locked(struct ieee802154_local *local,
+				  struct ieee802154_sub_if_data *sdata,
+				  struct sk_buff *skb)
+{
+	int ret;
+
+	ieee802154_mlme_op_pre(local);
+	ret = ieee802154_mlme_tx_locked(local, sdata, skb);
+	ieee802154_mlme_op_post(local);
+
+	return ret;
+}
+
 static bool ieee802154_queue_is_stopped(struct ieee802154_local *local)
 {
 	return test_bit(WPAN_PHY_FLAG_STATE_QUEUE_STOPPED, &local->phy->flags);
-- 
2.34.1

[PATCH wpan-next 6/6] mac802154: Handle passive scanning

[Miquel Raynal]

Implement the core hooks in order to provide the softMAC layer support
for passive scans. Scans are requested by the user and can be aborted.

Changing channels manually is prohibited during scans.

The implementation uses a workqueue triggered at a certain interval
depending on the symbol duration for the current channel and the
duration order provided. More advanced drivers with internal scheduling
capabilities might require additional care but there is none mainline
yet.

Received beacons during a passive scan are processed in a work queue and
their result forwarded to the upper layer.

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/cfg802154.h      |  16 ++
 net/mac802154/Makefile       |   2 +-
 net/mac802154/cfg.c          |  31 ++++
 net/mac802154/ieee802154_i.h |  37 ++++-
 net/mac802154/iface.c        |   3 +
 net/mac802154/main.c         |  16 +-
 net/mac802154/rx.c           |  36 ++++-
 net/mac802154/scan.c         | 286 +++++++++++++++++++++++++++++++++++
 9 files changed, 425 insertions(+), 6 deletions(-)
 create mode 100644 net/mac802154/scan.c

diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
index b22e4147d334..140f61ec0f5f 100644
--- a/include/linux/ieee802154.h
+++ b/include/linux/ieee802154.h
@@ -47,6 +47,10 @@
 /* Duration in superframe order */
 #define IEEE802154_MAX_SCAN_DURATION	14
 #define IEEE802154_ACTIVE_SCAN_DURATION	15
+/* 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/cfg802154.h b/include/net/cfg802154.h
index 5d4750d24f13..090e1ad64a55 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -314,6 +314,22 @@ struct cfg802154_scan_request {
 	struct wpan_phy *wpan_phy;
 };
 
+/**
+ * struct cfg802154_mac_pkt - MAC packet descriptor (beacon/command)
+ * @node: MAC packets to process list member
+ * @skb: the received sk_buff
+ * @sdata: the interface on which @skb was received
+ * @page: page configuration when @skb was received
+ * @channel: channel configuration when @skb was received
+ */
+struct cfg802154_mac_pkt {
+	struct list_head node;
+	struct sk_buff *skb;
+	struct ieee802154_sub_if_data *sdata;
+	u8 page;
+	u8 channel;
+};
+
 struct ieee802154_llsec_key_id {
 	u8 mode;
 	u8 id;
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 469d6e8dd2dd..187cebcaf233 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 (mac802154_is_scanning(local))
+		return -EBUSY;
+
 	ret = drv_set_channel(local, page, channel);
 	if (!ret) {
 		wpan_phy->current_page = page;
@@ -261,6 +265,31 @@ 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 *request)
+{
+	struct ieee802154_sub_if_data *sdata;
+
+	sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(request->wpan_dev);
+
+	ASSERT_RTNL();
+
+	return mac802154_trigger_scan_locked(sdata, request);
+}
+
+static int mac802154_abort_scan(struct wpan_phy *wpan_phy,
+				struct wpan_dev *wpan_dev)
+{
+	struct ieee802154_local *local = wpan_phy_priv(wpan_phy);
+	struct ieee802154_sub_if_data *sdata;
+
+	sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(wpan_dev);
+
+	ASSERT_RTNL();
+
+	return mac802154_abort_scan_locked(local, sdata);
+}
+
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 static void
 ieee802154_get_llsec_table(struct wpan_phy *wpan_phy,
@@ -468,6 +497,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 aeadee543a9c..0e4db967bd1d 100644
--- a/net/mac802154/ieee802154_i.h
+++ b/net/mac802154/ieee802154_i.h
@@ -21,6 +21,10 @@
 
 #include "llsec.h"
 
+enum ieee802154_ongoing {
+	IEEE802154_IS_SCANNING = BIT(0),
+};
+
 /* mac802154 device private data */
 struct ieee802154_local {
 	struct ieee802154_hw hw;
@@ -43,15 +47,26 @@ struct ieee802154_local {
 	struct list_head	interfaces;
 	struct mutex		iflist_mtx;
 
-	/* This one is used for scanning and other jobs not to be interfered
-	 * with serial driver.
-	 */
+	/* Data related workqueue */
 	struct workqueue_struct	*workqueue;
+	/* MAC commands related workqueue */
+	struct workqueue_struct	*mac_wq;
 
 	struct hrtimer ifs_timer;
 
+	/* Scanning */
+	u8 scan_page;
+	u8 scan_channel;
+	struct cfg802154_scan_request __rcu *scan_req;
+	struct delayed_work scan_work;
+
+	/* Asynchronous tasks */
+	struct list_head rx_beacon_list;
+	struct work_struct rx_beacon_work;
+
 	bool started;
 	bool suspended;
+	unsigned long ongoing;
 
 	struct tasklet_struct tasklet;
 	struct sk_buff_head skb_queue;
@@ -226,6 +241,22 @@ void mac802154_unlock_table(struct net_device *dev);
 
 int mac802154_wpan_update_llsec(struct net_device *dev);
 
+/* PAN management handling */
+void mac802154_scan_worker(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,
+				struct ieee802154_sub_if_data *sdata);
+int mac802154_process_beacon(struct ieee802154_local *local,
+			     struct sk_buff *skb,
+			     u8 page, u8 channel);
+void mac802154_rx_beacon_worker(struct work_struct *work);
+
+static inline bool mac802154_is_scanning(struct ieee802154_local *local)
+{
+	return test_bit(IEEE802154_IS_SCANNING, &local->ongoing);
+}
+
 /* interface handling */
 int ieee802154_iface_init(void);
 void ieee802154_iface_exit(void);
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 7de2f843379c..a5958d323ea3 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -302,6 +302,9 @@ static int mac802154_slave_close(struct net_device *dev)
 
 	ASSERT_RTNL();
 
+	if (mac802154_is_scanning(local))
+		mac802154_abort_scan_locked(local, sdata);
+
 	netif_stop_queue(dev);
 	local->open_count--;
 
diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index 12a13a850fdf..b1111279e06d 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -89,6 +89,7 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
 	local->ops = ops;
 
 	INIT_LIST_HEAD(&local->interfaces);
+	INIT_LIST_HEAD(&local->rx_beacon_list);
 	mutex_init(&local->iflist_mtx);
 
 	tasklet_setup(&local->tasklet, ieee802154_tasklet_handler);
@@ -96,6 +97,8 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
 	skb_queue_head_init(&local->skb_queue);
 
 	INIT_WORK(&local->sync_tx_work, ieee802154_xmit_sync_worker);
+	INIT_DELAYED_WORK(&local->scan_work, mac802154_scan_worker);
+	INIT_WORK(&local->rx_beacon_work, mac802154_rx_beacon_worker);
 
 	/* init supported flags with 802.15.4 default ranges */
 	phy->supported.max_minbe = 8;
@@ -185,6 +188,7 @@ static void ieee802154_setup_wpan_phy_pib(struct wpan_phy *wpan_phy)
 int ieee802154_register_hw(struct ieee802154_hw *hw)
 {
 	struct ieee802154_local *local = hw_to_local(hw);
+	char mac_wq_name[IFNAMSIZ + 10] = {};
 	struct net_device *dev;
 	int rc = -ENOSYS;
 
@@ -195,6 +199,13 @@ int ieee802154_register_hw(struct ieee802154_hw *hw)
 		goto out;
 	}
 
+	snprintf(mac_wq_name, IFNAMSIZ + 10, "%s-mac-cmds", wpan_phy_name(local->phy));
+	local->mac_wq =	create_singlethread_workqueue(mac_wq_name);
+	if (!local->mac_wq) {
+		rc = -ENOMEM;
+		goto out_wq;
+	}
+
 	hrtimer_init(&local->ifs_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	local->ifs_timer.function = ieee802154_xmit_ifs_timer;
 
@@ -224,7 +235,7 @@ int ieee802154_register_hw(struct ieee802154_hw *hw)
 
 	rc = wpan_phy_register(local->phy);
 	if (rc < 0)
-		goto out_wq;
+		goto out_mac_wq;
 
 	rtnl_lock();
 
@@ -243,6 +254,8 @@ int ieee802154_register_hw(struct ieee802154_hw *hw)
 
 out_phy:
 	wpan_phy_unregister(local->phy);
+out_mac_wq:
+	destroy_workqueue(local->mac_wq);
 out_wq:
 	destroy_workqueue(local->workqueue);
 out:
@@ -263,6 +276,7 @@ void ieee802154_unregister_hw(struct ieee802154_hw *hw)
 
 	rtnl_unlock();
 
+	destroy_workqueue(local->mac_wq);
 	destroy_workqueue(local->workqueue);
 	wpan_phy_unregister(local->phy);
 }
diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
index c2aae2a6d6a6..2b0a80571097 100644
--- a/net/mac802154/rx.c
+++ b/net/mac802154/rx.c
@@ -29,12 +29,31 @@ static int ieee802154_deliver_skb(struct sk_buff *skb)
 	return netif_receive_skb(skb);
 }
 
+void mac802154_rx_beacon_worker(struct work_struct *work)
+{
+	struct ieee802154_local *local =
+		container_of(work, struct ieee802154_local, rx_beacon_work);
+	struct cfg802154_mac_pkt *mac_pkt;
+
+	mac_pkt = list_first_entry_or_null(&local->rx_beacon_list,
+					   struct cfg802154_mac_pkt, node);
+	if (!mac_pkt)
+		return;
+
+	mac802154_process_beacon(local, mac_pkt->skb, mac_pkt->page, mac_pkt->channel);
+
+	list_del(&mac_pkt->node);
+	kfree_skb(mac_pkt->skb);
+	kfree(mac_pkt);
+}
+
 static int
 ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
 		       struct sk_buff *skb, const struct ieee802154_hdr *hdr)
 {
-	struct wpan_dev *wpan_dev = &sdata->wpan_dev;
 	struct wpan_phy *wpan_phy = sdata->local->hw.phy;
+	struct wpan_dev *wpan_dev = &sdata->wpan_dev;
+	struct cfg802154_mac_pkt *mac_pkt;
 	__le16 span, sshort;
 	int rc;
 
@@ -106,6 +125,21 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
 
 	switch (mac_cb(skb)->type) {
 	case IEEE802154_FC_TYPE_BEACON:
+		dev_dbg(&sdata->dev->dev, "BEACON received\n");
+		if (!mac802154_is_scanning(sdata->local))
+			goto fail;
+
+		mac_pkt = kzalloc(sizeof(*mac_pkt), GFP_ATOMIC);
+		if (!mac_pkt)
+			goto fail;
+
+		mac_pkt->skb = skb_get(skb);
+		mac_pkt->sdata = sdata;
+		mac_pkt->page = sdata->local->scan_page;
+		mac_pkt->channel = sdata->local->scan_channel;
+		list_add_tail(&mac_pkt->node, &sdata->local->rx_beacon_list);
+		queue_work(sdata->local->mac_wq, &sdata->local->rx_beacon_work);
+		return NET_RX_SUCCESS;
 	case IEEE802154_FC_TYPE_ACK:
 	case IEEE802154_FC_TYPE_MAC_CMD:
 		goto fail;
diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
new file mode 100644
index 000000000000..afe4e380058d
--- /dev/null
+++ b/net/mac802154/scan.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * IEEE 802.15.4 scanning management
+ *
+ * Copyright (C) 2021 Qorvo US, Inc
+ * Authors:
+ *   - David Girault <david.girault@qorvo.com>
+ *   - Miquel Raynal <miquel.raynal@bootlin.com>
+ */
+
+#include <linux/module.h>
+#include <linux/rtnetlink.h>
+#include <net/mac802154.h>
+
+#include "ieee802154_i.h"
+#include "driver-ops.h"
+#include "../ieee802154/nl802154.h"
+
+/*
+ * mac802154_scan_cleanup_locked() must be called upon scan completion or abort.
+ * - Completions are asynchronous, not locked by the rtnl and decided by the
+ *   scan worker.
+ * - Aborts are decided by userspace, and locked by the rtnl.
+ *
+ * Concurrent modifications to the PHY, the interfaces or the hardware is in
+ * general prevented by the rtnl. So in most cases we don't need additional
+ * protection.
+ *
+ * However, the scan worker get's triggered without anybody noticing and thus we
+ * must ensure the presence of the devices as well as data consistency:
+ * - The sub-interface and device driver module get both their reference
+ *   counters incremented whenever we start a scan, so they cannot disappear
+ *   during operation.
+ * - Data consistency is achieved by the use of rcu protected pointers.
+ */
+static int mac802154_scan_cleanup_locked(struct ieee802154_local *local,
+					 struct ieee802154_sub_if_data *sdata,
+					 bool aborted)
+{
+	struct wpan_dev *wpan_dev = &sdata->wpan_dev;
+	struct wpan_phy *wpan_phy = local->phy;
+	struct cfg802154_scan_request *request;
+	u8 cmd;
+
+	/* Prevent any further use of the scan request */
+	clear_bit(IEEE802154_IS_SCANNING, &local->ongoing);
+	cancel_delayed_work(&local->scan_work);
+	request = rcu_replace_pointer(local->scan_req, NULL, 1);
+	if (!request)
+		return 0;
+	kfree_rcu(request);
+
+	/* Advertize first, while we know the devices cannot be removed */
+	cmd = aborted ? NL802154_CMD_ABORT_SCAN : NL802154_CMD_SCAN_DONE;
+	nl802154_scan_done(wpan_phy, wpan_dev, cmd);
+
+	/* Cleanup software stack */
+	ieee802154_mlme_op_post(local);
+
+	/* Set the hardware back in its original state */
+	drv_set_channel(local, wpan_phy->current_page,
+			wpan_phy->current_channel);
+	ieee802154_configure_durations(wpan_phy, wpan_phy->current_page,
+				       wpan_phy->current_channel);
+	drv_stop(local);
+	synchronize_net();
+	sdata->required_filtering = sdata->iface_default_filtering;
+	drv_start(local, sdata->required_filtering, &local->addr_filt);
+
+	return 0;
+}
+
+int mac802154_abort_scan_locked(struct ieee802154_local *local,
+				struct ieee802154_sub_if_data *sdata)
+{
+	ASSERT_RTNL();
+
+	if (!mac802154_is_scanning(local))
+		return -ESRCH;
+
+	return mac802154_scan_cleanup_locked(local, sdata, true);
+}
+
+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));
+}
+
+static void mac802154_flush_queued_beacons(struct ieee802154_local *local)
+{
+	struct cfg802154_mac_pkt *mac_pkt, *tmp;
+
+	list_for_each_entry_safe(mac_pkt, tmp, &local->rx_beacon_list, node) {
+		list_del(&mac_pkt->node);
+		kfree_skb(mac_pkt->skb);
+		kfree(mac_pkt);
+	}
+}
+
+static void
+mac802154_scan_get_next_channel(struct ieee802154_local *local,
+                                struct cfg802154_scan_request *scan_req,
+				u8 *channel)
+{
+	(*channel)++;
+        *channel = find_next_bit((const unsigned long *)&scan_req->channels,
+				 IEEE802154_MAX_CHANNEL + 1,
+				 *channel);
+}
+
+static int mac802154_scan_find_next_chan(struct ieee802154_local *local,
+                                         struct cfg802154_scan_request *scan_req,
+                                         u8 page, u8 *channel)
+{
+	mac802154_scan_get_next_channel(local, scan_req, channel);
+	if (*channel > IEEE802154_MAX_CHANNEL)
+		return -EINVAL;
+
+	return 0;
+}
+
+void mac802154_scan_worker(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 = 0;
+	struct wpan_phy* wpan_phy;
+	u8 scan_req_duration;
+	u8 page, channel;
+	int ret;
+
+	/* Ensure the device receiver is turned off when changing channels
+	 * because there is no atomic way to change the channel and know on
+	 * which one a beacon might have been received.
+	 */
+	drv_stop(local);
+	synchronize_net();
+	mac802154_flush_queued_beacons(local);
+
+	rcu_read_lock();
+	scan_req = rcu_dereference(local->scan_req);
+	if (unlikely(!scan_req)) {
+		rcu_read_unlock();
+		return;
+	}
+
+	sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(scan_req->wpan_dev);
+
+	/* Wait an arbitrary amount of time in case we cannot use the device */
+	if (local->suspended || !ieee802154_sdata_running(sdata)) {
+		rcu_read_unlock();
+		queue_delayed_work(local->mac_wq, &local->scan_work,
+				   msecs_to_jiffies(1000));
+		return;
+	}
+
+	wpan_phy = scan_req->wpan_phy;
+	scan_req_duration = scan_req->duration;
+
+	/* Look for the next valid chan */
+	page = local->scan_page;
+	channel = local->scan_channel;
+	do {
+                ret = mac802154_scan_find_next_chan(local, scan_req, page, &channel);
+                if (ret) {
+			rcu_read_unlock();
+                        goto end_scan;
+                }
+        } while (!ieee802154_chan_is_valid(scan_req->wpan_phy, page, channel));
+
+        rcu_read_unlock();
+
+	/* Bypass the stack on purpose when changing the channel */
+	rtnl_lock();
+	ret = drv_set_channel(local, page, channel);
+	rtnl_unlock();
+	if (ret) {
+                dev_err(&sdata->dev->dev,
+                        "Channel change failure during scan, aborting (%d)\n", ret);
+		goto end_scan;
+	}
+
+	local->scan_page = page;
+	local->scan_channel = channel;
+
+	rtnl_lock();
+	ret = drv_start(local, IEEE802154_FILTERING_3_SCAN, &local->addr_filt);
+	rtnl_unlock();
+	if (ret) {
+                dev_err(&sdata->dev->dev,
+                        "Restarting failure after channel change, aborting (%d)\n", ret);
+		goto end_scan;
+	}
+
+	ieee802154_configure_durations(wpan_phy, page, channel);
+	scan_duration = mac802154_scan_get_channel_time(scan_req_duration,
+							wpan_phy->symbol_duration);
+	dev_dbg(&sdata->dev->dev,
+		"Scan page %u channel %u for %ums\n",
+		page, channel, jiffies_to_msecs(scan_duration));
+	queue_delayed_work(local->mac_wq, &local->scan_work, scan_duration);
+	return;
+
+end_scan:
+	rtnl_lock();
+	mac802154_scan_cleanup_locked(local, sdata, false);
+	rtnl_unlock();
+}
+
+int mac802154_trigger_scan_locked(struct ieee802154_sub_if_data *sdata,
+				  struct cfg802154_scan_request *request)
+{
+	struct ieee802154_local *local = sdata->local;
+
+	ASSERT_RTNL();
+
+	if (mac802154_is_scanning(local))
+		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);
+
+	/* Software scanning requires to set promiscuous mode, so we need to
+	 * pause the Tx queue during the entire operation.
+	 */
+	ieee802154_mlme_op_pre(local);
+
+	sdata->required_filtering = IEEE802154_FILTERING_3_SCAN;
+	local->scan_page = request->page;
+	local->scan_channel = -1;
+	set_bit(IEEE802154_IS_SCANNING, &local->ongoing);
+
+	nl802154_scan_started(request->wpan_phy, request->wpan_dev);
+
+	queue_delayed_work(local->mac_wq, &local->scan_work, 0);
+
+	return 0;
+}
+
+int mac802154_process_beacon(struct ieee802154_local *local,
+			     struct sk_buff *skb,
+			     u8 page, u8 channel)
+{
+	struct ieee802154_beacon_hdr *bh = (void *)skb->data;
+	struct ieee802154_addr *src = &mac_cb(skb)->source;
+	struct cfg802154_scan_request *scan_req;
+	struct ieee802154_coord_desc desc;
+
+	if (skb->len != sizeof(*bh))
+		return -EINVAL;
+
+	if (unlikely(src->mode == IEEE802154_ADDR_NONE))
+		return -EINVAL;
+
+	dev_dbg(&skb->dev->dev,
+		"BEACON received on page %u channel %u\n",
+		page, channel);
+
+	memcpy(&desc.addr, src, sizeof(desc.addr));
+	desc.page = page;
+	desc.channel = channel;
+	desc.link_quality = mac_cb(skb)->lqi;
+	desc.superframe_spec = get_unaligned_le16(skb->data);
+	desc.gts_permit = bh->gts_permit;
+
+	trace_802154_scan_event(&desc);
+
+	rcu_read_lock();
+	scan_req = rcu_dereference(local->scan_req);
+	if (likely(scan_req))
+		nl802154_scan_event(scan_req->wpan_phy, scan_req->wpan_dev, &desc);
+	rcu_read_unlock();
+
+	return 0;
+}
-- 
2.34.1

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

[Alexander Aring]

Hi,

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

Is NL802154_CMD_SCAN_DONE reserved now? I don't see it implemented in
this series and I think we had some discussion about the need of abort
vs done. Is the event now?

- Alex

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

[Miquel Raynal]

Hi Alexander,

aahringo@redhat.com wrote on Sun, 4 Dec 2022 17:44:24 -0500:

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

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

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

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

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

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

Is this working for you?

Thanks,
Miquèl

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

[Alexander Aring]

Hi,

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

I think it was also to implement a way to signal all listeners that
there is such an operation ongoing? Do we have something like that?

> * Patch 6/6 adds in mac802154/scan.c:
>
> +       cmd = aborted ? NL802154_CMD_ABORT_SCAN : NL802154_CMD_SCAN_DONE;
> +       nl802154_scan_done(wpan_phy, wpan_dev, cmd);
>
> * And in patch 1/6, in ieee802154/nl802154.c:
>
> +static int nl802154_send_scan_msg(struct cfg802154_registered_device *rdev,
> +                                 struct wpan_dev *wpan_dev, u8 cmd)
> +{
> [...]
> +
> +       ret = nl802154_prep_scan_msg(msg, rdev, wpan_dev, 0, 0, 0, cmd);
>
> Is this working for you?

Sure this works in some way, I would put a reason parameter on DONE,
but however...

- Alex

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

[Miquel Raynal]

Hi Alexander,

aahringo@redhat.com wrote on Wed, 7 Dec 2022 08:27:35 -0500:

> Hi,
> 
> On Mon, Dec 5, 2022 at 4:58 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > aahringo@redhat.com wrote on Sun, 4 Dec 2022 17:44:24 -0500:
> >  
> > > Hi,
> > >
> > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal
> > > <miquel.raynal@bootlin.com> wrote:  
> > > >
> > > > The ieee802154 layer should be able to scan a set of channels in order
> > > > to look for beacons advertizing PANs. Supporting this involves adding
> > > > two user commands: triggering scans and aborting scans. The user should
> > > > also be notified when a new beacon is received and also upon scan
> > > > termination.
> > > >
> > > > A scan request structure is created to list the requirements and to be
> > > > accessed asynchronously when changing channels or receiving beacons.
> > > >
> > > > Mac layers may now implement the ->trigger_scan() and ->abort_scan()
> > > > hooks.
> > > >
> > > > Co-developed-by: David Girault <david.girault@qorvo.com>
> > > > Signed-off-by: David Girault <david.girault@qorvo.com>
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  include/linux/ieee802154.h |   3 +
> > > >  include/net/cfg802154.h    |  25 +++++
> > > >  include/net/nl802154.h     |  49 +++++++++
> > > >  net/ieee802154/nl802154.c  | 215 +++++++++++++++++++++++++++++++++++++
> > > >  net/ieee802154/nl802154.h  |   3 +
> > > >  net/ieee802154/rdev-ops.h  |  28 +++++
> > > >  net/ieee802154/trace.h     |  40 +++++++
> > > >  7 files changed, 363 insertions(+)
> > > >
> > > > diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
> > > > index 0303eb84d596..b22e4147d334 100644
> > > > --- a/include/linux/ieee802154.h
> > > > +++ b/include/linux/ieee802154.h
> > > > @@ -44,6 +44,9 @@
> > > >  #define IEEE802154_SHORT_ADDR_LEN      2
> > > >  #define IEEE802154_PAN_ID_LEN          2
> > > >
> > > > +/* Duration in superframe order */
> > > > +#define IEEE802154_MAX_SCAN_DURATION   14
> > > > +#define IEEE802154_ACTIVE_SCAN_DURATION        15
> > > >  #define IEEE802154_LIFS_PERIOD         40
> > > >  #define IEEE802154_SIFS_PERIOD         12
> > > >  #define IEEE802154_MAX_SIFS_FRAME_SIZE 18
> > > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > > > index d09c393d229f..76d4f95e9974 100644
> > > > --- a/include/net/cfg802154.h
> > > > +++ b/include/net/cfg802154.h
> > > > @@ -18,6 +18,7 @@
> > > >
> > > >  struct wpan_phy;
> > > >  struct wpan_phy_cca;
> > > > +struct cfg802154_scan_request;
> > > >
> > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > >  struct ieee802154_llsec_device_key;
> > > > @@ -67,6 +68,10 @@ struct cfg802154_ops {
> > > >                                 struct wpan_dev *wpan_dev, bool mode);
> > > >         int     (*set_ackreq_default)(struct wpan_phy *wpan_phy,
> > > >                                       struct wpan_dev *wpan_dev, bool ackreq);
> > > > +       int     (*trigger_scan)(struct wpan_phy *wpan_phy,
> > > > +                               struct cfg802154_scan_request *request);
> > > > +       int     (*abort_scan)(struct wpan_phy *wpan_phy,
> > > > +                             struct wpan_dev *wpan_dev);
> > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > >         void    (*get_llsec_table)(struct wpan_phy *wpan_phy,
> > > >                                    struct wpan_dev *wpan_dev,
> > > > @@ -278,6 +283,26 @@ struct ieee802154_coord_desc {
> > > >         bool gts_permit;
> > > >  };
> > > >
> > > > +/**
> > > > + * struct cfg802154_scan_request - Scan request
> > > > + *
> > > > + * @type: type of scan to be performed
> > > > + * @page: page on which to perform the scan
> > > > + * @channels: channels in te %page to be scanned
> > > > + * @duration: time spent on each channel, calculated with:
> > > > + *            aBaseSuperframeDuration * (2 ^ duration + 1)
> > > > + * @wpan_dev: the wpan device on which to perform the scan
> > > > + * @wpan_phy: the wpan phy on which to perform the scan
> > > > + */
> > > > +struct cfg802154_scan_request {
> > > > +       enum nl802154_scan_types type;
> > > > +       u8 page;
> > > > +       u32 channels;
> > > > +       u8 duration;
> > > > +       struct wpan_dev *wpan_dev;
> > > > +       struct wpan_phy *wpan_phy;
> > > > +};
> > > > +
> > > >  struct ieee802154_llsec_key_id {
> > > >         u8 mode;
> > > >         u8 id;
> > > > diff --git a/include/net/nl802154.h b/include/net/nl802154.h
> > > > index b79a89d5207c..79fbd820b25a 100644
> > > > --- a/include/net/nl802154.h
> > > > +++ b/include/net/nl802154.h
> > > > @@ -73,6 +73,9 @@ enum nl802154_commands {
> > > >         NL802154_CMD_DEL_SEC_LEVEL,
> > > >
> > > >         NL802154_CMD_SCAN_EVENT,
> > > > +       NL802154_CMD_TRIGGER_SCAN,
> > > > +       NL802154_CMD_ABORT_SCAN,
> > > > +       NL802154_CMD_SCAN_DONE,  
> > >
> > > Is NL802154_CMD_SCAN_DONE reserved now? I don't see it implemented in
> > > this series and I think we had some discussion about the need of abort
> > > vs done. Is the event now?  
> >
> > To be very honest I went back and forth about the "abort" information
> > so I don't remember exactly what was supposed to be implemented. The
> > current implementation forwards to userspace the reason (whether the
> > scan was finished or was aborted for an external reason). So it is
> > implemented this way:
> >  
> 
> I think it was also to implement a way to signal all listeners that
> there is such an operation ongoing? Do we have something like that?

When the kernel receives a CMD_TRIGGER_SCAN it starts the scan and if
everything is on track sends to the userspace listeners a
CMD_TRIGGER_SCAN in return, meaning "a scan has started".

So any listener would see:
CMD_TRIGGER_SCAN
SCAN_EVENT (+ content of the beacon)
...
SCAN_EVENT (+ content of the beacon)
CMD_SCAN_DONE (+ reason, see below)

> > * Patch 6/6 adds in mac802154/scan.c:
> >
> > +       cmd = aborted ? NL802154_CMD_ABORT_SCAN : NL802154_CMD_SCAN_DONE;
> > +       nl802154_scan_done(wpan_phy, wpan_dev, cmd);
> >
> > * And in patch 1/6, in ieee802154/nl802154.c:
> >
> > +static int nl802154_send_scan_msg(struct cfg802154_registered_device *rdev,
> > +                                 struct wpan_dev *wpan_dev, u8 cmd)
> > +{
> > [...]
> > +
> > +       ret = nl802154_prep_scan_msg(msg, rdev, wpan_dev, 0, 0, 0, cmd);
> >
> > Is this working for you?  
> 
> Sure this works in some way, I would put a reason parameter on DONE,
> but however...

Of course, I can do that. So NL802154_CMD_SCAN_DONE plus an u8
parameter:
NL802154_SCAN_DONE_REASON_FINISHED
NL802154_SCAN_DONE_REASON_ABORTED
?

Thanks,
Miquèl

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

[Alexander Aring]

Hi,

On Wed, Dec 7, 2022 at 8:45 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> aahringo@redhat.com wrote on Wed, 7 Dec 2022 08:27:35 -0500:
>
> > Hi,
> >
> > On Mon, Dec 5, 2022 at 4:58 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Alexander,
> > >
> > > aahringo@redhat.com wrote on Sun, 4 Dec 2022 17:44:24 -0500:
> > >
> > > > Hi,
> > > >
> > > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal
> > > > <miquel.raynal@bootlin.com> wrote:
> > > > >
> > > > > The ieee802154 layer should be able to scan a set of channels in order
> > > > > to look for beacons advertizing PANs. Supporting this involves adding
> > > > > two user commands: triggering scans and aborting scans. The user should
> > > > > also be notified when a new beacon is received and also upon scan
> > > > > termination.
> > > > >
> > > > > A scan request structure is created to list the requirements and to be
> > > > > accessed asynchronously when changing channels or receiving beacons.
> > > > >
> > > > > Mac layers may now implement the ->trigger_scan() and ->abort_scan()
> > > > > hooks.
> > > > >
> > > > > Co-developed-by: David Girault <david.girault@qorvo.com>
> > > > > Signed-off-by: David Girault <david.girault@qorvo.com>
> > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > ---
> > > > >  include/linux/ieee802154.h |   3 +
> > > > >  include/net/cfg802154.h    |  25 +++++
> > > > >  include/net/nl802154.h     |  49 +++++++++
> > > > >  net/ieee802154/nl802154.c  | 215 +++++++++++++++++++++++++++++++++++++
> > > > >  net/ieee802154/nl802154.h  |   3 +
> > > > >  net/ieee802154/rdev-ops.h  |  28 +++++
> > > > >  net/ieee802154/trace.h     |  40 +++++++
> > > > >  7 files changed, 363 insertions(+)
> > > > >
> > > > > diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
> > > > > index 0303eb84d596..b22e4147d334 100644
> > > > > --- a/include/linux/ieee802154.h
> > > > > +++ b/include/linux/ieee802154.h
> > > > > @@ -44,6 +44,9 @@
> > > > >  #define IEEE802154_SHORT_ADDR_LEN      2
> > > > >  #define IEEE802154_PAN_ID_LEN          2
> > > > >
> > > > > +/* Duration in superframe order */
> > > > > +#define IEEE802154_MAX_SCAN_DURATION   14
> > > > > +#define IEEE802154_ACTIVE_SCAN_DURATION        15
> > > > >  #define IEEE802154_LIFS_PERIOD         40
> > > > >  #define IEEE802154_SIFS_PERIOD         12
> > > > >  #define IEEE802154_MAX_SIFS_FRAME_SIZE 18
> > > > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > > > > index d09c393d229f..76d4f95e9974 100644
> > > > > --- a/include/net/cfg802154.h
> > > > > +++ b/include/net/cfg802154.h
> > > > > @@ -18,6 +18,7 @@
> > > > >
> > > > >  struct wpan_phy;
> > > > >  struct wpan_phy_cca;
> > > > > +struct cfg802154_scan_request;
> > > > >
> > > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > > >  struct ieee802154_llsec_device_key;
> > > > > @@ -67,6 +68,10 @@ struct cfg802154_ops {
> > > > >                                 struct wpan_dev *wpan_dev, bool mode);
> > > > >         int     (*set_ackreq_default)(struct wpan_phy *wpan_phy,
> > > > >                                       struct wpan_dev *wpan_dev, bool ackreq);
> > > > > +       int     (*trigger_scan)(struct wpan_phy *wpan_phy,
> > > > > +                               struct cfg802154_scan_request *request);
> > > > > +       int     (*abort_scan)(struct wpan_phy *wpan_phy,
> > > > > +                             struct wpan_dev *wpan_dev);
> > > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > > >         void    (*get_llsec_table)(struct wpan_phy *wpan_phy,
> > > > >                                    struct wpan_dev *wpan_dev,
> > > > > @@ -278,6 +283,26 @@ struct ieee802154_coord_desc {
> > > > >         bool gts_permit;
> > > > >  };
> > > > >
> > > > > +/**
> > > > > + * struct cfg802154_scan_request - Scan request
> > > > > + *
> > > > > + * @type: type of scan to be performed
> > > > > + * @page: page on which to perform the scan
> > > > > + * @channels: channels in te %page to be scanned
> > > > > + * @duration: time spent on each channel, calculated with:
> > > > > + *            aBaseSuperframeDuration * (2 ^ duration + 1)
> > > > > + * @wpan_dev: the wpan device on which to perform the scan
> > > > > + * @wpan_phy: the wpan phy on which to perform the scan
> > > > > + */
> > > > > +struct cfg802154_scan_request {
> > > > > +       enum nl802154_scan_types type;
> > > > > +       u8 page;
> > > > > +       u32 channels;
> > > > > +       u8 duration;
> > > > > +       struct wpan_dev *wpan_dev;
> > > > > +       struct wpan_phy *wpan_phy;
> > > > > +};
> > > > > +
> > > > >  struct ieee802154_llsec_key_id {
> > > > >         u8 mode;
> > > > >         u8 id;
> > > > > diff --git a/include/net/nl802154.h b/include/net/nl802154.h
> > > > > index b79a89d5207c..79fbd820b25a 100644
> > > > > --- a/include/net/nl802154.h
> > > > > +++ b/include/net/nl802154.h
> > > > > @@ -73,6 +73,9 @@ enum nl802154_commands {
> > > > >         NL802154_CMD_DEL_SEC_LEVEL,
> > > > >
> > > > >         NL802154_CMD_SCAN_EVENT,
> > > > > +       NL802154_CMD_TRIGGER_SCAN,
> > > > > +       NL802154_CMD_ABORT_SCAN,
> > > > > +       NL802154_CMD_SCAN_DONE,
> > > >
> > > > Is NL802154_CMD_SCAN_DONE reserved now? I don't see it implemented in
> > > > this series and I think we had some discussion about the need of abort
> > > > vs done. Is the event now?
> > >
> > > To be very honest I went back and forth about the "abort" information
> > > so I don't remember exactly what was supposed to be implemented. The
> > > current implementation forwards to userspace the reason (whether the
> > > scan was finished or was aborted for an external reason). So it is
> > > implemented this way:
> > >
> >
> > I think it was also to implement a way to signal all listeners that
> > there is such an operation ongoing? Do we have something like that?
>
> When the kernel receives a CMD_TRIGGER_SCAN it starts the scan and if
> everything is on track sends to the userspace listeners a
> CMD_TRIGGER_SCAN in return, meaning "a scan has started".
>
> So any listener would see:
> CMD_TRIGGER_SCAN
> SCAN_EVENT (+ content of the beacon)
> ...
> SCAN_EVENT (+ content of the beacon)
> CMD_SCAN_DONE (+ reason, see below)
>

okay.

> > > * Patch 6/6 adds in mac802154/scan.c:
> > >
> > > +       cmd = aborted ? NL802154_CMD_ABORT_SCAN : NL802154_CMD_SCAN_DONE;
> > > +       nl802154_scan_done(wpan_phy, wpan_dev, cmd);
> > >
> > > * And in patch 1/6, in ieee802154/nl802154.c:
> > >
> > > +static int nl802154_send_scan_msg(struct cfg802154_registered_device *rdev,
> > > +                                 struct wpan_dev *wpan_dev, u8 cmd)
> > > +{
> > > [...]
> > > +
> > > +       ret = nl802154_prep_scan_msg(msg, rdev, wpan_dev, 0, 0, 0, cmd);
> > >
> > > Is this working for you?
> >
> > Sure this works in some way, I would put a reason parameter on DONE,
> > but however...
>
> Of course, I can do that. So NL802154_CMD_SCAN_DONE plus an u8
> parameter:
> NL802154_SCAN_DONE_REASON_FINISHED
> NL802154_SCAN_DONE_REASON_ABORTED
> ?

yes, please.

- Alex

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

[Jakub Kicinski]

On Tue, 29 Nov 2022 17:00:41 +0100 Miquel Raynal wrote:
> +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct cfg802154_registered_device *rdev = info->user_ptr[0];
> +	struct net_device *dev = info->user_ptr[1];
> +	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> +	struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> +	struct cfg802154_scan_request *request;
> +	u8 type;
> +	int err;
> +
> +	/* Monitors are not allowed to perform scans */
> +	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)

extack ?

> +		return -EPERM;
> +
> +	request = kzalloc(sizeof(*request), GFP_KERNEL);
> +	if (!request)
> +		return -ENOMEM;
> +
> +	request->wpan_dev = wpan_dev;
> +	request->wpan_phy = wpan_phy;
> +
> +	type = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_TYPE]);

what checks info->attrs[NL802154_ATTR_SCAN_TYPE] is not NULL?

> +	switch (type) {
> +	case NL802154_SCAN_PASSIVE:
> +		request->type = type;
> +		break;
> +	default:
> +		pr_err("Unsupported scan type: %d\n", type);
> +		err = -EINVAL;

extack (printfs are now supported)

> +		goto free_request;
> +	}
> +
> +	if (info->attrs[NL802154_ATTR_PAGE]) {
> +		request->page = nla_get_u8(info->attrs[NL802154_ATTR_PAGE]);
> +		if (request->page > IEEE802154_MAX_PAGE) {

bound check should be part of the policy NLA_POLICY_MAX()

> +			pr_err("Invalid page %d > %d\n",
> +			       request->page, IEEE802154_MAX_PAGE);
> +			err = -EINVAL;

extack

> +			goto free_request;
> +		}
> +	} else {
> +		/* Use current page by default */
> +		request->page = wpan_phy->current_page;
> +	}
> +
> +	if (info->attrs[NL802154_ATTR_SCAN_CHANNELS]) {
> +		request->channels = nla_get_u32(info->attrs[NL802154_ATTR_SCAN_CHANNELS]);
> +		if (request->channels >= BIT(IEEE802154_MAX_CHANNEL + 1)) {

policy as well

> +			pr_err("Invalid channels bitfield %x ≥ %lx\n",
> +			       request->channels,
> +			       BIT(IEEE802154_MAX_CHANNEL + 1));
> +			err = -EINVAL;
> +			goto free_request;
> +		}
> +	} else {
> +		/* Scan all supported channels by default */
> +		request->channels = wpan_phy->supported.channels[request->page];
> +	}
> +
> +	if (info->attrs[NL802154_ATTR_SCAN_PREAMBLE_CODES] ||
> +	    info->attrs[NL802154_ATTR_SCAN_MEAN_PRF]) {
> +		pr_err("Preamble codes and mean PRF not supported yet\n");

NLA_REJECT also in policy

> +		err = -EINVAL;
> +		goto free_request;
> +	}
> +
> +	if (info->attrs[NL802154_ATTR_SCAN_DURATION]) {
> +		request->duration = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_DURATION]);
> +		if (request->duration > IEEE802154_MAX_SCAN_DURATION) {
> +			pr_err("Duration is out of range\n");
> +			err = -EINVAL;
> +			goto free_request;
> +		}
> +	} else {
> +		/* Use maximum duration order by default */
> +		request->duration = IEEE802154_MAX_SCAN_DURATION;
> +	}
> +
> +	if (wpan_dev->netdev)
> +		dev_hold(wpan_dev->netdev);

Can we put a tracker in the request and use netdev_hold() ?

> +
> +	err = rdev_trigger_scan(rdev, request);
> +	if (err) {
> +		pr_err("Failure starting scanning (%d)\n", err);
> +		goto free_device;
> +	}

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

[Alexander Aring]

Hi,

On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
>
> The ieee802154 layer should be able to scan a set of channels in order
> to look for beacons advertizing PANs. Supporting this involves adding
> two user commands: triggering scans and aborting scans. The user should
> also be notified when a new beacon is received and also upon scan
> termination.
>
> A scan request structure is created to list the requirements and to be
> accessed asynchronously when changing channels or receiving beacons.
>
> Mac layers may now implement the ->trigger_scan() and ->abort_scan()
> hooks.
>
> Co-developed-by: David Girault <david.girault@qorvo.com>
> Signed-off-by: David Girault <david.girault@qorvo.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  include/linux/ieee802154.h |   3 +
>  include/net/cfg802154.h    |  25 +++++
>  include/net/nl802154.h     |  49 +++++++++
>  net/ieee802154/nl802154.c  | 215 +++++++++++++++++++++++++++++++++++++
>  net/ieee802154/nl802154.h  |   3 +
>  net/ieee802154/rdev-ops.h  |  28 +++++
>  net/ieee802154/trace.h     |  40 +++++++
>  7 files changed, 363 insertions(+)
>
> diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
> index 0303eb84d596..b22e4147d334 100644
> --- a/include/linux/ieee802154.h
> +++ b/include/linux/ieee802154.h
> @@ -44,6 +44,9 @@
>  #define IEEE802154_SHORT_ADDR_LEN      2
>  #define IEEE802154_PAN_ID_LEN          2
>
> +/* Duration in superframe order */
> +#define IEEE802154_MAX_SCAN_DURATION   14
> +#define IEEE802154_ACTIVE_SCAN_DURATION        15
>  #define IEEE802154_LIFS_PERIOD         40
>  #define IEEE802154_SIFS_PERIOD         12
>  #define IEEE802154_MAX_SIFS_FRAME_SIZE 18
> diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> index d09c393d229f..76d4f95e9974 100644
> --- a/include/net/cfg802154.h
> +++ b/include/net/cfg802154.h
> @@ -18,6 +18,7 @@
>
>  struct wpan_phy;
>  struct wpan_phy_cca;
> +struct cfg802154_scan_request;
>
>  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
>  struct ieee802154_llsec_device_key;
> @@ -67,6 +68,10 @@ struct cfg802154_ops {
>                                 struct wpan_dev *wpan_dev, bool mode);
>         int     (*set_ackreq_default)(struct wpan_phy *wpan_phy,
>                                       struct wpan_dev *wpan_dev, bool ackreq);
> +       int     (*trigger_scan)(struct wpan_phy *wpan_phy,
> +                               struct cfg802154_scan_request *request);
> +       int     (*abort_scan)(struct wpan_phy *wpan_phy,
> +                             struct wpan_dev *wpan_dev);
>  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
>         void    (*get_llsec_table)(struct wpan_phy *wpan_phy,
>                                    struct wpan_dev *wpan_dev,
> @@ -278,6 +283,26 @@ struct ieee802154_coord_desc {
>         bool gts_permit;
>  };
>
> +/**
> + * struct cfg802154_scan_request - Scan request
> + *
> + * @type: type of scan to be performed
> + * @page: page on which to perform the scan
> + * @channels: channels in te %page to be scanned
> + * @duration: time spent on each channel, calculated with:
> + *            aBaseSuperframeDuration * (2 ^ duration + 1)
> + * @wpan_dev: the wpan device on which to perform the scan
> + * @wpan_phy: the wpan phy on which to perform the scan
> + */
> +struct cfg802154_scan_request {
> +       enum nl802154_scan_types type;
> +       u8 page;
> +       u32 channels;
> +       u8 duration;
> +       struct wpan_dev *wpan_dev;
> +       struct wpan_phy *wpan_phy;
> +};
> +
>  struct ieee802154_llsec_key_id {
>         u8 mode;
>         u8 id;
> diff --git a/include/net/nl802154.h b/include/net/nl802154.h
> index b79a89d5207c..79fbd820b25a 100644
> --- a/include/net/nl802154.h
> +++ b/include/net/nl802154.h
> @@ -73,6 +73,9 @@ enum nl802154_commands {
>         NL802154_CMD_DEL_SEC_LEVEL,
>
>         NL802154_CMD_SCAN_EVENT,
> +       NL802154_CMD_TRIGGER_SCAN,
> +       NL802154_CMD_ABORT_SCAN,
> +       NL802154_CMD_SCAN_DONE,
>
>         /* add new commands above here */
>
> @@ -134,6 +137,12 @@ enum nl802154_attrs {
>         NL802154_ATTR_NETNS_FD,
>
>         NL802154_ATTR_COORDINATOR,
> +       NL802154_ATTR_SCAN_TYPE,
> +       NL802154_ATTR_SCAN_FLAGS,
> +       NL802154_ATTR_SCAN_CHANNELS,
> +       NL802154_ATTR_SCAN_PREAMBLE_CODES,
> +       NL802154_ATTR_SCAN_MEAN_PRF,
> +       NL802154_ATTR_SCAN_DURATION,
>
>         /* add attributes here, update the policy in nl802154.c */
>
> @@ -259,6 +268,46 @@ enum nl802154_coord {
>         NL802154_COORD_MAX,
>  };
>
> +/**
> + * enum nl802154_scan_types - Scan types
> + *
> + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved
> + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak
> + *     energy in each requested channel
> + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using
> + *     a Beacon Request command
> + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames
> + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation
> + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request
> + *     command instead of Beacon Request command
> + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames
> + *     instead of Beacon frames
> + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number
> + */
> +enum nl802154_scan_types {
> +       __NL802154_SCAN_INVALID,
> +       NL802154_SCAN_ED,
> +       NL802154_SCAN_ACTIVE,
> +       NL802154_SCAN_PASSIVE,
> +       NL802154_SCAN_ORPHAN,
> +       NL802154_SCAN_ENHANCED_ACTIVE,
> +       NL802154_SCAN_RIT_PASSIVE,
> +
> +       /* keep last */
> +       NL802154_SCAN_ATTR_MAX,
> +};
> +
> +/**
> + * enum nl802154_scan_flags - Scan request control flags
> + *
> + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie.
> + *     a different one for every scan iteration). When the flag is set, full
> + *     randomisation is assumed.
> + */
> +enum nl802154_scan_flags {
> +       NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0),
> +};
> +
>  /**
>   * enum nl802154_cca_modes - cca modes
>   *
> diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> index 80dc73182785..c497ffd8e897 100644
> --- a/net/ieee802154/nl802154.c
> +++ b/net/ieee802154/nl802154.c
> @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
>
>         [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED },
>
> +       [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 },
> +       [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 },
> +       [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 },
> +       [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 },
> +       [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 },
> +
>  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
>         [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
>         [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, },
> @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
>  }
>  EXPORT_SYMBOL_GPL(nl802154_scan_event);
>
> +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> +{
> +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> +       struct net_device *dev = info->user_ptr[1];
> +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> +       struct cfg802154_scan_request *request;
> +       u8 type;
> +       int err;
> +
> +       /* Monitors are not allowed to perform scans */
> +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> +               return -EPERM;

btw: why are monitors not allowed?

- Alex

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

[Miquel Raynal]

Hi Alexander,

aahringo@redhat.com wrote on Sun, 5 Feb 2023 20:39:32 -0500:

> Hi,
> 
> On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> >
> > The ieee802154 layer should be able to scan a set of channels in order
> > to look for beacons advertizing PANs. Supporting this involves adding
> > two user commands: triggering scans and aborting scans. The user should
> > also be notified when a new beacon is received and also upon scan
> > termination.
> >
> > A scan request structure is created to list the requirements and to be
> > accessed asynchronously when changing channels or receiving beacons.
> >
> > Mac layers may now implement the ->trigger_scan() and ->abort_scan()
> > hooks.
> >
> > Co-developed-by: David Girault <david.girault@qorvo.com>
> > Signed-off-by: David Girault <david.girault@qorvo.com>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  include/linux/ieee802154.h |   3 +
> >  include/net/cfg802154.h    |  25 +++++
> >  include/net/nl802154.h     |  49 +++++++++
> >  net/ieee802154/nl802154.c  | 215 +++++++++++++++++++++++++++++++++++++
> >  net/ieee802154/nl802154.h  |   3 +
> >  net/ieee802154/rdev-ops.h  |  28 +++++
> >  net/ieee802154/trace.h     |  40 +++++++
> >  7 files changed, 363 insertions(+)
> >
> > diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
> > index 0303eb84d596..b22e4147d334 100644
> > --- a/include/linux/ieee802154.h
> > +++ b/include/linux/ieee802154.h
> > @@ -44,6 +44,9 @@
> >  #define IEEE802154_SHORT_ADDR_LEN      2
> >  #define IEEE802154_PAN_ID_LEN          2
> >
> > +/* Duration in superframe order */
> > +#define IEEE802154_MAX_SCAN_DURATION   14
> > +#define IEEE802154_ACTIVE_SCAN_DURATION        15
> >  #define IEEE802154_LIFS_PERIOD         40
> >  #define IEEE802154_SIFS_PERIOD         12
> >  #define IEEE802154_MAX_SIFS_FRAME_SIZE 18
> > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > index d09c393d229f..76d4f95e9974 100644
> > --- a/include/net/cfg802154.h
> > +++ b/include/net/cfg802154.h
> > @@ -18,6 +18,7 @@
> >
> >  struct wpan_phy;
> >  struct wpan_phy_cca;
> > +struct cfg802154_scan_request;
> >
> >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> >  struct ieee802154_llsec_device_key;
> > @@ -67,6 +68,10 @@ struct cfg802154_ops {
> >                                 struct wpan_dev *wpan_dev, bool mode);
> >         int     (*set_ackreq_default)(struct wpan_phy *wpan_phy,
> >                                       struct wpan_dev *wpan_dev, bool ackreq);
> > +       int     (*trigger_scan)(struct wpan_phy *wpan_phy,
> > +                               struct cfg802154_scan_request *request);
> > +       int     (*abort_scan)(struct wpan_phy *wpan_phy,
> > +                             struct wpan_dev *wpan_dev);
> >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> >         void    (*get_llsec_table)(struct wpan_phy *wpan_phy,
> >                                    struct wpan_dev *wpan_dev,
> > @@ -278,6 +283,26 @@ struct ieee802154_coord_desc {
> >         bool gts_permit;
> >  };
> >
> > +/**
> > + * struct cfg802154_scan_request - Scan request
> > + *
> > + * @type: type of scan to be performed
> > + * @page: page on which to perform the scan
> > + * @channels: channels in te %page to be scanned
> > + * @duration: time spent on each channel, calculated with:
> > + *            aBaseSuperframeDuration * (2 ^ duration + 1)
> > + * @wpan_dev: the wpan device on which to perform the scan
> > + * @wpan_phy: the wpan phy on which to perform the scan
> > + */
> > +struct cfg802154_scan_request {
> > +       enum nl802154_scan_types type;
> > +       u8 page;
> > +       u32 channels;
> > +       u8 duration;
> > +       struct wpan_dev *wpan_dev;
> > +       struct wpan_phy *wpan_phy;
> > +};
> > +
> >  struct ieee802154_llsec_key_id {
> >         u8 mode;
> >         u8 id;
> > diff --git a/include/net/nl802154.h b/include/net/nl802154.h
> > index b79a89d5207c..79fbd820b25a 100644
> > --- a/include/net/nl802154.h
> > +++ b/include/net/nl802154.h
> > @@ -73,6 +73,9 @@ enum nl802154_commands {
> >         NL802154_CMD_DEL_SEC_LEVEL,
> >
> >         NL802154_CMD_SCAN_EVENT,
> > +       NL802154_CMD_TRIGGER_SCAN,
> > +       NL802154_CMD_ABORT_SCAN,
> > +       NL802154_CMD_SCAN_DONE,
> >
> >         /* add new commands above here */
> >
> > @@ -134,6 +137,12 @@ enum nl802154_attrs {
> >         NL802154_ATTR_NETNS_FD,
> >
> >         NL802154_ATTR_COORDINATOR,
> > +       NL802154_ATTR_SCAN_TYPE,
> > +       NL802154_ATTR_SCAN_FLAGS,
> > +       NL802154_ATTR_SCAN_CHANNELS,
> > +       NL802154_ATTR_SCAN_PREAMBLE_CODES,
> > +       NL802154_ATTR_SCAN_MEAN_PRF,
> > +       NL802154_ATTR_SCAN_DURATION,
> >
> >         /* add attributes here, update the policy in nl802154.c */
> >
> > @@ -259,6 +268,46 @@ enum nl802154_coord {
> >         NL802154_COORD_MAX,
> >  };
> >
> > +/**
> > + * enum nl802154_scan_types - Scan types
> > + *
> > + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved
> > + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak
> > + *     energy in each requested channel
> > + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using
> > + *     a Beacon Request command
> > + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames
> > + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation
> > + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request
> > + *     command instead of Beacon Request command
> > + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames
> > + *     instead of Beacon frames
> > + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number
> > + */
> > +enum nl802154_scan_types {
> > +       __NL802154_SCAN_INVALID,
> > +       NL802154_SCAN_ED,
> > +       NL802154_SCAN_ACTIVE,
> > +       NL802154_SCAN_PASSIVE,
> > +       NL802154_SCAN_ORPHAN,
> > +       NL802154_SCAN_ENHANCED_ACTIVE,
> > +       NL802154_SCAN_RIT_PASSIVE,
> > +
> > +       /* keep last */
> > +       NL802154_SCAN_ATTR_MAX,
> > +};
> > +
> > +/**
> > + * enum nl802154_scan_flags - Scan request control flags
> > + *
> > + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie.
> > + *     a different one for every scan iteration). When the flag is set, full
> > + *     randomisation is assumed.
> > + */
> > +enum nl802154_scan_flags {
> > +       NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0),
> > +};
> > +
> >  /**
> >   * enum nl802154_cca_modes - cca modes
> >   *
> > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> > index 80dc73182785..c497ffd8e897 100644
> > --- a/net/ieee802154/nl802154.c
> > +++ b/net/ieee802154/nl802154.c
> > @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
> >
> >         [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED },
> >
> > +       [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 },
> > +       [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 },
> > +       [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 },
> > +       [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 },
> > +       [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 },
> > +
> >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> >         [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
> >         [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, },
> > @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
> >  }
> >  EXPORT_SYMBOL_GPL(nl802154_scan_event);
> >
> > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > +{
> > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > +       struct net_device *dev = info->user_ptr[1];
> > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > +       struct cfg802154_scan_request *request;
> > +       u8 type;
> > +       int err;
> > +
> > +       /* Monitors are not allowed to perform scans */
> > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > +               return -EPERM;  
> 
> btw: why are monitors not allowed?

I guess I had the "active scan" use case in mind which of course does
not work with monitors. Maybe I can relax this a little bit indeed,
right now I don't remember why I strongly refused scans on monitors.

Thanks,
Miquèl

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

[Alexander Aring]

Hi,

On Mon, Feb 6, 2023 at 4:13 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> aahringo@redhat.com wrote on Sun, 5 Feb 2023 20:39:32 -0500:
>
> > Hi,
> >
> > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal
> > <miquel.raynal@bootlin.com> wrote:
> > >
> > > The ieee802154 layer should be able to scan a set of channels in order
> > > to look for beacons advertizing PANs. Supporting this involves adding
> > > two user commands: triggering scans and aborting scans. The user should
> > > also be notified when a new beacon is received and also upon scan
> > > termination.
> > >
> > > A scan request structure is created to list the requirements and to be
> > > accessed asynchronously when changing channels or receiving beacons.
> > >
> > > Mac layers may now implement the ->trigger_scan() and ->abort_scan()
> > > hooks.
> > >
> > > Co-developed-by: David Girault <david.girault@qorvo.com>
> > > Signed-off-by: David Girault <david.girault@qorvo.com>
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  include/linux/ieee802154.h |   3 +
> > >  include/net/cfg802154.h    |  25 +++++
> > >  include/net/nl802154.h     |  49 +++++++++
> > >  net/ieee802154/nl802154.c  | 215 +++++++++++++++++++++++++++++++++++++
> > >  net/ieee802154/nl802154.h  |   3 +
> > >  net/ieee802154/rdev-ops.h  |  28 +++++
> > >  net/ieee802154/trace.h     |  40 +++++++
> > >  7 files changed, 363 insertions(+)
> > >
> > > diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
> > > index 0303eb84d596..b22e4147d334 100644
> > > --- a/include/linux/ieee802154.h
> > > +++ b/include/linux/ieee802154.h
> > > @@ -44,6 +44,9 @@
> > >  #define IEEE802154_SHORT_ADDR_LEN      2
> > >  #define IEEE802154_PAN_ID_LEN          2
> > >
> > > +/* Duration in superframe order */
> > > +#define IEEE802154_MAX_SCAN_DURATION   14
> > > +#define IEEE802154_ACTIVE_SCAN_DURATION        15
> > >  #define IEEE802154_LIFS_PERIOD         40
> > >  #define IEEE802154_SIFS_PERIOD         12
> > >  #define IEEE802154_MAX_SIFS_FRAME_SIZE 18
> > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > > index d09c393d229f..76d4f95e9974 100644
> > > --- a/include/net/cfg802154.h
> > > +++ b/include/net/cfg802154.h
> > > @@ -18,6 +18,7 @@
> > >
> > >  struct wpan_phy;
> > >  struct wpan_phy_cca;
> > > +struct cfg802154_scan_request;
> > >
> > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > >  struct ieee802154_llsec_device_key;
> > > @@ -67,6 +68,10 @@ struct cfg802154_ops {
> > >                                 struct wpan_dev *wpan_dev, bool mode);
> > >         int     (*set_ackreq_default)(struct wpan_phy *wpan_phy,
> > >                                       struct wpan_dev *wpan_dev, bool ackreq);
> > > +       int     (*trigger_scan)(struct wpan_phy *wpan_phy,
> > > +                               struct cfg802154_scan_request *request);
> > > +       int     (*abort_scan)(struct wpan_phy *wpan_phy,
> > > +                             struct wpan_dev *wpan_dev);
> > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > >         void    (*get_llsec_table)(struct wpan_phy *wpan_phy,
> > >                                    struct wpan_dev *wpan_dev,
> > > @@ -278,6 +283,26 @@ struct ieee802154_coord_desc {
> > >         bool gts_permit;
> > >  };
> > >
> > > +/**
> > > + * struct cfg802154_scan_request - Scan request
> > > + *
> > > + * @type: type of scan to be performed
> > > + * @page: page on which to perform the scan
> > > + * @channels: channels in te %page to be scanned
> > > + * @duration: time spent on each channel, calculated with:
> > > + *            aBaseSuperframeDuration * (2 ^ duration + 1)
> > > + * @wpan_dev: the wpan device on which to perform the scan
> > > + * @wpan_phy: the wpan phy on which to perform the scan
> > > + */
> > > +struct cfg802154_scan_request {
> > > +       enum nl802154_scan_types type;
> > > +       u8 page;
> > > +       u32 channels;
> > > +       u8 duration;
> > > +       struct wpan_dev *wpan_dev;
> > > +       struct wpan_phy *wpan_phy;
> > > +};
> > > +
> > >  struct ieee802154_llsec_key_id {
> > >         u8 mode;
> > >         u8 id;
> > > diff --git a/include/net/nl802154.h b/include/net/nl802154.h
> > > index b79a89d5207c..79fbd820b25a 100644
> > > --- a/include/net/nl802154.h
> > > +++ b/include/net/nl802154.h
> > > @@ -73,6 +73,9 @@ enum nl802154_commands {
> > >         NL802154_CMD_DEL_SEC_LEVEL,
> > >
> > >         NL802154_CMD_SCAN_EVENT,
> > > +       NL802154_CMD_TRIGGER_SCAN,
> > > +       NL802154_CMD_ABORT_SCAN,
> > > +       NL802154_CMD_SCAN_DONE,
> > >
> > >         /* add new commands above here */
> > >
> > > @@ -134,6 +137,12 @@ enum nl802154_attrs {
> > >         NL802154_ATTR_NETNS_FD,
> > >
> > >         NL802154_ATTR_COORDINATOR,
> > > +       NL802154_ATTR_SCAN_TYPE,
> > > +       NL802154_ATTR_SCAN_FLAGS,
> > > +       NL802154_ATTR_SCAN_CHANNELS,
> > > +       NL802154_ATTR_SCAN_PREAMBLE_CODES,
> > > +       NL802154_ATTR_SCAN_MEAN_PRF,
> > > +       NL802154_ATTR_SCAN_DURATION,
> > >
> > >         /* add attributes here, update the policy in nl802154.c */
> > >
> > > @@ -259,6 +268,46 @@ enum nl802154_coord {
> > >         NL802154_COORD_MAX,
> > >  };
> > >
> > > +/**
> > > + * enum nl802154_scan_types - Scan types
> > > + *
> > > + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved
> > > + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak
> > > + *     energy in each requested channel
> > > + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using
> > > + *     a Beacon Request command
> > > + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames
> > > + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation
> > > + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request
> > > + *     command instead of Beacon Request command
> > > + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames
> > > + *     instead of Beacon frames
> > > + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number
> > > + */
> > > +enum nl802154_scan_types {
> > > +       __NL802154_SCAN_INVALID,
> > > +       NL802154_SCAN_ED,
> > > +       NL802154_SCAN_ACTIVE,
> > > +       NL802154_SCAN_PASSIVE,
> > > +       NL802154_SCAN_ORPHAN,
> > > +       NL802154_SCAN_ENHANCED_ACTIVE,
> > > +       NL802154_SCAN_RIT_PASSIVE,
> > > +
> > > +       /* keep last */
> > > +       NL802154_SCAN_ATTR_MAX,
> > > +};
> > > +
> > > +/**
> > > + * enum nl802154_scan_flags - Scan request control flags
> > > + *
> > > + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie.
> > > + *     a different one for every scan iteration). When the flag is set, full
> > > + *     randomisation is assumed.
> > > + */
> > > +enum nl802154_scan_flags {
> > > +       NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0),
> > > +};
> > > +
> > >  /**
> > >   * enum nl802154_cca_modes - cca modes
> > >   *
> > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> > > index 80dc73182785..c497ffd8e897 100644
> > > --- a/net/ieee802154/nl802154.c
> > > +++ b/net/ieee802154/nl802154.c
> > > @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
> > >
> > >         [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED },
> > >
> > > +       [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 },
> > > +       [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 },
> > > +       [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 },
> > > +       [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 },
> > > +       [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 },
> > > +
> > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > >         [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
> > >         [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, },
> > > @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
> > >  }
> > >  EXPORT_SYMBOL_GPL(nl802154_scan_event);
> > >
> > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > +{
> > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > +       struct net_device *dev = info->user_ptr[1];
> > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > +       struct cfg802154_scan_request *request;
> > > +       u8 type;
> > > +       int err;
> > > +
> > > +       /* Monitors are not allowed to perform scans */
> > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > +               return -EPERM;
> >
> > btw: why are monitors not allowed?
>
> I guess I had the "active scan" use case in mind which of course does
> not work with monitors. Maybe I can relax this a little bit indeed,
> right now I don't remember why I strongly refused scans on monitors.

Isn't it that scans really work close to phy level? Means in this case
we disable mostly everything of MAC filtering on the transceiver side.
Then I don't see any reasons why even monitors can't do anything, they
also can send something. But they really don't have any specific
source address set, so long addresses are none for source addresses, I
don't see any problem here. They also don't have AACK handling, but
it's not required for scan anyway...

If this gets too complicated right now, then I am also fine with
returning an error here, we can enable it later but would it be better
to use ENOTSUPP or something like that in this case? EPERM sounds like
you can do that, but you don't have the permissions.

- Alex

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

[Alexander Aring]

Hi,

On Mon, Feb 6, 2023 at 7:33 PM Alexander Aring <aahringo@redhat.com> wrote:
>
> Hi,
>
> On Mon, Feb 6, 2023 at 4:13 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > aahringo@redhat.com wrote on Sun, 5 Feb 2023 20:39:32 -0500:
> >
> > > Hi,
> > >
> > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal
> > > <miquel.raynal@bootlin.com> wrote:
> > > >
> > > > The ieee802154 layer should be able to scan a set of channels in order
> > > > to look for beacons advertizing PANs. Supporting this involves adding
> > > > two user commands: triggering scans and aborting scans. The user should
> > > > also be notified when a new beacon is received and also upon scan
> > > > termination.
> > > >
> > > > A scan request structure is created to list the requirements and to be
> > > > accessed asynchronously when changing channels or receiving beacons.
> > > >
> > > > Mac layers may now implement the ->trigger_scan() and ->abort_scan()
> > > > hooks.
> > > >
> > > > Co-developed-by: David Girault <david.girault@qorvo.com>
> > > > Signed-off-by: David Girault <david.girault@qorvo.com>
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  include/linux/ieee802154.h |   3 +
> > > >  include/net/cfg802154.h    |  25 +++++
> > > >  include/net/nl802154.h     |  49 +++++++++
> > > >  net/ieee802154/nl802154.c  | 215 +++++++++++++++++++++++++++++++++++++
> > > >  net/ieee802154/nl802154.h  |   3 +
> > > >  net/ieee802154/rdev-ops.h  |  28 +++++
> > > >  net/ieee802154/trace.h     |  40 +++++++
> > > >  7 files changed, 363 insertions(+)
> > > >
> > > > diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
> > > > index 0303eb84d596..b22e4147d334 100644
> > > > --- a/include/linux/ieee802154.h
> > > > +++ b/include/linux/ieee802154.h
> > > > @@ -44,6 +44,9 @@
> > > >  #define IEEE802154_SHORT_ADDR_LEN      2
> > > >  #define IEEE802154_PAN_ID_LEN          2
> > > >
> > > > +/* Duration in superframe order */
> > > > +#define IEEE802154_MAX_SCAN_DURATION   14
> > > > +#define IEEE802154_ACTIVE_SCAN_DURATION        15
> > > >  #define IEEE802154_LIFS_PERIOD         40
> > > >  #define IEEE802154_SIFS_PERIOD         12
> > > >  #define IEEE802154_MAX_SIFS_FRAME_SIZE 18
> > > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > > > index d09c393d229f..76d4f95e9974 100644
> > > > --- a/include/net/cfg802154.h
> > > > +++ b/include/net/cfg802154.h
> > > > @@ -18,6 +18,7 @@
> > > >
> > > >  struct wpan_phy;
> > > >  struct wpan_phy_cca;
> > > > +struct cfg802154_scan_request;
> > > >
> > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > >  struct ieee802154_llsec_device_key;
> > > > @@ -67,6 +68,10 @@ struct cfg802154_ops {
> > > >                                 struct wpan_dev *wpan_dev, bool mode);
> > > >         int     (*set_ackreq_default)(struct wpan_phy *wpan_phy,
> > > >                                       struct wpan_dev *wpan_dev, bool ackreq);
> > > > +       int     (*trigger_scan)(struct wpan_phy *wpan_phy,
> > > > +                               struct cfg802154_scan_request *request);
> > > > +       int     (*abort_scan)(struct wpan_phy *wpan_phy,
> > > > +                             struct wpan_dev *wpan_dev);
> > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > >         void    (*get_llsec_table)(struct wpan_phy *wpan_phy,
> > > >                                    struct wpan_dev *wpan_dev,
> > > > @@ -278,6 +283,26 @@ struct ieee802154_coord_desc {
> > > >         bool gts_permit;
> > > >  };
> > > >
> > > > +/**
> > > > + * struct cfg802154_scan_request - Scan request
> > > > + *
> > > > + * @type: type of scan to be performed
> > > > + * @page: page on which to perform the scan
> > > > + * @channels: channels in te %page to be scanned
> > > > + * @duration: time spent on each channel, calculated with:
> > > > + *            aBaseSuperframeDuration * (2 ^ duration + 1)
> > > > + * @wpan_dev: the wpan device on which to perform the scan
> > > > + * @wpan_phy: the wpan phy on which to perform the scan
> > > > + */
> > > > +struct cfg802154_scan_request {
> > > > +       enum nl802154_scan_types type;
> > > > +       u8 page;
> > > > +       u32 channels;
> > > > +       u8 duration;
> > > > +       struct wpan_dev *wpan_dev;
> > > > +       struct wpan_phy *wpan_phy;
> > > > +};
> > > > +
> > > >  struct ieee802154_llsec_key_id {
> > > >         u8 mode;
> > > >         u8 id;
> > > > diff --git a/include/net/nl802154.h b/include/net/nl802154.h
> > > > index b79a89d5207c..79fbd820b25a 100644
> > > > --- a/include/net/nl802154.h
> > > > +++ b/include/net/nl802154.h
> > > > @@ -73,6 +73,9 @@ enum nl802154_commands {
> > > >         NL802154_CMD_DEL_SEC_LEVEL,
> > > >
> > > >         NL802154_CMD_SCAN_EVENT,
> > > > +       NL802154_CMD_TRIGGER_SCAN,
> > > > +       NL802154_CMD_ABORT_SCAN,
> > > > +       NL802154_CMD_SCAN_DONE,
> > > >
> > > >         /* add new commands above here */
> > > >
> > > > @@ -134,6 +137,12 @@ enum nl802154_attrs {
> > > >         NL802154_ATTR_NETNS_FD,
> > > >
> > > >         NL802154_ATTR_COORDINATOR,
> > > > +       NL802154_ATTR_SCAN_TYPE,
> > > > +       NL802154_ATTR_SCAN_FLAGS,
> > > > +       NL802154_ATTR_SCAN_CHANNELS,
> > > > +       NL802154_ATTR_SCAN_PREAMBLE_CODES,
> > > > +       NL802154_ATTR_SCAN_MEAN_PRF,
> > > > +       NL802154_ATTR_SCAN_DURATION,
> > > >
> > > >         /* add attributes here, update the policy in nl802154.c */
> > > >
> > > > @@ -259,6 +268,46 @@ enum nl802154_coord {
> > > >         NL802154_COORD_MAX,
> > > >  };
> > > >
> > > > +/**
> > > > + * enum nl802154_scan_types - Scan types
> > > > + *
> > > > + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved
> > > > + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak
> > > > + *     energy in each requested channel
> > > > + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using
> > > > + *     a Beacon Request command
> > > > + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames
> > > > + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation
> > > > + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request
> > > > + *     command instead of Beacon Request command
> > > > + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames
> > > > + *     instead of Beacon frames
> > > > + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number
> > > > + */
> > > > +enum nl802154_scan_types {
> > > > +       __NL802154_SCAN_INVALID,
> > > > +       NL802154_SCAN_ED,
> > > > +       NL802154_SCAN_ACTIVE,
> > > > +       NL802154_SCAN_PASSIVE,
> > > > +       NL802154_SCAN_ORPHAN,
> > > > +       NL802154_SCAN_ENHANCED_ACTIVE,
> > > > +       NL802154_SCAN_RIT_PASSIVE,
> > > > +
> > > > +       /* keep last */
> > > > +       NL802154_SCAN_ATTR_MAX,
> > > > +};
> > > > +
> > > > +/**
> > > > + * enum nl802154_scan_flags - Scan request control flags
> > > > + *
> > > > + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie.
> > > > + *     a different one for every scan iteration). When the flag is set, full
> > > > + *     randomisation is assumed.
> > > > + */
> > > > +enum nl802154_scan_flags {
> > > > +       NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0),
> > > > +};
> > > > +
> > > >  /**
> > > >   * enum nl802154_cca_modes - cca modes
> > > >   *
> > > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> > > > index 80dc73182785..c497ffd8e897 100644
> > > > --- a/net/ieee802154/nl802154.c
> > > > +++ b/net/ieee802154/nl802154.c
> > > > @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
> > > >
> > > >         [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED },
> > > >
> > > > +       [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 },
> > > > +       [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 },
> > > > +       [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 },
> > > > +       [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 },
> > > > +       [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 },
> > > > +
> > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > >         [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
> > > >         [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, },
> > > > @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(nl802154_scan_event);
> > > >
> > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > +{
> > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > +       struct net_device *dev = info->user_ptr[1];
> > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > +       struct cfg802154_scan_request *request;
> > > > +       u8 type;
> > > > +       int err;
> > > > +
> > > > +       /* Monitors are not allowed to perform scans */
> > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > +               return -EPERM;
> > >
> > > btw: why are monitors not allowed?
> >
> > I guess I had the "active scan" use case in mind which of course does
> > not work with monitors. Maybe I can relax this a little bit indeed,
> > right now I don't remember why I strongly refused scans on monitors.
>
> Isn't it that scans really work close to phy level? Means in this case
> we disable mostly everything of MAC filtering on the transceiver side.
> Then I don't see any reasons why even monitors can't do anything, they
> also can send something. But they really don't have any specific
> source address set, so long addresses are none for source addresses, I
> don't see any problem here. They also don't have AACK handling, but
> it's not required for scan anyway...
>
> If this gets too complicated right now, then I am also fine with
> returning an error here, we can enable it later but would it be better
> to use ENOTSUPP or something like that in this case? EPERM sounds like
> you can do that, but you don't have the permissions.
>

For me a scan should also be possible from iwpan phy $PHY scan (or
whatever the scan command is, or just enable beacon)... to go over the
dev is just a shortcut for "I mean whatever the phy is under this dev"
?

- Alex

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

[Alexander Aring]

Hi,

On Tue, Feb 7, 2023 at 7:55 AM Alexander Aring <aahringo@redhat.com> wrote:
>
> Hi,
>
> On Mon, Feb 6, 2023 at 7:33 PM Alexander Aring <aahringo@redhat.com> wrote:
> >
> > Hi,
> >
> > On Mon, Feb 6, 2023 at 4:13 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Alexander,
> > >
> > > aahringo@redhat.com wrote on Sun, 5 Feb 2023 20:39:32 -0500:
> > >
> > > > Hi,
> > > >
> > > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal
> > > > <miquel.raynal@bootlin.com> wrote:
> > > > >
> > > > > The ieee802154 layer should be able to scan a set of channels in order
> > > > > to look for beacons advertizing PANs. Supporting this involves adding
> > > > > two user commands: triggering scans and aborting scans. The user should
> > > > > also be notified when a new beacon is received and also upon scan
> > > > > termination.
> > > > >
> > > > > A scan request structure is created to list the requirements and to be
> > > > > accessed asynchronously when changing channels or receiving beacons.
> > > > >
> > > > > Mac layers may now implement the ->trigger_scan() and ->abort_scan()
> > > > > hooks.
> > > > >
> > > > > Co-developed-by: David Girault <david.girault@qorvo.com>
> > > > > Signed-off-by: David Girault <david.girault@qorvo.com>
> > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > ---
> > > > >  include/linux/ieee802154.h |   3 +
> > > > >  include/net/cfg802154.h    |  25 +++++
> > > > >  include/net/nl802154.h     |  49 +++++++++
> > > > >  net/ieee802154/nl802154.c  | 215 +++++++++++++++++++++++++++++++++++++
> > > > >  net/ieee802154/nl802154.h  |   3 +
> > > > >  net/ieee802154/rdev-ops.h  |  28 +++++
> > > > >  net/ieee802154/trace.h     |  40 +++++++
> > > > >  7 files changed, 363 insertions(+)
> > > > >
> > > > > diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
> > > > > index 0303eb84d596..b22e4147d334 100644
> > > > > --- a/include/linux/ieee802154.h
> > > > > +++ b/include/linux/ieee802154.h
> > > > > @@ -44,6 +44,9 @@
> > > > >  #define IEEE802154_SHORT_ADDR_LEN      2
> > > > >  #define IEEE802154_PAN_ID_LEN          2
> > > > >
> > > > > +/* Duration in superframe order */
> > > > > +#define IEEE802154_MAX_SCAN_DURATION   14
> > > > > +#define IEEE802154_ACTIVE_SCAN_DURATION        15
> > > > >  #define IEEE802154_LIFS_PERIOD         40
> > > > >  #define IEEE802154_SIFS_PERIOD         12
> > > > >  #define IEEE802154_MAX_SIFS_FRAME_SIZE 18
> > > > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > > > > index d09c393d229f..76d4f95e9974 100644
> > > > > --- a/include/net/cfg802154.h
> > > > > +++ b/include/net/cfg802154.h
> > > > > @@ -18,6 +18,7 @@
> > > > >
> > > > >  struct wpan_phy;
> > > > >  struct wpan_phy_cca;
> > > > > +struct cfg802154_scan_request;
> > > > >
> > > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > > >  struct ieee802154_llsec_device_key;
> > > > > @@ -67,6 +68,10 @@ struct cfg802154_ops {
> > > > >                                 struct wpan_dev *wpan_dev, bool mode);
> > > > >         int     (*set_ackreq_default)(struct wpan_phy *wpan_phy,
> > > > >                                       struct wpan_dev *wpan_dev, bool ackreq);
> > > > > +       int     (*trigger_scan)(struct wpan_phy *wpan_phy,
> > > > > +                               struct cfg802154_scan_request *request);
> > > > > +       int     (*abort_scan)(struct wpan_phy *wpan_phy,
> > > > > +                             struct wpan_dev *wpan_dev);
> > > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > > >         void    (*get_llsec_table)(struct wpan_phy *wpan_phy,
> > > > >                                    struct wpan_dev *wpan_dev,
> > > > > @@ -278,6 +283,26 @@ struct ieee802154_coord_desc {
> > > > >         bool gts_permit;
> > > > >  };
> > > > >
> > > > > +/**
> > > > > + * struct cfg802154_scan_request - Scan request
> > > > > + *
> > > > > + * @type: type of scan to be performed
> > > > > + * @page: page on which to perform the scan
> > > > > + * @channels: channels in te %page to be scanned
> > > > > + * @duration: time spent on each channel, calculated with:
> > > > > + *            aBaseSuperframeDuration * (2 ^ duration + 1)
> > > > > + * @wpan_dev: the wpan device on which to perform the scan
> > > > > + * @wpan_phy: the wpan phy on which to perform the scan
> > > > > + */
> > > > > +struct cfg802154_scan_request {
> > > > > +       enum nl802154_scan_types type;
> > > > > +       u8 page;
> > > > > +       u32 channels;
> > > > > +       u8 duration;
> > > > > +       struct wpan_dev *wpan_dev;
> > > > > +       struct wpan_phy *wpan_phy;
> > > > > +};
> > > > > +
> > > > >  struct ieee802154_llsec_key_id {
> > > > >         u8 mode;
> > > > >         u8 id;
> > > > > diff --git a/include/net/nl802154.h b/include/net/nl802154.h
> > > > > index b79a89d5207c..79fbd820b25a 100644
> > > > > --- a/include/net/nl802154.h
> > > > > +++ b/include/net/nl802154.h
> > > > > @@ -73,6 +73,9 @@ enum nl802154_commands {
> > > > >         NL802154_CMD_DEL_SEC_LEVEL,
> > > > >
> > > > >         NL802154_CMD_SCAN_EVENT,
> > > > > +       NL802154_CMD_TRIGGER_SCAN,
> > > > > +       NL802154_CMD_ABORT_SCAN,
> > > > > +       NL802154_CMD_SCAN_DONE,
> > > > >
> > > > >         /* add new commands above here */
> > > > >
> > > > > @@ -134,6 +137,12 @@ enum nl802154_attrs {
> > > > >         NL802154_ATTR_NETNS_FD,
> > > > >
> > > > >         NL802154_ATTR_COORDINATOR,
> > > > > +       NL802154_ATTR_SCAN_TYPE,
> > > > > +       NL802154_ATTR_SCAN_FLAGS,
> > > > > +       NL802154_ATTR_SCAN_CHANNELS,
> > > > > +       NL802154_ATTR_SCAN_PREAMBLE_CODES,
> > > > > +       NL802154_ATTR_SCAN_MEAN_PRF,
> > > > > +       NL802154_ATTR_SCAN_DURATION,
> > > > >
> > > > >         /* add attributes here, update the policy in nl802154.c */
> > > > >
> > > > > @@ -259,6 +268,46 @@ enum nl802154_coord {
> > > > >         NL802154_COORD_MAX,
> > > > >  };
> > > > >
> > > > > +/**
> > > > > + * enum nl802154_scan_types - Scan types
> > > > > + *
> > > > > + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved
> > > > > + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak
> > > > > + *     energy in each requested channel
> > > > > + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using
> > > > > + *     a Beacon Request command
> > > > > + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames
> > > > > + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation
> > > > > + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request
> > > > > + *     command instead of Beacon Request command
> > > > > + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames
> > > > > + *     instead of Beacon frames
> > > > > + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number
> > > > > + */
> > > > > +enum nl802154_scan_types {
> > > > > +       __NL802154_SCAN_INVALID,
> > > > > +       NL802154_SCAN_ED,
> > > > > +       NL802154_SCAN_ACTIVE,
> > > > > +       NL802154_SCAN_PASSIVE,
> > > > > +       NL802154_SCAN_ORPHAN,
> > > > > +       NL802154_SCAN_ENHANCED_ACTIVE,
> > > > > +       NL802154_SCAN_RIT_PASSIVE,
> > > > > +
> > > > > +       /* keep last */
> > > > > +       NL802154_SCAN_ATTR_MAX,
> > > > > +};
> > > > > +
> > > > > +/**
> > > > > + * enum nl802154_scan_flags - Scan request control flags
> > > > > + *
> > > > > + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie.
> > > > > + *     a different one for every scan iteration). When the flag is set, full
> > > > > + *     randomisation is assumed.
> > > > > + */
> > > > > +enum nl802154_scan_flags {
> > > > > +       NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0),
> > > > > +};
> > > > > +
> > > > >  /**
> > > > >   * enum nl802154_cca_modes - cca modes
> > > > >   *
> > > > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> > > > > index 80dc73182785..c497ffd8e897 100644
> > > > > --- a/net/ieee802154/nl802154.c
> > > > > +++ b/net/ieee802154/nl802154.c
> > > > > @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
> > > > >
> > > > >         [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED },
> > > > >
> > > > > +       [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 },
> > > > > +       [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 },
> > > > > +       [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 },
> > > > > +       [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 },
> > > > > +       [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 },
> > > > > +
> > > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > > >         [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
> > > > >         [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, },
> > > > > @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
> > > > >  }
> > > > >  EXPORT_SYMBOL_GPL(nl802154_scan_event);
> > > > >
> > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > +{
> > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > +       struct cfg802154_scan_request *request;
> > > > > +       u8 type;
> > > > > +       int err;
> > > > > +
> > > > > +       /* Monitors are not allowed to perform scans */
> > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > +               return -EPERM;
> > > >
> > > > btw: why are monitors not allowed?
> > >
> > > I guess I had the "active scan" use case in mind which of course does
> > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > right now I don't remember why I strongly refused scans on monitors.
> >
> > Isn't it that scans really work close to phy level? Means in this case
> > we disable mostly everything of MAC filtering on the transceiver side.
> > Then I don't see any reasons why even monitors can't do anything, they
> > also can send something. But they really don't have any specific
> > source address set, so long addresses are none for source addresses, I
> > don't see any problem here. They also don't have AACK handling, but
> > it's not required for scan anyway...
> >
> > If this gets too complicated right now, then I am also fine with
> > returning an error here, we can enable it later but would it be better
> > to use ENOTSUPP or something like that in this case? EPERM sounds like
> > you can do that, but you don't have the permissions.
> >
>
> For me a scan should also be possible from iwpan phy $PHY scan (or
> whatever the scan command is, or just enable beacon)... to go over the

not enable beacon, this is for broadcasting valid pans around... as
said a monitor has no address. the user "could" do that to experiment
around via AF_PACKET raw.

- Alex

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

[Miquel Raynal]

Hi Stefan, Jakub,

kuba@kernel.org wrote on Fri, 3 Feb 2023 20:19:23 -0800:

> On Tue, 29 Nov 2022 17:00:41 +0100 Miquel Raynal wrote:
> > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > +{
> > +	struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > +	struct net_device *dev = info->user_ptr[1];
> > +	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > +	struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > +	struct cfg802154_scan_request *request;
> > +	u8 type;
> > +	int err;
> > +
> > +	/* Monitors are not allowed to perform scans */
> > +	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)  
> 
> extack ?

Thanks for pointing at it, I just did know about it. I did convert
most of the printk's into extack strings. Shall I keep both or is fine
to just keep the extack thing?

For now I've dropped the printk's, please tell me if this is wrong.

> 
> > +		return -EPERM;

Stefan, do you prefer a series of patches applying on top of your
current next or should I re-roll the entire series (scan + beacons)?

I am preparing a series applying on top of the current list of applied
patches. This means next PR to net maintainers will include this patch
as it is today + fixes on top. If this is fine for both parties, I will
send these (including the other changes discussed with Alexander). Just
let me know.

Sorry btw for the delay, I really had to finish other activities before
switching back.

> > +
> > +	request = kzalloc(sizeof(*request), GFP_KERNEL);
> > +	if (!request)
> > +		return -ENOMEM;
> > +
> > +	request->wpan_dev = wpan_dev;
> > +	request->wpan_phy = wpan_phy;
> > +
> > +	type = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_TYPE]);  
> 
> what checks info->attrs[NL802154_ATTR_SCAN_TYPE] is not NULL?
> 
> > +	switch (type) {
> > +	case NL802154_SCAN_PASSIVE:
> > +		request->type = type;
> > +		break;
> > +	default:
> > +		pr_err("Unsupported scan type: %d\n", type);
> > +		err = -EINVAL;  
> 
> extack (printfs are now supported)
> 
> > +		goto free_request;
> > +	}
> > +
> > +	if (info->attrs[NL802154_ATTR_PAGE]) {
> > +		request->page = nla_get_u8(info->attrs[NL802154_ATTR_PAGE]);
> > +		if (request->page > IEEE802154_MAX_PAGE) {  
> 
> bound check should be part of the policy NLA_POLICY_MAX()

I just improved the policies to make these checks useless and simplify a
lot the code there, thanks as well for pointing at it.

> > +			pr_err("Invalid page %d > %d\n",
> > +			       request->page, IEEE802154_MAX_PAGE);
> > +			err = -EINVAL;  
> 
> extack
> 
> > +			goto free_request;
> > +		}
> > +	} else {
> > +		/* Use current page by default */
> > +		request->page = wpan_phy->current_page;
> > +	}
> > +
> > +	if (info->attrs[NL802154_ATTR_SCAN_CHANNELS]) {
> > +		request->channels = nla_get_u32(info->attrs[NL802154_ATTR_SCAN_CHANNELS]);
> > +		if (request->channels >= BIT(IEEE802154_MAX_CHANNEL + 1)) {  
> 
> policy as well
> 
> > +			pr_err("Invalid channels bitfield %x ≥ %lx\n",
> > +			       request->channels,
> > +			       BIT(IEEE802154_MAX_CHANNEL + 1));
> > +			err = -EINVAL;
> > +			goto free_request;
> > +		}
> > +	} else {
> > +		/* Scan all supported channels by default */
> > +		request->channels = wpan_phy->supported.channels[request->page];
> > +	}
> > +
> > +	if (info->attrs[NL802154_ATTR_SCAN_PREAMBLE_CODES] ||
> > +	    info->attrs[NL802154_ATTR_SCAN_MEAN_PRF]) {
> > +		pr_err("Preamble codes and mean PRF not supported yet\n");  
> 
> NLA_REJECT also in policy
> 
> > +		err = -EINVAL;
> > +		goto free_request;
> > +	}
> > +
> > +	if (info->attrs[NL802154_ATTR_SCAN_DURATION]) {
> > +		request->duration = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_DURATION]);
> > +		if (request->duration > IEEE802154_MAX_SCAN_DURATION) {
> > +			pr_err("Duration is out of range\n");
> > +			err = -EINVAL;
> > +			goto free_request;
> > +		}
> > +	} else {
> > +		/* Use maximum duration order by default */
> > +		request->duration = IEEE802154_MAX_SCAN_DURATION;
> > +	}
> > +
> > +	if (wpan_dev->netdev)
> > +		dev_hold(wpan_dev->netdev);  
> 
> Can we put a tracker in the request and use netdev_hold() ?

I'll look into it.

Thanks,
Miquèl

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

[Stefan Schmidt]

Hello.

On 10.02.23 11:18, Miquel Raynal wrote:
> Hi Stefan, Jakub,
> 
> kuba@kernel.org wrote on Fri, 3 Feb 2023 20:19:23 -0800:
> 
>> On Tue, 29 Nov 2022 17:00:41 +0100 Miquel Raynal wrote:
>>> +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
>>> +{
>>> +	struct cfg802154_registered_device *rdev = info->user_ptr[0];
>>> +	struct net_device *dev = info->user_ptr[1];
>>> +	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
>>> +	struct wpan_phy *wpan_phy = &rdev->wpan_phy;
>>> +	struct cfg802154_scan_request *request;
>>> +	u8 type;
>>> +	int err;
>>> +
>>> +	/* Monitors are not allowed to perform scans */
>>> +	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
>>
>> extack ?
> 
> Thanks for pointing at it, I just did know about it. I did convert
> most of the printk's into extack strings. Shall I keep both or is fine
> to just keep the extack thing?
> 
> For now I've dropped the printk's, please tell me if this is wrong.
> 
>>
>>> +		return -EPERM;
> 
> Stefan, do you prefer a series of patches applying on top of your
> current next or should I re-roll the entire series (scan + beacons)?
> 
> I am preparing a series applying on top of the current list of applied
> patches. This means next PR to net maintainers will include this patch
> as it is today + fixes on top. If this is fine for both parties, I will
> send these (including the other changes discussed with Alexander). Just
> let me know.

On top please. The other patches are already sitting in a published git 
tree and I want to avoid doing a rebase on the published tree.

Once your new patches are in and Jakub is happy I will send an updated 
pull request with them included.

regards
Stefan Schmidt

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

[Miquel Raynal]

Hi Stefan,

stefan@datenfreihafen.org wrote on Fri, 10 Feb 2023 11:26:45 +0100:

> Hello.
> 
> On 10.02.23 11:18, Miquel Raynal wrote:
> > Hi Stefan, Jakub,
> > 
> > kuba@kernel.org wrote on Fri, 3 Feb 2023 20:19:23 -0800:
> >   
> >> On Tue, 29 Nov 2022 17:00:41 +0100 Miquel Raynal wrote:  
> >>> +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> >>> +{
> >>> +	struct cfg802154_registered_device *rdev = info->user_ptr[0];
> >>> +	struct net_device *dev = info->user_ptr[1];
> >>> +	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> >>> +	struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> >>> +	struct cfg802154_scan_request *request;
> >>> +	u8 type;
> >>> +	int err;
> >>> +
> >>> +	/* Monitors are not allowed to perform scans */
> >>> +	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)  
> >>
> >> extack ?  
> > 
> > Thanks for pointing at it, I just did know about it. I did convert
> > most of the printk's into extack strings. Shall I keep both or is fine
> > to just keep the extack thing?
> > 
> > For now I've dropped the printk's, please tell me if this is wrong.
> >   
> >>  
> >>> +		return -EPERM;  
> > 
> > Stefan, do you prefer a series of patches applying on top of your
> > current next or should I re-roll the entire series (scan + beacons)?
> > 
> > I am preparing a series applying on top of the current list of applied
> > patches. This means next PR to net maintainers will include this patch
> > as it is today + fixes on top. If this is fine for both parties, I will
> > send these (including the other changes discussed with Alexander). Just
> > let me know.  
> 
> On top please. The other patches are already sitting in a published git tree and I want to avoid doing a rebase on the published tree.
> 
> Once your new patches are in and Jakub is happy I will send an updated pull request with them included.

Thanks a lot for the quick answer!

Thanks,
Miquèl

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

[Miquel Raynal]

Hi Alexander,

aahringo@redhat.com wrote on Mon, 6 Feb 2023 19:33:24 -0500:

> Hi,
> 
> On Mon, Feb 6, 2023 at 4:13 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > aahringo@redhat.com wrote on Sun, 5 Feb 2023 20:39:32 -0500:
> >  
> > > Hi,
> > >
> > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal
> > > <miquel.raynal@bootlin.com> wrote:  
> > > >
> > > > The ieee802154 layer should be able to scan a set of channels in order
> > > > to look for beacons advertizing PANs. Supporting this involves adding
> > > > two user commands: triggering scans and aborting scans. The user should
> > > > also be notified when a new beacon is received and also upon scan
> > > > termination.
> > > >
> > > > A scan request structure is created to list the requirements and to be
> > > > accessed asynchronously when changing channels or receiving beacons.
> > > >
> > > > Mac layers may now implement the ->trigger_scan() and ->abort_scan()
> > > > hooks.
> > > >
> > > > Co-developed-by: David Girault <david.girault@qorvo.com>
> > > > Signed-off-by: David Girault <david.girault@qorvo.com>
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  include/linux/ieee802154.h |   3 +
> > > >  include/net/cfg802154.h    |  25 +++++
> > > >  include/net/nl802154.h     |  49 +++++++++
> > > >  net/ieee802154/nl802154.c  | 215 +++++++++++++++++++++++++++++++++++++
> > > >  net/ieee802154/nl802154.h  |   3 +
> > > >  net/ieee802154/rdev-ops.h  |  28 +++++
> > > >  net/ieee802154/trace.h     |  40 +++++++
> > > >  7 files changed, 363 insertions(+)
> > > >
> > > > diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
> > > > index 0303eb84d596..b22e4147d334 100644
> > > > --- a/include/linux/ieee802154.h
> > > > +++ b/include/linux/ieee802154.h
> > > > @@ -44,6 +44,9 @@
> > > >  #define IEEE802154_SHORT_ADDR_LEN      2
> > > >  #define IEEE802154_PAN_ID_LEN          2
> > > >
> > > > +/* Duration in superframe order */
> > > > +#define IEEE802154_MAX_SCAN_DURATION   14
> > > > +#define IEEE802154_ACTIVE_SCAN_DURATION        15
> > > >  #define IEEE802154_LIFS_PERIOD         40
> > > >  #define IEEE802154_SIFS_PERIOD         12
> > > >  #define IEEE802154_MAX_SIFS_FRAME_SIZE 18
> > > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > > > index d09c393d229f..76d4f95e9974 100644
> > > > --- a/include/net/cfg802154.h
> > > > +++ b/include/net/cfg802154.h
> > > > @@ -18,6 +18,7 @@
> > > >
> > > >  struct wpan_phy;
> > > >  struct wpan_phy_cca;
> > > > +struct cfg802154_scan_request;
> > > >
> > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > >  struct ieee802154_llsec_device_key;
> > > > @@ -67,6 +68,10 @@ struct cfg802154_ops {
> > > >                                 struct wpan_dev *wpan_dev, bool mode);
> > > >         int     (*set_ackreq_default)(struct wpan_phy *wpan_phy,
> > > >                                       struct wpan_dev *wpan_dev, bool ackreq);
> > > > +       int     (*trigger_scan)(struct wpan_phy *wpan_phy,
> > > > +                               struct cfg802154_scan_request *request);
> > > > +       int     (*abort_scan)(struct wpan_phy *wpan_phy,
> > > > +                             struct wpan_dev *wpan_dev);
> > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > >         void    (*get_llsec_table)(struct wpan_phy *wpan_phy,
> > > >                                    struct wpan_dev *wpan_dev,
> > > > @@ -278,6 +283,26 @@ struct ieee802154_coord_desc {
> > > >         bool gts_permit;
> > > >  };
> > > >
> > > > +/**
> > > > + * struct cfg802154_scan_request - Scan request
> > > > + *
> > > > + * @type: type of scan to be performed
> > > > + * @page: page on which to perform the scan
> > > > + * @channels: channels in te %page to be scanned
> > > > + * @duration: time spent on each channel, calculated with:
> > > > + *            aBaseSuperframeDuration * (2 ^ duration + 1)
> > > > + * @wpan_dev: the wpan device on which to perform the scan
> > > > + * @wpan_phy: the wpan phy on which to perform the scan
> > > > + */
> > > > +struct cfg802154_scan_request {
> > > > +       enum nl802154_scan_types type;
> > > > +       u8 page;
> > > > +       u32 channels;
> > > > +       u8 duration;
> > > > +       struct wpan_dev *wpan_dev;
> > > > +       struct wpan_phy *wpan_phy;
> > > > +};
> > > > +
> > > >  struct ieee802154_llsec_key_id {
> > > >         u8 mode;
> > > >         u8 id;
> > > > diff --git a/include/net/nl802154.h b/include/net/nl802154.h
> > > > index b79a89d5207c..79fbd820b25a 100644
> > > > --- a/include/net/nl802154.h
> > > > +++ b/include/net/nl802154.h
> > > > @@ -73,6 +73,9 @@ enum nl802154_commands {
> > > >         NL802154_CMD_DEL_SEC_LEVEL,
> > > >
> > > >         NL802154_CMD_SCAN_EVENT,
> > > > +       NL802154_CMD_TRIGGER_SCAN,
> > > > +       NL802154_CMD_ABORT_SCAN,
> > > > +       NL802154_CMD_SCAN_DONE,
> > > >
> > > >         /* add new commands above here */
> > > >
> > > > @@ -134,6 +137,12 @@ enum nl802154_attrs {
> > > >         NL802154_ATTR_NETNS_FD,
> > > >
> > > >         NL802154_ATTR_COORDINATOR,
> > > > +       NL802154_ATTR_SCAN_TYPE,
> > > > +       NL802154_ATTR_SCAN_FLAGS,
> > > > +       NL802154_ATTR_SCAN_CHANNELS,
> > > > +       NL802154_ATTR_SCAN_PREAMBLE_CODES,
> > > > +       NL802154_ATTR_SCAN_MEAN_PRF,
> > > > +       NL802154_ATTR_SCAN_DURATION,
> > > >
> > > >         /* add attributes here, update the policy in nl802154.c */
> > > >
> > > > @@ -259,6 +268,46 @@ enum nl802154_coord {
> > > >         NL802154_COORD_MAX,
> > > >  };
> > > >
> > > > +/**
> > > > + * enum nl802154_scan_types - Scan types
> > > > + *
> > > > + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved
> > > > + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak
> > > > + *     energy in each requested channel
> > > > + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using
> > > > + *     a Beacon Request command
> > > > + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames
> > > > + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation
> > > > + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request
> > > > + *     command instead of Beacon Request command
> > > > + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames
> > > > + *     instead of Beacon frames
> > > > + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number
> > > > + */
> > > > +enum nl802154_scan_types {
> > > > +       __NL802154_SCAN_INVALID,
> > > > +       NL802154_SCAN_ED,
> > > > +       NL802154_SCAN_ACTIVE,
> > > > +       NL802154_SCAN_PASSIVE,
> > > > +       NL802154_SCAN_ORPHAN,
> > > > +       NL802154_SCAN_ENHANCED_ACTIVE,
> > > > +       NL802154_SCAN_RIT_PASSIVE,
> > > > +
> > > > +       /* keep last */
> > > > +       NL802154_SCAN_ATTR_MAX,
> > > > +};
> > > > +
> > > > +/**
> > > > + * enum nl802154_scan_flags - Scan request control flags
> > > > + *
> > > > + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie.
> > > > + *     a different one for every scan iteration). When the flag is set, full
> > > > + *     randomisation is assumed.
> > > > + */
> > > > +enum nl802154_scan_flags {
> > > > +       NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0),
> > > > +};
> > > > +
> > > >  /**
> > > >   * enum nl802154_cca_modes - cca modes
> > > >   *
> > > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> > > > index 80dc73182785..c497ffd8e897 100644
> > > > --- a/net/ieee802154/nl802154.c
> > > > +++ b/net/ieee802154/nl802154.c
> > > > @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
> > > >
> > > >         [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED },
> > > >
> > > > +       [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 },
> > > > +       [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 },
> > > > +       [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 },
> > > > +       [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 },
> > > > +       [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 },
> > > > +
> > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > >         [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
> > > >         [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, },
> > > > @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(nl802154_scan_event);
> > > >
> > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > +{
> > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > +       struct net_device *dev = info->user_ptr[1];
> > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > +       struct cfg802154_scan_request *request;
> > > > +       u8 type;
> > > > +       int err;
> > > > +
> > > > +       /* Monitors are not allowed to perform scans */
> > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > +               return -EPERM;  
> > >
> > > btw: why are monitors not allowed?  
> >
> > I guess I had the "active scan" use case in mind which of course does
> > not work with monitors. Maybe I can relax this a little bit indeed,
> > right now I don't remember why I strongly refused scans on monitors.  
> 
> Isn't it that scans really work close to phy level? Means in this case
> we disable mostly everything of MAC filtering on the transceiver side.
> Then I don't see any reasons why even monitors can't do anything, they
> also can send something. But they really don't have any specific
> source address set, so long addresses are none for source addresses, I
> don't see any problem here. They also don't have AACK handling, but
> it's not required for scan anyway...

I think I remember why I did not want to enable scans on monitors: we
actually change the filtering level to "scan", which is very
different to what a monitor is supposed to receive, which means in scan
mode a monitor would no longer receive all what it is supposed to
receive. Nothing that cannot be workaround'ed by software, probably,
but I believe it is safer right now to avoid introducing potential
regressions. So I will just change the error code and still refuse
scans on monitor interfaces for now, until we figure out if it's
actually safe or not (and if we really want to allow it).

> If this gets too complicated right now, then I am also fine with
> returning an error here, we can enable it later but would it be better
> to use ENOTSUPP or something like that in this case? EPERM sounds like
> you can do that, but you don't have the permissions.

Got it.

Thanks,
Miquèl

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

[Jakub Kicinski]

On Fri, 10 Feb 2023 11:18:43 +0100 Miquel Raynal wrote:
> > > +	/* Monitors are not allowed to perform scans */
> > > +	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)    
> > 
> > extack ?  
> 
> Thanks for pointing at it, I just did know about it. I did convert
> most of the printk's into extack strings. Shall I keep both or is fine
> to just keep the extack thing?
> 
> For now I've dropped the printk's, please tell me if this is wrong.

That's right - just the extack, we don't want duplicated prints.

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

[Miquel Raynal]

Hi Jakub,

kuba@kernel.org wrote on Fri, 10 Feb 2023 10:59:25 -0800:

> On Fri, 10 Feb 2023 11:18:43 +0100 Miquel Raynal wrote:
> > > > +	/* Monitors are not allowed to perform scans */
> > > > +	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)      
> > > 
> > > extack ?    
> > 
> > Thanks for pointing at it, I just did know about it. I did convert
> > most of the printk's into extack strings. Shall I keep both or is fine
> > to just keep the extack thing?
> > 
> > For now I've dropped the printk's, please tell me if this is wrong.  
> 
> That's right - just the extack, we don't want duplicated prints.

Thanks for the confirmation.

Series ready, I need to test it further just to verify there is no
unwanted regression, I'll send it early next week.

Thanks,
Miquèl

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

[Alexander Aring]

Hi,

On Fri, Feb 10, 2023 at 12:21 PM Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> aahringo@redhat.com wrote on Mon, 6 Feb 2023 19:33:24 -0500:
>
> > Hi,
> >
> > On Mon, Feb 6, 2023 at 4:13 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Alexander,
> > >
> > > aahringo@redhat.com wrote on Sun, 5 Feb 2023 20:39:32 -0500:
> > >
> > > > Hi,
> > > >
> > > > On Tue, Nov 29, 2022 at 11:02 AM Miquel Raynal
> > > > <miquel.raynal@bootlin.com> wrote:
> > > > >
> > > > > The ieee802154 layer should be able to scan a set of channels in order
> > > > > to look for beacons advertizing PANs. Supporting this involves adding
> > > > > two user commands: triggering scans and aborting scans. The user should
> > > > > also be notified when a new beacon is received and also upon scan
> > > > > termination.
> > > > >
> > > > > A scan request structure is created to list the requirements and to be
> > > > > accessed asynchronously when changing channels or receiving beacons.
> > > > >
> > > > > Mac layers may now implement the ->trigger_scan() and ->abort_scan()
> > > > > hooks.
> > > > >
> > > > > Co-developed-by: David Girault <david.girault@qorvo.com>
> > > > > Signed-off-by: David Girault <david.girault@qorvo.com>
> > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > ---
> > > > >  include/linux/ieee802154.h |   3 +
> > > > >  include/net/cfg802154.h    |  25 +++++
> > > > >  include/net/nl802154.h     |  49 +++++++++
> > > > >  net/ieee802154/nl802154.c  | 215 +++++++++++++++++++++++++++++++++++++
> > > > >  net/ieee802154/nl802154.h  |   3 +
> > > > >  net/ieee802154/rdev-ops.h  |  28 +++++
> > > > >  net/ieee802154/trace.h     |  40 +++++++
> > > > >  7 files changed, 363 insertions(+)
> > > > >
> > > > > diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
> > > > > index 0303eb84d596..b22e4147d334 100644
> > > > > --- a/include/linux/ieee802154.h
> > > > > +++ b/include/linux/ieee802154.h
> > > > > @@ -44,6 +44,9 @@
> > > > >  #define IEEE802154_SHORT_ADDR_LEN      2
> > > > >  #define IEEE802154_PAN_ID_LEN          2
> > > > >
> > > > > +/* Duration in superframe order */
> > > > > +#define IEEE802154_MAX_SCAN_DURATION   14
> > > > > +#define IEEE802154_ACTIVE_SCAN_DURATION        15
> > > > >  #define IEEE802154_LIFS_PERIOD         40
> > > > >  #define IEEE802154_SIFS_PERIOD         12
> > > > >  #define IEEE802154_MAX_SIFS_FRAME_SIZE 18
> > > > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > > > > index d09c393d229f..76d4f95e9974 100644
> > > > > --- a/include/net/cfg802154.h
> > > > > +++ b/include/net/cfg802154.h
> > > > > @@ -18,6 +18,7 @@
> > > > >
> > > > >  struct wpan_phy;
> > > > >  struct wpan_phy_cca;
> > > > > +struct cfg802154_scan_request;
> > > > >
> > > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > > >  struct ieee802154_llsec_device_key;
> > > > > @@ -67,6 +68,10 @@ struct cfg802154_ops {
> > > > >                                 struct wpan_dev *wpan_dev, bool mode);
> > > > >         int     (*set_ackreq_default)(struct wpan_phy *wpan_phy,
> > > > >                                       struct wpan_dev *wpan_dev, bool ackreq);
> > > > > +       int     (*trigger_scan)(struct wpan_phy *wpan_phy,
> > > > > +                               struct cfg802154_scan_request *request);
> > > > > +       int     (*abort_scan)(struct wpan_phy *wpan_phy,
> > > > > +                             struct wpan_dev *wpan_dev);
> > > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > > >         void    (*get_llsec_table)(struct wpan_phy *wpan_phy,
> > > > >                                    struct wpan_dev *wpan_dev,
> > > > > @@ -278,6 +283,26 @@ struct ieee802154_coord_desc {
> > > > >         bool gts_permit;
> > > > >  };
> > > > >
> > > > > +/**
> > > > > + * struct cfg802154_scan_request - Scan request
> > > > > + *
> > > > > + * @type: type of scan to be performed
> > > > > + * @page: page on which to perform the scan
> > > > > + * @channels: channels in te %page to be scanned
> > > > > + * @duration: time spent on each channel, calculated with:
> > > > > + *            aBaseSuperframeDuration * (2 ^ duration + 1)
> > > > > + * @wpan_dev: the wpan device on which to perform the scan
> > > > > + * @wpan_phy: the wpan phy on which to perform the scan
> > > > > + */
> > > > > +struct cfg802154_scan_request {
> > > > > +       enum nl802154_scan_types type;
> > > > > +       u8 page;
> > > > > +       u32 channels;
> > > > > +       u8 duration;
> > > > > +       struct wpan_dev *wpan_dev;
> > > > > +       struct wpan_phy *wpan_phy;
> > > > > +};
> > > > > +
> > > > >  struct ieee802154_llsec_key_id {
> > > > >         u8 mode;
> > > > >         u8 id;
> > > > > diff --git a/include/net/nl802154.h b/include/net/nl802154.h
> > > > > index b79a89d5207c..79fbd820b25a 100644
> > > > > --- a/include/net/nl802154.h
> > > > > +++ b/include/net/nl802154.h
> > > > > @@ -73,6 +73,9 @@ enum nl802154_commands {
> > > > >         NL802154_CMD_DEL_SEC_LEVEL,
> > > > >
> > > > >         NL802154_CMD_SCAN_EVENT,
> > > > > +       NL802154_CMD_TRIGGER_SCAN,
> > > > > +       NL802154_CMD_ABORT_SCAN,
> > > > > +       NL802154_CMD_SCAN_DONE,
> > > > >
> > > > >         /* add new commands above here */
> > > > >
> > > > > @@ -134,6 +137,12 @@ enum nl802154_attrs {
> > > > >         NL802154_ATTR_NETNS_FD,
> > > > >
> > > > >         NL802154_ATTR_COORDINATOR,
> > > > > +       NL802154_ATTR_SCAN_TYPE,
> > > > > +       NL802154_ATTR_SCAN_FLAGS,
> > > > > +       NL802154_ATTR_SCAN_CHANNELS,
> > > > > +       NL802154_ATTR_SCAN_PREAMBLE_CODES,
> > > > > +       NL802154_ATTR_SCAN_MEAN_PRF,
> > > > > +       NL802154_ATTR_SCAN_DURATION,
> > > > >
> > > > >         /* add attributes here, update the policy in nl802154.c */
> > > > >
> > > > > @@ -259,6 +268,46 @@ enum nl802154_coord {
> > > > >         NL802154_COORD_MAX,
> > > > >  };
> > > > >
> > > > > +/**
> > > > > + * enum nl802154_scan_types - Scan types
> > > > > + *
> > > > > + * @__NL802154_SCAN_INVALID: scan type number 0 is reserved
> > > > > + * @NL802154_SCAN_ED: An ED scan allows a device to obtain a measure of the peak
> > > > > + *     energy in each requested channel
> > > > > + * @NL802154_SCAN_ACTIVE: Locate any coordinator transmitting Beacon frames using
> > > > > + *     a Beacon Request command
> > > > > + * @NL802154_SCAN_PASSIVE: Locate any coordinator transmitting Beacon frames
> > > > > + * @NL802154_SCAN_ORPHAN: Relocate coordinator following a loss of synchronisation
> > > > > + * @NL802154_SCAN_ENHANCED_ACTIVE: Same as Active using Enhanced Beacon Request
> > > > > + *     command instead of Beacon Request command
> > > > > + * @NL802154_SCAN_RIT_PASSIVE: Passive scan for RIT Data Request command frames
> > > > > + *     instead of Beacon frames
> > > > > + * @NL802154_SCAN_ATTR_MAX: Maximum SCAN attribute number
> > > > > + */
> > > > > +enum nl802154_scan_types {
> > > > > +       __NL802154_SCAN_INVALID,
> > > > > +       NL802154_SCAN_ED,
> > > > > +       NL802154_SCAN_ACTIVE,
> > > > > +       NL802154_SCAN_PASSIVE,
> > > > > +       NL802154_SCAN_ORPHAN,
> > > > > +       NL802154_SCAN_ENHANCED_ACTIVE,
> > > > > +       NL802154_SCAN_RIT_PASSIVE,
> > > > > +
> > > > > +       /* keep last */
> > > > > +       NL802154_SCAN_ATTR_MAX,
> > > > > +};
> > > > > +
> > > > > +/**
> > > > > + * enum nl802154_scan_flags - Scan request control flags
> > > > > + *
> > > > > + * @NL802154_SCAN_FLAG_RANDOM_ADDR: use a random MAC address for this scan (ie.
> > > > > + *     a different one for every scan iteration). When the flag is set, full
> > > > > + *     randomisation is assumed.
> > > > > + */
> > > > > +enum nl802154_scan_flags {
> > > > > +       NL802154_SCAN_FLAG_RANDOM_ADDR = BIT(0),
> > > > > +};
> > > > > +
> > > > >  /**
> > > > >   * enum nl802154_cca_modes - cca modes
> > > > >   *
> > > > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> > > > > index 80dc73182785..c497ffd8e897 100644
> > > > > --- a/net/ieee802154/nl802154.c
> > > > > +++ b/net/ieee802154/nl802154.c
> > > > > @@ -221,6 +221,12 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
> > > > >
> > > > >         [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED },
> > > > >
> > > > > +       [NL802154_ATTR_SCAN_TYPE] = { .type = NLA_U8 },
> > > > > +       [NL802154_ATTR_SCAN_CHANNELS] = { .type = NLA_U32 },
> > > > > +       [NL802154_ATTR_SCAN_PREAMBLE_CODES] = { .type = NLA_U64 },
> > > > > +       [NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 },
> > > > > +       [NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 },
> > > > > +
> > > > >  #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> > > > >         [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
> > > > >         [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, },
> > > > > @@ -1384,6 +1390,199 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
> > > > >  }
> > > > >  EXPORT_SYMBOL_GPL(nl802154_scan_event);
> > > > >
> > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > +{
> > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > +       struct cfg802154_scan_request *request;
> > > > > +       u8 type;
> > > > > +       int err;
> > > > > +
> > > > > +       /* Monitors are not allowed to perform scans */
> > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > +               return -EPERM;
> > > >
> > > > btw: why are monitors not allowed?
> > >
> > > I guess I had the "active scan" use case in mind which of course does
> > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > right now I don't remember why I strongly refused scans on monitors.
> >
> > Isn't it that scans really work close to phy level? Means in this case
> > we disable mostly everything of MAC filtering on the transceiver side.
> > Then I don't see any reasons why even monitors can't do anything, they
> > also can send something. But they really don't have any specific
> > source address set, so long addresses are none for source addresses, I
> > don't see any problem here. They also don't have AACK handling, but
> > it's not required for scan anyway...
>
> I think I remember why I did not want to enable scans on monitors: we
> actually change the filtering level to "scan", which is very
> different to what a monitor is supposed to receive, which means in scan
> mode a monitor would no longer receive all what it is supposed to
> receive. Nothing that cannot be workaround'ed by software, probably,
> but I believe it is safer right now to avoid introducing potential
> regressions. So I will just change the error code and still refuse
> scans on monitor interfaces for now, until we figure out if it's
> actually safe or not (and if we really want to allow it).
>

Okay, for scan yes we tell them to be in scan mode and then the
transceiver can filter whatever it delivers to the next level which is
necessary for filtering scan mac frames only. AACK handling is
disabled for scan mode for all types != MONITORS.

For monitors we mostly allow everything and AACK is _always_ disabled.
The transceiver filter is completely disabled for at least what looks
like a 802.15.4 MAC header (even malformed). There are some frame
length checks which are necessary for specific hardware because
otherwise they would read out the frame buffer. For me it can still
feed the mac802154 stack for scanning (with filtering level as what
the monitor sets to, but currently our scan filter is equal to the
monitor filter mode anyway (which probably can be changed in
future?)). So in my opinion the monitor can do both -> feed the scan
mac802154 deliver path and the packet layer. And I also think that on
a normal interface type the packet layer should be feeded by those
frames as well and do not hit the mac802154 layer scan path only.

However this can be done in future and I think, indeed there might be
other problems to tackle to enable such functionality.

- Alex

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

[Miquel Raynal]

Hi Alexander,

> > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > +{
> > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > +       struct cfg802154_scan_request *request;
> > > > > > +       u8 type;
> > > > > > +       int err;
> > > > > > +
> > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > +               return -EPERM;  
> > > > >
> > > > > btw: why are monitors not allowed?  
> > > >
> > > > I guess I had the "active scan" use case in mind which of course does
> > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > right now I don't remember why I strongly refused scans on monitors.  
> > >
> > > Isn't it that scans really work close to phy level? Means in this case
> > > we disable mostly everything of MAC filtering on the transceiver side.
> > > Then I don't see any reasons why even monitors can't do anything, they
> > > also can send something. But they really don't have any specific
> > > source address set, so long addresses are none for source addresses, I
> > > don't see any problem here. They also don't have AACK handling, but
> > > it's not required for scan anyway...  
> >
> > I think I remember why I did not want to enable scans on monitors: we
> > actually change the filtering level to "scan", which is very
> > different to what a monitor is supposed to receive, which means in scan
> > mode a monitor would no longer receive all what it is supposed to
> > receive. Nothing that cannot be workaround'ed by software, probably,
> > but I believe it is safer right now to avoid introducing potential
> > regressions. So I will just change the error code and still refuse
> > scans on monitor interfaces for now, until we figure out if it's
> > actually safe or not (and if we really want to allow it).
> >  
> 
> Okay, for scan yes we tell them to be in scan mode and then the
> transceiver can filter whatever it delivers to the next level which is
> necessary for filtering scan mac frames only. AACK handling is
> disabled for scan mode for all types != MONITORS.
> 
> For monitors we mostly allow everything and AACK is _always_ disabled.
> The transceiver filter is completely disabled for at least what looks
> like a 802.15.4 MAC header (even malformed). There are some frame
> length checks which are necessary for specific hardware because
> otherwise they would read out the frame buffer. For me it can still
> feed the mac802154 stack for scanning (with filtering level as what
> the monitor sets to, but currently our scan filter is equal to the
> monitor filter mode anyway (which probably can be changed in
> future?)). So in my opinion the monitor can do both -> feed the scan
> mac802154 deliver path and the packet layer. And I also think that on
> a normal interface type the packet layer should be feeded by those
> frames as well and do not hit the mac802154 layer scan path only.

Actually that would be an out-of-spec situation, here is a quote of
chapter "6.3.1.3 Active and passive channel scan"

	During an active or passive scan, the MAC sublayer shall
	discard all frames received over the PHY data service that are
	not Beacon frames.

I don't think this is possible to do anyway on devices with a single
hardware filter setting?

> However this can be done in future and I think, indeed there might be
> other problems to tackle to enable such functionality.

Indeed.

Thanks,
Miquèl

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

[Miquel Raynal]

Hi Alexander,

> > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > +{
> > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > +       struct cfg802154_scan_request *request;
> > > > > +       u8 type;
> > > > > +       int err;
> > > > > +
> > > > > +       /* Monitors are not allowed to perform scans */
> > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > +               return -EPERM;  
> > > >
> > > > btw: why are monitors not allowed?  
> > >
> > > I guess I had the "active scan" use case in mind which of course does
> > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > right now I don't remember why I strongly refused scans on monitors.  
> >
> > Isn't it that scans really work close to phy level? Means in this case
> > we disable mostly everything of MAC filtering on the transceiver side.
> > Then I don't see any reasons why even monitors can't do anything, they
> > also can send something. But they really don't have any specific
> > source address set, so long addresses are none for source addresses, I
> > don't see any problem here. They also don't have AACK handling, but
> > it's not required for scan anyway...
> >
> > If this gets too complicated right now, then I am also fine with
> > returning an error here, we can enable it later but would it be better
> > to use ENOTSUPP or something like that in this case? EPERM sounds like
> > you can do that, but you don't have the permissions.
> >  
> 
> For me a scan should also be possible from iwpan phy $PHY scan (or
> whatever the scan command is, or just enable beacon)... to go over the
> dev is just a shortcut for "I mean whatever the phy is under this dev"
> ?

Actually only coordinators (in a specific state) should be able to send
beacons, so I am kind of against allowing that shortcut, because there
are usually two dev interfaces on top of the phy's, a regular "NODE"
and a "COORD", so I don't think we should go that way.

For scans however it makes sense, I've added the necessary changes in
wpan-tools. The TOP_LEVEL(scan) macro however does not support using
the same command name twice because it creates a macro, so this one
only supports a device name (the interface command has kind of the same
situation and uses a HIDDEN() macro which cannot be used here).

So in summary here is what is supported:
- dev <dev> beacon
- dev <dev> scan trigger|abort
- phy <phy> scan trigger|abort
- dev <dev> scan (the blocking one, which triggers, listens and returns)

Do you agree?

Thanks,
Miquèl

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

[Alexander Aring]

Hi,

On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > +{
> > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > +       struct cfg802154_scan_request *request;
> > > > > > +       u8 type;
> > > > > > +       int err;
> > > > > > +
> > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > +               return -EPERM;
> > > > >
> > > > > btw: why are monitors not allowed?
> > > >
> > > > I guess I had the "active scan" use case in mind which of course does
> > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > right now I don't remember why I strongly refused scans on monitors.
> > >
> > > Isn't it that scans really work close to phy level? Means in this case
> > > we disable mostly everything of MAC filtering on the transceiver side.
> > > Then I don't see any reasons why even monitors can't do anything, they
> > > also can send something. But they really don't have any specific
> > > source address set, so long addresses are none for source addresses, I
> > > don't see any problem here. They also don't have AACK handling, but
> > > it's not required for scan anyway...
> > >
> > > If this gets too complicated right now, then I am also fine with
> > > returning an error here, we can enable it later but would it be better
> > > to use ENOTSUPP or something like that in this case? EPERM sounds like
> > > you can do that, but you don't have the permissions.
> > >
> >
> > For me a scan should also be possible from iwpan phy $PHY scan (or
> > whatever the scan command is, or just enable beacon)... to go over the
> > dev is just a shortcut for "I mean whatever the phy is under this dev"
> > ?
>
> Actually only coordinators (in a specific state) should be able to send
> beacons, so I am kind of against allowing that shortcut, because there
> are usually two dev interfaces on top of the phy's, a regular "NODE"
> and a "COORD", so I don't think we should go that way.
>
> For scans however it makes sense, I've added the necessary changes in
> wpan-tools. The TOP_LEVEL(scan) macro however does not support using
> the same command name twice because it creates a macro, so this one
> only supports a device name (the interface command has kind of the same
> situation and uses a HIDDEN() macro which cannot be used here).
>

Yes, I was thinking about scanning only.

> So in summary here is what is supported:
> - dev <dev> beacon
> - dev <dev> scan trigger|abort
> - phy <phy> scan trigger|abort
> - dev <dev> scan (the blocking one, which triggers, listens and returns)
>
> Do you agree?
>

Okay, yes. I trust you.

- Alex

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

[Alexander Aring]

Hi,

On Mon, Feb 13, 2023 at 5:16 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > > +{
> > > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > > +       struct cfg802154_scan_request *request;
> > > > > > > +       u8 type;
> > > > > > > +       int err;
> > > > > > > +
> > > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > > +               return -EPERM;
> > > > > >
> > > > > > btw: why are monitors not allowed?
> > > > >
> > > > > I guess I had the "active scan" use case in mind which of course does
> > > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > > right now I don't remember why I strongly refused scans on monitors.
> > > >
> > > > Isn't it that scans really work close to phy level? Means in this case
> > > > we disable mostly everything of MAC filtering on the transceiver side.
> > > > Then I don't see any reasons why even monitors can't do anything, they
> > > > also can send something. But they really don't have any specific
> > > > source address set, so long addresses are none for source addresses, I
> > > > don't see any problem here. They also don't have AACK handling, but
> > > > it's not required for scan anyway...
> > >
> > > I think I remember why I did not want to enable scans on monitors: we
> > > actually change the filtering level to "scan", which is very
> > > different to what a monitor is supposed to receive, which means in scan
> > > mode a monitor would no longer receive all what it is supposed to
> > > receive. Nothing that cannot be workaround'ed by software, probably,
> > > but I believe it is safer right now to avoid introducing potential
> > > regressions. So I will just change the error code and still refuse
> > > scans on monitor interfaces for now, until we figure out if it's
> > > actually safe or not (and if we really want to allow it).
> > >
> >
> > Okay, for scan yes we tell them to be in scan mode and then the
> > transceiver can filter whatever it delivers to the next level which is
> > necessary for filtering scan mac frames only. AACK handling is
> > disabled for scan mode for all types != MONITORS.
> >
> > For monitors we mostly allow everything and AACK is _always_ disabled.
> > The transceiver filter is completely disabled for at least what looks
> > like a 802.15.4 MAC header (even malformed). There are some frame
> > length checks which are necessary for specific hardware because
> > otherwise they would read out the frame buffer. For me it can still
> > feed the mac802154 stack for scanning (with filtering level as what
> > the monitor sets to, but currently our scan filter is equal to the
> > monitor filter mode anyway (which probably can be changed in
> > future?)). So in my opinion the monitor can do both -> feed the scan
> > mac802154 deliver path and the packet layer. And I also think that on
> > a normal interface type the packet layer should be feeded by those
> > frames as well and do not hit the mac802154 layer scan path only.
>
> Actually that would be an out-of-spec situation, here is a quote of
> chapter "6.3.1.3 Active and passive channel scan"
>
>         During an active or passive scan, the MAC sublayer shall
>         discard all frames received over the PHY data service that are
>         not Beacon frames.
>

Monitor interfaces are not anything that the spec describes, it is
some interface type which offers the user (mostly over AF_PACKET raw
socket) full phy level access with the _default_ options. I already
run user space stacks (for hacking/development only) on monitor
interfaces to connect with Linux 802.15.4 interfaces, e.g. see [0]
(newer came upstream, somewhere I have also a 2 years old updated
version, use hwsim not fakelb).

In other words, by default it should bypass 802.15.4 MAC and it still
conforms with your spec, just that it is in user space. However, there
exists problems to do that, but it kind of works for the most use
cases. I said here by default because some people have different use
cases of what they want to do in the kernel. e.g. encryption (so users
only get encrypted frames, etc.) We don't support that but we can,
same for doing a scan. It probably requires just more mac802154 layer
filtering.

There are enough examples in wireless that they do "crazy" things and
you can do that only with SoftMAC transceivers because it uses more
software parts like mac80211 and HardMAC transceivers only do what the
spec says and delivers it to the next layer. Some of them have more
functionality I guess, but then it depends on driver implementation
and a lot of other things.

Monitors also act as a sniffer device, but you _could_ also send
frames out and then the fun part begins.

> I don't think this is possible to do anyway on devices with a single
> hardware filter setting?
>

On SoftMAC it need to support a filtering level where we can disable
all filtering and get 802.15.4 MAC frames like it's on air (even
without non valid checksum, but we simply don't care if the
driver/transceiver does not support it we will always confirm it is
correct again until somebody comes around and say "oh we can do FCS
level then mac802154 does not need to check on this because it is
always correct")... This is currently the NONE filtering level I
think?

For HardMAC it is more complicated; they don't do that, they do the
"scan" operation on their transceiver and you can dump the result and
probably never forward any beacon related frames? (I had this question
once when Michael Richardson replied).

- Alex

[0] https://github.com/RIOT-OS/RIOT/pull/5582

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

[Alexander Aring]

Hi,

On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote:
>
> Hi,
>
> On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > > +{
> > > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > > +       struct cfg802154_scan_request *request;
> > > > > > > +       u8 type;
> > > > > > > +       int err;
> > > > > > > +
> > > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > > +               return -EPERM;
> > > > > >
> > > > > > btw: why are monitors not allowed?
> > > > >
> > > > > I guess I had the "active scan" use case in mind which of course does
> > > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > > right now I don't remember why I strongly refused scans on monitors.
> > > >
> > > > Isn't it that scans really work close to phy level? Means in this case
> > > > we disable mostly everything of MAC filtering on the transceiver side.
> > > > Then I don't see any reasons why even monitors can't do anything, they
> > > > also can send something. But they really don't have any specific
> > > > source address set, so long addresses are none for source addresses, I
> > > > don't see any problem here. They also don't have AACK handling, but
> > > > it's not required for scan anyway...
> > > >
> > > > If this gets too complicated right now, then I am also fine with
> > > > returning an error here, we can enable it later but would it be better
> > > > to use ENOTSUPP or something like that in this case? EPERM sounds like
> > > > you can do that, but you don't have the permissions.
> > > >
> > >
> > > For me a scan should also be possible from iwpan phy $PHY scan (or
> > > whatever the scan command is, or just enable beacon)... to go over the
> > > dev is just a shortcut for "I mean whatever the phy is under this dev"
> > > ?
> >
> > Actually only coordinators (in a specific state) should be able to send
> > beacons, so I am kind of against allowing that shortcut, because there
> > are usually two dev interfaces on top of the phy's, a regular "NODE"
> > and a "COORD", so I don't think we should go that way.
> >
> > For scans however it makes sense, I've added the necessary changes in
> > wpan-tools. The TOP_LEVEL(scan) macro however does not support using
> > the same command name twice because it creates a macro, so this one
> > only supports a device name (the interface command has kind of the same
> > situation and uses a HIDDEN() macro which cannot be used here).
> >
>
> Yes, I was thinking about scanning only.
>
> > So in summary here is what is supported:
> > - dev <dev> beacon
> > - dev <dev> scan trigger|abort
> > - phy <phy> scan trigger|abort
> > - dev <dev> scan (the blocking one, which triggers, listens and returns)
> >
> > Do you agree?
> >
>
> Okay, yes. I trust you.

btw: at the point when a scan requires a source address... it cannot
be done because then a scan is related to a MAC instance -> an wpan
interface and we need to bind to it. But I think it doesn't?

- Alex

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

[Miquel Raynal]

Hi Alexander,

aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500:

> Hi,
> 
> On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote:
> >
> > Hi,
> >
> > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal
> > <miquel.raynal@bootlin.com> wrote:  
> > >
> > > Hi Alexander,
> > >  
> > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > > > +{
> > > > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > > > +       struct cfg802154_scan_request *request;
> > > > > > > > +       u8 type;
> > > > > > > > +       int err;
> > > > > > > > +
> > > > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > > > +               return -EPERM;  
> > > > > > >
> > > > > > > btw: why are monitors not allowed?  
> > > > > >
> > > > > > I guess I had the "active scan" use case in mind which of course does
> > > > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > > > right now I don't remember why I strongly refused scans on monitors.  
> > > > >
> > > > > Isn't it that scans really work close to phy level? Means in this case
> > > > > we disable mostly everything of MAC filtering on the transceiver side.
> > > > > Then I don't see any reasons why even monitors can't do anything, they
> > > > > also can send something. But they really don't have any specific
> > > > > source address set, so long addresses are none for source addresses, I
> > > > > don't see any problem here. They also don't have AACK handling, but
> > > > > it's not required for scan anyway...
> > > > >
> > > > > If this gets too complicated right now, then I am also fine with
> > > > > returning an error here, we can enable it later but would it be better
> > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like
> > > > > you can do that, but you don't have the permissions.
> > > > >  
> > > >
> > > > For me a scan should also be possible from iwpan phy $PHY scan (or
> > > > whatever the scan command is, or just enable beacon)... to go over the
> > > > dev is just a shortcut for "I mean whatever the phy is under this dev"
> > > > ?  
> > >
> > > Actually only coordinators (in a specific state) should be able to send
> > > beacons, so I am kind of against allowing that shortcut, because there
> > > are usually two dev interfaces on top of the phy's, a regular "NODE"
> > > and a "COORD", so I don't think we should go that way.
> > >
> > > For scans however it makes sense, I've added the necessary changes in
> > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using
> > > the same command name twice because it creates a macro, so this one
> > > only supports a device name (the interface command has kind of the same
> > > situation and uses a HIDDEN() macro which cannot be used here).
> > >  
> >
> > Yes, I was thinking about scanning only.
> >  
> > > So in summary here is what is supported:
> > > - dev <dev> beacon
> > > - dev <dev> scan trigger|abort
> > > - phy <phy> scan trigger|abort
> > > - dev <dev> scan (the blocking one, which triggers, listens and returns)
> > >
> > > Do you agree?
> > >  
> >
> > Okay, yes. I trust you.  
> 
> btw: at the point when a scan requires a source address... it cannot
> be done because then a scan is related to a MAC instance -> an wpan
> interface and we need to bind to it. But I think it doesn't?

I'm not sure I follow you here. You mean in case of active scan? The
operation is always tight to a device in the end, even if you provide a
phy in userspace. So I guess it's not a problem. Or maybe I didn't get
the question right?

Thanks,
Miquèl

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

[Miquel Raynal]

Hi Alexander,

aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:51:12 -0500:

> Hi,
> 
> On Mon, Feb 13, 2023 at 5:16 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > > > +{
> > > > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > > > +       struct cfg802154_scan_request *request;
> > > > > > > > +       u8 type;
> > > > > > > > +       int err;
> > > > > > > > +
> > > > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > > > +               return -EPERM;
> > > > > > >
> > > > > > > btw: why are monitors not allowed?
> > > > > >
> > > > > > I guess I had the "active scan" use case in mind which of course does
> > > > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > > > right now I don't remember why I strongly refused scans on monitors.
> > > > >
> > > > > Isn't it that scans really work close to phy level? Means in this case
> > > > > we disable mostly everything of MAC filtering on the transceiver side.
> > > > > Then I don't see any reasons why even monitors can't do anything, they
> > > > > also can send something. But they really don't have any specific
> > > > > source address set, so long addresses are none for source addresses, I
> > > > > don't see any problem here. They also don't have AACK handling, but
> > > > > it's not required for scan anyway...
> > > >
> > > > I think I remember why I did not want to enable scans on monitors: we
> > > > actually change the filtering level to "scan", which is very
> > > > different to what a monitor is supposed to receive, which means in scan
> > > > mode a monitor would no longer receive all what it is supposed to
> > > > receive. Nothing that cannot be workaround'ed by software, probably,
> > > > but I believe it is safer right now to avoid introducing potential
> > > > regressions. So I will just change the error code and still refuse
> > > > scans on monitor interfaces for now, until we figure out if it's
> > > > actually safe or not (and if we really want to allow it).
> > > >
> > >
> > > Okay, for scan yes we tell them to be in scan mode and then the
> > > transceiver can filter whatever it delivers to the next level which is
> > > necessary for filtering scan mac frames only. AACK handling is
> > > disabled for scan mode for all types != MONITORS.
> > >
> > > For monitors we mostly allow everything and AACK is _always_ disabled.
> > > The transceiver filter is completely disabled for at least what looks
> > > like a 802.15.4 MAC header (even malformed). There are some frame
> > > length checks which are necessary for specific hardware because
> > > otherwise they would read out the frame buffer. For me it can still
> > > feed the mac802154 stack for scanning (with filtering level as what
> > > the monitor sets to, but currently our scan filter is equal to the
> > > monitor filter mode anyway (which probably can be changed in
> > > future?)). So in my opinion the monitor can do both -> feed the scan
> > > mac802154 deliver path and the packet layer. And I also think that on
> > > a normal interface type the packet layer should be feeded by those
> > > frames as well and do not hit the mac802154 layer scan path only.
> >
> > Actually that would be an out-of-spec situation, here is a quote of
> > chapter "6.3.1.3 Active and passive channel scan"
> >
> >         During an active or passive scan, the MAC sublayer shall
> >         discard all frames received over the PHY data service that are
> >         not Beacon frames.
> >
> 
> Monitor interfaces are not anything that the spec describes, it is
> some interface type which offers the user (mostly over AF_PACKET raw
> socket) full phy level access with the _default_ options. I already
> run user space stacks (for hacking/development only) on monitor
> interfaces to connect with Linux 802.15.4 interfaces, e.g. see [0]
> (newer came upstream, somewhere I have also a 2 years old updated
> version, use hwsim not fakelb).

:-)

> 
> In other words, by default it should bypass 802.15.4 MAC and it still
> conforms with your spec, just that it is in user space. However, there
> exists problems to do that, but it kind of works for the most use
> cases. I said here by default because some people have different use
> cases of what they want to do in the kernel. e.g. encryption (so users
> only get encrypted frames, etc.) We don't support that but we can,
> same for doing a scan. It probably requires just more mac802154 layer
> filtering.
> 
> There are enough examples in wireless that they do "crazy" things and
> you can do that only with SoftMAC transceivers because it uses more
> software parts like mac80211 and HardMAC transceivers only do what the
> spec says and delivers it to the next layer. Some of them have more
> functionality I guess, but then it depends on driver implementation
> and a lot of other things.
> 
> Monitors also act as a sniffer device, but you _could_ also send
> frames out and then the fun part begins.

Yes, you're right, it's up to us to allow monitor actions.

> > I don't think this is possible to do anyway on devices with a single
> > hardware filter setting?
> >
> 
> On SoftMAC it need to support a filtering level where we can disable
> all filtering and get 802.15.4 MAC frames like it's on air (even
> without non valid checksum, but we simply don't care if the
> driver/transceiver does not support it we will always confirm it is
> correct again until somebody comes around and say "oh we can do FCS
> level then mac802154 does not need to check on this because it is
> always correct")... This is currently the NONE filtering level I
> think?

But right now we ask for the "scan" filtering, which kind of discards
most frames. Would you like a special config for monitors, like
receiving everything on each channel you jump on? Or shall we stick to
only transmitting beacon frames during a scan on a monitor interface?

I guess it's rather easy to handle in each case. Just let me know what
you prefer. I think I have a small preference for the scan filtering
level, but I'm open.

> For HardMAC it is more complicated; they don't do that, they do the
> "scan" operation on their transceiver and you can dump the result and
> probably never forward any beacon related frames? (I had this question
> once when Michael Richardson replied).

Yes, in this case we'll have to figure out something else...

> 
> - Alex
> 
> [0] https://github.com/RIOT-OS/RIOT/pull/5582
> 

Thanks,
Miquèl

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

[Miquel Raynal]

miquel.raynal@bootlin.com wrote on Tue, 14 Feb 2023 15:06:00 +0100:

> Hi Alexander,
> 
> aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500:
> 
> > Hi,
> > 
> > On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote:  
> > >
> > > Hi,
> > >
> > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal
> > > <miquel.raynal@bootlin.com> wrote:    
> > > >
> > > > Hi Alexander,
> > > >    
> > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > > > > +{
> > > > > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > > > > +       struct cfg802154_scan_request *request;
> > > > > > > > > +       u8 type;
> > > > > > > > > +       int err;
> > > > > > > > > +
> > > > > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > > > > +               return -EPERM;    
> > > > > > > >
> > > > > > > > btw: why are monitors not allowed?    
> > > > > > >
> > > > > > > I guess I had the "active scan" use case in mind which of course does
> > > > > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > > > > right now I don't remember why I strongly refused scans on monitors.    
> > > > > >
> > > > > > Isn't it that scans really work close to phy level? Means in this case
> > > > > > we disable mostly everything of MAC filtering on the transceiver side.
> > > > > > Then I don't see any reasons why even monitors can't do anything, they
> > > > > > also can send something. But they really don't have any specific
> > > > > > source address set, so long addresses are none for source addresses, I
> > > > > > don't see any problem here. They also don't have AACK handling, but
> > > > > > it's not required for scan anyway...
> > > > > >
> > > > > > If this gets too complicated right now, then I am also fine with
> > > > > > returning an error here, we can enable it later but would it be better
> > > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like
> > > > > > you can do that, but you don't have the permissions.
> > > > > >    
> > > > >
> > > > > For me a scan should also be possible from iwpan phy $PHY scan (or
> > > > > whatever the scan command is, or just enable beacon)... to go over the
> > > > > dev is just a shortcut for "I mean whatever the phy is under this dev"
> > > > > ?    
> > > >
> > > > Actually only coordinators (in a specific state) should be able to send
> > > > beacons, so I am kind of against allowing that shortcut, because there
> > > > are usually two dev interfaces on top of the phy's, a regular "NODE"
> > > > and a "COORD", so I don't think we should go that way.
> > > >
> > > > For scans however it makes sense, I've added the necessary changes in
> > > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using
> > > > the same command name twice because it creates a macro, so this one
> > > > only supports a device name (the interface command has kind of the same
> > > > situation and uses a HIDDEN() macro which cannot be used here).
> > > >    
> > >
> > > Yes, I was thinking about scanning only.
> > >    
> > > > So in summary here is what is supported:
> > > > - dev <dev> beacon
> > > > - dev <dev> scan trigger|abort
> > > > - phy <phy> scan trigger|abort
> > > > - dev <dev> scan (the blocking one, which triggers, listens and returns)
> > > >
> > > > Do you agree?
> > > >    
> > >
> > > Okay, yes. I trust you.    
> > 
> > btw: at the point when a scan requires a source address... it cannot
> > be done because then a scan is related to a MAC instance -> an wpan
> > interface and we need to bind to it. But I think it doesn't?  
> 
> I'm not sure I follow you here. You mean in case of active scan?

Actually a beacon requests do not require any kind of source identifier,
so what do you mean by source address?

A regular beacon, however, does. Which means sending beacons would
include being able to set an address into a monitor interface. So if we
want to play with beacons on monitor interfaces, we should also relax
the pan_id/addressing rules.

> The operation is always tight to a device in the end, even if you
> provide a phy in userspace. So I guess it's not a problem. Or maybe I
> didn't get the question right?
> 
> Thanks,
> Miquèl


Thanks,
Miquèl

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

[Alexander Aring]

Hi,

On Tue, Feb 14, 2023 at 9:07 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500:
>
> > Hi,
> >
> > On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote:
> > >
> > > Hi,
> > >
> > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal
> > > <miquel.raynal@bootlin.com> wrote:
> > > >
> > > > Hi Alexander,
> > > >
> > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > > > > +{
> > > > > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > > > > +       struct cfg802154_scan_request *request;
> > > > > > > > > +       u8 type;
> > > > > > > > > +       int err;
> > > > > > > > > +
> > > > > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > > > > +               return -EPERM;
> > > > > > > >
> > > > > > > > btw: why are monitors not allowed?
> > > > > > >
> > > > > > > I guess I had the "active scan" use case in mind which of course does
> > > > > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > > > > right now I don't remember why I strongly refused scans on monitors.
> > > > > >
> > > > > > Isn't it that scans really work close to phy level? Means in this case
> > > > > > we disable mostly everything of MAC filtering on the transceiver side.
> > > > > > Then I don't see any reasons why even monitors can't do anything, they
> > > > > > also can send something. But they really don't have any specific
> > > > > > source address set, so long addresses are none for source addresses, I
> > > > > > don't see any problem here. They also don't have AACK handling, but
> > > > > > it's not required for scan anyway...
> > > > > >
> > > > > > If this gets too complicated right now, then I am also fine with
> > > > > > returning an error here, we can enable it later but would it be better
> > > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like
> > > > > > you can do that, but you don't have the permissions.
> > > > > >
> > > > >
> > > > > For me a scan should also be possible from iwpan phy $PHY scan (or
> > > > > whatever the scan command is, or just enable beacon)... to go over the
> > > > > dev is just a shortcut for "I mean whatever the phy is under this dev"
> > > > > ?
> > > >
> > > > Actually only coordinators (in a specific state) should be able to send
> > > > beacons, so I am kind of against allowing that shortcut, because there
> > > > are usually two dev interfaces on top of the phy's, a regular "NODE"
> > > > and a "COORD", so I don't think we should go that way.
> > > >
> > > > For scans however it makes sense, I've added the necessary changes in
> > > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using
> > > > the same command name twice because it creates a macro, so this one
> > > > only supports a device name (the interface command has kind of the same
> > > > situation and uses a HIDDEN() macro which cannot be used here).
> > > >
> > >
> > > Yes, I was thinking about scanning only.
> > >
> > > > So in summary here is what is supported:
> > > > - dev <dev> beacon
> > > > - dev <dev> scan trigger|abort
> > > > - phy <phy> scan trigger|abort
> > > > - dev <dev> scan (the blocking one, which triggers, listens and returns)
> > > >
> > > > Do you agree?
> > > >
> > >
> > > Okay, yes. I trust you.
> >
> > btw: at the point when a scan requires a source address... it cannot
> > be done because then a scan is related to a MAC instance -> an wpan
> > interface and we need to bind to it. But I think it doesn't?
>
> I'm not sure I follow you here. You mean in case of active scan? The
> operation is always tight to a device in the end, even if you provide a
> phy in userspace. So I guess it's not a problem. Or maybe I didn't get
> the question right?

As soon scan requires to put somewhere mib values inside e.g. address
information (which need to compared to source address settings (mib)?)
then it's no longer a phy operation -> wpan_phy, it is binded to a
wpan_dev (mac instance on a phy). But the addresses are set to NONE
address type?
I am not sure where all that data is stored right now for a scan
operation, if it's operating on a phy it should be stored on wpan_phy.

Note: there are also differences between wpan_phy and
ieee802154_local, also wpan_dev and ieee802154_sub_if_data structures.
It has something to do with visibility and SoftMAC vs HardMAC, however
the last one we don't really have an infrastructure for and we
probably need to move something around there. In short
wpan_phy/wpan_dev should be only visible by HardMAC (I think) and the
others are only additional data for the same instances used by
mac802154...

- Alex

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

[Alexander Aring]

Hi,

On Tue, Feb 14, 2023 at 9:46 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
>
> miquel.raynal@bootlin.com wrote on Tue, 14 Feb 2023 15:06:00 +0100:
>
> > Hi Alexander,
> >
> > aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500:
> >
> > > Hi,
> > >
> > > On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal
> > > > <miquel.raynal@bootlin.com> wrote:
> > > > >
> > > > > Hi Alexander,
> > > > >
> > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > > > > > +{
> > > > > > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > > > > > +       struct cfg802154_scan_request *request;
> > > > > > > > > > +       u8 type;
> > > > > > > > > > +       int err;
> > > > > > > > > > +
> > > > > > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > > > > > +               return -EPERM;
> > > > > > > > >
> > > > > > > > > btw: why are monitors not allowed?
> > > > > > > >
> > > > > > > > I guess I had the "active scan" use case in mind which of course does
> > > > > > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > > > > > right now I don't remember why I strongly refused scans on monitors.
> > > > > > >
> > > > > > > Isn't it that scans really work close to phy level? Means in this case
> > > > > > > we disable mostly everything of MAC filtering on the transceiver side.
> > > > > > > Then I don't see any reasons why even monitors can't do anything, they
> > > > > > > also can send something. But they really don't have any specific
> > > > > > > source address set, so long addresses are none for source addresses, I
> > > > > > > don't see any problem here. They also don't have AACK handling, but
> > > > > > > it's not required for scan anyway...
> > > > > > >
> > > > > > > If this gets too complicated right now, then I am also fine with
> > > > > > > returning an error here, we can enable it later but would it be better
> > > > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like
> > > > > > > you can do that, but you don't have the permissions.
> > > > > > >
> > > > > >
> > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or
> > > > > > whatever the scan command is, or just enable beacon)... to go over the
> > > > > > dev is just a shortcut for "I mean whatever the phy is under this dev"
> > > > > > ?
> > > > >
> > > > > Actually only coordinators (in a specific state) should be able to send
> > > > > beacons, so I am kind of against allowing that shortcut, because there
> > > > > are usually two dev interfaces on top of the phy's, a regular "NODE"
> > > > > and a "COORD", so I don't think we should go that way.
> > > > >
> > > > > For scans however it makes sense, I've added the necessary changes in
> > > > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using
> > > > > the same command name twice because it creates a macro, so this one
> > > > > only supports a device name (the interface command has kind of the same
> > > > > situation and uses a HIDDEN() macro which cannot be used here).
> > > > >
> > > >
> > > > Yes, I was thinking about scanning only.
> > > >
> > > > > So in summary here is what is supported:
> > > > > - dev <dev> beacon
> > > > > - dev <dev> scan trigger|abort
> > > > > - phy <phy> scan trigger|abort
> > > > > - dev <dev> scan (the blocking one, which triggers, listens and returns)
> > > > >
> > > > > Do you agree?
> > > > >
> > > >
> > > > Okay, yes. I trust you.
> > >
> > > btw: at the point when a scan requires a source address... it cannot
> > > be done because then a scan is related to a MAC instance -> an wpan
> > > interface and we need to bind to it. But I think it doesn't?
> >
> > I'm not sure I follow you here. You mean in case of active scan?
>
> Actually a beacon requests do not require any kind of source identifier,
> so what do you mean by source address?
>

Is this more clear now?

> A regular beacon, however, does. Which means sending beacons would
> include being able to set an address into a monitor interface. So if we
> want to play with beacons on monitor interfaces, we should also relax
> the pan_id/addressing rules.

mhhh, this is required for active scans? Then a scan operation cannot
be run on giving a phy only as a parameter...

- Alex

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

[Alexander Aring]

Hi,

On Tue, Feb 14, 2023 at 9:28 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:51:12 -0500:
>
> > Hi,
> >
> > On Mon, Feb 13, 2023 at 5:16 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Alexander,
> > >
> > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > > > > +{
> > > > > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > > > > +       struct cfg802154_scan_request *request;
> > > > > > > > > +       u8 type;
> > > > > > > > > +       int err;
> > > > > > > > > +
> > > > > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > > > > +               return -EPERM;
> > > > > > > >
> > > > > > > > btw: why are monitors not allowed?
> > > > > > >
> > > > > > > I guess I had the "active scan" use case in mind which of course does
> > > > > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > > > > right now I don't remember why I strongly refused scans on monitors.
> > > > > >
> > > > > > Isn't it that scans really work close to phy level? Means in this case
> > > > > > we disable mostly everything of MAC filtering on the transceiver side.
> > > > > > Then I don't see any reasons why even monitors can't do anything, they
> > > > > > also can send something. But they really don't have any specific
> > > > > > source address set, so long addresses are none for source addresses, I
> > > > > > don't see any problem here. They also don't have AACK handling, but
> > > > > > it's not required for scan anyway...
> > > > >
> > > > > I think I remember why I did not want to enable scans on monitors: we
> > > > > actually change the filtering level to "scan", which is very
> > > > > different to what a monitor is supposed to receive, which means in scan
> > > > > mode a monitor would no longer receive all what it is supposed to
> > > > > receive. Nothing that cannot be workaround'ed by software, probably,
> > > > > but I believe it is safer right now to avoid introducing potential
> > > > > regressions. So I will just change the error code and still refuse
> > > > > scans on monitor interfaces for now, until we figure out if it's
> > > > > actually safe or not (and if we really want to allow it).
> > > > >
> > > >
> > > > Okay, for scan yes we tell them to be in scan mode and then the
> > > > transceiver can filter whatever it delivers to the next level which is
> > > > necessary for filtering scan mac frames only. AACK handling is
> > > > disabled for scan mode for all types != MONITORS.
> > > >
> > > > For monitors we mostly allow everything and AACK is _always_ disabled.
> > > > The transceiver filter is completely disabled for at least what looks
> > > > like a 802.15.4 MAC header (even malformed). There are some frame
> > > > length checks which are necessary for specific hardware because
> > > > otherwise they would read out the frame buffer. For me it can still
> > > > feed the mac802154 stack for scanning (with filtering level as what
> > > > the monitor sets to, but currently our scan filter is equal to the
> > > > monitor filter mode anyway (which probably can be changed in
> > > > future?)). So in my opinion the monitor can do both -> feed the scan
> > > > mac802154 deliver path and the packet layer. And I also think that on
> > > > a normal interface type the packet layer should be feeded by those
> > > > frames as well and do not hit the mac802154 layer scan path only.
> > >
> > > Actually that would be an out-of-spec situation, here is a quote of
> > > chapter "6.3.1.3 Active and passive channel scan"
> > >
> > >         During an active or passive scan, the MAC sublayer shall
> > >         discard all frames received over the PHY data service that are
> > >         not Beacon frames.
> > >
> >
> > Monitor interfaces are not anything that the spec describes, it is
> > some interface type which offers the user (mostly over AF_PACKET raw
> > socket) full phy level access with the _default_ options. I already
> > run user space stacks (for hacking/development only) on monitor
> > interfaces to connect with Linux 802.15.4 interfaces, e.g. see [0]
> > (newer came upstream, somewhere I have also a 2 years old updated
> > version, use hwsim not fakelb).
>
> :-)
>
> >
> > In other words, by default it should bypass 802.15.4 MAC and it still
> > conforms with your spec, just that it is in user space. However, there
> > exists problems to do that, but it kind of works for the most use
> > cases. I said here by default because some people have different use
> > cases of what they want to do in the kernel. e.g. encryption (so users
> > only get encrypted frames, etc.) We don't support that but we can,
> > same for doing a scan. It probably requires just more mac802154 layer
> > filtering.
> >
> > There are enough examples in wireless that they do "crazy" things and
> > you can do that only with SoftMAC transceivers because it uses more
> > software parts like mac80211 and HardMAC transceivers only do what the
> > spec says and delivers it to the next layer. Some of them have more
> > functionality I guess, but then it depends on driver implementation
> > and a lot of other things.
> >
> > Monitors also act as a sniffer device, but you _could_ also send
> > frames out and then the fun part begins.
>
> Yes, you're right, it's up to us to allow monitor actions.
>
> > > I don't think this is possible to do anyway on devices with a single
> > > hardware filter setting?
> > >
> >
> > On SoftMAC it need to support a filtering level where we can disable
> > all filtering and get 802.15.4 MAC frames like it's on air (even
> > without non valid checksum, but we simply don't care if the
> > driver/transceiver does not support it we will always confirm it is
> > correct again until somebody comes around and say "oh we can do FCS
> > level then mac802154 does not need to check on this because it is
> > always correct")... This is currently the NONE filtering level I
> > think?
>
> But right now we ask for the "scan" filtering, which kind of discards
> most frames. Would you like a special config for monitors, like
> receiving everything on each channel you jump on? Or shall we stick to
> only transmitting beacon frames during a scan on a monitor interface?
>

good question...

> I guess it's rather easy to handle in each case. Just let me know what
> you prefer. I think I have a small preference for the scan filtering
> level, but I'm open.
>

I would capture and deliver everything to the user.. if the user does
a scan while doing whatever the user is doing with the monitor
interface at this time, the user need to live with the consequences
and you need to be root (okay probably every wireless manager will
give the normal user access to it, but still you need to know what you
are doing)

> > For HardMAC it is more complicated; they don't do that, they do the
> > "scan" operation on their transceiver and you can dump the result and
> > probably never forward any beacon related frames? (I had this question
> > once when Michael Richardson replied).
>
> Yes, in this case we'll have to figure out something else...
>

ok, I am curious. Probably it is very driver/device specific but yea,
HardMAC needs to at least support what 802.15.4 says, the rest is
optional and result in -ENOTSUPP?

- Alex

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

[Miquel Raynal]

Hi Alexander,

aahringo@redhat.com wrote on Thu, 16 Feb 2023 23:37:01 -0500:

> Hi,
> 
> On Tue, Feb 14, 2023 at 9:46 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> >
> > miquel.raynal@bootlin.com wrote on Tue, 14 Feb 2023 15:06:00 +0100:
> >  
> > > Hi Alexander,
> > >
> > > aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500:
> > >  
> > > > Hi,
> > > >
> > > > On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote:  
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal
> > > > > <miquel.raynal@bootlin.com> wrote:  
> > > > > >
> > > > > > Hi Alexander,
> > > > > >  
> > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > > > > > > +{
> > > > > > > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > > > > > > +       struct cfg802154_scan_request *request;
> > > > > > > > > > > +       u8 type;
> > > > > > > > > > > +       int err;
> > > > > > > > > > > +
> > > > > > > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > > > > > > +               return -EPERM;  
> > > > > > > > > >
> > > > > > > > > > btw: why are monitors not allowed?  
> > > > > > > > >
> > > > > > > > > I guess I had the "active scan" use case in mind which of course does
> > > > > > > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > > > > > > right now I don't remember why I strongly refused scans on monitors.  
> > > > > > > >
> > > > > > > > Isn't it that scans really work close to phy level? Means in this case
> > > > > > > > we disable mostly everything of MAC filtering on the transceiver side.
> > > > > > > > Then I don't see any reasons why even monitors can't do anything, they
> > > > > > > > also can send something. But they really don't have any specific
> > > > > > > > source address set, so long addresses are none for source addresses, I
> > > > > > > > don't see any problem here. They also don't have AACK handling, but
> > > > > > > > it's not required for scan anyway...
> > > > > > > >
> > > > > > > > If this gets too complicated right now, then I am also fine with
> > > > > > > > returning an error here, we can enable it later but would it be better
> > > > > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like
> > > > > > > > you can do that, but you don't have the permissions.
> > > > > > > >  
> > > > > > >
> > > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or
> > > > > > > whatever the scan command is, or just enable beacon)... to go over the
> > > > > > > dev is just a shortcut for "I mean whatever the phy is under this dev"
> > > > > > > ?  
> > > > > >
> > > > > > Actually only coordinators (in a specific state) should be able to send
> > > > > > beacons, so I am kind of against allowing that shortcut, because there
> > > > > > are usually two dev interfaces on top of the phy's, a regular "NODE"
> > > > > > and a "COORD", so I don't think we should go that way.
> > > > > >
> > > > > > For scans however it makes sense, I've added the necessary changes in
> > > > > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using
> > > > > > the same command name twice because it creates a macro, so this one
> > > > > > only supports a device name (the interface command has kind of the same
> > > > > > situation and uses a HIDDEN() macro which cannot be used here).
> > > > > >  
> > > > >
> > > > > Yes, I was thinking about scanning only.
> > > > >  
> > > > > > So in summary here is what is supported:
> > > > > > - dev <dev> beacon
> > > > > > - dev <dev> scan trigger|abort
> > > > > > - phy <phy> scan trigger|abort
> > > > > > - dev <dev> scan (the blocking one, which triggers, listens and returns)
> > > > > >
> > > > > > Do you agree?
> > > > > >  
> > > > >
> > > > > Okay, yes. I trust you.  
> > > >
> > > > btw: at the point when a scan requires a source address... it cannot
> > > > be done because then a scan is related to a MAC instance -> an wpan
> > > > interface and we need to bind to it. But I think it doesn't?  
> > >
> > > I'm not sure I follow you here. You mean in case of active scan?  
> >
> > Actually a beacon requests do not require any kind of source identifier,
> > so what do you mean by source address?
> >  
> 
> Is this more clear now?

Yes, thanks!

> > A regular beacon, however, does. Which means sending beacons would
> > include being able to set an address into a monitor interface. So if we
> > want to play with beacons on monitor interfaces, we should also relax
> > the pan_id/addressing rules.  
> 
> mhhh, this is required for active scans? Then a scan operation cannot
> be run on giving a phy only as a parameter...

No, this is not required for active scans. Scans do not require
anything else than phy parameters, AFAICS.

This is required for sending beacons though. So we cannot send beacons
from monitors if we don't relax the pan_id/addressing rules on these
interfaces. Do you think we should?

Thanks,
Miquèl

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

[Miquel Raynal]

Hi Alexander,

aahringo@redhat.com wrote on Thu, 16 Feb 2023 23:46:42 -0500:

> Hi,
> 
> On Tue, Feb 14, 2023 at 9:28 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:51:12 -0500:
> >  
> > > Hi,
> > >
> > > On Mon, Feb 13, 2023 at 5:16 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > > >
> > > > Hi Alexander,
> > > >  
> > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > > > > > +{
> > > > > > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > > > > > +       struct cfg802154_scan_request *request;
> > > > > > > > > > +       u8 type;
> > > > > > > > > > +       int err;
> > > > > > > > > > +
> > > > > > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > > > > > +               return -EPERM;  
> > > > > > > > >
> > > > > > > > > btw: why are monitors not allowed?  
> > > > > > > >
> > > > > > > > I guess I had the "active scan" use case in mind which of course does
> > > > > > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > > > > > right now I don't remember why I strongly refused scans on monitors.  
> > > > > > >
> > > > > > > Isn't it that scans really work close to phy level? Means in this case
> > > > > > > we disable mostly everything of MAC filtering on the transceiver side.
> > > > > > > Then I don't see any reasons why even monitors can't do anything, they
> > > > > > > also can send something. But they really don't have any specific
> > > > > > > source address set, so long addresses are none for source addresses, I
> > > > > > > don't see any problem here. They also don't have AACK handling, but
> > > > > > > it's not required for scan anyway...  
> > > > > >
> > > > > > I think I remember why I did not want to enable scans on monitors: we
> > > > > > actually change the filtering level to "scan", which is very
> > > > > > different to what a monitor is supposed to receive, which means in scan
> > > > > > mode a monitor would no longer receive all what it is supposed to
> > > > > > receive. Nothing that cannot be workaround'ed by software, probably,
> > > > > > but I believe it is safer right now to avoid introducing potential
> > > > > > regressions. So I will just change the error code and still refuse
> > > > > > scans on monitor interfaces for now, until we figure out if it's
> > > > > > actually safe or not (and if we really want to allow it).
> > > > > >  
> > > > >
> > > > > Okay, for scan yes we tell them to be in scan mode and then the
> > > > > transceiver can filter whatever it delivers to the next level which is
> > > > > necessary for filtering scan mac frames only. AACK handling is
> > > > > disabled for scan mode for all types != MONITORS.
> > > > >
> > > > > For monitors we mostly allow everything and AACK is _always_ disabled.
> > > > > The transceiver filter is completely disabled for at least what looks
> > > > > like a 802.15.4 MAC header (even malformed). There are some frame
> > > > > length checks which are necessary for specific hardware because
> > > > > otherwise they would read out the frame buffer. For me it can still
> > > > > feed the mac802154 stack for scanning (with filtering level as what
> > > > > the monitor sets to, but currently our scan filter is equal to the
> > > > > monitor filter mode anyway (which probably can be changed in
> > > > > future?)). So in my opinion the monitor can do both -> feed the scan
> > > > > mac802154 deliver path and the packet layer. And I also think that on
> > > > > a normal interface type the packet layer should be feeded by those
> > > > > frames as well and do not hit the mac802154 layer scan path only.  
> > > >
> > > > Actually that would be an out-of-spec situation, here is a quote of
> > > > chapter "6.3.1.3 Active and passive channel scan"
> > > >
> > > >         During an active or passive scan, the MAC sublayer shall
> > > >         discard all frames received over the PHY data service that are
> > > >         not Beacon frames.
> > > >  
> > >
> > > Monitor interfaces are not anything that the spec describes, it is
> > > some interface type which offers the user (mostly over AF_PACKET raw
> > > socket) full phy level access with the _default_ options. I already
> > > run user space stacks (for hacking/development only) on monitor
> > > interfaces to connect with Linux 802.15.4 interfaces, e.g. see [0]
> > > (newer came upstream, somewhere I have also a 2 years old updated
> > > version, use hwsim not fakelb).  
> >
> > :-)
> >  
> > >
> > > In other words, by default it should bypass 802.15.4 MAC and it still
> > > conforms with your spec, just that it is in user space. However, there
> > > exists problems to do that, but it kind of works for the most use
> > > cases. I said here by default because some people have different use
> > > cases of what they want to do in the kernel. e.g. encryption (so users
> > > only get encrypted frames, etc.) We don't support that but we can,
> > > same for doing a scan. It probably requires just more mac802154 layer
> > > filtering.
> > >
> > > There are enough examples in wireless that they do "crazy" things and
> > > you can do that only with SoftMAC transceivers because it uses more
> > > software parts like mac80211 and HardMAC transceivers only do what the
> > > spec says and delivers it to the next layer. Some of them have more
> > > functionality I guess, but then it depends on driver implementation
> > > and a lot of other things.
> > >
> > > Monitors also act as a sniffer device, but you _could_ also send
> > > frames out and then the fun part begins.  
> >
> > Yes, you're right, it's up to us to allow monitor actions.
> >  
> > > > I don't think this is possible to do anyway on devices with a single
> > > > hardware filter setting?
> > > >  
> > >
> > > On SoftMAC it need to support a filtering level where we can disable
> > > all filtering and get 802.15.4 MAC frames like it's on air (even
> > > without non valid checksum, but we simply don't care if the
> > > driver/transceiver does not support it we will always confirm it is
> > > correct again until somebody comes around and say "oh we can do FCS
> > > level then mac802154 does not need to check on this because it is
> > > always correct")... This is currently the NONE filtering level I
> > > think?  
> >
> > But right now we ask for the "scan" filtering, which kind of discards
> > most frames. Would you like a special config for monitors, like
> > receiving everything on each channel you jump on? Or shall we stick to
> > only transmitting beacon frames during a scan on a monitor interface?
> >  
> 
> good question...
> 
> > I guess it's rather easy to handle in each case. Just let me know what
> > you prefer. I think I have a small preference for the scan filtering
> > level, but I'm open.
> >  
> 
> I would capture and deliver everything to the user.. if the user does
> a scan while doing whatever the user is doing with the monitor
> interface at this time, the user need to live with the consequences
> and you need to be root (okay probably every wireless manager will
> give the normal user access to it, but still you need to know what you
> are doing)

Fair enough.

> > > For HardMAC it is more complicated; they don't do that, they do the
> > > "scan" operation on their transceiver and you can dump the result and
> > > probably never forward any beacon related frames? (I had this question
> > > once when Michael Richardson replied).  
> >
> > Yes, in this case we'll have to figure out something else...
> >  
> 
> ok, I am curious. Probably it is very driver/device specific but yea,
> HardMAC needs to at least support what 802.15.4 says, the rest is
> optional and result in -ENOTSUPP?

TBH this is still a gray area in my mental model. I'm not sure what
these devices will really offer in terms of interfaces.

Thanks,
Miquèl

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

[Miquel Raynal]

Hi Alexander,

aahringo@redhat.com wrote on Thu, 16 Feb 2023 23:34:30 -0500:

> Hi,
> 
> On Tue, Feb 14, 2023 at 9:07 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500:
> >  
> > > Hi,
> > >
> > > On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote:  
> > > >
> > > > Hi,
> > > >
> > > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal
> > > > <miquel.raynal@bootlin.com> wrote:  
> > > > >
> > > > > Hi Alexander,
> > > > >  
> > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > > > > > +{
> > > > > > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > > > > > +       struct cfg802154_scan_request *request;
> > > > > > > > > > +       u8 type;
> > > > > > > > > > +       int err;
> > > > > > > > > > +
> > > > > > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > > > > > +               return -EPERM;  
> > > > > > > > >
> > > > > > > > > btw: why are monitors not allowed?  
> > > > > > > >
> > > > > > > > I guess I had the "active scan" use case in mind which of course does
> > > > > > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > > > > > right now I don't remember why I strongly refused scans on monitors.  
> > > > > > >
> > > > > > > Isn't it that scans really work close to phy level? Means in this case
> > > > > > > we disable mostly everything of MAC filtering on the transceiver side.
> > > > > > > Then I don't see any reasons why even monitors can't do anything, they
> > > > > > > also can send something. But they really don't have any specific
> > > > > > > source address set, so long addresses are none for source addresses, I
> > > > > > > don't see any problem here. They also don't have AACK handling, but
> > > > > > > it's not required for scan anyway...
> > > > > > >
> > > > > > > If this gets too complicated right now, then I am also fine with
> > > > > > > returning an error here, we can enable it later but would it be better
> > > > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like
> > > > > > > you can do that, but you don't have the permissions.
> > > > > > >  
> > > > > >
> > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or
> > > > > > whatever the scan command is, or just enable beacon)... to go over the
> > > > > > dev is just a shortcut for "I mean whatever the phy is under this dev"
> > > > > > ?  
> > > > >
> > > > > Actually only coordinators (in a specific state) should be able to send
> > > > > beacons, so I am kind of against allowing that shortcut, because there
> > > > > are usually two dev interfaces on top of the phy's, a regular "NODE"
> > > > > and a "COORD", so I don't think we should go that way.
> > > > >
> > > > > For scans however it makes sense, I've added the necessary changes in
> > > > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using
> > > > > the same command name twice because it creates a macro, so this one
> > > > > only supports a device name (the interface command has kind of the same
> > > > > situation and uses a HIDDEN() macro which cannot be used here).
> > > > >  
> > > >
> > > > Yes, I was thinking about scanning only.
> > > >  
> > > > > So in summary here is what is supported:
> > > > > - dev <dev> beacon
> > > > > - dev <dev> scan trigger|abort
> > > > > - phy <phy> scan trigger|abort
> > > > > - dev <dev> scan (the blocking one, which triggers, listens and returns)
> > > > >
> > > > > Do you agree?
> > > > >  
> > > >
> > > > Okay, yes. I trust you.  
> > >
> > > btw: at the point when a scan requires a source address... it cannot
> > > be done because then a scan is related to a MAC instance -> an wpan
> > > interface and we need to bind to it. But I think it doesn't?  
> >
> > I'm not sure I follow you here. You mean in case of active scan? The
> > operation is always tight to a device in the end, even if you provide a
> > phy in userspace. So I guess it's not a problem. Or maybe I didn't get
> > the question right?  
> 
> As soon scan requires to put somewhere mib values inside e.g. address
> information (which need to compared to source address settings (mib)?)
> then it's no longer a phy operation -> wpan_phy, it is binded to a
> wpan_dev (mac instance on a phy). But the addresses are set to NONE
> address type?
> I am not sure where all that data is stored right now for a scan
> operation, if it's operating on a phy it should be stored on wpan_phy.
> 
> Note: there are also differences between wpan_phy and
> ieee802154_local, also wpan_dev and ieee802154_sub_if_data structures.
> It has something to do with visibility and SoftMAC vs HardMAC, however
> the last one we don't really have an infrastructure for and we
> probably need to move something around there. In short
> wpan_phy/wpan_dev should be only visible by HardMAC (I think) and the
> others are only additional data for the same instances used by
> mac802154...

Ok, I got what you meant.

So to be clear, I assume active and passive scans are phy activities,
they only involve phy parameters. Beaconing however need access to mac
parameters.

For now the structure defining user requests in terms of scanning and
beaconing is stored into ieee802154_local, but we can move it
away if needed at some point? For now I have no real example of
hardMAC device so it's a bit hard to anticipate all their
needs, but do you want me to move it to wpan_dev? (I would like to keep
both request descriptors aside from each other).

Thanks,
Miquèl

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

[Alexander Aring]

Hi,

On Fri, Feb 17, 2023 at 4:02 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> aahringo@redhat.com wrote on Thu, 16 Feb 2023 23:34:30 -0500:
>
> > Hi,
> >
> > On Tue, Feb 14, 2023 at 9:07 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Alexander,
> > >
> > > aahringo@redhat.com wrote on Tue, 14 Feb 2023 08:53:57 -0500:
> > >
> > > > Hi,
> > > >
> > > > On Tue, Feb 14, 2023 at 8:34 AM Alexander Aring <aahringo@redhat.com> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal
> > > > > <miquel.raynal@bootlin.com> wrote:
> > > > > >
> > > > > > Hi Alexander,
> > > > > >
> > > > > > > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > > > > > > +{
> > > > > > > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > > > > > > +       struct cfg802154_scan_request *request;
> > > > > > > > > > > +       u8 type;
> > > > > > > > > > > +       int err;
> > > > > > > > > > > +
> > > > > > > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > > > > > > +               return -EPERM;
> > > > > > > > > >
> > > > > > > > > > btw: why are monitors not allowed?
> > > > > > > > >
> > > > > > > > > I guess I had the "active scan" use case in mind which of course does
> > > > > > > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > > > > > > right now I don't remember why I strongly refused scans on monitors.
> > > > > > > >
> > > > > > > > Isn't it that scans really work close to phy level? Means in this case
> > > > > > > > we disable mostly everything of MAC filtering on the transceiver side.
> > > > > > > > Then I don't see any reasons why even monitors can't do anything, they
> > > > > > > > also can send something. But they really don't have any specific
> > > > > > > > source address set, so long addresses are none for source addresses, I
> > > > > > > > don't see any problem here. They also don't have AACK handling, but
> > > > > > > > it's not required for scan anyway...
> > > > > > > >
> > > > > > > > If this gets too complicated right now, then I am also fine with
> > > > > > > > returning an error here, we can enable it later but would it be better
> > > > > > > > to use ENOTSUPP or something like that in this case? EPERM sounds like
> > > > > > > > you can do that, but you don't have the permissions.
> > > > > > > >
> > > > > > >
> > > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or
> > > > > > > whatever the scan command is, or just enable beacon)... to go over the
> > > > > > > dev is just a shortcut for "I mean whatever the phy is under this dev"
> > > > > > > ?
> > > > > >
> > > > > > Actually only coordinators (in a specific state) should be able to send
> > > > > > beacons, so I am kind of against allowing that shortcut, because there
> > > > > > are usually two dev interfaces on top of the phy's, a regular "NODE"
> > > > > > and a "COORD", so I don't think we should go that way.
> > > > > >
> > > > > > For scans however it makes sense, I've added the necessary changes in
> > > > > > wpan-tools. The TOP_LEVEL(scan) macro however does not support using
> > > > > > the same command name twice because it creates a macro, so this one
> > > > > > only supports a device name (the interface command has kind of the same
> > > > > > situation and uses a HIDDEN() macro which cannot be used here).
> > > > > >
> > > > >
> > > > > Yes, I was thinking about scanning only.
> > > > >
> > > > > > So in summary here is what is supported:
> > > > > > - dev <dev> beacon
> > > > > > - dev <dev> scan trigger|abort
> > > > > > - phy <phy> scan trigger|abort
> > > > > > - dev <dev> scan (the blocking one, which triggers, listens and returns)
> > > > > >
> > > > > > Do you agree?
> > > > > >
> > > > >
> > > > > Okay, yes. I trust you.
> > > >
> > > > btw: at the point when a scan requires a source address... it cannot
> > > > be done because then a scan is related to a MAC instance -> an wpan
> > > > interface and we need to bind to it. But I think it doesn't?
> > >
> > > I'm not sure I follow you here. You mean in case of active scan? The
> > > operation is always tight to a device in the end, even if you provide a
> > > phy in userspace. So I guess it's not a problem. Or maybe I didn't get
> > > the question right?
> >
> > As soon scan requires to put somewhere mib values inside e.g. address
> > information (which need to compared to source address settings (mib)?)
> > then it's no longer a phy operation -> wpan_phy, it is binded to a
> > wpan_dev (mac instance on a phy). But the addresses are set to NONE
> > address type?
> > I am not sure where all that data is stored right now for a scan
> > operation, if it's operating on a phy it should be stored on wpan_phy.
> >
> > Note: there are also differences between wpan_phy and
> > ieee802154_local, also wpan_dev and ieee802154_sub_if_data structures.
> > It has something to do with visibility and SoftMAC vs HardMAC, however
> > the last one we don't really have an infrastructure for and we
> > probably need to move something around there. In short
> > wpan_phy/wpan_dev should be only visible by HardMAC (I think) and the
> > others are only additional data for the same instances used by
> > mac802154...
>
> Ok, I got what you meant.
>
> So to be clear, I assume active and passive scans are phy activities,
> they only involve phy parameters. Beaconing however need access to mac
> parameters.
>

ok.

> For now the structure defining user requests in terms of scanning and
> beaconing is stored into ieee802154_local, but we can move it
> away if needed at some point? For now I have no real example of
> hardMAC device so it's a bit hard to anticipate all their
> needs, but do you want me to move it to wpan_dev? (I would like to keep
> both request descriptors aside from each other).

yes, forget about moving things to the right structure... somehow I
think it's good to have them to not get in too much pain when trying
to introduce a hardmac driver...

- Alex

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

[Alexander Aring]

Hi,

On Fri, Feb 17, 2023 at 3:53 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
...
> >
> > ok, I am curious. Probably it is very driver/device specific but yea,
> > HardMAC needs to at least support what 802.15.4 says, the rest is
> > optional and result in -ENOTSUPP?
>
> TBH this is still a gray area in my mental model. I'm not sure what
> these devices will really offer in terms of interfaces.

ca8210 is one. They use those SAP-commands (MCPS-SAP and MLME-SAP)
which are described by 802.15.4 spec... there is this cfg802154_ops
structure which will redirect netlink to either SoftMAC or HardMAC it
should somehow conform to this...
However I think it should be the minimum functionality inside of this,
there might be a lot of optional things which only SoftMAC supports.
Also nl802154 should be oriented to this.

Are you agreeing here?

- Alex

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

[Alexander Aring]

Hi,

On Mon, Feb 20, 2023 at 9:54 PM Alexander Aring <aahringo@redhat.com> wrote:
>
> Hi,
>
> On Fri, Feb 17, 2023 at 3:53 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> ...
> > >
> > > ok, I am curious. Probably it is very driver/device specific but yea,
> > > HardMAC needs to at least support what 802.15.4 says, the rest is
> > > optional and result in -ENOTSUPP?
> >
> > TBH this is still a gray area in my mental model. I'm not sure what
> > these devices will really offer in terms of interfaces.
>
> ca8210 is one. They use those SAP-commands (MCPS-SAP and MLME-SAP)
> which are described by 802.15.4 spec... there is this cfg802154_ops
> structure which will redirect netlink to either SoftMAC or HardMAC it
> should somehow conform to this...
> However I think it should be the minimum functionality inside of this,
> there might be a lot of optional things which only SoftMAC supports.
> Also nl802154 should be oriented to this.
>
> Are you agreeing here?

it's just not nl802154/cfg802154 also sending specific kind of frames
out added to cfg802154 which we don't have yet inside of this
"callback structure to interfacing SoftMAC or HardMAC".

- Alex

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

[Miquel Raynal]

Hi Alexander,

aahringo@redhat.com wrote on Mon, 20 Feb 2023 21:54:41 -0500:

> Hi,
> 
> On Fri, Feb 17, 2023 at 3:53 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> ...
> > >
> > > ok, I am curious. Probably it is very driver/device specific but yea,
> > > HardMAC needs to at least support what 802.15.4 says, the rest is
> > > optional and result in -ENOTSUPP?  
> >
> > TBH this is still a gray area in my mental model. I'm not sure what
> > these devices will really offer in terms of interfaces.  
> 
> ca8210 is one. They use those SAP-commands (MCPS-SAP and MLME-SAP)
> which are described by 802.15.4 spec... there is this cfg802154_ops
> structure which will redirect netlink to either SoftMAC or HardMAC it
> should somehow conform to this...

Absolutely.

> However I think it should be the minimum functionality inside of this,
> there might be a lot of optional things which only SoftMAC supports.
> Also nl802154 should be oriented to this.
> 
> Are you agreeing here?

Yes. That support can also be improved if we ever have to support
advanced functionalities with "new" and compatible HardMAC devices.

Thanks,
Miquèl