diff mbox series

[v3,3/4] wifi: nl80211: update attributes netlink for S1G short beacon

Message ID 20221116020700.3907453-3-gilad.itzkovitch@morsemicro.com (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show
Series [v3,1/4] wifi: cfg80211: Add short_beacon_tail/head/period | expand

Commit Message

Gilad Itzkovitch Nov. 16, 2022, 2:06 a.m. UTC 
From: Kieran Frewen <kieran.frewen@morsemicro.com>

Add short beacon period, head and tail attributes for user
configuration

Signed-off-by: Kieran Frewen <kieran.frewen@morsemicro.com>
Co-developed-by: Gilad Itzkovitch <gilad.itzkovitch@morsemicro.com>
Signed-off-by: Gilad Itzkovitch <gilad.itzkovitch@morsemicro.com>
---
 include/uapi/linux/nl80211.h | 11 +++++++++++
 net/wireless/nl80211.c       | 32 +++++++++++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 1 deletion(-)

Comments

Johannes Berg March 30, 2023, 9:59 a.m. UTC | #1 
On Wed, 2022-11-16 at 15:06 +1300, Gilad Itzkovitch wrote:
> 
> + * @NL80211_ATTR_SHORT_BEACON_HEAD: portion of the short beacon before the TIM IE.
> + * @NL80211_ATTR_SHORT_BEACON_TAIL: portion of the short beacon after the TIM.

I would tend to say "TIM element" these days, since the spec changed
this, but I guess it doesn't matter much.

> + * @NL80211_BSS_SHORT_BEACON_PERIOD: S1G short beacon period in TUs
>   * @__NL80211_BSS_AFTER_LAST: internal
>   * @NL80211_BSS_MAX: highest BSS attribute
>   */
> @@ -4990,6 +5000,7 @@ enum nl80211_bss {
>  	NL80211_BSS_FREQUENCY_OFFSET,
>  	NL80211_BSS_MLO_LINK_ID,
>  	NL80211_BSS_MLD_ADDR,
> +	NL80211_BSS_SHORT_BEACON_PERIOD,

You don't use this.

> +++ b/net/wireless/nl80211.c
> @@ -231,12 +231,18 @@ static int validate_beacon_head(const struct nlattr *attr,
>  	const struct ieee80211_mgmt *mgmt = (void *)data;
>  	unsigned int fixedlen, hdrlen;
>  	bool s1g_bcn;
> +	bool s1g_short_bcn;
>  
>  	if (len < offsetofend(typeof(*mgmt), frame_control))
>  		goto err;
>  
>  	s1g_bcn = ieee80211_is_s1g_beacon(mgmt->frame_control);
> -	if (s1g_bcn) {
> +	s1g_short_bcn = ieee80211_is_s1g_short_beacon(mgmt->frame_control);
> +	if (s1g_short_bcn) {
> +		fixedlen = offsetof(struct ieee80211_ext,
> +				    u.s1g_short_beacon.variable);
> +		hdrlen = offsetof(struct ieee80211_ext, u.s1g_short_beacon);
> +	} else if (s1g_bcn) {
>  		fixedlen = offsetof(struct ieee80211_ext,
>  				    u.s1g_beacon.variable);
>  		hdrlen = offsetof(struct ieee80211_ext, u.s1g_beacon);

Even the old code here had me worried a bit, and the new code doesn't
make this better - what if you try to set an S1G (short or not) beacon
when the interface isn't using S1G really?

> +	[NL80211_ATTR_SHORT_BEACON_PERIOD] = { .type = NLA_U16 },

You can add a better range validation here, and then you don't need the
extra validation in the code later.

> +	[NL80211_ATTR_SHORT_BEACON_HEAD] =
> +		NLA_POLICY_VALIDATE_FN(NLA_BINARY, validate_beacon_head,
> +				       IEEE80211_MAX_DATA_LEN),
> +	[NL80211_ATTR_SHORT_BEACON_TAIL] =
> +		NLA_POLICY_VALIDATE_FN(NLA_BINARY, validate_ie_attr,
> +				       IEEE80211_MAX_DATA_LEN),
> +

nit: unnecessary blank line

> +	if (info->attrs[NL80211_ATTR_SHORT_BEACON_PERIOD])
> +		params->short_beacon_period =
> +			nla_get_u32(info->attrs[NL80211_ATTR_SHORT_BEACON_PERIOD]) == 0 ?
> +				1 : nla_get_u32(info->attrs[NL80211_ATTR_SHORT_BEACON_PERIOD]);

i.e. you don't need the == 0 check if you just reject == 0.

Also, you're confusing the types - using NLA_U16 above but nla_get_u32()
here.

johannes
diff mbox series

Patch

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index c14a91bbca7c..04bd39ee9d1d 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2751,6 +2751,11 @@  enum nl80211_commands {
  *	the incoming frame RX timestamp.
  * @NL80211_ATTR_TD_BITMAP: Transition Disable bitmap, for subsequent
  *	(re)associations.
+ *
+ * @NL80211_ATTR_SHORT_BEACON_PERIOD: S1G short beacon period in TUs.
+ * @NL80211_ATTR_SHORT_BEACON_HEAD: portion of the short beacon before the TIM IE.
+ * @NL80211_ATTR_SHORT_BEACON_TAIL: portion of the short beacon after the TIM.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -3280,6 +3285,10 @@  enum nl80211_attrs {
 	NL80211_ATTR_RX_HW_TIMESTAMP,
 	NL80211_ATTR_TD_BITMAP,
 
+	NL80211_ATTR_SHORT_BEACON_PERIOD,
+	NL80211_ATTR_SHORT_BEACON_HEAD,
+	NL80211_ATTR_SHORT_BEACON_TAIL,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -4963,6 +4972,7 @@  enum nl80211_bss_scan_width {
  * @NL80211_BSS_FREQUENCY_OFFSET: frequency offset in KHz
  * @NL80211_BSS_MLO_LINK_ID: MLO link ID of the BSS (u8).
  * @NL80211_BSS_MLD_ADDR: MLD address of this BSS if connected to it.
+ * @NL80211_BSS_SHORT_BEACON_PERIOD: S1G short beacon period in TUs
  * @__NL80211_BSS_AFTER_LAST: internal
  * @NL80211_BSS_MAX: highest BSS attribute
  */
@@ -4990,6 +5000,7 @@  enum nl80211_bss {
 	NL80211_BSS_FREQUENCY_OFFSET,
 	NL80211_BSS_MLO_LINK_ID,
 	NL80211_BSS_MLD_ADDR,
+	NL80211_BSS_SHORT_BEACON_PERIOD,
 
 	/* keep last */
 	__NL80211_BSS_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 148f66edb015..fca6e223c2c7 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -231,12 +231,18 @@  static int validate_beacon_head(const struct nlattr *attr,
 	const struct ieee80211_mgmt *mgmt = (void *)data;
 	unsigned int fixedlen, hdrlen;
 	bool s1g_bcn;
+	bool s1g_short_bcn;
 
 	if (len < offsetofend(typeof(*mgmt), frame_control))
 		goto err;
 
 	s1g_bcn = ieee80211_is_s1g_beacon(mgmt->frame_control);
-	if (s1g_bcn) {
+	s1g_short_bcn = ieee80211_is_s1g_short_beacon(mgmt->frame_control);
+	if (s1g_short_bcn) {
+		fixedlen = offsetof(struct ieee80211_ext,
+				    u.s1g_short_beacon.variable);
+		hdrlen = offsetof(struct ieee80211_ext, u.s1g_short_beacon);
+	} else if (s1g_bcn) {
 		fixedlen = offsetof(struct ieee80211_ext,
 				    u.s1g_beacon.variable);
 		hdrlen = offsetof(struct ieee80211_ext, u.s1g_beacon);
@@ -805,6 +811,14 @@  static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
 	[NL80211_ATTR_MLD_ADDR] = NLA_POLICY_EXACT_LEN(ETH_ALEN),
 	[NL80211_ATTR_MLO_SUPPORT] = { .type = NLA_FLAG },
 	[NL80211_ATTR_MAX_NUM_AKM_SUITES] = { .type = NLA_REJECT },
+	[NL80211_ATTR_SHORT_BEACON_PERIOD] = { .type = NLA_U16 },
+	[NL80211_ATTR_SHORT_BEACON_HEAD] =
+		NLA_POLICY_VALIDATE_FN(NLA_BINARY, validate_beacon_head,
+				       IEEE80211_MAX_DATA_LEN),
+	[NL80211_ATTR_SHORT_BEACON_TAIL] =
+		NLA_POLICY_VALIDATE_FN(NLA_BINARY, validate_ie_attr,
+				       IEEE80211_MAX_DATA_LEN),
+
 };
 
 /* policy for the key attributes */
@@ -5440,6 +5454,18 @@  static int nl80211_parse_beacon(struct cfg80211_registered_device *rdev,
 		haveinfo = true;
 	}
 
+	if (attrs[NL80211_ATTR_SHORT_BEACON_HEAD]) {
+		bcn->short_head = nla_data(attrs[NL80211_ATTR_SHORT_BEACON_HEAD]);
+		bcn->short_head_len = nla_len(attrs[NL80211_ATTR_SHORT_BEACON_HEAD]);
+		haveinfo = true;
+	}
+
+	if (attrs[NL80211_ATTR_SHORT_BEACON_TAIL]) {
+		bcn->short_tail = nla_data(attrs[NL80211_ATTR_SHORT_BEACON_TAIL]);
+		bcn->short_tail_len = nla_len(attrs[NL80211_ATTR_SHORT_BEACON_TAIL]);
+		haveinfo = true;
+	}
+
 	if (!haveinfo)
 		return -EINVAL;
 
@@ -5804,6 +5830,10 @@  static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 		nla_get_u32(info->attrs[NL80211_ATTR_BEACON_INTERVAL]);
 	params->dtim_period =
 		nla_get_u32(info->attrs[NL80211_ATTR_DTIM_PERIOD]);
+	if (info->attrs[NL80211_ATTR_SHORT_BEACON_PERIOD])
+		params->short_beacon_period =
+			nla_get_u32(info->attrs[NL80211_ATTR_SHORT_BEACON_PERIOD]) == 0 ?
+				1 : nla_get_u32(info->attrs[NL80211_ATTR_SHORT_BEACON_PERIOD]);
 
 	err = cfg80211_validate_beacon_int(rdev, dev->ieee80211_ptr->iftype,
 					   params->beacon_interval);