| Message ID | 20250120114551.1542812-1-pmartin-gomez@freebox.fr |
|---|---|
| State | New |
| Headers | show |
| Series | ieee80211: fix interopt issue with MT7927 chipset | expand |
On 20/01/2025 12:45, Pablo Martin-Gomez wrote: > - /* on 2.4 GHz, if it supports 40 MHz, the result is 3 */ > - if (he_cap->phy_cap_info[0] & > - IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_IN_2G) > - return 3; > + /* 20 MHz-only non-AP STA */ > + if (!from_ap && (he_cap->phy_cap_info[0] & > + (IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_IN_2G | > + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G | > + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G | > + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G)) == 0) > + return 4; > > - /* on 2.4 GHz, these three bits are reserved, so should be 0 */ > if (he_cap->phy_cap_info[0] & > - IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G) > + (IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_IN_2G | > + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G)) > count += 3; > > if (he_cap->phy_cap_info[0] & This patch is still reading reserved bits depending on the band being used. I wanted to do a new version of the patch to avoid doing that by passing the current band to ieee80211_eht_mcs_nss_size(). Unfortunately, ieee80211_eht_mcs_nss_size() is called by ieee80211_eht_capa_size_ok() which itself is called in places where the band is not known. So I'm not sure I can do better than this. Best regards, Pablo MG
On Mon, 2025-02-10 at 10:48 +0100, Pablo MARTIN-GOMEZ wrote: > On 20/01/2025 12:45, Pablo Martin-Gomez wrote: > > - /* on 2.4 GHz, if it supports 40 MHz, the result is 3 */ > > - if (he_cap->phy_cap_info[0] & > > - IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_IN_2G) > > - return 3; > > + /* 20 MHz-only non-AP STA */ > > + if (!from_ap && (he_cap->phy_cap_info[0] & > > + (IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_IN_2G | > > + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G | > > + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G | > > + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G)) == 0) > > + return 4; > > > > - /* on 2.4 GHz, these three bits are reserved, so should be 0 */ > > if (he_cap->phy_cap_info[0] & > > - IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G) > > + (IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_IN_2G | > > + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G)) > > count += 3; > > > > if (he_cap->phy_cap_info[0] & > > This patch is still reading reserved bits depending on the band being > used. I wanted to do a new version of the patch to avoid doing that by > passing the current band to ieee80211_eht_mcs_nss_size(). Unfortunately, > ieee80211_eht_mcs_nss_size() is called by ieee80211_eht_capa_size_ok() > which itself is called in places where the band is not known. So I'm not > sure I can do better than this. > Took me some time to look into this, but I don't think the band needs to be unknown. I'd still split this and use the existing version for *local* data, such as callers in net/wireless/nl80211.c, and have the band used for parsing in net/mac80211/parse.c. We already pass e.g. "from_ap" in the parsing context, so we can have the band too. Most callers aren't even affected since they don't need to parse this, it'd only be those in net/mac80211/mlme.c, and we can default to NUM_NL80211_BANDS (unknown) for ieee802_11_parse_elems_crc(). Want to take a stab at that? johannes
On 26/02/2025 15:33, Johannes Berg wrote: > On Mon, 2025-02-10 at 10:48 +0100, Pablo MARTIN-GOMEZ wrote: >> On 20/01/2025 12:45, Pablo Martin-Gomez wrote: >> [...] > Took me some time to look into this, but I don't think the band needs to > be unknown. I'd still split this and use the existing version for > *local* data, such as callers in net/wireless/nl80211.c, and have the > band used for parsing in net/mac80211/parse.c. We already pass e.g. > "from_ap" in the parsing context, so we can have the band too. Most > callers aren't even affected since they don't need to parse this, it'd > only be those in net/mac80211/mlme.c, and we can default to > NUM_NL80211_BANDS (unknown) for ieee802_11_parse_elems_crc(). > > Want to take a stab at that? I'm very new at any sort of kernel development, but I'm ready to try to figure it out given that you pre-chewed some of the work. I'll come back before the end of the month either with a RFC patch or a white flag. Pablo MG
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h index 16741e542e81..2e813824d52a 100644 --- a/include/linux/ieee80211.h +++ b/include/linux/ieee80211.h @@ -3196,14 +3196,17 @@ ieee80211_eht_mcs_nss_size(const struct ieee80211_he_cap_elem *he_cap, { u8 count = 0; - /* on 2.4 GHz, if it supports 40 MHz, the result is 3 */ - if (he_cap->phy_cap_info[0] & - IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_IN_2G) - return 3; + /* 20 MHz-only non-AP STA */ + if (!from_ap && (he_cap->phy_cap_info[0] & + (IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_IN_2G | + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G | + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G | + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G)) == 0) + return 4; - /* on 2.4 GHz, these three bits are reserved, so should be 0 */ if (he_cap->phy_cap_info[0] & - IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G) + (IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_IN_2G | + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G)) count += 3; if (he_cap->phy_cap_info[0] &
Mediatek's chipsets MT7927 and MT7925 with Windows driver 5.4.0.3044 (and earlier versions) set the IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_IN_2G bit in 5 & 6Ghz band, which is supposed to be reserved. Currently, the kernel assumes a reserved bit to be set to 0 and uses the bit value to deduce that the current band used is 2.4GHz. This causes the kernel to miscalculate mcs_nss_size to 3 bytes, resulting in incorrect rx/tx nss map, so the sta is believed to have 0 NSS for 160/320. Signed-off-by: Pablo Martin-Gomez <pmartin-gomez@freebox.fr> --- include/linux/ieee80211.h | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)