diff mbox series

wifi: mac80211: override HE/EHT capabilities if NSS is zero

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

Commit Message

Ajith C Aug. 7, 2024, 4:19 a.m. UTC
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(+)

Comments

Johannes Berg Aug. 23, 2024, 10:37 a.m. UTC | #1
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
Ajith C Aug. 23, 2024, 1:25 p.m. UTC | #2
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
Johannes Berg Aug. 28, 2024, 12:33 p.m. UTC | #3
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
diff mbox series

Patch

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,