- Add advertisement from driver to userspace to indicate whether
RU puncturing is supported.
- Validate and configure the userspace specified RU puncturing bitmap
Changes are only compile-tested.
Aloka Dixit (3):
nl80211: advertise RU puncturing support to userspace
cfg80211: validate RU puncturing bitmap
nl80211: validate RU puncturing bitmap
include/net/cfg80211.h | 18 ++++++-
include/uapi/linux/nl80211.h | 32 ++++++++++++
net/wireless/chan.c | 99 ++++++++++++++++++++++++++++++++++++
net/wireless/nl80211.c | 71 ++++++++++++++++++++++++++
4 files changed, 219 insertions(+), 1 deletion(-)
base-commit: 8aaaf2f3af2ae212428f4db1af34214225f5cec3
prerequisite-patch-id: 328a8dc53fc4ed611094648f8354ea51a220a469
prerequisite-patch-id: 9f6778fe7023b0f4d103e206ed80cb3074c4e4f9
prerequisite-patch-id: 40aa2131e1529282c8099c4e270155292802f4ff
prerequisite-patch-id: c9db940e5649f3586357ee608d24f518a7fadb00
prerequisite-patch-id: edb4a64395858a174dd6ad739cf065f4092f2da1
prerequisite-patch-id: eeacae90f17719f5546e53cd04082849491dcfc2
prerequisite-patch-id: b5c0bb8119aa46b6aba653fa60c173c6546556e0
prerequisite-patch-id: 4a58f779c257d9191efe94a7283c59eff683f7c3
prerequisite-patch-id: fd4e44527c2965ae943c68c7c96bde33777bde59
prerequisite-patch-id: 6c8a84b6e8e3a5c317ec3410d9f99b06a779304d
prerequisite-patch-id: f7e51eb9ee4da52a5f79248f0314ef23db3333bc
prerequisite-patch-id: eb4c3cfd53903f093f233af6bf00b88acab86595
prerequisite-patch-id: 7d85d1f530c496da1ad6769c59f8524f58edfca9
prerequisite-patch-id: ff5183a0e028a1041700c32992eb18a13dc2de8b
prerequisite-patch-id: 159188202e4afc341f8a566d45cdf2e2afcb2d2e
prerequisite-patch-id: e69273ed8e6c0cebb8288230fc781f345663035d
prerequisite-patch-id: 06cfb4718beb6d69f078744feef1b6b897fad0aa
prerequisite-patch-id: d17f643739d565fca4d8f9139da0083d522647ed
prerequisite-patch-id: bb5917bb77a3de7b6804d25b56043ff26184d49d
--
2.31.1
Add new attributes NL80211_ATTR_RU_PUNCT_BITMAP and
NL80211_ATTR_RU_PUNCT_SUPP_HE to receive RU puncturing information
from the userspace.
- Bitmap consists of 16 bits, each bit corresponding to a 20 MHz
channel in the operating bandwidth. Lowest bit corresponds to
the lowest frequency. Validate the bitmap against the minimum
bandwidth support advertised by the driver.
- HE support flag indicates whether OFDMA puncturing patterns
should be considered during validation.
Signed-off-by: Aloka Dixit <[email protected]>
---
include/uapi/linux/nl80211.h | 10 ++++++++
net/wireless/nl80211.c | 49 ++++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+)
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index b4849afede98..1cccc1e8dc29 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2667,6 +2667,14 @@ enum nl80211_commands {
* the driver supports preamble puncturing, value should be of type
* &enum nl80211_ru_punct_supp_bw
*
+ * @NL80211_ATTR_RU_PUNCT_SUPP_HE: flag attribute, used to indicate that RU
+ * puncturing bitmap validation should include OFDMA bitmaps.
+ *
+ * @NL80211_ATTR_RU_PUNCT_BITMAP: (u16) RU puncturing bitmap where the lowest
+ * bit corresponds to the lowest 20 MHz channel. Each bit set to 1
+ * indicates that the sub-channel is punctured, set 0 indicates that the
+ * channel is active.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -3180,6 +3188,8 @@ enum nl80211_attrs {
NL80211_ATTR_EHT_CAPABILITY,
NL80211_ATTR_RU_PUNCT_SUPP_BW,
+ NL80211_ATTR_RU_PUNCT_SUPP_HE,
+ NL80211_ATTR_RU_PUNCT_BITMAP,
/* add attributes here, update the policy in nl80211.c */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 3b244b96d15f..720e53917013 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -784,6 +784,8 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
NL80211_EHT_MAX_CAPABILITY_LEN),
[NL80211_ATTR_RU_PUNCT_SUPP_BW] =
NLA_POLICY_MAX(NLA_U8, NL80211_RU_PUNCT_SUPP_BW_320),
+ [NL80211_ATTR_RU_PUNCT_SUPP_HE] = { .type = NLA_FLAG },
+ [NL80211_ATTR_RU_PUNCT_BITMAP] = { .type = NLA_U16 },
};
/* policy for the key attributes */
@@ -3117,6 +3119,46 @@ static bool nl80211_can_set_dev_channel(struct wireless_dev *wdev)
wdev->iftype == NL80211_IFTYPE_P2P_GO;
}
+static int nl80211_parse_ru_punct_bitmap(struct cfg80211_registered_device *rdev,
+ struct genl_info *info,
+ struct cfg80211_chan_def *chandef)
+{
+ chandef->ru_punct_bitmap_supp_he =
+ nla_get_flag(info->attrs[NL80211_ATTR_RU_PUNCT_SUPP_HE]);
+
+ chandef->ru_punct_bitmap =
+ nla_get_u16(info->attrs[NL80211_ATTR_RU_PUNCT_BITMAP]);
+
+ if (!chandef->ru_punct_bitmap)
+ return 0;
+
+ if (!rdev->wiphy.ru_punct_supp_bw &&
+ (chandef->ru_punct_bitmap || chandef->ru_punct_bitmap_supp_he))
+ return -EOPNOTSUPP;
+
+ switch (chandef->width) {
+ case NL80211_CHAN_WIDTH_80:
+ if (rdev->wiphy.ru_punct_supp_bw >=
+ NL80211_RU_PUNCT_SUPP_BW_160)
+ return -EOPNOTSUPP;
+ break;
+
+ case NL80211_CHAN_WIDTH_160:
+ if (rdev->wiphy.ru_punct_supp_bw >=
+ NL80211_RU_PUNCT_SUPP_BW_320)
+ return -EOPNOTSUPP;
+ break;
+
+ case NL80211_CHAN_WIDTH_320:
+ break;
+
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
int nl80211_parse_chandef(struct cfg80211_registered_device *rdev,
struct genl_info *info,
struct cfg80211_chan_def *chandef)
@@ -3124,6 +3166,7 @@ int nl80211_parse_chandef(struct cfg80211_registered_device *rdev,
struct netlink_ext_ack *extack = info->extack;
struct nlattr **attrs = info->attrs;
u32 control_freq;
+ int err;
if (!attrs[NL80211_ATTR_WIPHY_FREQ])
return -EINVAL;
@@ -3230,6 +3273,12 @@ int nl80211_parse_chandef(struct cfg80211_registered_device *rdev,
return -EINVAL;
}
+ if (info->attrs[NL80211_ATTR_RU_PUNCT_BITMAP]) {
+ err = nl80211_parse_ru_punct_bitmap(rdev, info, chandef);
+ if (err)
+ return err;
+ }
+
return 0;
}
--
2.31.1
RU preamble puncturing is allowed for bandwidths more that 80 MHz
except 80+80. Drivers may not support puncturing at all or have
restrictions for a minimum bandwidth.
Add new attribute NL80211_ATTR_RU_PUNCT_SUPP_BW to advertise the
driver support to the userspace. Default value (0) will indicate that
RU puncturing is not supported.
Signed-off-by: Aloka Dixit <[email protected]>
---
include/net/cfg80211.h | 6 ++++++
include/uapi/linux/nl80211.h | 22 ++++++++++++++++++++++
net/wireless/nl80211.c | 22 ++++++++++++++++++++++
3 files changed, 50 insertions(+)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 136c0c537334..5605cf6d247b 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5129,6 +5129,10 @@ struct wiphy_iftype_akm_suites {
* @ema_max_profile_periodicity: maximum profile periodicity supported by
* the driver. Setting this field to a non-zero value indicates that the
* driver supports enhanced multi-BSSID advertisements (EMA AP).
+ *
+ * @ru_punct_supp_bw: Whether the driver supports RU puncturing, and if so,
+ * for which bandwidths. See &enum nl80211_ru_punct_supp_bw for the
+ * possible values for this field.
*/
struct wiphy {
struct mutex mtx;
@@ -5276,6 +5280,8 @@ struct wiphy {
u8 mbssid_max_interfaces;
u8 ema_max_profile_periodicity;
+ enum nl80211_ru_punct_supp_bw ru_punct_supp_bw;
+
char priv[] __aligned(NETDEV_ALIGN);
};
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 98ed52663d6b..b4849afede98 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2663,6 +2663,10 @@ enum nl80211_commands {
* association request when used with NL80211_CMD_NEW_STATION). Can be set
* only if %NL80211_STA_FLAG_WME is set.
*
+ * @NL80211_ATTR_RU_PUNCT_SUPP_BW: (u8) Minimum bandwidth for which
+ * the driver supports preamble puncturing, value should be of type
+ * &enum nl80211_ru_punct_supp_bw
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -3175,6 +3179,8 @@ enum nl80211_attrs {
NL80211_ATTR_EHT_CAPABILITY,
+ NL80211_ATTR_RU_PUNCT_SUPP_BW,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -7600,4 +7606,20 @@ enum nl80211_ap_settings_flags {
NL80211_AP_SETTINGS_SA_QUERY_OFFLOAD_SUPPORT = 1 << 1,
};
+/**
+ * enum nl80211_ru_punct_supp_bw - Bandwidths supporting preamble puncturing
+ *
+ * @NL80211_RU_PUNCT_NOT_SUPP: preamble puncturing is not supported
+ * @NL80211_RU_PUNCT_SUPP_BW_80: puncturing supported within channels of at
+ * least 80 MHz bandwidth
+ * @NL80211_RU_PUNCT_SUPP_BW_160: puncturing supported within channels of at
+ * least 160 MHz bandwidth
+ * @NL80211_RU_PUNCT_SUPP_BW_320: puncturing supported within 320 MHz.
+ */
+enum nl80211_ru_punct_supp_bw {
+ NL80211_RU_PUNCT_NOT_SUPP,
+ NL80211_RU_PUNCT_SUPP_BW_80,
+ NL80211_RU_PUNCT_SUPP_BW_160,
+ NL80211_RU_PUNCT_SUPP_BW_320,
+};
#endif /* __LINUX_NL80211_H */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 1595e14fd678..3b244b96d15f 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -782,6 +782,8 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
NLA_POLICY_RANGE(NLA_BINARY,
NL80211_EHT_MIN_CAPABILITY_LEN,
NL80211_EHT_MAX_CAPABILITY_LEN),
+ [NL80211_ATTR_RU_PUNCT_SUPP_BW] =
+ NLA_POLICY_MAX(NLA_U8, NL80211_RU_PUNCT_SUPP_BW_320),
};
/* policy for the key attributes */
@@ -2293,6 +2295,23 @@ static int nl80211_put_mbssid_support(struct wiphy *wiphy, struct sk_buff *msg)
return -ENOBUFS;
}
+static int
+nl80211_put_ru_punct_supp_bw(struct cfg80211_registered_device *rdev,
+ struct sk_buff *msg)
+{
+ if (!rdev->wiphy.ru_punct_supp_bw)
+ return 0;
+
+ if (rdev->wiphy.ru_punct_supp_bw > NL80211_RU_PUNCT_SUPP_BW_320)
+ return -EINVAL;
+
+ if (nla_put_u8(msg, NL80211_ATTR_RU_PUNCT_SUPP_BW,
+ rdev->wiphy.ru_punct_supp_bw))
+ return -ENOBUFS;
+
+ return 0;
+}
+
struct nl80211_dump_wiphy_state {
s64 filter_wiphy;
long start;
@@ -2881,6 +2900,9 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
if (nl80211_put_mbssid_support(&rdev->wiphy, msg))
goto nla_put_failure;
+ if (nl80211_put_ru_punct_supp_bw(rdev, msg))
+ goto nla_put_failure;
+
/* done */
state->split_start = 0;
break;
--
2.31.1
RU puncturing bitmap consists of 16 bits, each bit corresponding to
a 20 MHz channel in the operating bandwidth. Lowest bit corresponds to
the lowest frequency. Bit set to 1 indicates that the channel is
punctured otherwise it is active.
Validate the bitmap against following rules:
- Primary 20 MHz channel cannot be punctured
- As per IEEE P802.11be/D1.3, December 2021, 36.3.12.11.3 Preamble
puncturing for PPDUs in a non-OFDMA transmission
- As per IEEE P802.11be/D1.3, December 2021, 36.3.12.11.2 Preamble
puncturing for PPDUs in an OFDMA transmission.
Signed-off-by: Aloka Dixit <[email protected]>
---
include/net/cfg80211.h | 12 ++++-
net/wireless/chan.c | 99 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 110 insertions(+), 1 deletion(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 5605cf6d247b..c3246e989828 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -742,6 +742,11 @@ struct key_params {
* chan will define the primary channel and all other
* parameters are ignored.
* @freq1_offset: offset from @center_freq1, in KHz
+ * @ru_punct_bitmap: RU puncturing bitmap. Each bit represents a 20 MHz channel
+ * with lowest bit corresponding to the smallest frequency. Bit set to 1
+ * indicates that the channel is punctured, otherwise the channel is active
+ * @ru_punct_bitmap_supp_he: Indicates whether RU puncturing bitmap validation
+ * should include OFDMA bitmaps.
*/
struct cfg80211_chan_def {
struct ieee80211_channel *chan;
@@ -750,6 +755,8 @@ struct cfg80211_chan_def {
u32 center_freq2;
struct ieee80211_edmg edmg;
u16 freq1_offset;
+ u16 ru_punct_bitmap;
+ bool ru_punct_bitmap_supp_he;
};
/*
@@ -878,7 +885,10 @@ cfg80211_chandef_identical(const struct cfg80211_chan_def *chandef1,
chandef1->width == chandef2->width &&
chandef1->center_freq1 == chandef2->center_freq1 &&
chandef1->freq1_offset == chandef2->freq1_offset &&
- chandef1->center_freq2 == chandef2->center_freq2);
+ chandef1->center_freq2 == chandef2->center_freq2 &&
+ chandef1->ru_punct_bitmap == chandef2->ru_punct_bitmap &&
+ chandef1->ru_punct_bitmap_supp_he ==
+ chandef2->ru_punct_bitmap_supp_he);
}
/**
diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 8b7fb4a9e07b..4390e9297222 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -32,6 +32,7 @@ void cfg80211_chandef_create(struct cfg80211_chan_def *chandef,
chandef->center_freq2 = 0;
chandef->edmg.bw_config = 0;
chandef->edmg.channels = 0;
+ chandef->ru_punct_bitmap = 0;
switch (chan_type) {
case NL80211_CHAN_NO_HT:
@@ -196,6 +197,101 @@ static int cfg80211_chandef_get_width(const struct cfg80211_chan_def *c)
return nl80211_chan_width_to_mhz(c->width);
}
+/* IEEE P802.11be/D1.31, December 2021, Table 36-30 5-bit punctured channel
+ * indication for the non-OFDMA case in an EHT MU PPDU
+ */
+static const u16 ru_punct_bitmap_80[] = {0xF, 0xE, 0xD, 0xB, 0x7};
+static const u16 ru_punct_bitmap_160[] = {0xFF, 0xFE, 0xFD, 0xFB, 0xF7, 0xEF,
+ 0xDF, 0xBF, 0x7F, 0xFC, 0xF3, 0xCF,
+ 0x3F};
+static const u16 ru_punct_bitmap_320[] = {0xFFFF, 0xFFFC, 0xFFF3, 0xFFCF,
+ 0xFF3F, 0xFCFF, 0xF3FF, 0xCFFF,
+ 0x3FFF, 0xFFF0, 0xFF0F, 0xF0FF,
+ 0x0FFF, 0xFFC0, 0xFF30, 0xFCF0,
+ 0xF3F0, 0xCFF0, 0x3FF0, 0x0FFC,
+ 0x0FF3, 0x0FCF, 0x0F3F, 0x0CFF,
+ 0x03FF};
+
+bool cfg80211_ru_punct_bitmap_valid(const struct cfg80211_chan_def *chandef)
+{
+ u8 i, non_ofdma_bitmap_count, ofdma_block_count = 1;
+ u16 bitmap, pri_ch_bit_pos;
+ const u16 *non_ofdma_bitmap;
+ u32 start_freq;
+
+ if (!chandef->ru_punct_bitmap) /* All channels active */
+ return true;
+
+ bitmap = ~chandef->ru_punct_bitmap;
+ WARN_ON_ONCE(sizeof(bitmap) != sizeof(chandef->ru_punct_bitmap));
+
+ switch (chandef->width) {
+ case NL80211_CHAN_WIDTH_80:
+ bitmap &= 0xF;
+ non_ofdma_bitmap = &ru_punct_bitmap_80[0];
+ non_ofdma_bitmap_count = ARRAY_SIZE(ru_punct_bitmap_80);
+ start_freq = chandef->center_freq1 - 40;
+ break;
+
+ case NL80211_CHAN_WIDTH_160:
+ bitmap &= 0xFF;
+ non_ofdma_bitmap = &ru_punct_bitmap_160[0];
+ non_ofdma_bitmap_count = ARRAY_SIZE(ru_punct_bitmap_160);
+ ofdma_block_count = 2;
+ start_freq = chandef->center_freq1 - 80;
+ break;
+
+ case NL80211_CHAN_WIDTH_320:
+ bitmap &= 0xFFFF;
+ non_ofdma_bitmap = &ru_punct_bitmap_320[0];
+ non_ofdma_bitmap_count = ARRAY_SIZE(ru_punct_bitmap_320);
+ ofdma_block_count = 4;
+ start_freq = chandef->center_freq1 - 160;
+ break;
+
+ default:
+ return false;
+ }
+
+ if (!bitmap) /* No channel active */
+ return false;
+
+ pri_ch_bit_pos = ((chandef->chan->center_freq - start_freq) / 20);
+ if (!(bitmap & BIT(pri_ch_bit_pos)))
+ return false;
+
+ /* Check for non-OFDMA puncturing patterns */
+ for (i = 0; i < non_ofdma_bitmap_count; i++)
+ if (non_ofdma_bitmap[i] == bitmap)
+ return true;
+
+ if (!chandef->ru_punct_bitmap_supp_he)
+ return false;
+
+ /* Check for OFDMA puncturing patterns */
+ for (i = 0; i < ofdma_block_count; i++) {
+ switch ((bitmap >> (i * 4)) & 0xF) {
+ /* IEEE P802.11be/D1.31, December 2021, 36.3.12.11.2 Preamble
+ * puncturing for PPDUs in an OFDMA transmission
+ */
+ case 0xF:
+ case 0x7:
+ case 0xB:
+ case 0xD:
+ case 0xE:
+ case 0x3:
+ case 0xC:
+ case 0x9:
+ case 0x0:
+ break;
+ default:
+ return false;
+ }
+ }
+
+ return true;
+}
+
bool cfg80211_chandef_valid(const struct cfg80211_chan_def *chandef)
{
u32 control_freq, oper_freq;
@@ -316,6 +412,9 @@ bool cfg80211_chandef_valid(const struct cfg80211_chan_def *chandef)
!cfg80211_edmg_chandef_valid(chandef))
return false;
+ if (!cfg80211_ru_punct_bitmap_valid(chandef))
+ return false;
+
return true;
}
EXPORT_SYMBOL(cfg80211_chandef_valid);
--
2.31.1
On Mon, 2022-02-14 at 14:30 -0800, Aloka Dixit wrote:
>
> + if (rdev->wiphy.ru_punct_supp_bw > NL80211_RU_PUNCT_SUPP_BW_320)
> + return -EINVAL;
>
This check really belongs somewhere else, e.g. wiphy_register() or so?
johannes
On Mon, 2022-02-14 at 14:30 -0800, Aloka Dixit wrote:
>
> +/* IEEE P802.11be/D1.31, December 2021, Table 36-30 5-bit punctured channel
> + * indication for the non-OFDMA case in an EHT MU PPDU
> + */
> +static const u16 ru_punct_bitmap_80[] = {0xF, 0xE, 0xD, 0xB, 0x7};
> +static const u16 ru_punct_bitmap_160[] = {0xFF, 0xFE, 0xFD, 0xFB, 0xF7, 0xEF,
> + 0xDF, 0xBF, 0x7F, 0xFC, 0xF3, 0xCF,
> + 0x3F};
> +static const u16 ru_punct_bitmap_320[] = {0xFFFF, 0xFFFC, 0xFFF3, 0xFFCF,
> + 0xFF3F, 0xFCFF, 0xF3FF, 0xCFFF,
> + 0x3FFF, 0xFFF0, 0xFF0F, 0xF0FF,
> + 0x0FFF, 0xFFC0, 0xFF30, 0xFCF0,
> + 0xF3F0, 0xCFF0, 0x3FF0, 0x0FFC,
> + 0x0FF3, 0x0FCF, 0x0F3F, 0x0CFF,
> + 0x03FF};
> +
> +bool cfg80211_ru_punct_bitmap_valid(const struct cfg80211_chan_def *chandef)
>
Heh. We wrote basically the same code just the other day (except I think
inverting the bitmasks or something?) in mac80211 for the client side,
i.e. when receiving puncturing from the AP ...
Can we export this function maybe so mac80211 can use it?
Conceptually, I'm wondering if it really belongs into the chandef? Can
you explain why it's part of the channel configuration? If you've got
two chandefs with the same control channel, CCFS and bandwidth, but
different puncturing, does it really make sense to treat them as two
separate channel contexts, e.g. in mac80211? It seems strange to do
that.
johannes
On 2/15/2022 12:19 AM, Johannes Berg wrote:
> On Mon, 2022-02-14 at 14:30 -0800, Aloka Dixit wrote:
>>
>> +
>> +bool cfg80211_ru_punct_bitmap_valid(const struct cfg80211_chan_def *chandef)
>>
>
> Heh. We wrote basically the same code just the other day (except I think
> inverting the bitmasks or something?) in mac80211 for the client side,
> i.e. when receiving puncturing from the AP ...
>
I instead reverted the passed bitmap inside the function because that
way if the map is not provided at all, it will be all 0's by default and
will be considered as all channels active.
> Can we export this function maybe so mac80211 can use it?
>
Sure, will add in the next version.
>
> Conceptually, I'm wondering if it really belongs into the chandef? Can
> you explain why it's part of the channel configuration? If you've got
> two chandefs with the same control channel, CCFS and bandwidth, but
> different puncturing, does it really make sense to treat them as two
> separate channel contexts, e.g. in mac80211? It seems strange to do
> that.
>
> johannes
Added it here so that any command working with chandef will be updated
without any other change.
Example: During channel switch, user can provide a puncturing bitmap
with a new option I added in userspace, and because it is part of
chandef, the same code path validates it for that command automatically.
Regarding if different puncturing pattern should be considered as a new
context - yes, depending on if it is HE or non-HE mode, the new bitmap
may be invalid and the operation should fail.
Thanks,
Aloka
On 2/15/2022 4:37 PM, Aloka Dixit wrote:
> On 2/15/2022 12:19 AM, Johannes Berg wrote:
>> On Mon, 2022-02-14 at 14:30 -0800, Aloka Dixit wrote:
>>>
>>> +
>>> +bool cfg80211_ru_punct_bitmap_valid(const struct cfg80211_chan_def
>>> *chandef)
>>
>> Conceptually, I'm wondering if it really belongs into the chandef? Can
>> you explain why it's part of the channel configuration? If you've got
>> two chandefs with the same control channel, CCFS and bandwidth, but
>> different puncturing, does it really make sense to treat them as two
>> separate channel contexts, e.g. in mac80211? It seems strange to do
>> that.
>>
>> johannes
>
> Added it here so that any command working with chandef will be updated
> without any other change.
> Example: During channel switch, user can provide a puncturing bitmap
> with a new option I added in userspace, and because it is part of
> chandef, the same code path validates it for that command automatically.
>
> Regarding if different puncturing pattern should be considered as a new
> context - yes, depending on if it is HE or non-HE mode, the new bitmap
> may be invalid and the operation should fail.
>
> Thanks,
> Aloka
Hi Johannes,
Any comments on the above. Thanks.
On Tue, 2022-02-15 at 16:37 -0800, Aloka Dixit wrote:
> >
> > Conceptually, I'm wondering if it really belongs into the chandef? Can
> > you explain why it's part of the channel configuration? If you've got
> > two chandefs with the same control channel, CCFS and bandwidth, but
> > different puncturing, does it really make sense to treat them as two
> > separate channel contexts, e.g. in mac80211? It seems strange to do
> > that.
>
> Added it here so that any command working with chandef will be updated
> without any other change.
> Example: During channel switch, user can provide a puncturing bitmap
> with a new option I added in userspace, and because it is part of
> chandef, the same code path validates it for that command automatically.
Yeah but is it really a CSA/chanswitch if the puncturing changes? I
don't think so?
> Regarding if different puncturing pattern should be considered as a new
> context - yes, depending on if it is HE or non-HE mode, the new bitmap
> may be invalid and the operation should fail.
>
That wasn't really the question though. Consider this:
Say you have STA + STA, if the first STA is connected to an AP with no
puncturing, and the second STA is connected to an AP where the channel
and bandwidth are the same, but some puncturing is done, should that
really be two channel contexts as far as mac80211 is concerned, and thus
require channels=2 in the interface combinations etc.? It doesn't seem
right to me.
Or consider AP + STA, where the AP is set up for some channel but the
STA is connecting to an AP on the exact same channel, but with
puncturing... Again, same thing, I don't think it should consume two
channel resources.
johannes
Hi Aloka, all,
Reviving an ancient thread, but this is where we discussed these things
last I think ...
So I said, back at the beginning of 2022:
> > > Conceptually, I'm wondering if it really belongs into the chandef? Can
> > > you explain why it's part of the channel configuration? If you've got
> > > two chandefs with the same control channel, CCFS and bandwidth, but
> > > different puncturing, does it really make sense to treat them as two
> > > separate channel contexts, e.g. in mac80211? It seems strange to do
> > > that.
And you replied two things:
> > Added it here so that any command working with chandef will be updated
> > without any other change.
> > Example: During channel switch, user can provide a puncturing bitmap
> > with a new option I added in userspace, and because it is part of
> > chandef, the same code path validates it for that command automatically.
>
> Yeah but is it really a CSA/chanswitch if the puncturing changes? I
> don't think so?
Which, well, I think I was confused about - it could we CSA/chanswitch
depending on what you actually want to do, i.e. it's up to the AP to do
this as just a puncturing bitmap update in the beacon, or with CSA. It
might do one way or the other depending on what it wants...
But also, I read this as being a bit more about the software POV, which
I didn't really think was the most important part.
And also, you said:
> > Regarding if different puncturing pattern should be considered as a new
> > context - yes, depending on if it is HE or non-HE mode, the new bitmap
> > may be invalid and the operation should fail.
Which I'm not sure I understood then, and certainly not sure I
understand now, but I said:
> That wasn't really the question though. Consider this:
>
> Say you have STA + STA, if the first STA is connected to an AP with no
> puncturing, and the second STA is connected to an AP where the channel
> and bandwidth are the same, but some puncturing is done, should that
> really be two channel contexts as far as mac80211 is concerned, and thus
> require channels=2 in the interface combinations etc.? It doesn't seem
> right to me.
>
> Or consider AP + STA, where the AP is set up for some channel but the
> STA is connecting to an AP on the exact same channel, but with
> puncturing... Again, same thing, I don't think it should consume two
> channel resources.
Which, actually, I've learned since that I was completely wrong about!
It should, and likely must, in fact be two separate channel contexts,
with all the limitations that implies.
The thing is - back then I was making not just one, but in fact *two*
wrong assumptions:
1. The DSP/radio can receive punctured PPDUs if listening on the non
punctured channel.
At least for our device that's not true, not sure about ath12k? It
seems you have a per-peer puncturing configuration even, but that
seems odd, and it's always just set to the vif puncturing
configuration.
2. You can simply transmit punctured PPDUs when on a non-punctured
channel, i.e. it's just a rate control decision.
This is perhaps less important, but it's also not really true.
While you can clearly _transmit_ this way, that's not the only
thing - you also need to do the CCA before transmitting, and if
there's noise/interference on the punctured channel, you'd much
more rarely find the channel to be clear and be able to transmit
if this doesn't consider the puncturing, but that's something to
do sort of generally in the background for the transmit.
It might be possible to work around #2, but I'm not sure it's possible
to work around #1?
So I think I have two questions:
A. Would you object if I moved the puncturing into the chandef after
all?
B. How does ath12k cope #1/#2 above? Would we need to have a callback
to the driver to compare if two channel contexts are compatible or
not (e.g. if they have different puncturing), or does ath12k also
have limitations on RX/TX that mean it would actually prefer two
channel contexts for the cases I had outlined in the quoted text
above (STA+STA/AP+STA)?
Thanks,
johannes
And another thing - separately because it's not related to puncturing,
but in the same thread because it's related to the channel context
handling...
We also realized that a very similar thing might happen for the OFDMA
trigger frame processing. Obviously I don't know if this is hardware or
firmware in ath12k, but given the speed this happens at, at least in our
device we can't really control it up-front.
As a client, you might (for various reasons) connect to an AP with a
lower bandwidth than the AP has configured. In this case, OFDMA UL/DL is
still mostly mandatory (80 MHz STA in a 160/320 MHz BSS, 160 MHz STA in
a 320 MHz BSS). As a result, if you end up being a STA with two EH
connections (*), you have to be able to parse the trigger frame /
downlink OFDMA RU allocation according to the *AP bandwidth* (**).
(*) And expect OFDMA on them, so RU allocations are relative to the AP
bandwidth; we're assuming for now that P2P will not use OFDMA.
(**) See 36.3.2.7 80 MHz operating non-AP EHT STAs participating in
wider bandwidth OFDMA and 36.3.2.8 for 160 MHz.
As a result, I'm thinking of adding the OFDMA bandwidth to the chandef,
and then treating two different ones as incompatible, though due to (*)
we might want driver-local decision on whether OFDMA is possible or not,
at least unless you agree that we should ignore it in P2P. For HE, OFDMA
exists but you don't participate in wider-than-your-bandwidth OFMDA, so
I guess we'd treat it as not relevant there.
Then we might end up with a situation where we have two different say 80
MHz channel contexts in mac80211 because one is with a 320 MHz AP and
the other with a 160 MHz AP, but that's necessary, I think.
Thoughts?
johannes
On 10/12/2023 4:53 AM, Johannes Berg wrote:
> Hi Aloka, all,
>
> Reviving an ancient thread, but this is where we discussed these things
> last I think ...
>
> So I said, back at the beginning of 2022:
>
>>>> Conceptually, I'm wondering if it really belongs into the chandef? Can
>>>> you explain why it's part of the channel configuration? If you've got
>>>> two chandefs with the same control channel, CCFS and bandwidth, but
>>>> different puncturing, does it really make sense to treat them as two
>>>> separate channel contexts, e.g. in mac80211? It seems strange to do
>>>> that.
>
>
> And you replied two things:
>
>>> Added it here so that any command working with chandef will be updated
>>> without any other change.
>>> Example: During channel switch, user can provide a puncturing bitmap
>>> with a new option I added in userspace, and because it is part of
>>> chandef, the same code path validates it for that command automatically.
>>
>> Yeah but is it really a CSA/chanswitch if the puncturing changes? I
>> don't think so?
>
> Which, well, I think I was confused about - it could we CSA/chanswitch
> depending on what you actually want to do, i.e. it's up to the AP to do
> this as just a puncturing bitmap update in the beacon, or with CSA. It
> might do one way or the other depending on what it wants...
>
>
> But also, I read this as being a bit more about the software POV, which
> I didn't really think was the most important part.
> >
> And also, you said:
>
>>> Regarding if different puncturing pattern should be considered as a new
>>> context - yes, depending on if it is HE or non-HE mode, the new bitmap
>>> may be invalid and the operation should fail.
>
> Which I'm not sure I understood then, and certainly not sure I
> understand now, but I said:
>
802.11be allows only few patterns when AP is operating in non-OFDMA mode
but if OFDMA is used then each 80 MHz sub-block can have a different
puncturing pattern when BW > 80MHz.
I know *_HE was not the best terminology, originally it was *_OFDMA but
later changed because we decided to base the puncturing bitmap
validation based on HE vs older modes.
Function "cfg80211_ru_punct_bitmap_valid" added in this version first
checks for non-OFDMA patterns, and only if "ru_punct_bitmap_supp_he"
attribute is set by the userspace then it goes further to also check
against patterns allowed for OFDMA.
I could not find any other way to decide OFDMA vs non-OFDMA than letting
userspace explicitly indicate latter.
It would be great if you can provide your inputs on this.
>> That wasn't really the question though. Consider this:
>>
>> Say you have STA + STA, if the first STA is connected to an AP with no
>> puncturing, and the second STA is connected to an AP where the channel
>> and bandwidth are the same, but some puncturing is done, should that
>> really be two channel contexts as far as mac80211 is concerned, and thus
>> require channels=2 in the interface combinations etc.? It doesn't seem
>> right to me.
>>
>> Or consider AP + STA, where the AP is set up for some channel but the
>> STA is connecting to an AP on the exact same channel, but with
>> puncturing... Again, same thing, I don't think it should consume two
>> channel resources.
>
> Which, actually, I've learned since that I was completely wrong about!
> It should, and likely must, in fact be two separate channel contexts,
> with all the limitations that implies.
>
> The thing is - back then I was making not just one, but in fact *two*
> wrong assumptions:
>
> 1. The DSP/radio can receive punctured PPDUs if listening on the non
> punctured channel.
>
> At least for our device that's not true, not sure about ath12k? It
> seems you have a per-peer puncturing configuration even, but that
> seems odd, and it's always just set to the vif puncturing
> configuration.
>
Yes, same vif puncturing pattern is assigned for all the peers
associated on that vif, but firmware requires it to be sent separately
for each peer.
> 2. You can simply transmit punctured PPDUs when on a non-punctured
> channel, i.e. it's just a rate control decision.
>
> This is perhaps less important, but it's also not really true.
> While you can clearly _transmit_ this way, that's not the only
> thing - you also need to do the CCA before transmitting, and if
> there's noise/interference on the punctured channel, you'd much
> more rarely find the channel to be clear and be able to transmit
> if this doesn't consider the puncturing, but that's something to
> do sort of generally in the background for the transmit.
>
> It might be possible to work around #2, but I'm not sure it's possible
> to work around #1?
>
>
> So I think I have two questions:
> A. Would you object if I moved the puncturing into the chandef after
> all?
>
This is where I'm getting confused.
The main reason to put in chandef was that I thought of the bitmap as a
radio characteristic (not vif). But after you brought up that AP+STA
mode can have different bitmaps, even though all other channel
characteristics (width, cf etc) are same, I realized my original
assumption wrong incorrect.
Moving the bitmap to cfg80211_ap_settings() meant that each AP vif can
have different bitmap, and I'm guessing you similarly added for each STA
vif context.
Now if you move it back into chandef, how exactly will this work if you
need different bitmaps?
> B. How does ath12k cope #1/#2 above? Would we need to have a callback
> to the driver to compare if two channel contexts are compatible or
> not (e.g. if they have different puncturing), or does ath12k also
> have limitations on RX/TX that mean it would actually prefer two
> channel contexts for the cases I had outlined in the quoted text
> above (STA+STA/AP+STA)?
>
If we do end up moving the bitmap back to chandef, we may need some
changes, because as I said above, when I originally added it I hadn't
thought of different bitmaps for each vif.
But can you give an example of what you would consider as compatible
channel contexts and what would be incompatible? I'm not clear on that part.
Thanks,
Aloka.
Hi,
> > > > Regarding if different puncturing pattern should be considered as a new
> > > > context - yes, depending on if it is HE or non-HE mode, the new bitmap
> > > > may be invalid and the operation should fail.
> >
> > Which I'm not sure I understood then, and certainly not sure I
> > understand now, but I said:
> >
>
> 802.11be allows only few patterns when AP is operating in non-OFDMA mode
> but if OFDMA is used then each 80 MHz sub-block can have a different
> puncturing pattern when BW > 80MHz.
Right, but that's not the same, it's per PPDU more or less, no? I mean,
you say in the trigger frame for example that some RU is not allocated.
So is that relevant here?
> I know *_HE was not the best terminology, originally it was *_OFDMA but
> later changed because we decided to base the puncturing bitmap
> validation based on HE vs older modes.
> Function "cfg80211_ru_punct_bitmap_valid" added in this version first
> checks for non-OFDMA patterns, and only if "ru_punct_bitmap_supp_he"
> attribute is set by the userspace then it goes further to also check
> against patterns allowed for OFDMA.
> I could not find any other way to decide OFDMA vs non-OFDMA than letting
> userspace explicitly indicate latter.
> It would be great if you can provide your inputs on this.
This ... doesn't exist upstream?
OK actually it did exist in this patch though.
Not sure I understand. Let's go back to what we have in the tree now.
The only thing we check there is
cfg80211_valid_disable_subchannel_bitmap(), which on the client at least
is just for the "disabled subchannel bitmap" in the EHT operation.
Are you thinking about (separately?) configuring the OFDMA puncturing?
Which spec-wise you do per PPDU, controlled by the AP (trigger frame), I
think?
> > Which, actually, I've learned since that I was completely wrong about!
> > It should, and likely must, in fact be two separate channel contexts,
> > with all the limitations that implies.
> >
> > The thing is - back then I was making not just one, but in fact *two*
> > wrong assumptions:
> >
> > 1. The DSP/radio can receive punctured PPDUs if listening on the non
> > punctured channel.
> >
> > At least for our device that's not true, not sure about ath12k? It
> > seems you have a per-peer puncturing configuration even, but that
> > seems odd, and it's always just set to the vif puncturing
> > configuration.
> >
>
> Yes, same vif puncturing pattern is assigned for all the peers
> associated on that vif, but firmware requires it to be sent separately
> for each peer.
OK, thanks.
What if it differs for different vifs?
> > 2. You can simply transmit punctured PPDUs when on a non-punctured
> > channel, i.e. it's just a rate control decision.
> >
> > This is perhaps less important, but it's also not really true.
> > While you can clearly _transmit_ this way, that's not the only
> > thing - you also need to do the CCA before transmitting, and if
> > there's noise/interference on the punctured channel, you'd much
> > more rarely find the channel to be clear and be able to transmit
> > if this doesn't consider the puncturing, but that's something to
> > do sort of generally in the background for the transmit.
> >
> > It might be possible to work around #2, but I'm not sure it's possible
> > to work around #1?
> >
> >
> > So I think I have two questions:
> > A. Would you object if I moved the puncturing into the chandef after
> > all?
> >
>
> This is where I'm getting confused.
:)
> The main reason to put in chandef was that I thought of the bitmap as a
> radio characteristic (not vif).
Right.
> But after you brought up that AP+STA
> mode can have different bitmaps, even though all other channel
> characteristics (width, cf etc) are same, I realized my original
> assumption wrong incorrect.
So I convinced you, I guess, but what I'm saying is that - at least as
far as our hardware is concerned - I was wrong!
Thing is: you're not just transmitting with this bitmap, you're also
listening - for both CCA and RX - in a specific way. And at least the
way our hardware works, we apparently can't do puncturing just based on
the preamble, and can't do CCA depending on the next frame.
So that means the (non-OFDMA) puncturing bitmap *does* in fact become a
radio characteristic.
I don't know though is if that's really true for all hardware in
general, or just a side effect of our design. I could see that it might
be possible to receive punctured/non-punctured without changing hardware
configuration, and certainly that it might be doable to do CCA depending
on which frame you're going to transmit.
But in any case, as far as I'm told the hardware design we have doesn't
allow that, so I think I'd like to move this to the chandef/chanctx, and
then perhaps define a driver callback to determine compatibility, if
needed?
> Moving the bitmap to cfg80211_ap_settings() meant that each AP vif can
> have different bitmap, and I'm guessing you similarly added for each STA
> vif context.
Yes.
> Now if you move it back into chandef, how exactly will this work if you
> need different bitmaps?
You'd get two chanctx since it's not compatible, unless we define some
extra callback or hw flags to determine what's treated as compatible and
what isn't. But see above - I actually want that, now that I know how
the HW works :)
> > B. How does ath12k cope #1/#2 above? Would we need to have a callback
> > to the driver to compare if two channel contexts are compatible or
> > not (e.g. if they have different puncturing), or does ath12k also
> > have limitations on RX/TX that mean it would actually prefer two
> > channel contexts for the cases I had outlined in the quoted text
> > above (STA+STA/AP+STA)?
> >
>
> If we do end up moving the bitmap back to chandef, we may need some
> changes, because as I said above, when I originally added it I hadn't
> thought of different bitmaps for each vif.
> But can you give an example of what you would consider as compatible
> channel contexts and what would be incompatible? I'm not clear on that part.
Easy example:
* control channel 36, 80 MHz, puncturing bitmap 0x2
* control channel 36, 80 MHz, puncturing bitmap 0
Contrary to what I thought and said before, I want to treat these as
*not* compatible now, and allocate two channel contexts if I end up
having to do this.
johannes
On 10/18/2023 5:58 AM, Johannes Berg wrote:
>
> Are you thinking about (separately?) configuring the OFDMA puncturing?
> Which spec-wise you do per PPDU, controlled by the AP (trigger frame), I
> think?
>
Need to study the spec again so not any time soon.
Will send a new series if it is needed.
>
>>> 1. The DSP/radio can receive punctured PPDUs if listening on the non
>>> punctured channel.
>>>
>>> At least for our device that's not true, not sure about ath12k? It
>>> seems you have a per-peer puncturing configuration even, but that
>>> seems odd, and it's always just set to the vif puncturing
>>> configuration.
>>>
>>
>> Yes, same vif puncturing pattern is assigned for all the peers
>> associated on that vif, but firmware requires it to be sent separately
>> for each peer.
>
> OK, thanks.
>
> What if it differs for different vifs?
>
So far that use-case hasn't come up but I'm confirming if we really need
that support or not. Will get back you.
>
>> The main reason to put in chandef was that I thought of the bitmap as a
>> radio characteristic (not vif).
>
> Right.
>
>> But after you brought up that AP+STA
>> mode can have different bitmaps, even though all other channel
>> characteristics (width, cf etc) are same, I realized my original
>> assumption wrong incorrect.
>
> So I convinced you, I guess, but what I'm saying is that - at least as
> far as our hardware is concerned - I was wrong!
>
> Thing is: you're not just transmitting with this bitmap, you're also
> listening - for both CCA and RX - in a specific way. And at least the
> way our hardware works, we apparently can't do puncturing just based on
> the preamble, and can't do CCA depending on the next frame.
>
> So that means the (non-OFDMA) puncturing bitmap *does* in fact become a
> radio characteristic.
>
Got it.
>
>> Now if you move it back into chandef, how exactly will this work if you
>> need different bitmaps?
>
> You'd get two chanctx since it's not compatible, unless we define some
> extra callback or hw flags to determine what's treated as compatible and
> what isn't. But see above - I actually want that, now that I know how
> the HW works :)
>
>>> B. How does ath12k cope #1/#2 above? Would we need to have a callback
>>> to the driver to compare if two channel contexts are compatible or
>>> not (e.g. if they have different puncturing), or does ath12k also
>>> have limitations on RX/TX that mean it would actually prefer two
>>> channel contexts for the cases I had outlined in the quoted text
>>> above (STA+STA/AP+STA)?
>>>
>>
>> If we do end up moving the bitmap back to chandef, we may need some
>> changes, because as I said above, when I originally added it I hadn't
>> thought of different bitmaps for each vif.
>> But can you give an example of what you would consider as compatible
>> channel contexts and what would be incompatible? I'm not clear on that part.
>
> Easy example:
>
> * control channel 36, 80 MHz, puncturing bitmap 0x2
> * control channel 36, 80 MHz, puncturing bitmap 0
>
> Contrary to what I thought and said before, I want to treat these as
> *not* compatible now, and allocate two channel contexts if I end up
> having to do this.
>
> johannes
I'm okay if you want to move it back to chandef, in fact I myself can
send a series for it.
As far as two contexts are concerned, sounds like you don't need that
for your use-case. And I will confirm if we need it or not.
Thanks.