| Message ID | 20240807041933.3196761-1-quic_ajithc@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: mac80211: override HE/EHT capabilities if NSS is zero | expand |
On Wed, 2024-08-07 at 09:49 +0530, Ajith C wrote: > Currently, when some stations send association requests > with the "Support for 320 MHz in 6 GHz band" flag enabled > in the EHT PHY Capabilities Information, but the Supported > EHT-MCS And NSS Set for 320 MHz is filled with zeros, or > Support for 160MHz in the 5GHz / Support for 80+80MHz in > the 5GHz band flags enabled in HE PHY Capabilities > Information, but the Supported EHT-MCS And NSS Set for > 160 MHz is filled with zeros, Causes the > ieee80211_sta_cap_rx_bw() function to choose a bandwidth > which has Supported NSS value zero. Not following here ... are you saying stations are sending bad association requests, hostapd/mac80211 accept them, and then we get 0 NSS? Why are we accepting them? Or are you saying we're sending them, but then that seems like a driver bug? johannes
On 8/23/2024 4:07 PM, Johannes Berg wrote: > On Wed, 2024-08-07 at 09:49 +0530, Ajith C wrote: >> Currently, when some stations send association requests >> with the "Support for 320 MHz in 6 GHz band" flag enabled >> in the EHT PHY Capabilities Information, but the Supported >> EHT-MCS And NSS Set for 320 MHz is filled with zeros, or >> Support for 160MHz in the 5GHz / Support for 80+80MHz in >> the 5GHz band flags enabled in HE PHY Capabilities >> Information, but the Supported EHT-MCS And NSS Set for >> 160 MHz is filled with zeros, Causes the >> ieee80211_sta_cap_rx_bw() function to choose a bandwidth >> which has Supported NSS value zero. > > Not following here ... are you saying stations are sending bad > association requests, hostapd/mac80211 accept them, and then we get 0 > NSS? Why are we accepting them? > > Or are you saying we're sending them, but then that seems like a driver > bug? > > johannes Hi Johannes, I’ve noticed that stations are sending association requests with zeros in the EHT-MCS and NSS fields. According to draft 6.0 (Table 9-417p), a value of zero is allowed for NSS to indicate ‘Not supported.’ Therefore, I believe we shouldn’t consider these as invalid requests. Additionally, since other lower bandwidths are supported, I thought it would be more appropriate to select the next available bandwidth rather than dropping the request. Thanks, Ajith C
On Fri, 2024-08-23 at 18:55 +0530, Ajith C wrote: > > I’ve noticed that stations are sending association requests with zeros > in the EHT-MCS and NSS fields. According to draft 6.0 (Table 9-417p), > a value of zero is allowed for NSS to indicate ‘Not supported.’ > Therefore, I believe we shouldn’t consider these as invalid requests. OK, that sounds different... > Additionally, since other lower bandwidths are supported, I thought > it would be more appropriate to select the next available bandwidth > rather than dropping the request. I'm not sure I see why. You're talking about ieee80211_sta_cap_rx_bw(), and if the STA says it has a certain capability we should probably believe it? Munging the capabilities there seems pretty wrong, and *especially* doing it if it e.g. has no RX or TX for a given bandwidth - I guess in theory then it's possible that it's saying it can receive but won't transmit (which we should probably not care about), or it can transmit but not receive (which should impact rate control). It doesn't seem right to assume that it will not use say 160 MHz if it doesn't have RX MCS/NSS support for 160 MHz, I'd say? Or only has partial support, for some NSSes? It seems you should solve whatever problem you have here in rate control instead, but I'm not even sure what problem you have. johannes
Circling back to this, still had it on my list (but am going to remove it now) On Fri, 2024-08-23 at 18:55 +0530, Ajith C wrote: > > I’ve noticed that stations are sending association requests with zeros > in the EHT-MCS and NSS fields. According to draft 6.0 (Table 9-417p), > a value of zero is allowed for NSS to indicate ‘Not supported.’ > Therefore, I believe we shouldn’t consider these as invalid requests. I still don't think it's valid to connect in 320 MHz and then not have any rates for 320 MHz, so I'd be totally on board with hostapd simply refusing this. johannes
diff --git a/net/mac80211/eht.c b/net/mac80211/eht.c index ddc7acc68335..ed9a3d492414 100644 --- a/net/mac80211/eht.c +++ b/net/mac80211/eht.c @@ -7,6 +7,46 @@ #include "ieee80211_i.h" +static void +ieee80211_eht_override_peer_capabilities(u8 *he_info, u8 *eht_info, + const u8 *mcs_set) +{ + u8 offset_320mhz_set = 3; + + if (((*he_info) & + (IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G | + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G))) { + /* for 160 MHz bandwidth, if none of the MCS-NSS set + * has a minimum NSS of 1 for both Rx and Tx, disable + * support for 160 MHz bandwidth by resetting + * corresponding flag bits of HE capabilities IE + */ + if (((mcs_set[3] & 0x0F) == 0x00 || (mcs_set[3] & 0xF0) == 0x00) && + ((mcs_set[4] & 0x0F) == 0x00 || (mcs_set[4] & 0xF0) == 0x00) && + ((mcs_set[5] & 0x0F) == 0x00 || (mcs_set[5] & 0xF0) == 0x00)) { + (*he_info) &= + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G; + (*he_info) &= + ~IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G; + } + offset_320mhz_set += 3; + } + + /* for 320 MHz bandwidth, if none of the MCS-NSS set + * has a minimum NSS of 1 for both Rx and Tx, disable + * support for 320 MHz bandwidth by resetting + * corresponding flag bit of EHT capabilities IE + */ + if (((*eht_info) & IEEE80211_EHT_PHY_CAP0_320MHZ_IN_6GHZ) && + ((mcs_set[offset_320mhz_set] & 0x0F) == 0x00 || + (mcs_set[offset_320mhz_set] & 0xF0) == 0x00) && + ((mcs_set[offset_320mhz_set + 1] & 0x0F) == 0x00 || + (mcs_set[offset_320mhz_set + 1] & 0xF0) == 0x00) && + ((mcs_set[offset_320mhz_set + 2] & 0x0F) == 0x00 || + (mcs_set[offset_320mhz_set + 2] & 0xF0) == 0x00)) + (*eht_info) &= ~IEEE80211_EHT_PHY_CAP0_320MHZ_IN_6GHZ; +} + void ieee80211_eht_cap_ie_to_sta_eht_cap(struct ieee80211_sub_if_data *sdata, struct ieee80211_supported_band *sband, @@ -65,6 +105,13 @@ ieee80211_eht_cap_ie_to_sta_eht_cap(struct ieee80211_sub_if_data *sdata, memset(&eht_cap->eht_mcs_nss_supp, 0, sizeof(eht_cap->eht_mcs_nss_supp)); memcpy(&eht_cap->eht_mcs_nss_supp, pos, mcs_nss_size); + /* Override station bandwidth capabilities + * if bandwidth is unsupported in MCS-NSS set + */ + ieee80211_eht_override_peer_capabilities + (&link_sta->pub->he_cap.he_cap_elem.phy_cap_info[0], + &eht_cap->eht_cap_elem.phy_cap_info[0], + eht_cap_ie_elem->optional); if (eht_ppe_size) memcpy(eht_cap->eht_ppe_thres,
Currently, when some stations send association requests with the "Support for 320 MHz in 6 GHz band" flag enabled in the EHT PHY Capabilities Information, but the Supported EHT-MCS And NSS Set for 320 MHz is filled with zeros, or Support for 160MHz in the 5GHz / Support for 80+80MHz in the 5GHz band flags enabled in HE PHY Capabilities Information, but the Supported EHT-MCS And NSS Set for 160 MHz is filled with zeros, Causes the ieee80211_sta_cap_rx_bw() function to choose a bandwidth which has Supported NSS value zero. This leads to obtaining an Rx NSS value of 0 in the drivers which obtains Rx NSS from the Supported EHT-MCS And NSS Set corresponding to the selected bandwidth. Address association requests with conflicts between capabilities flags and MCS-NSS set by overriding the station capabilities flags to disable the bandwidth support which has NSS 0. Signed-off-by: Ajith C <quic_ajithc@quicinc.com> --- net/mac80211/eht.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)