[wireless-next v2 0/4] [v2] wifi: S1G short beacon support

[wireless-next v2 0/4] [v2] wifi: S1G short beacon support

[Lachlan Hodges]

This patchset implements S1G short beaconing within mac80211 and
cfg80211. I don't think its worth going into the technical detail
again as that can be found in the commit messages or in v1:

https://patchwork.kernel.org/project/linux-wireless/cover/20250714051403.597090-1-lachlan.hodges@morsemicro.com/

The changes since v1 are as follows:

(1) Rather than introduce new validation routines for the short
    beacons, we leverage the existing routines since they can
    handle all beacon formats and ensure both long and short
    beacons are well formed without validating the existence of
    particular elements.

(2) The long beacon period (renamed from short beacon period to
    better describe what it actually is i.e the number of beacon
    intervals between each long beacon) is now an individual
    attribute. The reasoning behind this is that this attribute
    cannot be updated dynamically, unlike the beacon template. So
    we have taken inspiration for how the regular beacon interval,
    DTIM period etc. are handled. This allows us to use the same
    routine for both updating and setting the beacon data, but
    require this attribute when bringing up the interface if it
    is to be using short beacons. We think this is a much cleaner
    approach.

NB: It was mentioned it would be good to find a better way (or
    introduce) a new method for determining if we are bringing up
    an S1G interface. This patch does not handle that, though it is
    something I briefly looked into and will probably be done in a
    future patchset.

(3) We no longer introduce the short beacon variant for the beacon
    interval or DTIM period - and instead just reuse the existing
    parameters since - fundamentally - they don't change anything
    besides add more code complexity to various key code paths plus
    require changes within mac80211_hwsim. This patch now no longer
    requires hwsim changes.

(4) Drop further validation from within mac80211, and just perform
    it within cfg80211.

(5) Due to (3), we can drop all mac80211_hwsim changes.

(6) Fix up a kernel-doc error and properly describe the max
    short beacon nested attributes.

Overall this patch is much leaner, and has less of an affect on
non S1G interfaces so that is obviously preferable for everyone.
Even then it is more inline with what is currently done when setting
vs updating an interface and handling optional attributes.

I've sent this as a non-RFC as I think it's a "finished patchset"
but obviously still open to any feedback to resend another version.

Lachlan Hodges (4):
  wifi: cfg80211: support configuring an S1G short beaconing BSS
  wifi: mac80211: support initialising an S1G short beaconing BSS
  wifi: mac80211: support initialising current short beacon index
  wifi: mac80211: support returning the S1G short beacon skb

 include/net/cfg80211.h        |  23 ++++++++
 include/net/mac80211.h        |   9 +++
 include/uapi/linux/nl80211.h  |  39 +++++++++++++
 net/mac80211/cfg.c            |  93 +++++++++++++++++++++++++++++-
 net/mac80211/debugfs_netdev.c |   2 +-
 net/mac80211/ieee80211_i.h    |  15 ++++-
 net/mac80211/mesh.c           |   2 +-
 net/mac80211/tx.c             | 104 ++++++++++++++++++++++++++++++----
 net/mac80211/util.c           |  31 +++++++++-
 net/wireless/nl80211.c        |  72 +++++++++++++++++++++++
 10 files changed, 370 insertions(+), 20 deletions(-)

-- 
2.43.0

[wireless-next v2 1/4] wifi: cfg80211: support configuring an S1G short beaconing BSS

[Lachlan Hodges]

S1G short beacons are an optional frame type used in an S1G BSS
that contain a limited set of elements. While they are optional,
they are a fundamental part of S1G that enables significant
power saving.

Expose 2 additional netlink attributes,
NL80211_ATTR_S1G_LONG_BEACON_PERIOD which denotes the number of beacon
intervals between each long beacon and NL80211_ATTR_S1G_SHORT_BEACON
which is a nested attribute containing the short beacon tail and
head. We split them as the long beacon period cannot be updated,
and is only used when initialisng the interface, whereas the short
beacon data can be used to both initialise and update the templates.
This follows how things such as the beacon interval and DTIM period
currently operate.

During the initialisation path, we ensure we have the long beacon
period if the short beacon data is being passed down, whereas
the update path will simply update the template if its sent down.

The short beacon data is validated by the same routines for regular
beacons as they support correctly parsing the short beacons.

Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
v1 -> v2:

- Remove additional short beacon element validation and use existing
  validation routines.
- Remove the short beacon DTIM period and short beacon interval
  attributes
- Remove the long beacon period from the nested attribute to allow
  easier updates, but make it mandatory for bringing up the interface.
- Rename "short beacon period" to "long beacon period"
- Fix missing nested attribute kernel doc
- Fix attribute naming to ...SHORT_BEACON_ATTR_HEAD for nested
  attributes

---
 include/net/cfg80211.h       | 23 ++++++++++++
 include/uapi/linux/nl80211.h | 39 +++++++++++++++++++
 net/wireless/nl80211.c       | 72 ++++++++++++++++++++++++++++++++++++
 3 files changed, 134 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 6ec9a8865b8b..a7da04e13290 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1423,6 +1423,23 @@ struct cfg80211_unsol_bcast_probe_resp {
 	const u8 *tmpl;
 };
 
+/**
+ * struct cfg80211_s1g_short_beacon - S1G short beacon data.
+ *
+ * @update: Set to true if the feature configuration should be updated.
+ * @short_head: Short beacon head.
+ * @short_tail: Short beacon tail.
+ * @short_head_len: Short beacon head len.
+ * @short_tail_len: Short beacon tail len.
+ */
+struct cfg80211_s1g_short_beacon {
+	bool update;
+	const u8 *short_head;
+	const u8 *short_tail;
+	size_t short_head_len;
+	size_t short_tail_len;
+};
+
 /**
  * struct cfg80211_ap_settings - AP configuration
  *
@@ -1463,6 +1480,8 @@ struct cfg80211_unsol_bcast_probe_resp {
  * @fils_discovery: FILS discovery transmission parameters
  * @unsol_bcast_probe_resp: Unsolicited broadcast probe response parameters
  * @mbssid_config: AP settings for multiple bssid
+ * @s1g_long_beacon_period: S1G long beacon period
+ * @s1g_short_beacon: S1G short beacon data
  */
 struct cfg80211_ap_settings {
 	struct cfg80211_chan_def chandef;
@@ -1496,6 +1515,8 @@ struct cfg80211_ap_settings {
 	struct cfg80211_fils_discovery fils_discovery;
 	struct cfg80211_unsol_bcast_probe_resp unsol_bcast_probe_resp;
 	struct cfg80211_mbssid_config mbssid_config;
+	u8 s1g_long_beacon_period;
+	struct cfg80211_s1g_short_beacon s1g_short_beacon;
 };
 
 
@@ -1507,11 +1528,13 @@ struct cfg80211_ap_settings {
  * @beacon: beacon data
  * @fils_discovery: FILS discovery transmission parameters
  * @unsol_bcast_probe_resp: Unsolicited broadcast probe response parameters
+ * @s1g_short_beacon: S1G short beacon data
  */
 struct cfg80211_ap_update {
 	struct cfg80211_beacon_data beacon;
 	struct cfg80211_fils_discovery fils_discovery;
 	struct cfg80211_unsol_bcast_probe_resp unsol_bcast_probe_resp;
+	struct cfg80211_s1g_short_beacon s1g_short_beacon;
 };
 
 /**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 39460334dafb..d1a14f2892d9 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2915,6 +2915,19 @@ enum nl80211_commands {
  *	applicable to that specific radio only. If the radio id is greater
  *	thank the number of radios, error denoting invalid value is returned.
  *
+ * @NL80211_ATTR_S1G_LONG_BEACON_PERIOD: (u8) Integer attribute that represents
+ *	the number of beacon intervals between each long beacon transmission
+ *	for an S1G BSS with short beaconing enabled. This is a required
+ *	attribute for initialising an S1G short beaconing BSS. When updating
+ *	the short beacon data, this is not required. It has a minimum value of
+ *	2 (i.e 2 beacon intervals).
+ *
+ * @NL80211_ATTR_S1G_SHORT_BEACON: Nested attribute containing the short beacon
+ *	head and tail used to set or update the short beacon templates. When
+ *	bringing up a new interface, %NL80211_ATTR_S1G_LONG_BEACON_PERIOD is
+ *	required alongside this attribute. Refer to
+ *	@enum nl80211_s1g_short_beacon_attrs for the attribute definitions.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -3474,6 +3487,9 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_WIPHY_RADIO_INDEX,
 
+	NL80211_ATTR_S1G_LONG_BEACON_PERIOD,
+	NL80211_ATTR_S1G_SHORT_BEACON,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -8148,4 +8164,27 @@ enum nl80211_wiphy_radio_freq_range {
 	NL80211_WIPHY_RADIO_FREQ_ATTR_MAX = __NL80211_WIPHY_RADIO_FREQ_ATTR_LAST - 1,
 };
 
+/**
+ * enum nl80211_s1g_short_beacon_attrs - S1G short beacon data
+ *
+ * @__NL80211_S1G_SHORT_BEACON_ATTR_INVALID: Invalid
+ *
+ * @NL80211_S1G_SHORT_BEACON_ATTR_HEAD: Short beacon head (binary).
+ * @NL80211_S1G_SHORT_BEACON_ATTR_TAIL: Short beacon tail (binary).
+ *
+ * @__NL80211_S1G_SHORT_BEACON_ATTR_LAST: Internal
+ * @NL80211_S1G_SHORT_BEACON_ATTR_MAX: Highest attribute
+ */
+enum nl80211_s1g_short_beacon_attrs {
+	__NL80211_S1G_SHORT_BEACON_ATTR_INVALID,
+
+	NL80211_S1G_SHORT_BEACON_ATTR_HEAD,
+	NL80211_S1G_SHORT_BEACON_ATTR_TAIL,
+
+	/* keep last */
+	__NL80211_S1G_SHORT_BEACON_ATTR_LAST,
+	NL80211_S1G_SHORT_BEACON_ATTR_MAX =
+		__NL80211_S1G_SHORT_BEACON_ATTR_LAST - 1
+};
+
 #endif /* __LINUX_NL80211_H */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 4e6c0a4e2a82..47c35b741124 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -482,6 +482,16 @@ nl80211_sta_wme_policy[NL80211_STA_WME_MAX + 1] = {
 	[NL80211_STA_WME_MAX_SP] = { .type = NLA_U8 },
 };
 
+static const struct nla_policy
+nl80211_s1g_short_beacon[NL80211_S1G_SHORT_BEACON_ATTR_MAX + 1] = {
+	[NL80211_S1G_SHORT_BEACON_ATTR_HEAD] =
+		NLA_POLICY_VALIDATE_FN(NLA_BINARY, validate_beacon_head,
+				       IEEE80211_MAX_DATA_LEN),
+	[NL80211_S1G_SHORT_BEACON_ATTR_TAIL] =
+		NLA_POLICY_VALIDATE_FN(NLA_BINARY, validate_ie_attr,
+				       IEEE80211_MAX_DATA_LEN),
+};
+
 static const struct netlink_range_validation nl80211_punct_bitmap_range = {
 	.min = 0,
 	.max = 0xffff,
@@ -858,6 +868,9 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
 	[NL80211_ATTR_EPCS] = { .type = NLA_FLAG },
 	[NL80211_ATTR_ASSOC_MLD_EXT_CAPA_OPS] = { .type = NLA_U16 },
 	[NL80211_ATTR_WIPHY_RADIO_INDEX] = { .type = NLA_U8 },
+	[NL80211_ATTR_S1G_LONG_BEACON_PERIOD] = NLA_POLICY_MIN(NLA_U8, 2),
+	[NL80211_ATTR_S1G_SHORT_BEACON] =
+		NLA_POLICY_NESTED(nl80211_s1g_short_beacon),
 };
 
 /* policy for the key attributes */
@@ -6202,6 +6215,41 @@ static int nl80211_validate_ap_phy_operation(struct cfg80211_ap_settings *params
 	return 0;
 }
 
+static int
+nl80211_parse_s1g_short_beacon(struct cfg80211_registered_device *rdev,
+			       struct nlattr *attrs,
+			       struct cfg80211_s1g_short_beacon *sb)
+{
+	struct nlattr *tb[NL80211_S1G_SHORT_BEACON_ATTR_MAX + 1];
+	int ret;
+
+	if (!rdev->wiphy.bands[NL80211_BAND_S1GHZ])
+		return -EINVAL;
+
+	ret = nla_parse_nested(tb, NL80211_S1G_SHORT_BEACON_ATTR_MAX, attrs,
+			       NULL, NULL);
+	if (ret)
+		return ret;
+
+	/* Short beacon tail is optional (i.e might only include the TIM) */
+	if (!tb[NL80211_S1G_SHORT_BEACON_ATTR_HEAD])
+		return -EINVAL;
+
+	sb->short_head = nla_data(tb[NL80211_S1G_SHORT_BEACON_ATTR_HEAD]);
+	sb->short_head_len = nla_len(tb[NL80211_S1G_SHORT_BEACON_ATTR_HEAD]);
+	sb->short_tail_len = 0;
+
+	if (tb[NL80211_S1G_SHORT_BEACON_ATTR_TAIL]) {
+		sb->short_tail =
+			nla_data(tb[NL80211_S1G_SHORT_BEACON_ATTR_TAIL]);
+		sb->short_tail_len =
+			nla_len(tb[NL80211_S1G_SHORT_BEACON_ATTR_TAIL]);
+	}
+
+	sb->update = true;
+	return 0;
+}
+
 static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
@@ -6442,6 +6490,22 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 		goto out;
 	}
 
+	if (info->attrs[NL80211_ATTR_S1G_SHORT_BEACON]) {
+		if (!info->attrs[NL80211_ATTR_S1G_LONG_BEACON_PERIOD]) {
+			err = -EINVAL;
+			goto out;
+		}
+
+		params->s1g_long_beacon_period = nla_get_u8(
+			info->attrs[NL80211_ATTR_S1G_LONG_BEACON_PERIOD]);
+
+		err = nl80211_parse_s1g_short_beacon(
+			rdev, info->attrs[NL80211_ATTR_S1G_SHORT_BEACON],
+			&params->s1g_short_beacon);
+		if (err)
+			goto out;
+	}
+
 	err = nl80211_calculate_ap_params(params);
 	if (err)
 		goto out;
@@ -6550,6 +6614,14 @@ static int nl80211_set_beacon(struct sk_buff *skb, struct genl_info *info)
 			goto out;
 	}
 
+	attr = info->attrs[NL80211_ATTR_S1G_SHORT_BEACON];
+	if (attr) {
+		err = nl80211_parse_s1g_short_beacon(rdev, attr,
+						     &params->s1g_short_beacon);
+		if (err)
+			goto out;
+	}
+
 	err = rdev_change_beacon(rdev, dev, params);
 
 out:
-- 
2.43.0

[wireless-next v2 2/4] wifi: mac80211: support initialising an S1G short beaconing BSS

[Lachlan Hodges]

Introduce the ability to parse the short beacon data and long
beacon period. Ensure during the initialisation path we set the
long beacon period and set the s1g_short_beaconing boolean to true,
which determines if the S1G BSS is short beaconing. Additionally,
the update path only updates the beacon templates if there is
an update present, as the long beacon period cannot be
dynamically updated without tearing down the interface.

Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
v1 -> v2:

- Remove validation not needed within mac80211 (its performed in
  cfg80211)
- remove short beacon DTIM and short beacon interval fields as we
  are using the existing beacon int and dtim period variables

---
 include/net/mac80211.h     |  9 ++++
 net/mac80211/cfg.c         | 85 ++++++++++++++++++++++++++++++++++++++
 net/mac80211/ieee80211_i.h |  9 ++++
 3 files changed, 103 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 577fd6a8c372..119466e64c42 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -365,6 +365,7 @@ struct ieee80211_vif_chanctx_switch {
  * @BSS_CHANGED_MLD_VALID_LINKS: MLD valid links status changed.
  * @BSS_CHANGED_MLD_TTLM: negotiated TID to link mapping was changed
  * @BSS_CHANGED_TPE: transmit power envelope changed
+ * @BSS_CHANGED_S1G_SHORT_BEACON: S1G short beacon changed
  */
 enum ieee80211_bss_change {
 	BSS_CHANGED_ASSOC		= 1<<0,
@@ -402,6 +403,7 @@ enum ieee80211_bss_change {
 	BSS_CHANGED_MLD_VALID_LINKS	= BIT_ULL(33),
 	BSS_CHANGED_MLD_TTLM		= BIT_ULL(34),
 	BSS_CHANGED_TPE			= BIT_ULL(35),
+	BSS_CHANGED_S1G_SHORT_BEACON	= BIT_ULL(36),
 
 	/* when adding here, make sure to change ieee80211_reconfig */
 };
@@ -758,6 +760,10 @@ struct ieee80211_parsed_tpe {
  *	be updated to 1, even if bss_param_ch_cnt didn't change. This allows
  *	the link to know that it heard the latest value from its own beacon
  *	(as opposed to hearing its value from another link's beacon).
+ * @s1g_short_beaconing: determines if short beaconing is enabled for an S1G
+ *	BSS.
+ * @s1g_long_beacon_period: number of beacon intervals between each long
+ *	beacon transmission.
  */
 struct ieee80211_bss_conf {
 	struct ieee80211_vif *vif;
@@ -857,6 +863,9 @@ struct ieee80211_bss_conf {
 
 	u8 bss_param_ch_cnt;
 	u8 bss_param_ch_cnt_link_id;
+
+	bool s1g_short_beaconing;
+	u8 s1g_long_beacon_period;
 };
 
 /**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index d76643d46150..aeceac5747b6 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1071,6 +1071,60 @@ ieee80211_set_unsol_bcast_probe_resp(struct ieee80211_sub_if_data *sdata,
 	return 0;
 }
 
+static int
+ieee80211_set_s1g_short_beacon(struct ieee80211_sub_if_data *sdata,
+			       struct cfg80211_s1g_short_beacon *params,
+			       struct ieee80211_link_data *link,
+			       struct ieee80211_bss_conf *link_conf,
+			       u64 *changed)
+{
+	struct s1g_short_beacon_data *new, *old;
+	int new_head_len, new_tail_len, size;
+
+	if (!params->update)
+		return 0;
+
+	old = sdata_dereference(link->u.ap.s1g_short_beacon, sdata);
+	if (!params->short_head && !old)
+		return -EINVAL;
+
+	new_head_len = params->short_head ? params->short_head_len :
+					    old->short_head_len;
+	new_tail_len = (params->short_tail || !old) ? params->short_tail_len :
+						      old->short_tail_len;
+
+	size = sizeof(*new) + new_head_len + new_tail_len;
+
+	new = kzalloc(size, GFP_KERNEL);
+	if (!new)
+		return -ENOMEM;
+
+	/* Memory layout: |struct|head|tail| */
+	new->short_head = ((u8 *)new) + sizeof(*new);
+	new->short_tail = new->short_head + new_head_len;
+
+	new->short_head_len = new_head_len;
+	new->short_tail_len = new_tail_len;
+
+	if (params->short_head)
+		memcpy(new->short_head, params->short_head, new_head_len);
+	else
+		memcpy(new->short_head, old->short_head, new_head_len);
+
+	if (params->short_tail)
+		memcpy(new->short_tail, params->short_tail, new_tail_len);
+	else if (old && old->short_tail)
+		memcpy(new->short_tail, old->short_tail, new_tail_len);
+
+	rcu_assign_pointer(link->u.ap.s1g_short_beacon, new);
+
+	if (old)
+		kfree_rcu(old, rcu_head);
+
+	*changed |= BSS_CHANGED_S1G_SHORT_BEACON;
+	return 0;
+}
+
 static int ieee80211_set_ftm_responder_params(
 				struct ieee80211_sub_if_data *sdata,
 				const u8 *lci, size_t lci_len,
@@ -1493,9 +1547,16 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 	link_conf->twt_responder = params->twt_responder;
 	link_conf->he_obss_pd = params->he_obss_pd;
 	link_conf->he_bss_color = params->beacon.he_bss_color;
+	link_conf->s1g_short_beaconing = false;
 	sdata->vif.cfg.s1g = params->chandef.chan->band ==
 				  NL80211_BAND_S1GHZ;
 
+	if (sdata->vif.cfg.s1g && params->s1g_long_beacon_period) {
+		link_conf->s1g_short_beaconing = true;
+		link_conf->s1g_long_beacon_period =
+			params->s1g_long_beacon_period;
+	}
+
 	sdata->vif.cfg.ssid_len = params->ssid_len;
 	if (params->ssid_len)
 		memcpy(sdata->vif.cfg.ssid, params->ssid,
@@ -1541,6 +1602,14 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 	if (err < 0)
 		goto error;
 
+	if (sdata->vif.cfg.s1g) {
+		err = ieee80211_set_s1g_short_beacon(sdata,
+						     &params->s1g_short_beacon,
+						     link, link_conf, &changed);
+		if (err < 0)
+			goto error;
+	}
+
 	err = drv_start_ap(sdata->local, sdata, link_conf);
 	if (err) {
 		old = sdata_dereference(link->u.ap.beacon, sdata);
@@ -1619,6 +1688,14 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
 	if (err < 0)
 		return err;
 
+	if (link_conf->s1g_short_beaconing) {
+		err = ieee80211_set_s1g_short_beacon(sdata,
+						     &params->s1g_short_beacon,
+						     link, link_conf, &changed);
+		if (err < 0)
+			return err;
+	}
+
 	if (beacon->he_bss_color_valid &&
 	    beacon->he_bss_color.enabled != link_conf->he_bss_color.enabled) {
 		link_conf->he_bss_color.enabled = beacon->he_bss_color.enabled;
@@ -1650,6 +1727,7 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev,
 	struct probe_resp *old_probe_resp;
 	struct fils_discovery_data *old_fils_discovery;
 	struct unsol_bcast_probe_resp_data *old_unsol_bcast_probe_resp;
+	struct s1g_short_beacon_data *old_s1g_short_beacon;
 	struct cfg80211_chan_def chandef;
 	struct ieee80211_link_data *link =
 		sdata_dereference(sdata->link[link_id], sdata);
@@ -1668,6 +1746,8 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev,
 	old_unsol_bcast_probe_resp =
 		sdata_dereference(link->u.ap.unsol_bcast_probe_resp,
 				  sdata);
+	old_s1g_short_beacon =
+		sdata_dereference(link->u.ap.s1g_short_beacon, sdata);
 
 	/* abort any running channel switch or color change */
 	link_conf->csa_active = false;
@@ -1690,6 +1770,7 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev,
 	RCU_INIT_POINTER(link->u.ap.probe_resp, NULL);
 	RCU_INIT_POINTER(link->u.ap.fils_discovery, NULL);
 	RCU_INIT_POINTER(link->u.ap.unsol_bcast_probe_resp, NULL);
+	RCU_INIT_POINTER(link->u.ap.s1g_short_beacon, NULL);
 	kfree_rcu(old_beacon, rcu_head);
 	if (old_probe_resp)
 		kfree_rcu(old_probe_resp, rcu_head);
@@ -1697,6 +1778,8 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev,
 		kfree_rcu(old_fils_discovery, rcu_head);
 	if (old_unsol_bcast_probe_resp)
 		kfree_rcu(old_unsol_bcast_probe_resp, rcu_head);
+	if (old_s1g_short_beacon)
+		kfree_rcu(old_s1g_short_beacon, rcu_head);
 
 	kfree(link_conf->ftmr_params);
 	link_conf->ftmr_params = NULL;
@@ -1718,8 +1801,10 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev,
 	RCU_INIT_POINTER(link_conf->tx_bss_conf, NULL);
 
 	link_conf->enable_beacon = false;
+	link_conf->s1g_short_beaconing = false;
 	sdata->beacon_rate_set = false;
 	sdata->vif.cfg.ssid_len = 0;
+	sdata->vif.cfg.s1g = false;
 	clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED, &sdata->state);
 	ieee80211_link_info_change_notify(sdata, link,
 					  BSS_CHANGED_BEACON_ENABLED);
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ec68204fddc9..47b51c7eb09c 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -296,6 +296,14 @@ struct unsol_bcast_probe_resp_data {
 	u8 data[];
 };
 
+struct s1g_short_beacon_data {
+	struct rcu_head rcu_head;
+	u8 *short_head;
+	u8 *short_tail;
+	int short_head_len;
+	int short_tail_len;
+};
+
 struct ps_data {
 	/* yes, this looks ugly, but guarantees that we can later use
 	 * bitmap_empty :)
@@ -1042,6 +1050,7 @@ struct ieee80211_link_data_ap {
 	struct probe_resp __rcu *probe_resp;
 	struct fils_discovery_data __rcu *fils_discovery;
 	struct unsol_bcast_probe_resp_data __rcu *unsol_bcast_probe_resp;
+	struct s1g_short_beacon_data __rcu *s1g_short_beacon;
 
 	/* to be used after channel switch. */
 	struct cfg80211_beacon_data *next_beacon;
-- 
2.43.0

[wireless-next v2 3/4] wifi: mac80211: support initialising current short beacon index

[Lachlan Hodges]

Introduce the sb_count variable which tracks the number of
beacon intervals until the next long beacon. Similarly to the DTIM
count, initialise the current index into the long beacon period
using the current TSF. Utilise the same TSF value used to initialise
the DTIM count to ensure they are in sync as its common for the long
beacon period and DTIM period to be equal.

Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
v1 -> v2:

- Rely on existing beacon_int and dtim_period values rather then
  selecting the short variants (removed).

---
 net/mac80211/cfg.c            |  8 +++++++-
 net/mac80211/debugfs_netdev.c |  2 +-
 net/mac80211/ieee80211_i.h    |  6 +++---
 net/mac80211/mesh.c           |  2 +-
 net/mac80211/util.c           | 31 ++++++++++++++++++++++++++++---
 5 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index aeceac5747b6..8fb73952bf39 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1395,6 +1395,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 	struct ieee80211_link_data *link;
 	struct ieee80211_bss_conf *link_conf;
 	struct ieee80211_chan_req chanreq = { .oper = params->chandef };
+	u64 tsf;
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
@@ -1624,7 +1625,12 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 		goto error;
 	}
 
-	ieee80211_recalc_dtim(local, sdata);
+	tsf = drv_get_tsf(local, sdata);
+	ieee80211_recalc_dtim(sdata, tsf);
+
+	if (link_conf->s1g_short_beaconing)
+		ieee80211_recalc_sb_count(sdata, tsf);
+
 	ieee80211_vif_cfg_change_notify(sdata, BSS_CHANGED_SSID);
 	ieee80211_link_info_change_notify(sdata, link, changed);
 
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index 54c479910d05..1dac78271045 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -704,7 +704,7 @@ static ssize_t ieee80211_if_parse_tsf(
 		}
 	}
 
-	ieee80211_recalc_dtim(local, sdata);
+	ieee80211_recalc_dtim(sdata, drv_get_tsf(local, sdata));
 	return buflen;
 }
 IEEE80211_IF_FILE_RW(tsf);
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 47b51c7eb09c..297ae35a1e79 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -314,6 +314,7 @@ struct ps_data {
 	atomic_t num_sta_ps; /* number of stations in PS mode */
 	int dtim_count;
 	bool dtim_bc_mc;
+	int sb_count; /* num short beacons til next long beacon */
 };
 
 struct ieee80211_if_ap {
@@ -2740,9 +2741,8 @@ void ieee80211_dfs_radar_detected_work(struct wiphy *wiphy,
 				       struct wiphy_work *work);
 int ieee80211_send_action_csa(struct ieee80211_sub_if_data *sdata,
 			      struct cfg80211_csa_settings *csa_settings);
-
-void ieee80211_recalc_dtim(struct ieee80211_local *local,
-			   struct ieee80211_sub_if_data *sdata);
+void ieee80211_recalc_sb_count(struct ieee80211_sub_if_data *sdata, u64 tsf);
+void ieee80211_recalc_dtim(struct ieee80211_sub_if_data *sdata, u64 tsf);
 int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 				 const struct cfg80211_chan_def *chandef,
 				 enum ieee80211_chanctx_mode chanmode,
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index d00d9d413c5c..a4a715f6f1c3 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1202,7 +1202,7 @@ int ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 		return -ENOMEM;
 	}
 
-	ieee80211_recalc_dtim(local, sdata);
+	ieee80211_recalc_dtim(sdata, drv_get_tsf(local, sdata));
 	ieee80211_link_info_change_notify(sdata, &sdata->deflink, changed);
 
 	netif_carrier_on(sdata->dev);
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 0d85a382746f..d5dbfd37dd85 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -3913,10 +3913,8 @@ int ieee80211_parse_p2p_noa(const struct ieee80211_p2p_noa_attr *attr,
 }
 EXPORT_SYMBOL(ieee80211_parse_p2p_noa);
 
-void ieee80211_recalc_dtim(struct ieee80211_local *local,
-			   struct ieee80211_sub_if_data *sdata)
+void ieee80211_recalc_dtim(struct ieee80211_sub_if_data *sdata, u64 tsf)
 {
-	u64 tsf = drv_get_tsf(local, sdata);
 	u64 dtim_count = 0;
 	u32 beacon_int = sdata->vif.bss_conf.beacon_int * 1024;
 	u8 dtim_period = sdata->vif.bss_conf.dtim_period;
@@ -3954,6 +3952,33 @@ void ieee80211_recalc_dtim(struct ieee80211_local *local,
 	ps->dtim_count = dtim_count;
 }
 
+/*
+ * Given a long beacon period, calculate the current index into
+ * that period to determine the number of TSBTTs until the next TBTT.
+ * It is completely valid to have a short beacon period that differs
+ * from the dtim period (i.e a TBTT thats not a DTIM).
+ */
+void ieee80211_recalc_sb_count(struct ieee80211_sub_if_data *sdata, u64 tsf)
+{
+	u32 sb_idx;
+	struct ps_data *ps = &sdata->bss->ps;
+	u16 lb_period = sdata->vif.bss_conf.s1g_long_beacon_period;
+	u32 beacon_int = sdata->vif.bss_conf.beacon_int * 1024;
+
+	/* No mesh / IBSS support for short beaconing */
+	if (tsf == -1ULL || !lb_period ||
+	    (sdata->vif.type != NL80211_IFTYPE_AP &&
+	     sdata->vif.type != NL80211_IFTYPE_AP_VLAN))
+		return;
+
+	/* find the current TSBTT index in our lb_period */
+	do_div(tsf, beacon_int);
+	sb_idx = do_div(tsf, lb_period);
+
+	/* num TSBTTs until the next TBTT */
+	ps->sb_count = sb_idx ? lb_period - sb_idx : 0;
+}
+
 static u8 ieee80211_chanctx_radar_detect(struct ieee80211_local *local,
 					 struct ieee80211_chanctx *ctx)
 {
-- 
2.43.0

[wireless-next v2 4/4] wifi: mac80211: support returning the S1G short beacon skb

[Lachlan Hodges]

When short beaconing is enabled, check the value of the sb_count
to determine whether we are to send a long beacon or short beacon.
sb_count represents the number of short beacons until the next
long beacon, where if its value is 0 we are to send a long beacon.
The value is then reset to the long beacon period, which represents
the number of beacon intervals between each long beacon. The decrement
process follows the same cadence as the decrement of the DTIM count value.

Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
v1 -> v2:

- short_beacon_period -> long_beacon_period

---
 net/mac80211/tx.c | 104 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 93 insertions(+), 11 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 6fa883a9250d..d59cf87c6d96 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -5290,14 +5290,14 @@ ieee80211_beacon_add_mbssid(struct sk_buff *skb, struct beacon_data *beacon,
 }
 
 static struct sk_buff *
-ieee80211_beacon_get_ap(struct ieee80211_hw *hw,
-			struct ieee80211_vif *vif,
-			struct ieee80211_link_data *link,
-			struct ieee80211_mutable_offsets *offs,
-			bool is_template,
-			struct beacon_data *beacon,
-			struct ieee80211_chanctx_conf *chanctx_conf,
-			u8 ema_index)
+__ieee80211_beacon_get_ap(struct ieee80211_hw *hw,
+			  struct ieee80211_vif *vif,
+			  struct ieee80211_link_data *link,
+			  struct ieee80211_mutable_offsets *offs,
+			  bool is_template,
+			  struct beacon_data *beacon,
+			  struct ieee80211_chanctx_conf *chanctx_conf,
+			  u8 ema_index)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
 	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
@@ -5358,6 +5358,80 @@ ieee80211_beacon_get_ap(struct ieee80211_hw *hw,
 	return skb;
 }
 
+static bool ieee80211_s1g_need_long_beacon(struct ieee80211_sub_if_data *sdata,
+					   struct ieee80211_link_data *link)
+{
+	struct ps_data *ps = &sdata->u.ap.ps;
+
+	if (ps->sb_count == 0)
+		ps->sb_count = link->conf->s1g_long_beacon_period - 1;
+	else
+		ps->sb_count--;
+
+	return ps->sb_count == 0;
+}
+
+static struct sk_buff *
+ieee80211_s1g_short_beacon_get(struct ieee80211_hw *hw,
+			       struct ieee80211_vif *vif,
+			       struct ieee80211_link_data *link,
+			       struct ieee80211_chanctx_conf *chanctx_conf,
+			       struct s1g_short_beacon_data *sb,
+			       bool is_template)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+	struct ieee80211_if_ap *ap = &sdata->u.ap;
+	struct sk_buff *skb;
+
+	skb = dev_alloc_skb(local->tx_headroom + sb->short_head_len +
+			    sb->short_tail_len + 256 +
+			    local->hw.extra_beacon_tailroom);
+	if (!skb)
+		return NULL;
+
+	skb_reserve(skb, local->tx_headroom);
+	skb_put_data(skb, sb->short_head, sb->short_head_len);
+
+	ieee80211_beacon_add_tim(sdata, link, &ap->ps, skb, is_template);
+
+	if (sb->short_tail)
+		skb_put_data(skb, sb->short_tail, sb->short_tail_len);
+
+	ieee80211_beacon_get_finish(hw, vif, link, NULL, NULL, skb,
+				    chanctx_conf, 0);
+	return skb;
+}
+
+static struct sk_buff *
+ieee80211_beacon_get_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+			struct ieee80211_link_data *link,
+			struct ieee80211_mutable_offsets *offs,
+			bool is_template, struct beacon_data *beacon,
+			struct ieee80211_chanctx_conf *chanctx_conf,
+			u8 ema_index, struct s1g_short_beacon_data *s1g_sb)
+{
+	struct sk_buff *skb = NULL;
+	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+
+	if (!vif->cfg.s1g || !link->conf->s1g_short_beaconing || !s1g_sb)
+		return __ieee80211_beacon_get_ap(hw, vif, link, offs,
+						 is_template, beacon,
+						 chanctx_conf, ema_index);
+
+	if (ieee80211_s1g_need_long_beacon(sdata, link)) {
+		skb = __ieee80211_beacon_get_ap(hw, vif, link, offs,
+						is_template, beacon,
+						chanctx_conf, ema_index);
+	} else {
+		skb = ieee80211_s1g_short_beacon_get(hw, vif, link,
+						     chanctx_conf, s1g_sb,
+						     is_template);
+	}
+
+	return skb;
+}
+
 static struct ieee80211_ema_beacons *
 ieee80211_beacon_get_ap_ema_list(struct ieee80211_hw *hw,
 				 struct ieee80211_vif *vif,
@@ -5381,7 +5455,7 @@ ieee80211_beacon_get_ap_ema_list(struct ieee80211_hw *hw,
 			ieee80211_beacon_get_ap(hw, vif, link,
 						&ema->bcn[ema->cnt].offs,
 						is_template, beacon,
-						chanctx_conf, ema->cnt);
+						chanctx_conf, ema->cnt, NULL);
 		if (!ema->bcn[ema->cnt].skb)
 			break;
 	}
@@ -5410,6 +5484,7 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw,
 	struct ieee80211_sub_if_data *sdata = NULL;
 	struct ieee80211_chanctx_conf *chanctx_conf;
 	struct ieee80211_link_data *link;
+	struct s1g_short_beacon_data *s1g_short_bcn = NULL;
 
 	rcu_read_lock();
 
@@ -5431,6 +5506,13 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw,
 		if (!beacon)
 			goto out;
 
+		if (vif->cfg.s1g && link->conf->s1g_short_beaconing) {
+			s1g_short_bcn =
+				rcu_dereference(link->u.ap.s1g_short_beacon);
+			if (!s1g_short_bcn)
+				goto out;
+		}
+
 		if (ema_beacons) {
 			*ema_beacons =
 				ieee80211_beacon_get_ap_ema_list(hw, vif, link,
@@ -5451,8 +5533,8 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw,
 
 			skb = ieee80211_beacon_get_ap(hw, vif, link, offs,
 						      is_template, beacon,
-						      chanctx_conf,
-						      ema_index);
+						      chanctx_conf, ema_index,
+						      s1g_short_bcn);
 		}
 	} else if (sdata->vif.type == NL80211_IFTYPE_ADHOC) {
 		struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
-- 
2.43.0

Re: [wireless-next v2 2/4] wifi: mac80211: support initialising an S1G short beaconing BSS

[Johannes Berg]

On Wed, 2025-07-16 at 15:32 +1000, Lachlan Hodges wrote:
> 
> +++ b/include/net/mac80211.h
> @@ -365,6 +365,7 @@ struct ieee80211_vif_chanctx_switch {
>   * @BSS_CHANGED_MLD_VALID_LINKS: MLD valid links status changed.
>   * @BSS_CHANGED_MLD_TTLM: negotiated TID to link mapping was changed
>   * @BSS_CHANGED_TPE: transmit power envelope changed
> + * @BSS_CHANGED_S1G_SHORT_BEACON: S1G short beacon changed
>   */
>  enum ieee80211_bss_change {
>  	BSS_CHANGED_ASSOC		= 1<<0,
> @@ -402,6 +403,7 @@ enum ieee80211_bss_change {
>  	BSS_CHANGED_MLD_VALID_LINKS	= BIT_ULL(33),
>  	BSS_CHANGED_MLD_TTLM		= BIT_ULL(34),
>  	BSS_CHANGED_TPE			= BIT_ULL(35),
> +	BSS_CHANGED_S1G_SHORT_BEACON	= BIT_ULL(36),

I feel like at the moment the driver has no way of using this, is it
even needed? It's (currently) designed to really only work with drivers
that pull each individual beacon, since you have no "get short beacon
template" and such.

> + * @s1g_short_beaconing: determines if short beaconing is enabled for an S1G
> + *	BSS.
> + * @s1g_long_beacon_period: number of beacon intervals between each long
> + *	beacon transmission.
>   */
>  struct ieee80211_bss_conf {
>  	struct ieee80211_vif *vif;
> @@ -857,6 +863,9 @@ struct ieee80211_bss_conf {
>  
>  	u8 bss_param_ch_cnt;
>  	u8 bss_param_ch_cnt_link_id;
> +
> +	bool s1g_short_beaconing;
> +	u8 s1g_long_beacon_period;
>  };

Given that, should these even be visible to the driver?

And is s1g_short_beaconing needed at all, I don't see it ever being
read? Almost feels like it shouldn't - could get out of date vs. the
link->u.ap.s1g_short_beacon pointer, which internally indicates the
same.


> +static int
> +ieee80211_set_s1g_short_beacon(struct ieee80211_sub_if_data *sdata,
> +			       struct cfg80211_s1g_short_beacon *params,
> +			       struct ieee80211_link_data *link,
> +			       struct ieee80211_bss_conf *link_conf,
> +			       u64 *changed)
> +{
> +	struct s1g_short_beacon_data *new, *old;
> +	int new_head_len, new_tail_len, size;
> +
> +	if (!params->update)
> +		return 0;
> +
> +	old = sdata_dereference(link->u.ap.s1g_short_beacon, sdata);
> +	if (!params->short_head && !old)
> +		return -EINVAL;

Not sure I understand this logic. If there's no update, it returns
anyway.

This should probably be on the cfg80211 patch, but now that I'm writing
here ... If there is no new short beacon update cannot currently be set
to true, I think? And also, right now by the policy you can't set the
long_beacon_interval == 1 from userspace, but what if you actively want
to _remove_ the short beacon entirely?

Maybe that's not legal? Or maybe it should be allowed, and then I think
the easiest way of achieving it would be to allow the long beacon
interval to be set to 1, which effectively removes the short beacon?

Either way - I'm not sure I follow the "!new && !old" part here since
you can't actually have params->update && !params->head right now? And
even if you could, I'm not sure what the old matters.

> +	new_head_len = params->short_head ? params->short_head_len :
> +					    old->short_head_len;
> +	new_tail_len = (params->short_tail || !old) ? params->short_tail_len :
> +						      old->short_tail_len;

This seems similarly odd to me, if you set a new short head but no short
tail you surely don't want to reuse the old short tail?

Seems to me really this should never use "old" at all, since you want to
update everything (or even remove the short beacon if that's allowed per
above), but never mix things?

And if update is false nothing ever happens here anyway.

> +	if (old)
> +		kfree_rcu(old, rcu_head);

Though of course this is still needed.

johannes

Re: [wireless-next v2 2/4] wifi: mac80211: support initialising an S1G short beaconing BSS

[Lachlan Hodges]

On Wed, Jul 16, 2025 at 09:55:58AM +0200, Johannes Berg wrote:
> On Wed, 2025-07-16 at 15:32 +1000, Lachlan Hodges wrote:
> > 
> > +++ b/include/net/mac80211.h
> > @@ -365,6 +365,7 @@ struct ieee80211_vif_chanctx_switch {
> >   * @BSS_CHANGED_MLD_VALID_LINKS: MLD valid links status changed.
> >   * @BSS_CHANGED_MLD_TTLM: negotiated TID to link mapping was changed
> >   * @BSS_CHANGED_TPE: transmit power envelope changed
> > + * @BSS_CHANGED_S1G_SHORT_BEACON: S1G short beacon changed
> >   */
> >  enum ieee80211_bss_change {
> >  	BSS_CHANGED_ASSOC		= 1<<0,
> > @@ -402,6 +403,7 @@ enum ieee80211_bss_change {
> >  	BSS_CHANGED_MLD_VALID_LINKS	= BIT_ULL(33),
> >  	BSS_CHANGED_MLD_TTLM		= BIT_ULL(34),
> >  	BSS_CHANGED_TPE			= BIT_ULL(35),
> > +	BSS_CHANGED_S1G_SHORT_BEACON	= BIT_ULL(36),
> 
> I feel like at the moment the driver has no way of using this, is it
> even needed? It's (currently) designed to really only work with drivers
> that pull each individual beacon, since you have no "get short beacon
> template" and such.
> 

This is correct and an oversight on my part. Looking at our driver we
retrieve the beacon each time, so theres no reason to be notified for
a short beacon update. Will fix this.

> > + * @s1g_short_beaconing: determines if short beaconing is enabled for an S1G
> > + *	BSS.
> > + * @s1g_long_beacon_period: number of beacon intervals between each long
> > + *	beacon transmission.
> >   */
> >  struct ieee80211_bss_conf {
> >  	struct ieee80211_vif *vif;
> > @@ -857,6 +863,9 @@ struct ieee80211_bss_conf {
> >  
> >  	u8 bss_param_ch_cnt;
> >  	u8 bss_param_ch_cnt_link_id;
> > +
> > +	bool s1g_short_beaconing;
> > +	u8 s1g_long_beacon_period;
> >  };
> 
> Given that, should these even be visible to the driver?
> 
> And is s1g_short_beaconing needed at all, I don't see it ever being
> read? Almost feels like it shouldn't - could get out of date vs. the
> link->u.ap.s1g_short_beacon pointer, which internally indicates the
> same.
> 

An interface can't be modified such that we can disable short beaconing
without tearing it down. This is how I've reworked the initial
configuration within the first 2 patches. As for whether it needs to be
exposed to the driver, it does not. I kept it included as I find it
easier to read then validaing against the link->u.ap.s1g_short_beacon
pointer though I have no opposition to doing what you suggest (though
maybe in an struct not exposed to the driver). I suppose since this is
only checked twice (once here, and once when we check if we need to return
a short beacon SKB) we can just check the link->u.ap.s1g_short_beacon.

> 
> > +static int
> > +ieee80211_set_s1g_short_beacon(struct ieee80211_sub_if_data *sdata,
> > +			       struct cfg80211_s1g_short_beacon *params,
> > +			       struct ieee80211_link_data *link,
> > +			       struct ieee80211_bss_conf *link_conf,
> > +			       u64 *changed)
> > +{
> > +	struct s1g_short_beacon_data *new, *old;
> > +	int new_head_len, new_tail_len, size;
> > +
> > +	if (!params->update)
> > +		return 0;
> > +
> > +	old = sdata_dereference(link->u.ap.s1g_short_beacon, sdata);
> > +	if (!params->short_head && !old)
> > +		return -EINVAL;
> 
> Not sure I understand this logic. If there's no update, it returns
> anyway.
> 
> This should probably be on the cfg80211 patch, but now that I'm writing
> here ... If there is no new short beacon update cannot currently be set
> to true, I think? And also, right now by the policy you can't set the
> long_beacon_interval == 1 from userspace, but what if you actively want
> to _remove_ the short beacon entirely?
> 

I initially did think so that this should be in a cfg80211 targetted
patch but since cfg.c since within mac80211 I did such. Can do either.

> Maybe that's not legal? Or maybe it should be allowed, and then I think
> the easiest way of achieving it would be to allow the long beacon
> interval to be set to 1, which effectively removes the short beacon?
> 
> Either way - I'm not sure I follow the "!new && !old" part here since
> you can't actually have params->update && !params->head right now? And
> even if you could, I'm not sure what the old matters.
> 

So you are correct, It's not legal to disable short beaconing without
tearing the interface down (as mentioned above). Thats why within
ieee80211_change_beacon() we check if we are short beaconing first
and if theres an update proceed with said update. This function should
really just be setting the new pointers and discarding the old pointers
if they exist i.e during an update. 

> > +	new_head_len = params->short_head ? params->short_head_len :
> > +					    old->short_head_len;
> > +	new_tail_len = (params->short_tail || !old) ? params->short_tail_len :
> > +						      old->short_tail_len;
> 
> This seems similarly odd to me, if you set a new short head but no short
> tail you surely don't want to reuse the old short tail?
> 
> Seems to me really this should never use "old" at all, since you want to
> update everything (or even remove the short beacon if that's allowed per
> above), but never mix things?
> 
> And if update is false nothing ever happens here anyway.
>

Yea.. this seems to be a case of me stealing the beacon change code...
:). We essentially just want to be setting the new pointers, freeing
the old and that is it. A quick rewrite to something like this:

static int
ieee80211_set_s1g_short_beacon(struct ieee80211_sub_if_data *sdata,
			       struct cfg80211_s1g_short_beacon *params,
			       struct ieee80211_link_data *link)
{
	struct s1g_short_beacon_data *new;
	struct s1g_short_beacon_data *old =
		sdata_dereference(link->u.ap.s1g_short_beacon, sdata);
	size_t new_len =
		sizeof(*new) + params->short_head_len + params->short_tail_len;

	if (!params->update)
		return 0;

	if (!params->short_head)
		return -EINVAL;

	new = kzalloc(new_len, GFP_KERNEL);
	if (!new)
		return -ENOMEM;

	/* Memory layout: | struct | head | tail | */
	new->short_head = ((u8 *)new) + sizeof(*new);
	new->short_head_len = params->short_head_len;
	memcpy(new->short_head, params->short_head, params->short_head_len);

	if (params->short_tail) {
		new->short_tail = new->short_head + params->short_head_len;
		new->short_tail_len = params->short_tail_len;
		memcpy(new->short_tail, params->short_tail,
		       params->short_tail_len);
	}

	rcu_assign_pointer(link->u.ap.s1g_short_beacon, new);

	if (old)
		kfree_rcu(old, rcu_head);

	return 0;
}

seems more robust, where we simply update the new beacon given
the parameters and discard the old, else if theres no update
we do nothing.

lachlan

Re: [wireless-next v2 2/4] wifi: mac80211: support initialising an S1G short beaconing BSS

[Johannes Berg]

On Wed, 2025-07-16 at 19:01 +1000, Lachlan Hodges wrote:
> An interface can't be modified such that we can disable short beaconing
> without tearing it down.

OK, that's fair. No objection to that, just wanted to clarify if that
was what you intended.

> > This should probably be on the cfg80211 patch, but now that I'm writing
> > here ... If there is no new short beacon update cannot currently be set
> > to true, I think? And also, right now by the policy you can't set the
> > long_beacon_interval == 1 from userspace, but what if you actively want
> > to _remove_ the short beacon entirely?
> > 
> 
> I initially did think so that this should be in a cfg80211 targetted
> patch but since cfg.c since within mac80211 I did such. Can do either.

Oh, sorry, what I said was confusing. I meant I should be _commenting_
on the cfg80211 patch instead (about the logic of being able to remove
the short beacon or not). But you clarified that above.

> So you are correct, It's not legal to disable short beaconing without
> tearing the interface down (as mentioned above). Thats why within
> ieee80211_change_beacon() we check if we are short beaconing first
> and if theres an update proceed with said update. This function should
> really just be setting the new pointers and discarding the old pointers
> if they exist i.e during an update. 

Right, makes sense.

> Yea.. this seems to be a case of me stealing the beacon change code...
> :).

OK, I don't know what that code does off-hand, maybe there we have some
other things that don't always need to change.

> static int
> ieee80211_set_s1g_short_beacon(struct ieee80211_sub_if_data *sdata,
> 			       struct cfg80211_s1g_short_beacon *params,
> 			       struct ieee80211_link_data *link)
> {
...

> 	/* Memory layout: | struct | head | tail | */
> 	new->short_head = ((u8 *)new) + sizeof(*new);

You probably don't need the extra () around "(u8 *)new".

> seems more robust, where we simply update the new beacon given
> the parameters and discard the old, else if theres no update
> we do nothing.

Yeah, that looks good to me.

johannes