2023-01-30 07:23:05

by Aloka Dixit

[permalink] [raw]
Subject: [PATCH v3 1/6] wifi: nl80211: configure puncturing bitmap in NL80211_CMD_START_AP

Add a new attribute NL80211_ATTR_PUNCT_BITMAP to receive a puncturing
bitmap from the userspace. Each bit corresponds to a 20 MHz channel
in the operating bandwidth, lowest bit for the lowest frequency.
Bit set to 1 indicates that the channel is punctured.

Signed-off-by: Aloka Dixit <[email protected]>
Signed-off-by: Muna Sinada <[email protected]>
---
v3: Validation and storing the bitmap moved to MAC80211.
v2: Puncturing bitmap added to struct cfg80211_chan_def and
validated in CFG80211.

include/net/cfg80211.h | 5 +++++
include/uapi/linux/nl80211.h | 13 +++++++++++++
net/wireless/nl80211.c | 21 +++++++++++++++++++++
3 files changed, 39 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 54a77d906b2d..c25a558d50ea 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1316,6 +1316,10 @@ 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
+ * @punct_bitmap: Preamble puncturing bitmap. Each bit represents a 20 MHz
+ * channel, lowest bit corresponding to the lowest frequency. Bit set
+ * to 1 indicates that the channel is punctured. Higher 16 bits are
+ * reserved.
*/
struct cfg80211_ap_settings {
struct cfg80211_chan_def chandef;
@@ -1350,6 +1354,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;
+ u32 punct_bitmap;
};

/**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 8ecb0fbee721..b029a5b30c52 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2752,6 +2752,12 @@ enum nl80211_commands {
* the incoming frame RX timestamp.
* @NL80211_ATTR_TD_BITMAP: Transition Disable bitmap, for subsequent
* (re)associations.
+ *
+ * @NL80211_ATTR_PUNCT_BITMAP: (u32) Preamble puncturing bitmap, lowest
+ * bit corresponds to the lowest 20 MHz channel. Each bit set to 1
+ * indicates that the sub-channel is punctured. Higher 16 bits are
+ * reserved.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -3281,6 +3287,8 @@ enum nl80211_attrs {
NL80211_ATTR_RX_HW_TIMESTAMP,
NL80211_ATTR_TD_BITMAP,

+ NL80211_ATTR_PUNCT_BITMAP,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
@@ -6296,6 +6304,9 @@ enum nl80211_feature_flags {
* might apply, e.g. no scans in progress, no offchannel operations
* in progress, and no active connections.
*
+ * @NL80211_EXT_FEATURE_EHT_PUNCTURING: Driver supports preamble puncturing in
+ * EHT.
+ *
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
*/
@@ -6365,6 +6376,8 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_RADAR_BACKGROUND,
NL80211_EXT_FEATURE_POWERED_ADDR_CHANGE,

+ NL80211_EXT_FEATURE_EHT_PUNCTURING,
+
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
MAX_NL80211_EXT_FEATURES = NUM_NL80211_EXT_FEATURES - 1
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 64cf6110ce9d..351c4cc5ec92 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -805,6 +805,7 @@ 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_PUNCT_BITMAP] = { .type = NLA_U32 },
};

/* policy for the key attributes */
@@ -3173,6 +3174,19 @@ static bool nl80211_can_set_dev_channel(struct wireless_dev *wdev)
wdev->iftype == NL80211_IFTYPE_P2P_GO;
}

+static int nl80211_parse_punct_bitmap(struct cfg80211_registered_device *rdev,
+ struct genl_info *info,
+ u32 *bitmap)
+{
+ if (!bitmap ||
+ !wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_EHT_PUNCTURING))
+ return -EINVAL;
+
+ *bitmap = nla_get_u32(info->attrs[NL80211_ATTR_PUNCT_BITMAP]) & 0xFFFF;
+ return 0;
+}
+
int nl80211_parse_chandef(struct cfg80211_registered_device *rdev,
struct genl_info *info,
struct cfg80211_chan_def *chandef)
@@ -5918,6 +5932,13 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
goto out;
}

+ if (info->attrs[NL80211_ATTR_PUNCT_BITMAP]) {
+ err = nl80211_parse_punct_bitmap(rdev, info,
+ &params->punct_bitmap);
+ if (err)
+ goto out;
+ }
+
if (!cfg80211_reg_can_beacon_relax(&rdev->wiphy, &params->chandef,
wdev->iftype)) {
err = -EINVAL;
--
2.39.0



2023-01-30 08:40:12

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] wifi: nl80211: configure puncturing bitmap in NL80211_CMD_START_AP

On Sun, 2023-01-29 at 23:22 -0800, Aloka Dixit wrote:
>
> v3: Validation and storing the bitmap moved to MAC80211.

I think I'd prefer we move the validation function to cfg80211 so both
can use it, this way all potential non-mac80211 drivers have to do it as
well, and then they'll move the function _anyway_ to do the validation
in a single place, I'd hope?

> + * @punct_bitmap: Preamble puncturing bitmap. Each bit represents a 20 MHz
> + * channel, lowest bit corresponding to the lowest frequency. Bit set
> + * to 1 indicates that the channel is punctured. Higher 16 bits are
> + * reserved.
> */
> struct cfg80211_ap_settings {
> struct cfg80211_chan_def chandef;
> @@ -1350,6 +1354,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;
> + u32 punct_bitmap;

Internally I think we can continue to use u16, that's trivial to change
later.

> + * @NL80211_EXT_FEATURE_EHT_PUNCTURING: Driver supports preamble puncturing in
> + * EHT.

That should probably make some mention of AP mode? It's not optional in
any way for client, after all, and also not relevant to the API how
client does it.

> +static int nl80211_parse_punct_bitmap(struct cfg80211_registered_device *rdev,
> + struct genl_info *info,
> + u32 *bitmap)
> +{
> + if (!bitmap ||
> + !wiphy_ext_feature_isset(&rdev->wiphy,
> + NL80211_EXT_FEATURE_EHT_PUNCTURING))
> + return -EINVAL;
> +
> + *bitmap = nla_get_u32(info->attrs[NL80211_ATTR_PUNCT_BITMAP]) & 0xFFFF;

As the top bits are *reserved* then you should check that they're indeed
zero - now they're ignored, which is generally bad. They might not
always be.

johannes

2023-01-30 19:36:03

by Aloka Dixit

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] wifi: nl80211: configure puncturing bitmap in NL80211_CMD_START_AP

On 1/30/2023 12:40 AM, Johannes Berg wrote:
> On Sun, 2023-01-29 at 23:22 -0800, Aloka Dixit wrote:
>>
>> v3: Validation and storing the bitmap moved to MAC80211.
>
> I think I'd prefer we move the validation function to cfg80211 so both
> can use it, this way all potential non-mac80211 drivers have to do it as
> well, and then they'll move the function _anyway_ to do the validation
> in a single place, I'd hope?
>
>> + u32 punct_bitmap;
>
> Internally I think we can continue to use u16, that's trivial to change
> later.
> >> + * @NL80211_EXT_FEATURE_EHT_PUNCTURING: Driver supports preamble
puncturing in
>> + * EHT.
>
> That should probably make some mention of AP mode? It's not optional in
> any way for client, after all, and also not relevant to the API how
> client does it.
>
>> +
>> + *bitmap = nla_get_u32(info->attrs[NL80211_ATTR_PUNCT_BITMAP]) & 0xFFFF;
>
> As the top bits are *reserved* then you should check that they're indeed
> zero - now they're ignored, which is generally bad. They might not
> always be.
>

I will address all next version.
Will you be sending another patch which moves validation from mac80211
to cfg80211 or should I add that as the first patch?

2023-01-30 19:41:06

by Aloka Dixit

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] wifi: nl80211: configure puncturing bitmap in NL80211_CMD_START_AP

On 1/30/2023 11:35 AM, Aloka Dixit wrote:
> On 1/30/2023 12:40 AM, Johannes Berg wrote:
>> On Sun, 2023-01-29 at 23:22 -0800, Aloka Dixit wrote:
>>>
>>> v3: Validation and storing the bitmap moved to MAC80211.
>>
>> I think I'd prefer we move the validation function to cfg80211 so both
>> can use it, this way all potential non-mac80211 drivers have to do it as
>> well, and then they'll move the function _anyway_ to do the validation
>> in a single place, I'd hope?
>>
>>> +    u32 punct_bitmap;
>>
>> Internally I think we can continue to use u16, that's trivial to change
>> later.
>> >> + * @NL80211_EXT_FEATURE_EHT_PUNCTURING: Driver supports preamble
> puncturing in
>>> + *    EHT.
>>
>> That should probably make some mention of AP mode? It's not optional in
>> any way for client, after all, and also not relevant to the API how
>> client does it.
>>
>>> +
>>> +    *bitmap = nla_get_u32(info->attrs[NL80211_ATTR_PUNCT_BITMAP]) &
>>> 0xFFFF;
>>
>> As the top bits are *reserved* then you should check that they're indeed
>> zero - now they're ignored, which is generally bad. They might not
>> always be.
>>
>
> I will address all next version.
> Will you be sending another patch which moves validation from mac80211
> to cfg80211 or should I add that as the first patch?

Okay, saw your comments on 0/6 and one other late.
Will add 1/7 for moving the validation in next version.
Thanks!