[RFC 0/5] wifi: S1G short beacon support

[RFC 0/5] wifi: S1G short beacon support

[Lachlan Hodges]

Short beaconing is a feature within S1G that allows beacons
to contain a limited set of elements, significantly decreasing
beacon size. This set of patches adds support for short beaconing
within mac80211 and cfg80211 aswell as mac80211_hwsim.

Preface:

Previously, some work was done ~2 years ago to get short beaconing
in the kernel but it was never successful. The patches can be found
here:

1/2: https://patchwork.kernel.org/project/linux-wireless/patch/20230810093556.33800-1-bassem@morsemicro.com/
2/2: https://patchwork.kernel.org/project/linux-wireless/patch/20230810093556.33800-2-bassem@morsemicro.com/

As a result of this, we have independently rewritten the patchset
based on feedback from the last attempt.

Implementation:

Note that all standards exerts reference below are from
IEEE80211-2024.

(1) S1G long beacons:

What makes S1G long beacons "weird" is that functionally, they
are the same as regular beacons - however they are defined as
a type of extension frame in the standard. While they do slightly
differ in terms of how they are parsed due to variable length
optional elements, as of now the way they are treated by mac80211
is equivalent to regular beacons.

This leads to a decision whether we build out some new infrastructure
to parse and store S1G long beacons separately or use the existing
beacon path (from cfg80211 all the way to beacon retrieval). We have
opted to use the existing code paths as there is already support for
parsing S1G beacons in the validate_beacon_head path and no changes
are required for S1G long beacons as functionally they are equivalent
to regular beacons.

There is something to say regarding whether this is "correct" as
after all they are extension frames and while we feel this is the best
approach when all factors are taken into account - ultimately it is
up to maintainers to decide.

(2) S1G short beacons:

Short beacons on the other hand are indeed functionally different
to long beacons alongside being optional. Primarily related to the
fact that the set of elements available to them is different to long
beacons and thus separate validation is required. We have based
the implementation on Alokas FILS Discovery patchset:

https://lore.kernel.org/linux-wireless/1912863dcd17aa50b09d1ddfc889478eb323f901.camel@sipsolutions.net/T/#m86511f184d40ab36221f4ceae066900233ceb84e

We have created a new nested attribute specifically for the short
beacon data alongside the various BSS parameters needed. This comes
with new validation policies for both the beacon elements and
the beacon head to ensure only elements permitted in a short beacon
are used. In addition, we also prevent short beacons from being
sent down the regular beacon path and vice versa.

We then introduce cfg80211_s1g_short_beacon and s1g_short_beacon_data
which store the related short beacon data, where the former is used
on AP bring up and to initialise the BSS related information and
the latter used when retrieving a beacon. This keeps the optional
short beacon related data separate from the beacon_data type which
makes sense as it is optional, aswell as introduces it into the
cfg80211_ap_update structure for beacon updates such as an SSID
change which may change the short beacon template if it contains
a CSSID field.

(3) S1G BSS:

When an S1G BSS is initialised with short beaconing enabled, the
standard dictates a few things:

9.4.2.5.1:

"If dot11ShortBeaconInterval is equal to true, the DTIM Period field
is set to dot11ShortBeaconDTIMPeriod. If dot11ShortBeaconInterval is
equal to false, the DTIM Period field is set to dot11DTIMPeriod"

11.1.2.1:

"In an S1G BSS, the S1G AP shall generate S1G Beacon frames every
dot11BeaconPeriod TUs; and if dot11ShortBeaconInterval is true, it
shall additionally generate S1G Beacon frames every
dot11ShortBeaconPeriod TUs"

Its important to note that dot11ShortBeaconInterval is a boolean
that states whether short beaconing is enabled or not, and
dot11ShortBeaconPeriod is the interval between each short beacon
transmission (TSBTT). We have opted to use the notion of "short beacon
interval" to represent the time in TUs between each TSBTT as
the kernel currently uses beacon_int to determine the time between
each TBTT, so using s1g_short_beacon_int seems appropriate here.

From the standard exerts above, it infers that when we are short
beaconing, decisions made regarding the DTIM period and beacon
transmission times use TSBTT's rather then TBTT's - so make sure
any calculations when short beaconing is enabled that involve
the DTIM period or beacon interval use the equivalent s1g_short_xyz
parameters.

In addition to this, we now need to know whether we need to send
a short beacon or a long beacon. From the standard it states:

11.1.3.10.2:

"The value for the dot11ShortBeaconPeriod shall be such that
dot11BeaconPeriod = n * dot11ShortBeaconPeriod, where n is a
positive integer. This defines a series of TSBTTs exactly
dot11ShortBeaconPeriod TUs apart"

the value for n here is what we are denoting as
s1g_short_beacon_period (another deviation from the naming
within the standard) which represents the number of short
beacons between each long beacon. To keep track of the current
state, we introduce a new parameter sb_count within the
struct ps_data structure to track the current index into this
period. This is what is used to determine whether we send a
long or short beacon on beacon retrieval where a value of 0
indicates a long beacon (following the same cadence of
decrementing the DTIM count).

We have opted to place it in the struct ps_data since,
fundamentally, it is a power save mechanism and is initialised
using the same TSF value the DTIM count is initialised with
as its very common for the short beacon DTIM period and
short beacon period to be equal - so this ensures we
initialise these values in sync.

(4) Beacon retrieval

There were essentially two options we could take with regards to
retrieving the beacons:

1.implement it in the traditional beacon path via
  ieee80211_beacon_get_ap

2.Implement an S1G (or maybe extension frame?) specific path
  such as ieee80211_s1g_beacon_get_ap where the S1G specific
  handling can be done in its own function, leaving the regular
  beacon path untouched

We have opted to take method (1) but this really comes down to
maintainers preference. New conditionals will be introduced
in the beacon path regardless and we feel this is the best
approach but are open to feedback.

(5) Testing

This patchset has been tested on the following configurations:

1. Multi-sta setup with real Morse Micro hardware.
2. S1G hwsim configuration with a single AP and > 10 STAs
3. 2.4GHz hwsim configuration to test for regressions along the
   non-s1g path consisting of a single AP and STA.
4. hostapd hwsim tests were run to ensure no regression for
   regular 2.4/5/6 radios.

(6) Other notes:

1. The update mechanism is not really that nice. Though that may
   stem from a misunderstanding with how FILS discovery and unsol
   bcast update mechanisms work. Since short beacons rely on a BSS
   to be configured we ensure that an update is only performed
   when this is the case, and when the BSS has not been configured
   we do not allow a set of parameters used for updating the beacon
   to be used when the BSS has not been initialised. Would like some
   feedback on our implementation here as Im not really convinved
   it's the right way, but it seems this is how we currently handle
   it. My understanding of an "update" is that we are updating an
   existing interface. When an interface is being initialised, we
   are not updating - we are setting... An option here is to just
   not even allow updates since this is how S1G is done now and this
   can be ammended in the future.

2. The power save patch (patch #3) is a bit ugly to split, so have
   kept as one as they are fundamentally related.

Lachlan Hodges (5):
  wifi: cfg80211: support configuring an S1G short beaconing BSS
  wifi: mac80211: support initialising an S1G short beaconing BSS
  wifi: mac80211: configure power save for an S1G short beaconing BSS
  wifi: mac80211: support returning the S1G short beacon skb
  wifi: mac80211_hwsim: add S1G short beacon support

 drivers/net/wireless/virtual/mac80211_hwsim.c | 109 ++++++---
 include/linux/ieee80211.h                     |   4 +
 include/net/cfg80211.h                        |  32 +++
 include/net/mac80211.h                        |  17 ++
 include/uapi/linux/nl80211.h                  |  32 +++
 net/mac80211/cfg.c                            | 132 ++++++++++-
 net/mac80211/debugfs_netdev.c                 |   2 +-
 net/mac80211/ieee80211_i.h                    |  15 +-
 net/mac80211/mesh.c                           |   2 +-
 net/mac80211/tx.c                             | 115 ++++++++--
 net/mac80211/util.c                           |  50 ++++-
 net/wireless/nl80211.c                        | 208 ++++++++++++++++++
 12 files changed, 661 insertions(+), 57 deletions(-)

-- 
2.43.0

[RFC 1/5] 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.

When an S1G BSS is configured with short beaconing enabled, the
BSS uses the TSBTT (Target Short Beacon Transmission Time) as the
base unit for the DTIM period and the beacon interval as per
IEEE80211-2024 11.1.2.1:

"In an S1G BSS, the S1G AP shall generate S1G Beacon frames
every dot11BeaconPeriod TUs; and if dot11ShortBeaconInterval is
true, it shall additionally generate S1G Beacon frames every
dot11ShortBeaconPeriod TUs"

Note: As the kernel uses the notion of beacon interval rather then
beacon period, we do the same with short beaconing - where
dot11ShortBeaconPeriod is known as the short beacon interval.

and per IEEE80211-2024 Annex C.3 MIB detail:

dot11ShortBeaconPeriod:

This attribute specifies the time period that a STA uses for
scheduling S1G Beacon transmissions in a TSBTT that is not a TBTT.

dot11ShortBeaconDTIMPeriod:

This attribute specifies the number of short beacon intervals that
elapse between transmission of S1G Beacon frames containing a TIM
element whose DTIM Count field is 0.

Expose 2 additional attributes alongside the short beacon frame data,
short_interval which is the time in TUs between each TSBTT and the
short_dtim_period which is the number of TSBTTs between each DTIM
beacon. Additionally, validate the elements present in the short
beacon as per IEEE80211-2024 9.3.4.3 Table 9-76 which describes
the elements permitted within a short beacon. Introduce these new
attributes as an optional netlink attribute to allow an S1G BSS
to be configured for short beaconing.

Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
 include/linux/ieee80211.h    |   4 +
 include/net/cfg80211.h       |  32 ++++++
 include/uapi/linux/nl80211.h |  32 ++++++
 net/wireless/nl80211.c       | 208 +++++++++++++++++++++++++++++++++++
 4 files changed, 276 insertions(+)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index e5a2096e022e..16e76cbf6bd6 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -3823,14 +3823,18 @@ enum ieee80211_eid {
 
 	WLAN_EID_REDUCED_NEIGHBOR_REPORT = 201,
 
+	WLAN_EID_RPS = 208,
+	WLAN_EID_PAGE_SLICE = 209,
 	WLAN_EID_AID_REQUEST = 210,
 	WLAN_EID_AID_RESPONSE = 211,
 	WLAN_EID_S1G_BCN_COMPAT = 213,
 	WLAN_EID_S1G_SHORT_BCN_INTERVAL = 214,
 	WLAN_EID_S1G_TWT = 216,
 	WLAN_EID_S1G_CAPABILITIES = 217,
+	WLAN_EID_SST = 220,
 	WLAN_EID_VENDOR_SPECIFIC = 221,
 	WLAN_EID_QOS_PARAMETER = 222,
+	WLAN_EID_S1G_RELAY = 224,
 	WLAN_EID_S1G_OPERATION = 232,
 	WLAN_EID_CAG_NUMBER = 237,
 	WLAN_EID_AP_CSN = 239,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 6ec9a8865b8b..c7c9a297753b 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1423,6 +1423,34 @@ struct cfg80211_unsol_bcast_probe_resp {
 	const u8 *tmpl;
 };
 
+/**
+ * struct cfg80211_s1g_short_beacon - S1G short beacon data.
+ *
+ * @update: Set to true if the beacon head/tail has been updated. In this
+ *	case the BSS has already been configured to be short beaconing.
+ * @short_head: Short beacon head.
+ * @short_tail: Short beacon tail.
+ * @short_head_len: Short beacon head len.
+ * @short_tail_len: Short beacon tail len.
+ * @short_interval: Time in TU between each TSBTT (Target Short Beacon
+ *	Transmission Time). When short beaconing is enabled for an S1G
+ *	BSS, this value will be used to determine beacon transmission
+ *	times for both long and short beacons.
+ * @short_dtim_period: Specifies the number of TSBTTs that elapse between
+ *	transmission of S1G beacon frames containing a TIM element whose
+ *	DTIM count field is 0. When short beaconing is enabled for an S1G
+ *	BSS, this value is used to initialise the current DTIM count.
+ */
+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;
+	u32 short_interval;
+	u8 short_dtim_period;
+};
+
 /**
  * struct cfg80211_ap_settings - AP configuration
  *
@@ -1463,6 +1491,7 @@ 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_short_beacon: S1G short beacon data
  */
 struct cfg80211_ap_settings {
 	struct cfg80211_chan_def chandef;
@@ -1496,6 +1525,7 @@ 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;
+	struct cfg80211_s1g_short_beacon s1g_short_beacon;
 };
 
 
@@ -1507,11 +1537,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..ccfc5ab1a42f 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2915,6 +2915,10 @@ 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_SHORT_BEACON: Optional parameter to configure an S1G BSS
+ *	with short beaconing support. It is a nested attribute, see
+ *	@enum nl80211_s1g_short_beacon_attrs.
+ *
  * @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 +3478,8 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_WIPHY_RADIO_INDEX,
 
+	NL80211_ATTR_S1G_SHORT_BEACON,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -8148,4 +8154,30 @@ 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_HEAD: Short beacon head (binary).
+ * @NL80211_S1G_SHORT_BEACON_TAIL: Short beacon tail (binary).
+ * @NL80211_S1G_SHORT_BEACON_INTERVAL: Time in TUs between each short
+ *	beacon transmission (u32).
+ * @NL80211_S1G_SHORT_BEACON_DTIM_PERIOD: DTIM period for a short
+ *	beaconing BSS (u8).
+ */
+enum nl80211_s1g_short_beacon_attrs {
+	__NL80211_S1G_SHORT_BEACON_ATTR_INVALID,
+
+	NL80211_S1G_SHORT_BEACON_HEAD,
+	NL80211_S1G_SHORT_BEACON_TAIL,
+	NL80211_S1G_SHORT_BEACON_INTERVAL,
+	NL80211_S1G_SHORT_BEACON_DTIM_PERIOD,
+
+	/* 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..519f8fe9f686 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -258,6 +258,10 @@ static int validate_beacon_head(const struct nlattr *attr,
 	data += fixedlen;
 	len -= fixedlen;
 
+	if (s1g_bcn &&
+	    ieee80211_is_s1g_short_beacon(ext->frame_control, data, len))
+		goto err;
+
 	for_each_element(elem, data, len) {
 		/* nothing */
 	}
@@ -288,6 +292,92 @@ static int validate_ie_attr(const struct nlattr *attr,
 	return -EINVAL;
 }
 
+/*
+ * Short beacons contain a limited set of allowed elements as per
+ * IEEE80211-2024 9.3.4.3 Table 9-76. The TIM element is allowed,
+ * but as it is inserted by mac80211, we do not check for it.
+ */
+static int is_valid_s1g_short_elem(const struct element *elem)
+{
+	switch (elem->id) {
+	case WLAN_EID_FMS_DESCRIPTOR:
+	case WLAN_EID_RPS:
+	case WLAN_EID_SST:
+	case WLAN_EID_S1G_RELAY:
+	case WLAN_EID_PAGE_SLICE:
+	case WLAN_EID_VENDOR_SPECIFIC:
+	case WLAN_EID_MMIE:
+	case WLAN_EID_MIC:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static int validate_s1g_short_beacon_head(const struct nlattr *attr,
+					  struct netlink_ext_ack *extack)
+{
+	const u8 *data = nla_data(attr);
+	unsigned int len = nla_len(attr);
+	const struct element *elem;
+	unsigned int fixedlen, hdrlen;
+	const struct ieee80211_ext *ext = (void *)data;
+
+	if (len < offsetofend(typeof(*ext), frame_control))
+		goto err;
+
+	if (!ieee80211_is_s1g_beacon(ext->frame_control))
+		goto err;
+
+	fixedlen = offsetof(struct ieee80211_ext, u.s1g_beacon.variable) +
+		   ieee80211_s1g_optional_len(ext->frame_control);
+	hdrlen = offsetof(struct ieee80211_ext, u.s1g_beacon);
+
+	if (len < fixedlen)
+		goto err;
+
+	if (ieee80211_hdrlen(ext->frame_control) != hdrlen)
+		goto err;
+
+	data += fixedlen;
+	len -= fixedlen;
+
+	if (!ieee80211_is_s1g_short_beacon(ext->frame_control, data, len))
+		goto err;
+
+	for_each_element(elem, data, len) {
+		if (!is_valid_s1g_short_elem(elem))
+			goto err;
+	}
+
+	if (for_each_element_completed(elem, data, len))
+		return 0;
+
+err:
+	NL_SET_ERR_MSG_ATTR(extack, attr, "malformed short beacon head");
+	return -EINVAL;
+}
+
+static int validate_s1g_short_beacon_ie_attr(const struct nlattr *attr,
+					     struct netlink_ext_ack *extack)
+{
+	const u8 *data = nla_data(attr);
+	unsigned int len = nla_len(attr);
+	const struct element *elem;
+
+	for_each_element(elem, data, len) {
+		if (!is_valid_s1g_short_elem(elem))
+			goto err;
+	}
+
+	if (for_each_element_completed(elem, data, len))
+		return 0;
+
+err:
+	NL_SET_ERR_MSG_ATTR(extack, attr, "malformed short beacon elements");
+	return -EINVAL;
+}
+
 static int validate_he_capa(const struct nlattr *attr,
 			    struct netlink_ext_ack *extack)
 {
@@ -482,6 +572,20 @@ 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_HEAD] =
+		NLA_POLICY_VALIDATE_FN(NLA_BINARY,
+				       validate_s1g_short_beacon_head,
+				       IEEE80211_MAX_DATA_LEN),
+	[NL80211_S1G_SHORT_BEACON_TAIL] =
+		NLA_POLICY_VALIDATE_FN(NLA_BINARY,
+				       validate_s1g_short_beacon_ie_attr,
+				       IEEE80211_MAX_DATA_LEN),
+	[NL80211_S1G_SHORT_BEACON_INTERVAL] = { .type = NLA_U32 },
+	[NL80211_S1G_SHORT_BEACON_DTIM_PERIOD] = { .type = NLA_U8 },
+};
+
 static const struct netlink_range_validation nl80211_punct_bitmap_range = {
 	.min = 0,
 	.max = 0xffff,
@@ -858,6 +962,8 @@ 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_SHORT_BEACON] =
+		NLA_POLICY_NESTED(nl80211_s1g_short_beacon),
 };
 
 /* policy for the key attributes */
@@ -6202,6 +6308,73 @@ static int nl80211_validate_ap_phy_operation(struct cfg80211_ap_settings *params
 	return 0;
 }
 
+static int nl80211_validate_s1g_short_conf(struct cfg80211_ap_settings *params)
+{
+	struct cfg80211_s1g_short_beacon *sb = &params->s1g_short_beacon;
+	u64 beacon_int = params->beacon_interval;
+	u32 short_int = sb->short_interval;
+
+	/*
+	 * As per IEEE80211 11.1.3.10.2, beacon_interval = n * short_interval
+	 * where n is a positive integer. Meaning a BSS can be configured such
+	 * that both the short beacon interval and long beacon interval are
+	 * equivalent. This effectively means we aren't short beaconing. In
+	 * this case, since the short beacon data is irellevent its probably a
+	 * misconfiguration and we should reject it.
+	 */
+	if (sb->short_interval >= params->beacon_interval)
+		return -EINVAL;
+
+	if (do_div(beacon_int, short_int))
+		return -EINVAL;
+
+	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_HEAD])
+		return -EINVAL;
+
+	sb->update = false;
+	sb->short_head = nla_data(tb[NL80211_S1G_SHORT_BEACON_HEAD]);
+	sb->short_head_len = nla_len(tb[NL80211_S1G_SHORT_BEACON_HEAD]);
+	sb->short_tail_len = 0;
+
+	if (tb[NL80211_S1G_SHORT_BEACON_TAIL]) {
+		sb->short_tail = nla_data(tb[NL80211_S1G_SHORT_BEACON_TAIL]);
+		sb->short_tail_len = nla_len(tb[NL80211_S1G_SHORT_BEACON_TAIL]);
+	}
+
+	if (!tb[NL80211_S1G_SHORT_BEACON_INTERVAL] ||
+	    !tb[NL80211_S1G_SHORT_BEACON_DTIM_PERIOD]) {
+		sb->update = true;
+		return 0;
+	}
+
+	sb->short_interval =
+		nla_get_u32(tb[NL80211_S1G_SHORT_BEACON_INTERVAL]);
+	sb->short_dtim_period =
+		nla_get_u8(tb[NL80211_S1G_SHORT_BEACON_DTIM_PERIOD]);
+
+	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 +6615,28 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 		goto out;
 	}
 
+	if (info->attrs[NL80211_ATTR_S1G_SHORT_BEACON]) {
+		err = nl80211_parse_s1g_short_beacon(
+			rdev, info->attrs[NL80211_ATTR_S1G_SHORT_BEACON],
+			&params->s1g_short_beacon);
+		if (err)
+			goto out;
+
+		/*
+		 * If we have only received the parameters to perform a
+		 * short beacon update, return an error to usermode as
+		 * the BSS has not yet been configured for short beaconing.
+		 */
+		if (params->s1g_short_beacon.update) {
+			err = -EINVAL;
+			goto out;
+		}
+
+		err = nl80211_validate_s1g_short_conf(params);
+		if (err)
+			goto out;
+	}
+
 	err = nl80211_calculate_ap_params(params);
 	if (err)
 		goto out;
@@ -6550,6 +6745,19 @@ 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;
+
+		if (!params->s1g_short_beacon.update) {
+			err = -EINVAL;
+			goto out;
+		}
+	}
+
 	err = rdev_change_beacon(rdev, dev, params);
 
 out:
-- 
2.43.0

[RFC 2/5] wifi: mac80211: support initialising an S1G short beaconing BSS

[Lachlan Hodges]

When an S1G BSS has been configured with short beaconing enabled,
utilise the short beacon DTIM period and short beacon interval as
opposed to the regular DTIM period and beacon interval as per
IEEE80211-2024 9.4.2.5.1 and 11.1.2.1 respectively. These values use
the base unit of TSBTT's (Target Short Beacon Transmission Time)
rather then TBTT. Additionally, introduce a new BSS parameter
s1g_short_beacon_period which represents the number of short beacons
sent per long beacon. This value is calcuated based on the ratio of
TSBTT's per TBTT such that:

short_beacon_period = beacon_int / short_beacon_int

as per IEEE80211-2024 11.1.3.10.2. This value will be used to
initialise the short beacon count every TBTT.

Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
 include/net/mac80211.h     |  17 +++++
 net/mac80211/cfg.c         | 124 +++++++++++++++++++++++++++++++++++++
 net/mac80211/ieee80211_i.h |   9 +++
 3 files changed, 150 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 577fd6a8c372..3782a3d12dff 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,16 @@ 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_short_beacon_int: short beacon interval in TUs. When short beaconing is
+ *	enabled beacon transmission times are computed using this value as
+ *	opposed to beacon_int as per IEEE80211-2024 11.1.2.1.
+ * @s1g_short_beacon_dtim_period: number of short beacon intervals that elapse
+ *	between each beacon with a TIM element whose DTIM count is 0. When
+ *	short beaconing is enabled, this value is used as opposed to
+ *	dtim_period as per IEEE80211-2024 9.4.2.5.1.
+ * @s1g_short_beacon_period: number of short beacons sent per long beacon.
  */
 struct ieee80211_bss_conf {
 	struct ieee80211_vif *vif;
@@ -857,6 +869,11 @@ struct ieee80211_bss_conf {
 
 	u8 bss_param_ch_cnt;
 	u8 bss_param_ch_cnt_link_id;
+
+	bool s1g_short_beaconing;
+	u16 s1g_short_beacon_int;
+	u8 s1g_short_beacon_dtim_period;
+	u16 s1g_short_beacon_period;
 };
 
 /**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index d76643d46150..d955bf8a6654 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 (!sdata->vif.bss_conf.s1g_short_beaconing)
+		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,
@@ -1489,6 +1543,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 
 	link_conf->dtim_period = params->dtim_period;
 	link_conf->enable_beacon = true;
+	link_conf->s1g_short_beaconing = false;
 	link_conf->allow_p2p_go_ps = sdata->vif.p2p;
 	link_conf->twt_responder = params->twt_responder;
 	link_conf->he_obss_pd = params->he_obss_pd;
@@ -1496,6 +1551,41 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 	sdata->vif.cfg.s1g = params->chandef.chan->band ==
 				  NL80211_BAND_S1GHZ;
 
+	if (sdata->vif.cfg.s1g) {
+		u64 beacon_interval = link_conf->beacon_int;
+
+		/*
+		 * If we have only passed in the short beacon head/tail when
+		 * the BSS has not been configured, error out.
+		 */
+		if (params->s1g_short_beacon.update) {
+			err = -EINVAL;
+			goto error;
+		}
+
+		if (params->s1g_short_beacon.short_interval) {
+			/*
+			 * IEEE80211-2024 11.1.3.10.2:
+			 * beacon_int = n * short_beacon_int where n is a
+			 * positive integer and represents the short beacon
+			 * period which is the number of short beacons
+			 * transmitted per long beacon.
+			 */
+			if (do_div(beacon_interval,
+				   params->s1g_short_beacon.short_interval)) {
+				err = -EINVAL;
+				goto error;
+			}
+
+			link_conf->s1g_short_beaconing = true;
+			link_conf->s1g_short_beacon_int =
+				params->s1g_short_beacon.short_interval;
+			link_conf->s1g_short_beacon_dtim_period =
+				params->s1g_short_beacon.short_dtim_period;
+			link_conf->s1g_short_beacon_period = beacon_interval;
+		}
+	}
+
 	sdata->vif.cfg.ssid_len = params->ssid_len;
 	if (params->ssid_len)
 		memcpy(sdata->vif.cfg.ssid, params->ssid,
@@ -1541,6 +1631,14 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 	if (err < 0)
 		goto error;
 
+	if (sdata->vif.cfg.s1g && link_conf->s1g_short_beaconing) {
+		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 +1717,24 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
 	if (err < 0)
 		return err;
 
+	if (sdata->vif.cfg.s1g) {
+		/*
+		 * If we are updating the short beacon and the BSS has not
+		 * been configured for short beaconing, error out.
+		 */
+		if (!link_conf->s1g_short_beaconing &&
+		    params->s1g_short_beacon.update)
+			return -EINVAL;
+
+		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 +1766,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 +1785,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 +1809,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 +1817,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 +1840,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

[RFC 3/5] wifi: mac80211: configure power save for an S1G short beaconing BSS

[Lachlan Hodges]

Configure the short beacon count and DTIM count using the
s1g_short_beacon_int and s1g_short_beacon_dtim_period values
when short beaconing is enabled as per:

(1) IEEE80211-2024 9.4.2.5.1 for the short beacon interval
(2) IEEE80211-2024 11.1.2.1 for the short beacon DTIM period

The short beacon count represents the number of short beacons
that will be sent until the next long beacon. This is stored
in the struct ps_data as it is fundamentally a power save
feature.

In addition, use the short beacon variants rather then the
traditional variants. Also initialise the short beacon count
using the same TSF value to ensure they are in sync at
initilisation. The DTIM count and short beacon count are initialised
separately as it is perfectly valid to have a short beacon period
that differs from the short beacon dtim period.

Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
 net/mac80211/cfg.c            |  8 +++++-
 net/mac80211/debugfs_netdev.c |  2 +-
 net/mac80211/ieee80211_i.h    |  6 ++---
 net/mac80211/mesh.c           |  2 +-
 net/mac80211/tx.c             | 11 ++++++--
 net/mac80211/util.c           | 50 +++++++++++++++++++++++++++++++----
 6 files changed, 66 insertions(+), 13 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index d955bf8a6654..4f6755244030 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);
 
@@ -1653,7 +1654,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/tx.c b/net/mac80211/tx.c
index 6fa883a9250d..4c457b715b22 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -4881,6 +4881,13 @@ static void __ieee80211_beacon_add_tim(struct ieee80211_sub_if_data *sdata,
 	int aid0 = 0;
 	int i, have_bits = 0, n1, n2;
 	struct ieee80211_bss_conf *link_conf = link->conf;
+	/*
+	 * via IEEE80211-2024 11.1.3.10.2 if we are short beaconing,
+	 * use dot11ShortBeaconDTIMPeriod, else use dot11DTIMPeriod.
+	 */
+	u8 dtim_period = link_conf->s1g_short_beaconing ?
+			      link_conf->s1g_short_beacon_dtim_period :
+			      link_conf->dtim_period;
 
 	/* Generate bitmap for TIM only if there are any STAs in power save
 	 * mode. */
@@ -4891,7 +4898,7 @@ static void __ieee80211_beacon_add_tim(struct ieee80211_sub_if_data *sdata,
 					  IEEE80211_MAX_AID+1);
 	if (!is_template) {
 		if (ps->dtim_count == 0)
-			ps->dtim_count = link_conf->dtim_period - 1;
+			ps->dtim_count = dtim_period - 1;
 		else
 			ps->dtim_count--;
 	}
@@ -4900,7 +4907,7 @@ static void __ieee80211_beacon_add_tim(struct ieee80211_sub_if_data *sdata,
 	*pos++ = WLAN_EID_TIM;
 	*pos++ = 3;
 	*pos++ = ps->dtim_count;
-	*pos++ = link_conf->dtim_period;
+	*pos++ = dtim_period;
 
 	if (ps->dtim_count == 0 && !skb_queue_empty(&ps->bc_buf))
 		aid0 = 1;
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 0d85a382746f..47e86461c209 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -3913,15 +3913,28 @@ 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;
 	struct ps_data *ps;
 	u8 bcns_from_dtim;
+	bool short_beaconing = sdata->vif.bss_conf.s1g_short_beaconing;
+	/*
+	 * When short beaconing, ensure we use the short beacon interval
+	 * as opposed to the beacon interval as per IEEE80211-2024 11.1.2.1.
+	 */
+	u32 beacon_int = (short_beaconing ?
+				  sdata->vif.bss_conf.s1g_short_beacon_int :
+				  sdata->vif.bss_conf.beacon_int) * 1024;
+	/*
+	 * When short beaconing, all DTIM calculations are made using the
+	 * dot11ShortBeaconDTIMPeriod value and the dot11DTIMPeriod value
+	 * is unused as per IEEE80211-2024 9.4.2.5.1.
+	 */
+	u8 dtim_period =
+		short_beaconing ?
+			sdata->vif.bss_conf.s1g_short_beacon_dtim_period :
+			sdata->vif.bss_conf.dtim_period;
 
 	if (tsf == -1ULL || !beacon_int || !dtim_period)
 		return;
@@ -3954,6 +3967,33 @@ void ieee80211_recalc_dtim(struct ieee80211_local *local,
 	ps->dtim_count = dtim_count;
 }
 
+/*
+ * Given a short 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 sb_period = sdata->vif.bss_conf.s1g_short_beacon_period;
+	u32 beacon_int = sdata->vif.bss_conf.s1g_short_beacon_int * 1024;
+
+	/* No mesh / IBSS support for short beaconing */
+	if (tsf == -1ULL || !sb_period ||
+	    (sdata->vif.type != NL80211_IFTYPE_AP &&
+	     sdata->vif.type != NL80211_IFTYPE_AP_VLAN))
+		return;
+
+	/* find the current TSBTT index in our sb_period */
+	do_div(tsf, beacon_int);
+	sb_idx = do_div(tsf, sb_period);
+
+	/* num TSBTTs until the next TBTT */
+	ps->sb_count = sb_idx ? sb_period - sb_idx : 0;
+}
+
 static u8 ieee80211_chanctx_radar_detect(struct ieee80211_local *local,
 					 struct ieee80211_chanctx *ctx)
 {
-- 
2.43.0

[RFC 4/5] 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 short beacon period, which is the
number of short beacons sent per 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>
---
 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 4c457b715b22..c04f79875eda 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -5297,14 +5297,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);
@@ -5365,6 +5365,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_short_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,
@@ -5388,7 +5462,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;
 	}
@@ -5417,6 +5491,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();
 
@@ -5438,6 +5513,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,
@@ -5458,8 +5540,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

[RFC 5/5] wifi: mac80211_hwsim: add S1G short beacon support

[Lachlan Hodges]

When short beaconing is enabled, correctly initialise the beacon
timer using the short beacon interval. Additionally, utilise the
correct short beaconing BSS configuration values when short beaconing
is enabled for an S1G BSS within mac80211_hwsim.

Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
 drivers/net/wireless/virtual/mac80211_hwsim.c | 109 ++++++++++++------
 1 file changed, 76 insertions(+), 33 deletions(-)

diff --git a/drivers/net/wireless/virtual/mac80211_hwsim.c b/drivers/net/wireless/virtual/mac80211_hwsim.c
index eefe8da3b14d..daeb7ff23384 100644
--- a/drivers/net/wireless/virtual/mac80211_hwsim.c
+++ b/drivers/net/wireless/virtual/mac80211_hwsim.c
@@ -655,6 +655,8 @@ static struct platform_driver mac80211_hwsim_driver = {
 struct mac80211_hwsim_link_data {
 	u32 link_id;
 	u64 beacon_int	/* beacon interval in us */;
+	bool s1g_short_beaconing; /* whether this link is short beaconing */
+	u64 s1g_short_beacon_int; /* S1G short beacon interval in us */
 	struct hrtimer beacon_timer;
 };
 
@@ -1226,15 +1228,18 @@ static void mac80211_hwsim_set_tsf(struct ieee80211_hw *hw,
 {
 	struct mac80211_hwsim_data *data = hw->priv;
 	u64 now = mac80211_hwsim_get_tsf(hw, vif);
-	/* MLD not supported here */
-	u32 bcn_int = data->link_data[0].beacon_int;
 	u64 delta = abs(tsf - now);
 	struct ieee80211_bss_conf *conf;
+	u32 bcn_int;
 
 	conf = link_conf_dereference_protected(vif, data->link_data[0].link_id);
-	if (conf && !conf->enable_beacon)
+	if (!conf || !conf->enable_beacon)
 		return;
 
+	/* MLD not supported here */
+	bcn_int = conf->s1g_short_beaconing ?
+			     data->link_data[0].s1g_short_beacon_int :
+			     data->link_data[0].beacon_int;
 	/* adjust after beaconing with new timestamp at old TBTT */
 	if (tsf > now) {
 		data->tsf_offset += delta;
@@ -2345,7 +2350,9 @@ mac80211_hwsim_beacon(struct hrtimer *timer)
 		container_of(link_data, struct mac80211_hwsim_data,
 			     link_data[link_data->link_id]);
 	struct ieee80211_hw *hw = data->hw;
-	u64 bcn_int = link_data->beacon_int;
+	u64 bcn_int = link_data->s1g_short_beaconing ?
+			      link_data->s1g_short_beacon_int :
+			      link_data->beacon_int;
 
 	if (!data->started)
 		return HRTIMER_NORESTART;
@@ -2449,11 +2456,14 @@ static int mac80211_hwsim_config(struct ieee80211_hw *hw, int radio_idx,
 		struct mac80211_hwsim_link_data *link_data =
 			&data->link_data[idx];
 
-		if (!data->started || !link_data->beacon_int) {
+		if (!data->started || (!link_data->beacon_int &&
+				       !link_data->s1g_short_beacon_int)) {
 			hrtimer_cancel(&link_data->beacon_timer);
 		} else if (!hrtimer_active(&link_data->beacon_timer)) {
 			u64 tsf = mac80211_hwsim_get_tsf(hw, NULL);
-			u32 bcn_int = link_data->beacon_int;
+			u32 bcn_int = link_data->s1g_short_beaconing ?
+					      link_data->s1g_short_beacon_int :
+					      link_data->beacon_int;
 			u64 until_tbtt = bcn_int - do_div(tsf, bcn_int);
 
 			hrtimer_start(&link_data->beacon_timer,
@@ -2520,6 +2530,55 @@ static void mac80211_hwsim_vif_info_changed(struct ieee80211_hw *hw,
 	}
 }
 
+static void
+mac80211_hwsim_link_change_beacon(struct ieee80211_hw *hw,
+				  struct ieee80211_vif *vif,
+				  struct ieee80211_bss_conf *info,
+				  struct mac80211_hwsim_link_data *link_data,
+				  struct mac80211_hwsim_data *data,
+				  struct hwsim_vif_priv *vp)
+{
+	u64 tsf, until_tbtt;
+	u32 bcn_interval_us;
+	bool short_beaconing = vif->cfg.s1g && info->s1g_short_beaconing;
+
+	vp->bcn_en = info->enable_beacon;
+	if (info->enable_beacon && data->started &&
+	    !hrtimer_active(&link_data->beacon_timer)) {
+		if (short_beaconing) {
+			link_data->s1g_short_beacon_int =
+				info->s1g_short_beacon_int * 1024;
+			bcn_interval_us = link_data->s1g_short_beacon_int;
+			link_data->s1g_short_beaconing = true;
+		} else {
+			link_data->beacon_int = info->beacon_int * 1024;
+			bcn_interval_us = link_data->beacon_int;
+		}
+
+		tsf = mac80211_hwsim_get_tsf(hw, vif);
+		until_tbtt = bcn_interval_us - do_div(tsf, bcn_interval_us);
+
+		hrtimer_start(&link_data->beacon_timer,
+			      ns_to_ktime(until_tbtt * NSEC_PER_USEC),
+			      HRTIMER_MODE_REL_SOFT);
+	} else if (!info->enable_beacon) {
+		unsigned int count = 0;
+
+		ieee80211_iterate_active_interfaces_atomic(
+			data->hw, IEEE80211_IFACE_ITER_NORMAL,
+			mac80211_hwsim_bcn_en_iter, &count);
+
+		wiphy_dbg(hw->wiphy, "  beaconing vifs remaining: %u", count);
+
+		if (count == 0) {
+			hrtimer_cancel(&link_data->beacon_timer);
+			link_data->beacon_int = 0;
+			link_data->s1g_short_beacon_int = 0;
+			link_data->s1g_short_beaconing = false;
+		}
+	}
+}
+
 static void mac80211_hwsim_link_info_changed(struct ieee80211_hw *hw,
 					     struct ieee80211_vif *vif,
 					     struct ieee80211_bss_conf *info,
@@ -2542,34 +2601,18 @@ static void mac80211_hwsim_link_info_changed(struct ieee80211_hw *hw,
 	}
 
 	if (changed & BSS_CHANGED_BEACON_ENABLED) {
-		wiphy_dbg(hw->wiphy, "  BCN EN: %d (BI=%u)\n",
-			  info->enable_beacon, info->beacon_int);
-		vp->bcn_en = info->enable_beacon;
-		if (data->started &&
-		    !hrtimer_active(&link_data->beacon_timer) &&
-		    info->enable_beacon) {
-			u64 tsf, until_tbtt;
-			u32 bcn_int;
-			link_data->beacon_int = info->beacon_int * 1024;
-			tsf = mac80211_hwsim_get_tsf(hw, vif);
-			bcn_int = link_data->beacon_int;
-			until_tbtt = bcn_int - do_div(tsf, bcn_int);
-
-			hrtimer_start(&link_data->beacon_timer,
-				      ns_to_ktime(until_tbtt * NSEC_PER_USEC),
-				      HRTIMER_MODE_REL_SOFT);
-		} else if (!info->enable_beacon) {
-			unsigned int count = 0;
-			ieee80211_iterate_active_interfaces_atomic(
-				data->hw, IEEE80211_IFACE_ITER_NORMAL,
-				mac80211_hwsim_bcn_en_iter, &count);
-			wiphy_dbg(hw->wiphy, "  beaconing vifs remaining: %u",
-				  count);
-			if (count == 0) {
-				hrtimer_cancel(&link_data->beacon_timer);
-				link_data->beacon_int = 0;
-			}
+		if (info->s1g_short_beaconing) {
+			wiphy_dbg(hw->wiphy,
+				  "  SHORT BCN EN: %d (BI=%u, SBI=%u)\n",
+				  info->enable_beacon, info->beacon_int,
+				  info->s1g_short_beacon_int);
+		} else {
+			wiphy_dbg(hw->wiphy, "  BCN EN: %d (BI=%u)\n",
+				  info->enable_beacon, info->beacon_int);
 		}
+
+		mac80211_hwsim_link_change_beacon(hw, vif, info, link_data,
+						  data, vp);
 	}
 
 	if (changed & BSS_CHANGED_ERP_CTS_PROT) {
-- 
2.43.0

Re: [RFC 0/5] wifi: S1G short beacon support

[Johannes Berg]

Hi,

So just to say this up front I have very little interest per se in this
feature directly, so I'll mostly leave it to you :)

> Preface:
> 
> Previously, some work was done ~2 years ago to get short beaconing
> in the kernel but it was never successful. The patches can be found
> here:
> 
> 1/2: https://patchwork.kernel.org/project/linux-wireless/patch/20230810093556.33800-1-bassem@morsemicro.com/
> 2/2: https://patchwork.kernel.org/project/linux-wireless/patch/20230810093556.33800-2-bassem@morsemicro.com/

Heh, I didn't even remember.

> There is something to say regarding whether this is "correct" as
> after all they are extension frames and while we feel this is the best
> approach when all factors are taken into account - ultimately it is
> up to maintainers to decide.

Seems reasonable to me, and anyway probably necessary so that we don't
regress.

> 11.1.3.10.2:
> 
> "The value for the dot11ShortBeaconPeriod shall be such that
> dot11BeaconPeriod = n * dot11ShortBeaconPeriod, where n is a
> positive integer. This defines a series of TSBTTs exactly
> dot11ShortBeaconPeriod TUs apart"
> 
> the value for n here is what we are denoting as
> s1g_short_beacon_period (another deviation from the naming
> within the standard) which represents the number of short
> beacons between each long beacon.

That seems slightly confusing - I've have interpreted a 'period' either
as a span of time (as in the spec), or the number of steps between
events, so maybe that's rather 's1g_long_beacon_period'? But if the spec
uses period as a span of time, then perhaps 's1g_long_beacon_step' or
something would be easier to understand? Not sure ...

> To keep track of the current
> state, we introduce a new parameter sb_count within the
> struct ps_data structure to track the current index into this
> period. This is what is used to determine whether we send a
> long or short beacon on beacon retrieval where a value of 0
> indicates a long beacon (following the same cadence of
> decrementing the DTIM count).

Seems fair.

> (4) Beacon retrieval
> 
> There were essentially two options we could take with regards to
> retrieving the beacons:
> 
> 1.implement it in the traditional beacon path via
>   ieee80211_beacon_get_ap
> 
> 2.Implement an S1G (or maybe extension frame?) specific path
>   such as ieee80211_s1g_beacon_get_ap where the S1G specific
>   handling can be done in its own function, leaving the regular
>   beacon path untouched
> 
> We have opted to take method (1) but this really comes down to
> maintainers preference. New conditionals will be introduced
> in the beacon path regardless and we feel this is the best
> approach but are open to feedback.

That seems reasonable to me, why bother the driver with the difference,
it's likely simply going to transmit the frame anyway, regardless of
what it is.

> (5) Testing
> 
> This patchset has been tested on the following configurations:
> 
> 1. Multi-sta setup with real Morse Micro hardware.
> 2. S1G hwsim configuration with a single AP and > 10 STAs
> 3. 2.4GHz hwsim configuration to test for regressions along the
>    non-s1g path consisting of a single AP and STA.
> 4. hostapd hwsim tests were run to ensure no regression for
>    regular 2.4/5/6 radios.

Cool.

> (6) Other notes:
> 
> 1. The update mechanism is not really that nice. Though that may
>    stem from a misunderstanding with how FILS discovery and unsol
>    bcast update mechanisms work. Since short beacons rely on a BSS
>    to be configured we ensure that an update is only performed
>    when this is the case, and when the BSS has not been configured
>    we do not allow a set of parameters used for updating the beacon
>    to be used when the BSS has not been initialised. Would like some
>    feedback on our implementation here as Im not really convinved
>    it's the right way, but it seems this is how we currently handle
>    it. My understanding of an "update" is that we are updating an
>    existing interface. When an interface is being initialised, we
>    are not updating - we are setting... An option here is to just
>    not even allow updates since this is how S1G is done now and this
>    can be ammended in the future.

Noted. I'll go through the commits and shout if I find anything :)


johannes

Re: [RFC 1/5] wifi: cfg80211: support configuring an S1G short beaconing BSS

[Johannes Berg]

On Mon, 2025-07-14 at 15:13 +1000, Lachlan Hodges wrote:
> 
> +/**
> + * enum nl80211_s1g_short_beacon_attrs - S1G short beacon data
> + *
> + * @__NL80211_S1G_SHORT_BEACON_ATTR_INVALID: Invalid
> + *
> + * @NL80211_S1G_SHORT_BEACON_HEAD: Short beacon head (binary).
> + * @NL80211_S1G_SHORT_BEACON_TAIL: Short beacon tail (binary).
> + * @NL80211_S1G_SHORT_BEACON_INTERVAL: Time in TUs between each short
> + *	beacon transmission (u32).
> + * @NL80211_S1G_SHORT_BEACON_DTIM_PERIOD: DTIM period for a short
> + *	beaconing BSS (u8).
> + */
> +enum nl80211_s1g_short_beacon_attrs {
> +	__NL80211_S1G_SHORT_BEACON_ATTR_INVALID,
> +
> +	NL80211_S1G_SHORT_BEACON_HEAD,
> +	NL80211_S1G_SHORT_BEACON_TAIL,
> +	NL80211_S1G_SHORT_BEACON_INTERVAL,
> +	NL80211_S1G_SHORT_BEACON_DTIM_PERIOD,

nit: we usually have _ATTR_ in there after the qualification, so
something like NL80211_S1G_SHORT_BEACON_ATTR_HEAD.

Also, the bot complained about some missing kernel-doc.
 
> +/*
> + * Short beacons contain a limited set of allowed elements as per
> + * IEEE80211-2024 9.3.4.3 Table 9-76. The TIM element is allowed,
> + * but as it is inserted by mac80211, we do not check for it.
> + */
> +static int is_valid_s1g_short_elem(const struct element *elem)
> +{
> +	switch (elem->id) {
> +	case WLAN_EID_FMS_DESCRIPTOR:
> +	case WLAN_EID_RPS:
> +	case WLAN_EID_SST:
> +	case WLAN_EID_S1G_RELAY:
> +	case WLAN_EID_PAGE_SLICE:
> +	case WLAN_EID_VENDOR_SPECIFIC:
> +	case WLAN_EID_MMIE:
> +	case WLAN_EID_MIC:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}

Is that really worth it? We don't have to protect userspace from
shooting it self into the foot _too_ much, just make sure that we don't
get into a mess in the kernel itself. As long as the elements are not
malformed, I'd argue we're fine from a kernel perspective?

This also prevents future updates and experimentation, and I see little
value in it?

> +static int nl80211_validate_s1g_short_conf(struct cfg80211_ap_settings *params)
> +{
> +	struct cfg80211_s1g_short_beacon *sb = &params->s1g_short_beacon;
> +	u64 beacon_int = params->beacon_interval;

Why a u64, it's not a u64 in the params? and it makes the division
harder (well, really you wanted the remainder)

> +	u32 short_int = sb->short_interval;
> +
> +	/*
> +	 * As per IEEE80211 11.1.3.10.2, beacon_interval = n * short_interval
> +	 * where n is a positive integer. Meaning a BSS can be configured such
> +	 * that both the short beacon interval and long beacon interval are
> +	 * equivalent. This effectively means we aren't short beaconing. In
> +	 * this case, since the short beacon data is irellevent its probably a

typo: irrelevant

> +	 * misconfiguration and we should reject it.
> +	 */
> +	if (sb->short_interval >= params->beacon_interval)
> +		return -EINVAL;
> +
> +	if (do_div(beacon_int, short_int))
> +		return -EINVAL;

so that could just be

	if (params->beacon_interval % sb->short_interval)
		return -EINVAL;

no?

> +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;

Maybe we should (instead?) check that it's operating as or being set up
for s1g? Not sure how easy that is, but I've actually wanted it in other
places before, I think.

> +
> +	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_HEAD])
> +		return -EINVAL;
> +
> +	sb->update = false;
> +	sb->short_head = nla_data(tb[NL80211_S1G_SHORT_BEACON_HEAD]);
> +	sb->short_head_len = nla_len(tb[NL80211_S1G_SHORT_BEACON_HEAD]);
> +	sb->short_tail_len = 0;
> +
> +	if (tb[NL80211_S1G_SHORT_BEACON_TAIL]) {
> +		sb->short_tail = nla_data(tb[NL80211_S1G_SHORT_BEACON_TAIL]);
> +		sb->short_tail_len = nla_len(tb[NL80211_S1G_SHORT_BEACON_TAIL]);
> +	}
> +
> +	if (!tb[NL80211_S1G_SHORT_BEACON_INTERVAL] ||
> +	    !tb[NL80211_S1G_SHORT_BEACON_DTIM_PERIOD]) {
> +		sb->update = true;
> +		return 0;
> +	}
> +
> +	sb->short_interval =
> +		nla_get_u32(tb[NL80211_S1G_SHORT_BEACON_INTERVAL]);
> +	sb->short_dtim_period =
> +		nla_get_u8(tb[NL80211_S1G_SHORT_BEACON_DTIM_PERIOD]);
> +
> +	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 +6615,28 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
>  		goto out;
>  	}
>  
> +	if (info->attrs[NL80211_ATTR_S1G_SHORT_BEACON]) {
> +		err = nl80211_parse_s1g_short_beacon(
> +			rdev, info->attrs[NL80211_ATTR_S1G_SHORT_BEACON],
> +			&params->s1g_short_beacon);
> +		if (err)
> +			goto out;
> +
> +		/*
> +		 * If we have only received the parameters to perform a
> +		 * short beacon update, return an error to usermode as
> +		 * the BSS has not yet been configured for short beaconing.
> +		 */
> +		if (params->s1g_short_beacon.update) {
> +			err = -EINVAL;
> +			goto out;
> +		}
> +
> +		err = nl80211_validate_s1g_short_conf(params);
> +		if (err)
> +			goto out;

why not call the validation inside nl80211_parse_s1g_short_beacon()?
seems harder to misuse later then, and the order shouldn't matter much?

> @@ -6550,6 +6745,19 @@ 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;
> +
> +		if (!params->s1g_short_beacon.update) {
> +			err = -EINVAL;
> +			goto out;
> +		}
> +	}

And you already forgot the validation here, it seems?

Looks fine from the perspective of when/how you parse it, I think,
unless you were going to need channel switch and/or BSS color change.

johannes

Re: [RFC 2/5] wifi: mac80211: support initialising an S1G short beaconing BSS

[Johannes Berg]

On Mon, 2025-07-14 at 15:14 +1000, Lachlan Hodges wrote:
> 
> @@ -758,6 +760,16 @@ 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_short_beacon_int: short beacon interval in TUs. When short beaconing is
> + *	enabled beacon transmission times are computed using this value as
> + *	opposed to beacon_int as per IEEE80211-2024 11.1.2.1.
> + * @s1g_short_beacon_dtim_period: number of short beacon intervals that elapse
> + *	between each beacon with a TIM element whose DTIM count is 0. When
> + *	short beaconing is enabled, this value is used as opposed to
> + *	dtim_period as per IEEE80211-2024 9.4.2.5.1.
> + * @s1g_short_beacon_period: number of short beacons sent per long beacon.
>   */
>  struct ieee80211_bss_conf {
>  	struct ieee80211_vif *vif;
> @@ -857,6 +869,11 @@ struct ieee80211_bss_conf {
>  
>  	u8 bss_param_ch_cnt;
>  	u8 bss_param_ch_cnt_link_id;
> +
> +	bool s1g_short_beaconing;
> +	u16 s1g_short_beacon_int;
> +	u8 s1g_short_beacon_dtim_period;
> +	u16 s1g_short_beacon_period;

Does the driver even need to know? For hwsim this is just additional
complexity - mac80211 could set the beacon interval to the short beacon
interval, and simply return the long beacon every
"s1g_short_beacon_period" (which perhaps should be called
"s1g_long_beacon_period")?

But depends on how your driver works I guess. Given these parameters
though, you can't really offload it entirely either since there's no way
to get a short beacon template now.


> +		if (params->s1g_short_beacon.short_interval) {
> +			/*
> +			 * IEEE80211-2024 11.1.3.10.2:
> +			 * beacon_int = n * short_beacon_int where n is a
> +			 * positive integer and represents the short beacon
> +			 * period which is the number of short beacons
> +			 * transmitted per long beacon.
> +			 */
> +			if (do_div(beacon_interval,
> +				   params->s1g_short_beacon.short_interval)) {

Again not sure I understand the use of do_div, and wouldn't it be enough
for cfg80211 to check?

johannes

Re: [RFC 3/5] wifi: mac80211: configure power save for an S1G short beaconing BSS

[Johannes Berg]

On Mon, 2025-07-14 at 15:14 +1000, Lachlan Hodges wrote:
> 
> +++ b/net/mac80211/tx.c
> @@ -4881,6 +4881,13 @@ static void __ieee80211_beacon_add_tim(struct ieee80211_sub_if_data *sdata,
>  	int aid0 = 0;
>  	int i, have_bits = 0, n1, n2;
>  	struct ieee80211_bss_conf *link_conf = link->conf;
> +	/*
> +	 * via IEEE80211-2024 11.1.3.10.2 if we are short beaconing,
> +	 * use dot11ShortBeaconDTIMPeriod, else use dot11DTIMPeriod.
> +	 */
> +	u8 dtim_period = link_conf->s1g_short_beaconing ?
> +			      link_conf->s1g_short_beacon_dtim_period :
> +			      link_conf->dtim_period;

Also here I wonder if the complexity is worth it?

If we just say the beacon interval and DTIM period are when we send
_any_ beacon, then none of this is needed and we simply send long
beacons and DTIMs according their own schedule each?

You've certainly thought about this more than I have though :-)

johannes

Re: [RFC 0/5] wifi: S1G short beacon support

[Lachlan Hodges]

Mon, Jul 14, 2025 at 02:50:42PM +0200, Johannes Berg wrote:
> Hi,
> 
> So just to say this up front I have very little interest per se in this
> feature directly, so I'll mostly leave it to you :)

Hi Johannes,

Firstly appreciate the thorough and quick reply... and of course that is
fine. We still want to make sure we implement it correctly :)

> > 11.1.3.10.2:
> > 
> > "The value for the dot11ShortBeaconPeriod shall be such that
> > dot11BeaconPeriod = n * dot11ShortBeaconPeriod, where n is a
> > positive integer. This defines a series of TSBTTs exactly
> > dot11ShortBeaconPeriod TUs apart"
> > 
> > the value for n here is what we are denoting as
> > s1g_short_beacon_period (another deviation from the naming
> > within the standard) which represents the number of short
> > beacons between each long beacon.
> 
> That seems slightly confusing - I've have interpreted a 'period' either
> as a span of time (as in the spec), or the number of steps between
> events, so maybe that's rather 's1g_long_beacon_period'? But if the spec
> uses period as a span of time, then perhaps 's1g_long_beacon_step' or
> something would be easier to understand? Not sure ...

You are right, it should be the long beacon period (similar to how the
DTIM period describes the number of intervals between each DTIM beacon).

lachlan

Re: [RFC 1/5] wifi: cfg80211: support configuring an S1G short beaconing BSS

[Lachlan Hodges]

On Mon, Jul 14, 2025 at 03:00:11PM +0200, Johannes Berg wrote:
> On Mon, 2025-07-14 at 15:13 +1000, Lachlan Hodges wrote:
> > 
> > +/**
> > + * enum nl80211_s1g_short_beacon_attrs - S1G short beacon data
> > + *
> > + * @__NL80211_S1G_SHORT_BEACON_ATTR_INVALID: Invalid
> > + *
> > + * @NL80211_S1G_SHORT_BEACON_HEAD: Short beacon head (binary).
> > + * @NL80211_S1G_SHORT_BEACON_TAIL: Short beacon tail (binary).
> > + * @NL80211_S1G_SHORT_BEACON_INTERVAL: Time in TUs between each short
> > + *	beacon transmission (u32).
> > + * @NL80211_S1G_SHORT_BEACON_DTIM_PERIOD: DTIM period for a short
> > + *	beaconing BSS (u8).
> > + */
> > +enum nl80211_s1g_short_beacon_attrs {
> > +	__NL80211_S1G_SHORT_BEACON_ATTR_INVALID,
> > +
> > +	NL80211_S1G_SHORT_BEACON_HEAD,
> > +	NL80211_S1G_SHORT_BEACON_TAIL,
> > +	NL80211_S1G_SHORT_BEACON_INTERVAL,
> > +	NL80211_S1G_SHORT_BEACON_DTIM_PERIOD,
> 
> nit: we usually have _ATTR_ in there after the qualification, so
> something like NL80211_S1G_SHORT_BEACON_ATTR_HEAD.
> 
> Also, the bot complained about some missing kernel-doc.
>

Yea, forgot to describe the max attributes :( Will fix for next set.

> > +/*
> > + * Short beacons contain a limited set of allowed elements as per
> > + * IEEE80211-2024 9.3.4.3 Table 9-76. The TIM element is allowed,
> > + * but as it is inserted by mac80211, we do not check for it.
> > + */
> > +static int is_valid_s1g_short_elem(const struct element *elem)
> > +{
> > +	switch (elem->id) {
> > +	case WLAN_EID_FMS_DESCRIPTOR:
> > +	case WLAN_EID_RPS:
> > +	case WLAN_EID_SST:
> > +	case WLAN_EID_S1G_RELAY:
> > +	case WLAN_EID_PAGE_SLICE:
> > +	case WLAN_EID_VENDOR_SPECIFIC:
> > +	case WLAN_EID_MMIE:
> > +	case WLAN_EID_MIC:
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
> 
> Is that really worth it? We don't have to protect userspace from
> shooting it self into the foot _too_ much, just make sure that we don't
> get into a mess in the kernel itself. As long as the elements are not
> malformed, I'd argue we're fine from a kernel perspective?
> 
> This also prevents future updates and experimentation, and I see little
> value in it?

In that case, would you have any opposition to using the regular
validate_beacon_head and validate_ie_attr for short beacon validation?
 
> why not call the validation inside nl80211_parse_s1g_short_beacon()?
> seems harder to misuse later then, and the order shouldn't matter much?
> 
> > @@ -6550,6 +6745,19 @@ 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;
> > +
> > +		if (!params->s1g_short_beacon.update) {
> > +			err = -EINVAL;
> > +			goto out;
> > +		}
> > +	}
> 
> And you already forgot the validation here, it seems?

The lack of validation is one of the issues I have with doing this
"correctly". I've made some changes here so I'll leave that to be
discussed in the forthcoming patchset.

lachlan

Re: [RFC 2/5] wifi: mac80211: support initialising an S1G short beaconing BSS

[Lachlan Hodges]

On Mon, Jul 14, 2025 at 03:03:44PM +0200, Johannes Berg wrote:
> On Mon, 2025-07-14 at 15:14 +1000, Lachlan Hodges wrote:
> > 
> > @@ -758,6 +760,16 @@ 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_short_beacon_int: short beacon interval in TUs. When short beaconing is
> > + *	enabled beacon transmission times are computed using this value as
> > + *	opposed to beacon_int as per IEEE80211-2024 11.1.2.1.
> > + * @s1g_short_beacon_dtim_period: number of short beacon intervals that elapse
> > + *	between each beacon with a TIM element whose DTIM count is 0. When
> > + *	short beaconing is enabled, this value is used as opposed to
> > + *	dtim_period as per IEEE80211-2024 9.4.2.5.1.
> > + * @s1g_short_beacon_period: number of short beacons sent per long beacon.
> >   */
> >  struct ieee80211_bss_conf {
> >  	struct ieee80211_vif *vif;
> > @@ -857,6 +869,11 @@ struct ieee80211_bss_conf {
> >  
> >  	u8 bss_param_ch_cnt;
> >  	u8 bss_param_ch_cnt_link_id;
> > +
> > +	bool s1g_short_beaconing;
> > +	u16 s1g_short_beacon_int;
> > +	u8 s1g_short_beacon_dtim_period;
> > +	u16 s1g_short_beacon_period;
> 
> Does the driver even need to know? For hwsim this is just additional
> complexity - mac80211 could set the beacon interval to the short beacon
> interval, and simply return the long beacon every
> "s1g_short_beacon_period" (which perhaps should be called
> "s1g_long_beacon_period")?
> 
> But depends on how your driver works I guess. Given these parameters
> though, you can't really offload it entirely either since there's no way
> to get a short beacon template now.

So this is something I forgot to mention in the cover letter, since yes
it simply adds code bloat (mentioned here and in the other patch). I
guess my intention was the "follow the standard" but maybe a bit too
much :). Without these new parameters the patchset is much leaner
and probably makes more sense anyway since at the end of the day,
the beacon interval is still the beacon interval and same for the
DTIM period. New patchset will exclude these new parameters, which
definitely cleans up much of the code.

lachlan.

Re: [RFC 1/5] wifi: cfg80211: support configuring an S1G short beaconing BSS

[Johannes Berg]

On Tue, 2025-07-15 at 19:12 +1000, Lachlan Hodges wrote:
> 
> > 
> > Is that really worth it? We don't have to protect userspace from
> > shooting it self into the foot _too_ much, just make sure that we don't
> > get into a mess in the kernel itself. As long as the elements are not
> > malformed, I'd argue we're fine from a kernel perspective?
> > 
> > This also prevents future updates and experimentation, and I see little
> > value in it?
> 
> In that case, would you have any opposition to using the regular
> validate_beacon_head and validate_ie_attr for short beacon validation?

No, don't think so, though I didn't check now that
validate_beacon_head() can deal with the differently sized non-element
fields in the different formats, but I suppose it could just do that.

johannes

Re: [RFC 1/5] wifi: cfg80211: support configuring an S1G short beaconing BSS

[Johannes Berg]

On Tue, 2025-07-15 at 11:18 +0200, Johannes Berg wrote:
> On Tue, 2025-07-15 at 19:12 +1000, Lachlan Hodges wrote:
> > 
> > > 
> > > Is that really worth it? We don't have to protect userspace from
> > > shooting it self into the foot _too_ much, just make sure that we don't
> > > get into a mess in the kernel itself. As long as the elements are not
> > > malformed, I'd argue we're fine from a kernel perspective?
> > > 
> > > This also prevents future updates and experimentation, and I see little
> > > value in it?
> > 
> > In that case, would you have any opposition to using the regular
> > validate_beacon_head and validate_ie_attr for short beacon validation?
> 
> No, don't think so, though I didn't check now that
> validate_beacon_head() can deal with the differently sized non-element
> fields in the different formats, but I suppose it could just do that.

Maybe that came out confusing, to clarify, I have no objection assuming
it handles the different formats :)

I don't even think it would be relevant to force the short beacon to be
short and the long beacon to be long since we already have a mix of the
three beacon formats everywhere, and all the code now (since your fixes)
handles all the possible formats/permutations thereof.

johannes