[PATCH wpan-next v2 0/2] ieee802154: Beaconing support

[PATCH wpan-next v2 0/2] ieee802154: Beaconing support

[Miquel Raynal]

Scanning being now supported, we can eg. play with hwsim to verify
everything works as soon as this series including beaconing support gets
merged.

Thanks,
Miquèl

Changes in v2:
* Clearly state in the commit log llsec is not supported yet.
* Do not use mlme transmission helpers because we don't really need to
  stop the queue when sending a beacon, as we don't expect any feedback
  from the PHY nor from the peers. However, we don't want to go through
  the whole net stack either, so we bypass it calling the subif helper
  directly.

Miquel Raynal (2):
  ieee802154: Add support for user beaconing requests
  mac802154: Handle basic beaconing

 include/net/cfg802154.h         |  23 +++++
 include/net/ieee802154_netdev.h |  16 ++++
 include/net/nl802154.h          |   3 +
 net/ieee802154/header_ops.c     |  24 +++++
 net/ieee802154/nl802154.c       |  93 ++++++++++++++++++++
 net/ieee802154/nl802154.h       |   1 +
 net/ieee802154/rdev-ops.h       |  28 ++++++
 net/ieee802154/trace.h          |  21 +++++
 net/mac802154/cfg.c             |  31 ++++++-
 net/mac802154/ieee802154_i.h    |  18 ++++
 net/mac802154/iface.c           |   3 +
 net/mac802154/llsec.c           |   5 +-
 net/mac802154/main.c            |   1 +
 net/mac802154/scan.c            | 151 ++++++++++++++++++++++++++++++++
 14 files changed, 415 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH wpan-next v2 1/2] ieee802154: Add support for user beaconing requests

[Miquel Raynal]

Parse user requests for sending beacons, start sending beacons at a
regular pace. If needed, the pace can be updated with a new request. The
process can also be interrupted at any moment.

The page and channel must be changed beforehands if needed. Interval
orders above 14 are reserved to tell a device it must answer BEACON_REQ
coming from another device as part of an active scan procedure and this
is not yet supported.

A netlink "beacon request" structure is created to list the
requirements.

Mac layers may now implement the ->send_beacons() and
->stop_beacons() hooks.

Co-developed-by: David Girault <david.girault@qorvo.com>
Signed-off-by: David Girault <david.girault@qorvo.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/cfg802154.h   | 23 ++++++++++
 include/net/nl802154.h    |  3 ++
 net/ieee802154/nl802154.c | 93 +++++++++++++++++++++++++++++++++++++++
 net/ieee802154/nl802154.h |  1 +
 net/ieee802154/rdev-ops.h | 28 ++++++++++++
 net/ieee802154/trace.h    | 21 +++++++++
 6 files changed, 169 insertions(+)

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 0b0f81a945b6..0c2778a836db 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -19,6 +19,7 @@
 struct wpan_phy;
 struct wpan_phy_cca;
 struct cfg802154_scan_request;
+struct cfg802154_beacon_request;
 
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 struct ieee802154_llsec_device_key;
@@ -72,6 +73,10 @@ struct cfg802154_ops {
 				struct cfg802154_scan_request *request);
 	int	(*abort_scan)(struct wpan_phy *wpan_phy,
 			      struct wpan_dev *wpan_dev);
+	int	(*send_beacons)(struct wpan_phy *wpan_phy,
+				struct cfg802154_beacon_request *request);
+	int	(*stop_beacons)(struct wpan_phy *wpan_phy,
+				struct wpan_dev *wpan_dev);
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	void	(*get_llsec_table)(struct wpan_phy *wpan_phy,
 				   struct wpan_dev *wpan_dev,
@@ -314,6 +319,24 @@ struct cfg802154_scan_request {
 	struct wpan_phy *wpan_phy;
 };
 
+/**
+ * struct cfg802154_beacon_request - Beacon request descriptor
+ *
+ * @interval: interval n between sendings, in multiple order of the super frame
+ *            duration: aBaseSuperframeDuration * (2^n) unless the interval
+ *            order is greater or equal to 15, in this case beacons won't be
+ *            passively sent out at a fixed rate but instead inform the device
+ *            that it should answer beacon requests as part of active scan
+ *            procedures
+ * @wpan_dev: the concerned wpan device
+ * @wpan_phy: the wpan phy this was for
+ */
+struct cfg802154_beacon_request {
+	u8 interval;
+	struct wpan_dev *wpan_dev;
+	struct wpan_phy *wpan_phy;
+};
+
 /**
  * struct cfg802154_mac_pkt - MAC packet descriptor (beacon/command)
  * @node: MAC packets to process list member
diff --git a/include/net/nl802154.h b/include/net/nl802154.h
index c267fa1c5aac..8cd9d141f5af 100644
--- a/include/net/nl802154.h
+++ b/include/net/nl802154.h
@@ -76,6 +76,8 @@ enum nl802154_commands {
 	NL802154_CMD_TRIGGER_SCAN,
 	NL802154_CMD_ABORT_SCAN,
 	NL802154_CMD_SCAN_DONE,
+	NL802154_CMD_SEND_BEACONS,
+	NL802154_CMD_STOP_BEACONS,
 
 	/* add new commands above here */
 
@@ -144,6 +146,7 @@ enum nl802154_attrs {
 	NL802154_ATTR_SCAN_MEAN_PRF,
 	NL802154_ATTR_SCAN_DURATION,
 	NL802154_ATTR_SCAN_DONE_REASON,
+	NL802154_ATTR_BEACON_INTERVAL,
 
 	/* add attributes here, update the policy in nl802154.c */
 
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 1d703251f74a..0d9becd678e3 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -227,6 +227,7 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
 	[NL802154_ATTR_SCAN_MEAN_PRF] = { .type = NLA_U8 },
 	[NL802154_ATTR_SCAN_DURATION] = { .type = NLA_U8 },
 	[NL802154_ATTR_SCAN_DONE_REASON] = { .type = NLA_U8 },
+	[NL802154_ATTR_BEACON_INTERVAL] = { .type = NLA_U8 },
 
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	[NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
@@ -1587,6 +1588,82 @@ static int nl802154_abort_scan(struct sk_buff *skb, struct genl_info *info)
 	return rdev_abort_scan(rdev, wpan_dev);
 }
 
+static int
+nl802154_send_beacons(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg802154_registered_device *rdev = info->user_ptr[0];
+	struct net_device *dev = info->user_ptr[1];
+	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
+	struct wpan_phy *wpan_phy = &rdev->wpan_phy;
+	struct cfg802154_beacon_request *request;
+	int err;
+
+	/* Only coordinators can send beacons */
+	if (wpan_dev->iftype != NL802154_IFTYPE_COORD)
+		return -EOPNOTSUPP;
+
+	if (wpan_dev->pan_id == cpu_to_le16(IEEE802154_PANID_BROADCAST)) {
+		pr_err("Device is not part of any PAN\n");
+		return -EPERM;
+	}
+
+	request = kzalloc(sizeof(*request), GFP_KERNEL);
+	if (!request)
+		return -ENOMEM;
+
+	request->wpan_dev = wpan_dev;
+	request->wpan_phy = wpan_phy;
+
+	if (info->attrs[NL802154_ATTR_BEACON_INTERVAL]) {
+		request->interval = nla_get_u8(info->attrs[NL802154_ATTR_BEACON_INTERVAL]);
+		if (request->interval > IEEE802154_MAX_SCAN_DURATION) {
+			pr_err("Interval is out of range\n");
+			err = -EINVAL;
+			goto free_request;
+		}
+	} else {
+		/* Use maximum duration order by default */
+		request->interval = IEEE802154_MAX_SCAN_DURATION;
+	}
+
+	if (wpan_dev->netdev)
+		dev_hold(wpan_dev->netdev);
+
+	err = rdev_send_beacons(rdev, request);
+	if (err) {
+		pr_err("Failure starting sending beacons (%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;
+}
+
+void nl802154_beaconing_done(struct wpan_dev *wpan_dev)
+{
+	if (wpan_dev->netdev)
+		dev_put(wpan_dev->netdev);
+}
+EXPORT_SYMBOL_GPL(nl802154_beaconing_done);
+
+static int
+nl802154_stop_beacons(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg802154_registered_device *rdev = info->user_ptr[0];
+	struct net_device *dev = info->user_ptr[1];
+	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
+
+	/* Resources are released in the notification helper above */
+	return rdev_stop_beacons(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 },
@@ -2691,6 +2768,22 @@ static const struct genl_ops nl802154_ops[] = {
 				  NL802154_FLAG_CHECK_NETDEV_UP |
 				  NL802154_FLAG_NEED_RTNL,
 	},
+	{
+		.cmd = NL802154_CMD_SEND_BEACONS,
+		.doit = nl802154_send_beacons,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL802154_FLAG_NEED_NETDEV |
+				  NL802154_FLAG_CHECK_NETDEV_UP |
+				  NL802154_FLAG_NEED_RTNL,
+	},
+	{
+		.cmd = NL802154_CMD_STOP_BEACONS,
+		.doit = nl802154_stop_beacons,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL802154_FLAG_NEED_NETDEV |
+				  NL802154_FLAG_CHECK_NETDEV_UP |
+				  NL802154_FLAG_NEED_RTNL,
+	},
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	{
 		.cmd = NL802154_CMD_SET_SEC_PARAMS,
diff --git a/net/ieee802154/nl802154.h b/net/ieee802154/nl802154.h
index cfa7134be747..d69d950f9a6a 100644
--- a/net/ieee802154/nl802154.h
+++ b/net/ieee802154/nl802154.h
@@ -9,5 +9,6 @@ int nl802154_scan_event(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
 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,
 		       enum nl802154_scan_done_reasons reason);
+void nl802154_beaconing_done(struct wpan_dev *wpan_dev);
 
 #endif /* __IEEE802154_NL802154_H */
diff --git a/net/ieee802154/rdev-ops.h b/net/ieee802154/rdev-ops.h
index e171d74c3251..5eaae15c610e 100644
--- a/net/ieee802154/rdev-ops.h
+++ b/net/ieee802154/rdev-ops.h
@@ -237,6 +237,34 @@ static inline int rdev_abort_scan(struct cfg802154_registered_device *rdev,
 	return ret;
 }
 
+static inline int rdev_send_beacons(struct cfg802154_registered_device *rdev,
+				    struct cfg802154_beacon_request *request)
+{
+	int ret;
+
+	if (!rdev->ops->send_beacons)
+		return -EOPNOTSUPP;
+
+	trace_802154_rdev_send_beacons(&rdev->wpan_phy, request);
+	ret = rdev->ops->send_beacons(&rdev->wpan_phy, request);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
+}
+
+static inline int rdev_stop_beacons(struct cfg802154_registered_device *rdev,
+				    struct wpan_dev *wpan_dev)
+{
+	int ret;
+
+	if (!rdev->ops->stop_beacons)
+		return -EOPNOTSUPP;
+
+	trace_802154_rdev_stop_beacons(&rdev->wpan_phy, wpan_dev);
+	ret = rdev->ops->stop_beacons(&rdev->wpan_phy, wpan_dev);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
+}
+
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 /* TODO this is already a nl802154, so move into ieee802154 */
 static inline void
diff --git a/net/ieee802154/trace.h b/net/ieee802154/trace.h
index e5405f737ded..e5d8439b9e45 100644
--- a/net/ieee802154/trace.h
+++ b/net/ieee802154/trace.h
@@ -315,6 +315,22 @@ TRACE_EVENT(802154_rdev_trigger_scan,
 		  WPAN_PHY_PR_ARG, __entry->page, __entry->channels, __entry->duration)
 );
 
+TRACE_EVENT(802154_rdev_send_beacons,
+	TP_PROTO(struct wpan_phy *wpan_phy,
+		 struct cfg802154_beacon_request *request),
+	TP_ARGS(wpan_phy, request),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		__field(u8, interval)
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		__entry->interval = request->interval;
+	),
+	TP_printk(WPAN_PHY_PR_FMT ", sending beacons (interval order: %d)",
+		  WPAN_PHY_PR_ARG, __entry->interval)
+);
+
 DECLARE_EVENT_CLASS(802154_wdev_template,
 	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev),
 	TP_ARGS(wpan_phy, wpan_dev),
@@ -335,6 +351,11 @@ DEFINE_EVENT(802154_wdev_template, 802154_rdev_abort_scan,
 	TP_ARGS(wpan_phy, wpan_dev)
 );
 
+DEFINE_EVENT(802154_wdev_template, 802154_rdev_stop_beacons,
+	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev),
+	TP_ARGS(wpan_phy, wpan_dev)
+);
+
 TRACE_EVENT(802154_rdev_return_int,
 	TP_PROTO(struct wpan_phy *wpan_phy, int ret),
 	TP_ARGS(wpan_phy, ret),
-- 
2.34.1

[PATCH wpan-next v2 2/2] mac802154: Handle basic beaconing

[Miquel Raynal]

Implement the core hooks in order to provide the softMAC layer support
for sending beacons. Coordinators may be requested to send beacons in a
beacon enabled PAN in order for the other devices around to self
discover the available PANs automatically.

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

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

Sending beacons in response to a BEACON_REQ frame (ie. answering active
scans) is not yet supported.

This initial patchset has no security support (llsec).

Co-developed-by: David Girault <david.girault@qorvo.com>
Signed-off-by: David Girault <david.girault@qorvo.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/ieee802154_netdev.h |  16 ++++
 net/ieee802154/header_ops.c     |  24 +++++
 net/mac802154/cfg.c             |  31 ++++++-
 net/mac802154/ieee802154_i.h    |  18 ++++
 net/mac802154/iface.c           |   3 +
 net/mac802154/llsec.c           |   5 +-
 net/mac802154/main.c            |   1 +
 net/mac802154/scan.c            | 151 ++++++++++++++++++++++++++++++++
 8 files changed, 246 insertions(+), 3 deletions(-)

diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h
index 2f2196049a86..da8a3e648c7a 100644
--- a/include/net/ieee802154_netdev.h
+++ b/include/net/ieee802154_netdev.h
@@ -129,6 +129,13 @@ enum ieee802154_frame_version {
 	IEEE802154_MULTIPURPOSE_STD = IEEE802154_2003_STD,
 };
 
+enum ieee802154_addressing_mode {
+	IEEE802154_NO_ADDRESSING,
+	IEEE802154_RESERVED,
+	IEEE802154_SHORT_ADDRESSING,
+	IEEE802154_EXTENDED_ADDRESSING,
+};
+
 struct ieee802154_hdr {
 	struct ieee802154_hdr_fc fc;
 	u8 seq;
@@ -137,6 +144,11 @@ struct ieee802154_hdr {
 	struct ieee802154_sechdr sec;
 };
 
+struct ieee802154_beacon_frame {
+	struct ieee802154_hdr mhr;
+	struct ieee802154_beacon_hdr mac_pl;
+};
+
 /* pushes hdr onto the skb. fields of hdr->fc that can be calculated from
  * the contents of hdr will be, and the actual value of those bits in
  * hdr->fc will be ignored. this includes the INTRA_PAN bit and the frame
@@ -162,6 +174,10 @@ int ieee802154_hdr_peek_addrs(const struct sk_buff *skb,
  */
 int ieee802154_hdr_peek(const struct sk_buff *skb, struct ieee802154_hdr *hdr);
 
+/* pushes a beacon frame into an skb */
+int ieee802154_beacon_push(struct sk_buff *skb,
+			   struct ieee802154_beacon_frame *beacon);
+
 int ieee802154_max_payload(const struct ieee802154_hdr *hdr);
 
 static inline int
diff --git a/net/ieee802154/header_ops.c b/net/ieee802154/header_ops.c
index af337cf62764..35d384dfe29d 100644
--- a/net/ieee802154/header_ops.c
+++ b/net/ieee802154/header_ops.c
@@ -120,6 +120,30 @@ ieee802154_hdr_push(struct sk_buff *skb, struct ieee802154_hdr *hdr)
 }
 EXPORT_SYMBOL_GPL(ieee802154_hdr_push);
 
+int ieee802154_beacon_push(struct sk_buff *skb,
+			   struct ieee802154_beacon_frame *beacon)
+{
+	struct ieee802154_beacon_hdr *mac_pl = &beacon->mac_pl;
+	struct ieee802154_hdr *mhr = &beacon->mhr;
+	int ret;
+
+	skb_reserve(skb, sizeof(*mhr));
+	ret = ieee802154_hdr_push(skb, mhr);
+	if (ret < 0)
+		return ret;
+
+	skb_reset_mac_header(skb);
+	skb->mac_len = ret;
+
+	skb_put_data(skb, mac_pl, sizeof(*mac_pl));
+
+	if (mac_pl->pend_short_addr_count || mac_pl->pend_ext_addr_count)
+		return -EOPNOTSUPP;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ieee802154_beacon_push);
+
 static int
 ieee802154_hdr_get_addr(const u8 *buf, int mode, bool omit_pan,
 			struct ieee802154_addr *addr)
diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c
index 187cebcaf233..5c3cb019f751 100644
--- a/net/mac802154/cfg.c
+++ b/net/mac802154/cfg.c
@@ -114,8 +114,8 @@ 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))
+	/* Refuse to change channels during scanning or beaconing */
+	if (mac802154_is_scanning(local) || mac802154_is_beaconing(local))
 		return -EBUSY;
 
 	ret = drv_set_channel(local, page, channel);
@@ -290,6 +290,31 @@ static int mac802154_abort_scan(struct wpan_phy *wpan_phy,
 	return mac802154_abort_scan_locked(local, sdata);
 }
 
+static int mac802154_send_beacons(struct wpan_phy *wpan_phy,
+				  struct cfg802154_beacon_request *request)
+{
+	struct ieee802154_sub_if_data *sdata;
+
+	sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(request->wpan_dev);
+
+	ASSERT_RTNL();
+
+	return mac802154_send_beacons_locked(sdata, request);
+}
+
+static int mac802154_stop_beacons(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_stop_beacons_locked(local, sdata);
+}
+
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 static void
 ieee802154_get_llsec_table(struct wpan_phy *wpan_phy,
@@ -499,6 +524,8 @@ const struct cfg802154_ops mac802154_config_ops = {
 	.set_ackreq_default = ieee802154_set_ackreq_default,
 	.trigger_scan = mac802154_trigger_scan,
 	.abort_scan = mac802154_abort_scan,
+	.send_beacons = mac802154_send_beacons,
+	.stop_beacons = mac802154_stop_beacons,
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
 	.get_llsec_table = ieee802154_get_llsec_table,
 	.lock_llsec_table = ieee802154_lock_llsec_table,
diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h
index 0e4db967bd1d..63bab99ed368 100644
--- a/net/mac802154/ieee802154_i.h
+++ b/net/mac802154/ieee802154_i.h
@@ -23,6 +23,7 @@
 
 enum ieee802154_ongoing {
 	IEEE802154_IS_SCANNING = BIT(0),
+	IEEE802154_IS_BEACONING = BIT(1),
 };
 
 /* mac802154 device private data */
@@ -60,6 +61,12 @@ struct ieee802154_local {
 	struct cfg802154_scan_request __rcu *scan_req;
 	struct delayed_work scan_work;
 
+	/* Beaconing */
+	unsigned int beacon_interval;
+	struct ieee802154_beacon_frame beacon;
+	struct cfg802154_beacon_request __rcu *beacon_req;
+	struct delayed_work beacon_work;
+
 	/* Asynchronous tasks */
 	struct list_head rx_beacon_list;
 	struct work_struct rx_beacon_work;
@@ -257,6 +264,17 @@ static inline bool mac802154_is_scanning(struct ieee802154_local *local)
 	return test_bit(IEEE802154_IS_SCANNING, &local->ongoing);
 }
 
+void mac802154_beacon_worker(struct work_struct *work);
+int mac802154_send_beacons_locked(struct ieee802154_sub_if_data *sdata,
+				  struct cfg802154_beacon_request *request);
+int mac802154_stop_beacons_locked(struct ieee802154_local *local,
+				  struct ieee802154_sub_if_data *sdata);
+
+static inline bool mac802154_is_beaconing(struct ieee802154_local *local)
+{
+	return test_bit(IEEE802154_IS_BEACONING, &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 a5958d323ea3..9d59caeb74e0 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -305,6 +305,9 @@ static int mac802154_slave_close(struct net_device *dev)
 	if (mac802154_is_scanning(local))
 		mac802154_abort_scan_locked(local, sdata);
 
+	if (mac802154_is_beaconing(local))
+		mac802154_stop_beacons_locked(local, sdata);
+
 	netif_stop_queue(dev);
 	local->open_count--;
 
diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c
index 55550ead2ced..8d2eabc71bbe 100644
--- a/net/mac802154/llsec.c
+++ b/net/mac802154/llsec.c
@@ -707,7 +707,10 @@ int mac802154_llsec_encrypt(struct mac802154_llsec *sec, struct sk_buff *skb)
 
 	hlen = ieee802154_hdr_pull(skb, &hdr);
 
-	if (hlen < 0 || hdr.fc.type != IEEE802154_FC_TYPE_DATA)
+	/* TODO: control frames security support */
+	if (hlen < 0 ||
+	    (hdr.fc.type != IEEE802154_FC_TYPE_DATA &&
+	     hdr.fc.type != IEEE802154_FC_TYPE_BEACON))
 		return -EINVAL;
 
 	if (!hdr.fc.security_enabled ||
diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index b1111279e06d..ee23e234b998 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -99,6 +99,7 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
 	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_DELAYED_WORK(&local->beacon_work, mac802154_beacon_worker);
 
 	/* init supported flags with 802.15.4 default ranges */
 	phy->supported.max_minbe = 8;
diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
index 56056b9c93c1..cfbe20b1ec5e 100644
--- a/net/mac802154/scan.c
+++ b/net/mac802154/scan.c
@@ -16,6 +16,11 @@
 #include "driver-ops.h"
 #include "../ieee802154/nl802154.h"
 
+#define IEEE802154_BEACON_MHR_SZ 13
+#define IEEE802154_BEACON_PL_SZ 4
+#define IEEE802154_BEACON_SKB_SZ (IEEE802154_BEACON_MHR_SZ + \
+				  IEEE802154_BEACON_PL_SZ)
+
 /* 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.
@@ -286,3 +291,149 @@ int mac802154_process_beacon(struct ieee802154_local *local,
 
 	return 0;
 }
+
+static int mac802154_transmit_beacon(struct ieee802154_local *local,
+				     struct wpan_dev *wpan_dev)
+{
+	struct cfg802154_beacon_request *beacon_req;
+	struct ieee802154_sub_if_data *sdata;
+	struct sk_buff *skb;
+	int ret;
+
+	/* Update the sequence number */
+	local->beacon.mhr.seq = atomic_inc_return(&wpan_dev->bsn) & 0xFF;
+
+	skb = alloc_skb(IEEE802154_BEACON_SKB_SZ, GFP_KERNEL);
+	if (!skb)
+		return -ENOBUFS;
+
+	rcu_read_lock();
+	beacon_req = rcu_dereference(local->beacon_req);
+	if (unlikely(!beacon_req)) {
+		rcu_read_unlock();
+		kfree_skb(skb);
+		return -EINVAL;
+	}
+
+	sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(beacon_req->wpan_dev);
+	skb->dev = sdata->dev;
+
+	rcu_read_unlock();
+
+	ret = ieee802154_beacon_push(skb, &local->beacon);
+	if (ret) {
+		kfree_skb(skb);
+		return ret;
+	}
+
+	return ieee802154_subif_start_xmit(skb, sdata->dev);
+}
+
+void mac802154_beacon_worker(struct work_struct *work)
+{
+	struct ieee802154_local *local =
+		container_of(work, struct ieee802154_local, beacon_work.work);
+	struct cfg802154_beacon_request *beacon_req;
+	struct ieee802154_sub_if_data *sdata;
+	struct wpan_dev *wpan_dev;
+	int ret;
+
+	rcu_read_lock();
+	beacon_req = rcu_dereference(local->beacon_req);
+	if (unlikely(!beacon_req)) {
+		rcu_read_unlock();
+		return;
+	}
+
+	sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(beacon_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->beacon_work,
+				   msecs_to_jiffies(1000));
+		return;
+	}
+
+	wpan_dev = beacon_req->wpan_dev;
+
+	rcu_read_unlock();
+
+	dev_dbg(&sdata->dev->dev, "Sending beacon\n");
+	ret = mac802154_transmit_beacon(local, wpan_dev);
+	if (ret)
+		dev_err(&sdata->dev->dev,
+			"Beacon could not be transmitted (%d)\n", ret);
+
+	if (local->beacon_interval >= 0)
+		queue_delayed_work(local->mac_wq, &local->beacon_work,
+				   local->beacon_interval);
+}
+
+int mac802154_stop_beacons_locked(struct ieee802154_local *local,
+				  struct ieee802154_sub_if_data *sdata)
+{
+	struct wpan_dev *wpan_dev = &sdata->wpan_dev;
+	struct cfg802154_beacon_request *request;
+
+	ASSERT_RTNL();
+
+	if (!mac802154_is_beaconing(local))
+		return -ESRCH;
+
+	clear_bit(IEEE802154_IS_BEACONING, &local->ongoing);
+	cancel_delayed_work(&local->beacon_work);
+	request = rcu_replace_pointer(local->beacon_req, NULL, 1);
+	if (!request)
+		return 0;
+	kfree_rcu(request);
+
+	nl802154_beaconing_done(wpan_dev);
+
+	return 0;
+}
+
+int mac802154_send_beacons_locked(struct ieee802154_sub_if_data *sdata,
+				  struct cfg802154_beacon_request *request)
+{
+	struct ieee802154_local *local = sdata->local;
+
+	ASSERT_RTNL();
+
+	if (mac802154_is_beaconing(local))
+		mac802154_stop_beacons_locked(local, sdata);
+
+	/* Store beaconing parameters */
+	rcu_assign_pointer(local->beacon_req, request);
+
+	set_bit(IEEE802154_IS_BEACONING, &local->ongoing);
+
+	memset(&local->beacon, 0, sizeof(local->beacon));
+	local->beacon.mhr.fc.type = IEEE802154_FC_TYPE_BEACON;
+	local->beacon.mhr.fc.security_enabled = 0;
+	local->beacon.mhr.fc.frame_pending = 0;
+	local->beacon.mhr.fc.ack_request = 0;
+	local->beacon.mhr.fc.intra_pan = 0;
+	local->beacon.mhr.fc.dest_addr_mode = IEEE802154_NO_ADDRESSING;
+	local->beacon.mhr.fc.version = IEEE802154_2003_STD;
+	local->beacon.mhr.fc.source_addr_mode = IEEE802154_EXTENDED_ADDRESSING;
+	atomic_set(&request->wpan_dev->bsn, -1);
+	local->beacon.mhr.source.mode = IEEE802154_ADDR_LONG;
+	local->beacon.mhr.source.pan_id = cpu_to_le16(request->wpan_dev->pan_id);
+	local->beacon.mhr.source.extended_addr = cpu_to_le64(request->wpan_dev->extended_addr);
+	local->beacon.mac_pl.beacon_order = request->interval;
+	local->beacon.mac_pl.superframe_order = request->interval;
+	local->beacon.mac_pl.final_cap_slot = 0xf;
+	local->beacon.mac_pl.battery_life_ext = 0;
+	/* TODO: Fill this field depending on the coordinator capacity */
+	local->beacon.mac_pl.pan_coordinator = 1;
+	local->beacon.mac_pl.assoc_permit = 1;
+
+	/* Start the beacon work */
+	local->beacon_interval =
+		mac802154_scan_get_channel_time(request->interval,
+						request->wpan_phy->symbol_duration);
+	queue_delayed_work(local->mac_wq, &local->beacon_work, 0);
+
+	return 0;
+}
-- 
2.34.1

Re: [PATCH wpan-next v2 0/2] ieee802154: Beaconing support

[Alexander Aring]

Hi,

On Wed, Jan 25, 2023 at 5:31 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Scanning being now supported, we can eg. play with hwsim to verify
> everything works as soon as this series including beaconing support gets
> merged.
>
> Thanks,
> Miquèl
>
> Changes in v2:
> * Clearly state in the commit log llsec is not supported yet.
> * Do not use mlme transmission helpers because we don't really need to
>   stop the queue when sending a beacon, as we don't expect any feedback
>   from the PHY nor from the peers. However, we don't want to go through
>   the whole net stack either, so we bypass it calling the subif helper
>   directly.
>

Acked-by: Alexander Aring <aahringo@redhat.com>

- Alex

Re: [PATCH wpan-next v2 0/2] ieee802154: Beaconing support

[Alexander Aring]

Hi,

On Thu, Jan 26, 2023 at 8:45 PM Alexander Aring <aahringo@redhat.com> wrote:
>
> Hi,
>
> On Wed, Jan 25, 2023 at 5:31 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Scanning being now supported, we can eg. play with hwsim to verify
> > everything works as soon as this series including beaconing support gets
> > merged.
> >
> > Thanks,
> > Miquèl
> >
> > Changes in v2:
> > * Clearly state in the commit log llsec is not supported yet.
> > * Do not use mlme transmission helpers because we don't really need to
> >   stop the queue when sending a beacon, as we don't expect any feedback
> >   from the PHY nor from the peers. However, we don't want to go through
> >   the whole net stack either, so we bypass it calling the subif helper
> >   directly.
> >

moment, we use the mlme helpers to stop tx but we use the
ieee802154_subif_start_xmit() because of the possibility to invoke
current 802.15.4 hooks like llsec? That's how I understand it.

- Alex

Re: [PATCH wpan-next v2 0/2] ieee802154: Beaconing support

[Stefan Schmidt]

Hello.

On 25.01.23 11:29, Miquel Raynal wrote:
> Scanning being now supported, we can eg. play with hwsim to verify
> everything works as soon as this series including beaconing support gets
> merged.
> 
> Thanks,
> Miquèl
> 
> Changes in v2:
> * Clearly state in the commit log llsec is not supported yet.
> * Do not use mlme transmission helpers because we don't really need to
>    stop the queue when sending a beacon, as we don't expect any feedback
>    from the PHY nor from the peers. However, we don't want to go through
>    the whole net stack either, so we bypass it calling the subif helper
>    directly.
> 
> Miquel Raynal (2):
>    ieee802154: Add support for user beaconing requests
>    mac802154: Handle basic beaconing
> 
>   include/net/cfg802154.h         |  23 +++++
>   include/net/ieee802154_netdev.h |  16 ++++
>   include/net/nl802154.h          |   3 +
>   net/ieee802154/header_ops.c     |  24 +++++
>   net/ieee802154/nl802154.c       |  93 ++++++++++++++++++++
>   net/ieee802154/nl802154.h       |   1 +
>   net/ieee802154/rdev-ops.h       |  28 ++++++
>   net/ieee802154/trace.h          |  21 +++++
>   net/mac802154/cfg.c             |  31 ++++++-
>   net/mac802154/ieee802154_i.h    |  18 ++++
>   net/mac802154/iface.c           |   3 +
>   net/mac802154/llsec.c           |   5 +-
>   net/mac802154/main.c            |   1 +
>   net/mac802154/scan.c            | 151 ++++++++++++++++++++++++++++++++
>   14 files changed, 415 insertions(+), 3 deletions(-)

These patches have been applied to the wpan-next tree and will be
part of the next pull request to net-next. Thanks!

regards
Stefan Schmidt

Re: [PATCH wpan-next v2 0/2] ieee802154: Beaconing support

[Miquel Raynal]

Hi Alexander,

aahringo@redhat.com wrote on Thu, 26 Jan 2023 20:48:02 -0500:

> Hi,
> 
> On Thu, Jan 26, 2023 at 8:45 PM Alexander Aring <aahringo@redhat.com> wrote:
> >
> > Hi,
> >
> > On Wed, Jan 25, 2023 at 5:31 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > >
> > > Scanning being now supported, we can eg. play with hwsim to verify
> > > everything works as soon as this series including beaconing support gets
> > > merged.
> > >
> > > Thanks,
> > > Miquèl
> > >
> > > Changes in v2:
> > > * Clearly state in the commit log llsec is not supported yet.
> > > * Do not use mlme transmission helpers because we don't really need to
> > >   stop the queue when sending a beacon, as we don't expect any feedback
> > >   from the PHY nor from the peers. However, we don't want to go through
> > >   the whole net stack either, so we bypass it calling the subif helper
> > >   directly.
> > >  
> 
> moment, we use the mlme helpers to stop tx 

No, we no longer use the mlme helpers to stop tx when sending beacons
(but true MLME transmissions, we ack handling and return codes will be
used for other purposes).

> but we use the
> ieee802154_subif_start_xmit() because of the possibility to invoke
> current 802.15.4 hooks like llsec? That's how I understand it.

We go through llsec (see ieee802154_subif_start_xmit() implementation)
when we send data or beacons. When we send beacons, for now, we just
discard the llsec logic. This needs of course to be improved. We will
probably need some llsec handling in the mlme case as well in the near
future.

Thanks,
Miquèl

Re: [PATCH wpan-next v2 0/2] ieee802154: Beaconing support

[Alexander Aring]

Hi,

On Mon, Jan 30, 2023 at 4:55 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> aahringo@redhat.com wrote on Thu, 26 Jan 2023 20:48:02 -0500:
>
> > Hi,
> >
> > On Thu, Jan 26, 2023 at 8:45 PM Alexander Aring <aahringo@redhat.com> wrote:
> > >
> > > Hi,
> > >
> > > On Wed, Jan 25, 2023 at 5:31 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >
> > > > Scanning being now supported, we can eg. play with hwsim to verify
> > > > everything works as soon as this series including beaconing support gets
> > > > merged.
> > > >
> > > > Thanks,
> > > > Miquèl
> > > >
> > > > Changes in v2:
> > > > * Clearly state in the commit log llsec is not supported yet.
> > > > * Do not use mlme transmission helpers because we don't really need to
> > > >   stop the queue when sending a beacon, as we don't expect any feedback
> > > >   from the PHY nor from the peers. However, we don't want to go through
> > > >   the whole net stack either, so we bypass it calling the subif helper
> > > >   directly.
> > > >
> >
> > moment, we use the mlme helpers to stop tx
>
> No, we no longer use the mlme helpers to stop tx when sending beacons
> (but true MLME transmissions, we ack handling and return codes will be
> used for other purposes).
>

then we run into an issue overwriting the framebuffer while the normal
transmit path is active?

> > but we use the
> > ieee802154_subif_start_xmit() because of the possibility to invoke
> > current 802.15.4 hooks like llsec? That's how I understand it.
>
> We go through llsec (see ieee802154_subif_start_xmit() implementation)
> when we send data or beacons. When we send beacons, for now, we just
> discard the llsec logic. This needs of course to be improved. We will
> probably need some llsec handling in the mlme case as well in the near
> future.
>

i agree, thanks.

- Alex

Re: [PATCH wpan-next v2 0/2] ieee802154: Beaconing support

[Miquel Raynal]

Hi Alexander,

> > > > > Changes in v2:
> > > > > * Clearly state in the commit log llsec is not supported yet.
> > > > > * Do not use mlme transmission helpers because we don't really need to
> > > > >   stop the queue when sending a beacon, as we don't expect any feedback
> > > > >   from the PHY nor from the peers. However, we don't want to go through
> > > > >   the whole net stack either, so we bypass it calling the subif helper
> > > > >   directly.
> > > > >  
> > >
> > > moment, we use the mlme helpers to stop tx  
> >
> > No, we no longer use the mlme helpers to stop tx when sending beacons
> > (but true MLME transmissions, we ack handling and return codes will be
> > used for other purposes).
> >  
> 
> then we run into an issue overwriting the framebuffer while the normal
> transmit path is active?

Crap, yes you're right. That's not gonna work.

The net core acquires HARD_TX_LOCK() to avoid these issues and we are
no bypassing the net core without taking care of the proper frame
transmissions either (which would have worked with mlme_tx_one()). So I
guess there are two options:

* Either we deal with the extra penalty of stopping the queue and
  waiting for the beacon to be transmitted with an mlme_tx_one() call,
  as proposed initially.

* Or we hardcode our own "net" transmit helper, something like:

mac802154_fast_mlme_tx() {
	struct net_device *dev = skb->dev;
	struct netdev_queue *txq;

	txq = netdev_core_pick_tx(dev, skb, NULL);
	cpu = smp_processor_id();
	HARD_TX_LOCK(dev, txq, cpu);
	if (!netif_xmit_frozen_or_drv_stopped(txq))
		netdev_start_xmit(skb, dev, txq, 0);
	HARD_TX_UNLOCK(dev, txq);
}

Note1: this is very close to generic_xdp_tx() which tries to achieve the
same goal: sending packets, bypassing qdisc et al. I don't know whether
it makes sense to define it under mac802154/tx.c or core/dev.c and give
it another name, like generic_tx() or whatever would be more
appropriate. Or even adapting generic_xdp_tx() to make it look more
generic and use that function instead (without the xdp struct pointer).

Note2: I am wondering if it makes sense to disable bh here as well?

Once we settle, I send a patch.

Thanks,
Miquèl

Re: [PATCH wpan-next v2 0/2] ieee802154: Beaconing support

[Alexander Aring]

Hi,

On Tue, Jan 31, 2023 at 6:25 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> > > > > > Changes in v2:
> > > > > > * Clearly state in the commit log llsec is not supported yet.
> > > > > > * Do not use mlme transmission helpers because we don't really need to
> > > > > >   stop the queue when sending a beacon, as we don't expect any feedback
> > > > > >   from the PHY nor from the peers. However, we don't want to go through
> > > > > >   the whole net stack either, so we bypass it calling the subif helper
> > > > > >   directly.
> > > > > >
> > > >
> > > > moment, we use the mlme helpers to stop tx
> > >
> > > No, we no longer use the mlme helpers to stop tx when sending beacons
> > > (but true MLME transmissions, we ack handling and return codes will be
> > > used for other purposes).
> > >
> >
> > then we run into an issue overwriting the framebuffer while the normal
> > transmit path is active?
>
> Crap, yes you're right. That's not gonna work.
>
> The net core acquires HARD_TX_LOCK() to avoid these issues and we are
> no bypassing the net core without taking care of the proper frame
> transmissions either (which would have worked with mlme_tx_one()). So I
> guess there are two options:
>
> * Either we deal with the extra penalty of stopping the queue and
>   waiting for the beacon to be transmitted with an mlme_tx_one() call,
>   as proposed initially.
>
> * Or we hardcode our own "net" transmit helper, something like:
>
> mac802154_fast_mlme_tx() {
>         struct net_device *dev = skb->dev;
>         struct netdev_queue *txq;
>
>         txq = netdev_core_pick_tx(dev, skb, NULL);
>         cpu = smp_processor_id();
>         HARD_TX_LOCK(dev, txq, cpu);
>         if (!netif_xmit_frozen_or_drv_stopped(txq))
>                 netdev_start_xmit(skb, dev, txq, 0);
>         HARD_TX_UNLOCK(dev, txq);
> }
>
> Note1: this is very close to generic_xdp_tx() which tries to achieve the
> same goal: sending packets, bypassing qdisc et al. I don't know whether
> it makes sense to define it under mac802154/tx.c or core/dev.c and give
> it another name, like generic_tx() or whatever would be more
> appropriate. Or even adapting generic_xdp_tx() to make it look more
> generic and use that function instead (without the xdp struct pointer).
>

The problem here is that the transmit handling is completely
asynchronous. Calling netdev_start_xmit() is not "transmit and wait
until transmit is done", it is "start transmit here is the buffer" an
interrupt is coming up to report transmit is done. Until the time the
interrupt isn't arrived the framebuffer on the device is in use, we
don't know when the transceiver is done reading it. Only after tx done
isr. The time until the isr isn't arrived is for us a -EBUSY case due
hardware resource limitation. Currently we do that with stop/wake
queue to avoid calling of xmit_do() to not run into such -EBUSY
cases...

There might be clever things to do here to avoid this issue... I am
not sure how XDP does that.

> Note2: I am wondering if it makes sense to disable bh here as well?

May HARD_TX_LOCK() already do that? If they use spin_lock_bh() they
disable local softirqs until the lock isn't held anymore.

>
> Once we settle, I send a patch.
>

Not sure how to preceded here, but do see the problem? Or maybe I
overlooked something here...

- Alex

Re: [PATCH wpan-next v2 0/2] ieee802154: Beaconing support

[Miquel Raynal]

Hi Alexander,

aahringo@redhat.com wrote on Wed, 1 Feb 2023 12:15:42 -0500:

> Hi,
> 
> On Tue, Jan 31, 2023 at 6:25 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >  
> > > > > > > Changes in v2:
> > > > > > > * Clearly state in the commit log llsec is not supported yet.
> > > > > > > * Do not use mlme transmission helpers because we don't really need to
> > > > > > >   stop the queue when sending a beacon, as we don't expect any feedback
> > > > > > >   from the PHY nor from the peers. However, we don't want to go through
> > > > > > >   the whole net stack either, so we bypass it calling the subif helper
> > > > > > >   directly.
> > > > > > >  
> > > > >
> > > > > moment, we use the mlme helpers to stop tx  
> > > >
> > > > No, we no longer use the mlme helpers to stop tx when sending beacons
> > > > (but true MLME transmissions, we ack handling and return codes will be
> > > > used for other purposes).
> > > >  
> > >
> > > then we run into an issue overwriting the framebuffer while the normal
> > > transmit path is active?  
> >
> > Crap, yes you're right. That's not gonna work.
> >
> > The net core acquires HARD_TX_LOCK() to avoid these issues and we are
> > no bypassing the net core without taking care of the proper frame
> > transmissions either (which would have worked with mlme_tx_one()). So I
> > guess there are two options:
> >
> > * Either we deal with the extra penalty of stopping the queue and
> >   waiting for the beacon to be transmitted with an mlme_tx_one() call,
> >   as proposed initially.
> >
> > * Or we hardcode our own "net" transmit helper, something like:
> >
> > mac802154_fast_mlme_tx() {
> >         struct net_device *dev = skb->dev;
> >         struct netdev_queue *txq;
> >
> >         txq = netdev_core_pick_tx(dev, skb, NULL);
> >         cpu = smp_processor_id();
> >         HARD_TX_LOCK(dev, txq, cpu);
> >         if (!netif_xmit_frozen_or_drv_stopped(txq))
> >                 netdev_start_xmit(skb, dev, txq, 0);
> >         HARD_TX_UNLOCK(dev, txq);
> > }
> >
> > Note1: this is very close to generic_xdp_tx() which tries to achieve the
> > same goal: sending packets, bypassing qdisc et al. I don't know whether
> > it makes sense to define it under mac802154/tx.c or core/dev.c and give
> > it another name, like generic_tx() or whatever would be more
> > appropriate. Or even adapting generic_xdp_tx() to make it look more
> > generic and use that function instead (without the xdp struct pointer).
> >  
> 
> The problem here is that the transmit handling is completely
> asynchronous. Calling netdev_start_xmit() is not "transmit and wait
> until transmit is done", it is "start transmit here is the buffer" an
> interrupt is coming up to report transmit is done. Until the time the
> interrupt isn't arrived the framebuffer on the device is in use, we
> don't know when the transceiver is done reading it. Only after tx done
> isr. The time until the isr isn't arrived is for us a -EBUSY case due
> hardware resource limitation. Currently we do that with stop/wake
> queue to avoid calling of xmit_do() to not run into such -EBUSY
> cases...
> 
> There might be clever things to do here to avoid this issue... I am
> not sure how XDP does that.
> 
> > Note2: I am wondering if it makes sense to disable bh here as well?  
> 
> May HARD_TX_LOCK() already do that? If they use spin_lock_bh() they
> disable local softirqs until the lock isn't held anymore.

I saw a case where both are called so I guess the short answer is "no":
https://elixir.bootlin.com/linux/latest/source/net/core/dev.c#L4307

> 
> >
> > Once we settle, I send a patch.
> >  
> 
> Not sure how to preceded here, but do see the problem? Or maybe I
> overlooked something here...

No you clearly had a sharp eye on that one, I totally see the problem.

Maybe the safest and simplest approach would be to be back using
the proper mlme transmission helpers for beacons (like in the initial
proposal). TBH I don't think there is a huge performance hit because in
both cases we wait for that ISR saying "the packet has been consumed by
the transceiver". It's just that in one case we wait for the return
code (MLME) and then return, in the other case we return but no
more packets will go through until the queue is released by the ISR (as
you said, in order to avoid the -EBUSY case). So in practice I don't
expect any performance hit. It is true however that we might want to
optimize this a little bit if we ever add something like an async
callback saying "skb consumed by the transceiver, another can be
queued" and gain a few us. Maybe a comment could be useful here (I'll
add it to my fix if we agree).

Thanks,
Miquèl

Re: [PATCH wpan-next v2 0/2] ieee802154: Beaconing support

[Alexander Aring]

Hi,

On Fri, Feb 3, 2023 at 10:19 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> aahringo@redhat.com wrote on Wed, 1 Feb 2023 12:15:42 -0500:
>
> > Hi,
> >
> > On Tue, Jan 31, 2023 at 6:25 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Alexander,
> > >
> > > > > > > > Changes in v2:
> > > > > > > > * Clearly state in the commit log llsec is not supported yet.
> > > > > > > > * Do not use mlme transmission helpers because we don't really need to
> > > > > > > >   stop the queue when sending a beacon, as we don't expect any feedback
> > > > > > > >   from the PHY nor from the peers. However, we don't want to go through
> > > > > > > >   the whole net stack either, so we bypass it calling the subif helper
> > > > > > > >   directly.
> > > > > > > >
> > > > > >
> > > > > > moment, we use the mlme helpers to stop tx
> > > > >
> > > > > No, we no longer use the mlme helpers to stop tx when sending beacons
> > > > > (but true MLME transmissions, we ack handling and return codes will be
> > > > > used for other purposes).
> > > > >
> > > >
> > > > then we run into an issue overwriting the framebuffer while the normal
> > > > transmit path is active?
> > >
> > > Crap, yes you're right. That's not gonna work.
> > >
> > > The net core acquires HARD_TX_LOCK() to avoid these issues and we are
> > > no bypassing the net core without taking care of the proper frame
> > > transmissions either (which would have worked with mlme_tx_one()). So I
> > > guess there are two options:
> > >
> > > * Either we deal with the extra penalty of stopping the queue and
> > >   waiting for the beacon to be transmitted with an mlme_tx_one() call,
> > >   as proposed initially.
> > >
> > > * Or we hardcode our own "net" transmit helper, something like:
> > >
> > > mac802154_fast_mlme_tx() {
> > >         struct net_device *dev = skb->dev;
> > >         struct netdev_queue *txq;
> > >
> > >         txq = netdev_core_pick_tx(dev, skb, NULL);
> > >         cpu = smp_processor_id();
> > >         HARD_TX_LOCK(dev, txq, cpu);
> > >         if (!netif_xmit_frozen_or_drv_stopped(txq))
> > >                 netdev_start_xmit(skb, dev, txq, 0);
> > >         HARD_TX_UNLOCK(dev, txq);
> > > }
> > >
> > > Note1: this is very close to generic_xdp_tx() which tries to achieve the
> > > same goal: sending packets, bypassing qdisc et al. I don't know whether
> > > it makes sense to define it under mac802154/tx.c or core/dev.c and give
> > > it another name, like generic_tx() or whatever would be more
> > > appropriate. Or even adapting generic_xdp_tx() to make it look more
> > > generic and use that function instead (without the xdp struct pointer).
> > >
> >
> > The problem here is that the transmit handling is completely
> > asynchronous. Calling netdev_start_xmit() is not "transmit and wait
> > until transmit is done", it is "start transmit here is the buffer" an
> > interrupt is coming up to report transmit is done. Until the time the
> > interrupt isn't arrived the framebuffer on the device is in use, we
> > don't know when the transceiver is done reading it. Only after tx done
> > isr. The time until the isr isn't arrived is for us a -EBUSY case due
> > hardware resource limitation. Currently we do that with stop/wake
> > queue to avoid calling of xmit_do() to not run into such -EBUSY
> > cases...
> >
> > There might be clever things to do here to avoid this issue... I am
> > not sure how XDP does that.
> >
> > > Note2: I am wondering if it makes sense to disable bh here as well?
> >
> > May HARD_TX_LOCK() already do that? If they use spin_lock_bh() they
> > disable local softirqs until the lock isn't held anymore.
>
> I saw a case where both are called so I guess the short answer is "no":
> https://elixir.bootlin.com/linux/latest/source/net/core/dev.c#L4307
>
> >
> > >
> > > Once we settle, I send a patch.
> > >
> >
> > Not sure how to preceded here, but do see the problem? Or maybe I
> > overlooked something here...
>
> No you clearly had a sharp eye on that one, I totally see the problem.
>
> Maybe the safest and simplest approach would be to be back using
> the proper mlme transmission helpers for beacons (like in the initial
> proposal).

ok.

> TBH I don't think there is a huge performance hit because in
> both cases we wait for that ISR saying "the packet has been consumed by
> the transceiver". It's just that in one case we wait for the return
> code (MLME) and then return, in the other case we return but no
> more packets will go through until the queue is released by the ISR (as
> you said, in order to avoid the -EBUSY case). So in practice I don't
> expect any performance hit. It is true however that we might want to
> optimize this a little bit if we ever add something like an async
> callback saying "skb consumed by the transceiver, another can be
> queued" and gain a few us. Maybe a comment could be useful here (I'll
> add it to my fix if we agree).

the future will show how things work out here. I am fine with the
initial proposal.

- Alex