| Message ID | 20240424064019.4847-1-quic_bqiang@quicinc.com (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [v2] wifi: ath11k: move power type check to ASSOC stage when connecting to 6 GHz AP | expand |
Baochen Qiang <quic_bqiang@quicinc.com> writes: > With commit bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") > ath11k fails to connect to 6 GHz AP. > > This is because currently ath11k checks AP's power type in > ath11k_mac_op_assign_vif_chanctx() which would be called in AUTH stage. > However with above commit power type is not available until ASSOC stage. > As a result power type check fails and therefore connection fails. > > Fix this by moving power type check to ASSOC stage, also move regulatory > rules update there because it depends on power type. > > Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.30 > > Fixes: bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") > Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com> > --- > v2: > - add check on 'arvif->chanctx.def.chan' in ath11k_mac_op_sta_state() > to avoid NULL pointer dereference. > - add 'Fixes:' tag. Thanks, this seems to fix the crash I saw.
On 4/23/2024 11:40 PM, Baochen Qiang wrote: > With commit bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") > ath11k fails to connect to 6 GHz AP. > > This is because currently ath11k checks AP's power type in > ath11k_mac_op_assign_vif_chanctx() which would be called in AUTH stage. > However with above commit power type is not available until ASSOC stage. > As a result power type check fails and therefore connection fails. > > Fix this by moving power type check to ASSOC stage, also move regulatory > rules update there because it depends on power type. > > Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.30 > > Fixes: bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") > Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
Baochen Qiang <quic_bqiang@quicinc.com> writes: > With commit bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") > ath11k fails to connect to 6 GHz AP. > > This is because currently ath11k checks AP's power type in > ath11k_mac_op_assign_vif_chanctx() which would be called in AUTH stage. > However with above commit power type is not available until ASSOC stage. > As a result power type check fails and therefore connection fails. > > Fix this by moving power type check to ASSOC stage, also move regulatory > rules update there because it depends on power type. > > Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.30 > > Fixes: bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") > Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com> Oh, this fell through the cracks. Commit bc8a0fac8677 was introduced in v6.9-rc1 so I should have sent this to v6.9 but it's too late now. I'll need to queue this for v6.10 via wireless tree. Adding the regression also to regzbot: #regzbot introduced: bc8a0fac8677 #regzbot title: ath11k: connection to 6 GHz AP fails
On 11.05.24 11:54, Kalle Valo wrote: > Baochen Qiang <quic_bqiang@quicinc.com> writes: > >> With commit bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") >> ath11k fails to connect to 6 GHz AP. >> >> This is because currently ath11k checks AP's power type in >> ath11k_mac_op_assign_vif_chanctx() which would be called in AUTH stage. >> However with above commit power type is not available until ASSOC stage. >> As a result power type check fails and therefore connection fails. >> >> Fix this by moving power type check to ASSOC stage, also move regulatory >> rules update there because it depends on power type. >> >> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.30 >> >> Fixes: bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") >> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com> > > Oh, this fell through the cracks. Commit bc8a0fac8677 was introduced in > v6.9-rc1 so I should have sent this to v6.9 but it's too late now. I'll > need to queue this for v6.10 via wireless tree. I might have missed something, but from here it looks like that hasn't happened yet. If I missed something or if you are busy with more critical stuff, feel free to ignore this mail; if it fell through the cracks again, consider this a friendly reminder that this is still waiting for processing. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. #regzbot poke
On 5/11/2024 5:54 PM, Kalle Valo wrote: > Baochen Qiang <quic_bqiang@quicinc.com> writes: > >> With commit bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") >> ath11k fails to connect to 6 GHz AP. >> >> This is because currently ath11k checks AP's power type in >> ath11k_mac_op_assign_vif_chanctx() which would be called in AUTH stage. >> However with above commit power type is not available until ASSOC stage. >> As a result power type check fails and therefore connection fails. >> >> Fix this by moving power type check to ASSOC stage, also move regulatory >> rules update there because it depends on power type. >> >> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.30 >> >> Fixes: bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") >> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com> > > Oh, this fell through the cracks. Commit bc8a0fac8677 was introduced in > v6.9-rc1 so I should have sent this to v6.9 but it's too late now. I'll > need to queue this for v6.10 via wireless tree. > > Adding the regression also to regzbot: > > #regzbot introduced: bc8a0fac8677 > #regzbot title: ath11k: connection to 6 GHz AP fails > Hi Kalle, with an upcoming patch this regression is expected to be fixed: https://lore.kernel.org/all/20240506214536.310434f55f76.I6aca291ee06265e3f63e0f9024ba19a850b53a33@changeid/#t So here the ath11k fix would not be needed any more once above patch got merged. But I don't have time to test this, so suggest keeping it pending. We could drop it once above analysis got verified.
On 17.05.24 04:14, Baochen Qiang wrote: > On 5/11/2024 5:54 PM, Kalle Valo wrote: >> Baochen Qiang <quic_bqiang@quicinc.com> writes: >> >>> With commit bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") >>> ath11k fails to connect to 6 GHz AP. >>> >>> This is because currently ath11k checks AP's power type in >>> ath11k_mac_op_assign_vif_chanctx() which would be called in AUTH stage. >>> However with above commit power type is not available until ASSOC stage. >>> As a result power type check fails and therefore connection fails. >>> >>> Fix this by moving power type check to ASSOC stage, also move regulatory >>> rules update there because it depends on power type. >>> >>> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.30 >>> >>> Fixes: bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") >>> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com> Quick question: how does the error actually look like? I noticed a bug report where someone complained problems connecting "to my Wifi 7 AP on 6GHz band using my Lenovo P14s Gen 4 laptop" and wonder if it's the same or a different problem: https://bugzilla.kernel.org/show_bug.cgi?id=218838 To quote part of it: """ Sometimes when the connection fails the following appears on the logs invalid HE MCS: bw:6, ru:6 WARNING: CPU: 13 PID: 1242 at net/wireless/util.c:1551 cfg80211_calculate_bitrate_he+0x1b7/0x3c0 [cfg80211] CPU: 13 PID: 1242 Comm: NetworkManager Not tainted 6.9.0-0.rc7.20240510git448b3fe5a0ea.62.fc41.x86_64 #1 Hardware name: LENOVO 21K5000DMX/21K5000DMX, BIOS R2FET56W (1.36 ) 03/13/2024 RIP: 0010:cfg80211_calculate_bitrate_he+0x1b7/0x3c0 [cfg80211] Code: 00 40 80 fe 06 0f 84 a1 00 00 00 40 80 fe 03 0f 84 d6 00 00 00 40 84 f6 0f 84 0b 01 00 00 48 c7 c7 41 33 96 c1 e8 59 cd 8d e8 <0f> 0b 31 c0 eb 53 44 0f b6 e8 3c 02 0f 87 62 01 00 00 42 8b 44 ac RSP: 0018:ffffa5f2c6df7320 EFLAGS: 00010286 RAX: 0000000000000000 RBX: ffffa5f2c6df7518 RCX: 0000000000000027 RDX: ffff9166df0a18c8 RSI: 0000000000000001 RDI: ffff9166df0a18c0 RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000 R10: ffffffffc196335e R11: 0000000000000001 R12: 0000000000000009 R13: ffff91580eae0000 R14: ffff915b0ed33ef0 R15: ffff915a8b051b00 FS: 00007f7d396cd580(0000) GS:ffff9166df080000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00003d7c6e07d000 CR3: 0000000137282000 CR4: 0000000000f50ef0 PKRU: 55555554 Call Trace: <TASK> ? cfg80211_calculate_bitrate_he+0x1b7/0x3c0 [cfg80211] ? __warn.cold+0x8e/0xe8 ? cfg80211_calculate_bitrate_he+0x1b7/0x3c0 [cfg80211] ? report_bug+0xff/0x140 ? handle_bug+0x3c/0x80 ? exc_invalid_op+0x17/0x70 ? asm_exc_invalid_op+0x1a/0x20 ? cfg80211_calculate_bitrate_he+0x1b7/0x3c0 [cfg80211] nl80211_put_sta_rate+0x5b/0x4f0 [cfg80211] nl80211_send_station+0x9f0/0x1060 [cfg80211] nl80211_dump_station+0xef/0x280 [cfg80211] genl_dumpit+0x33/0x90 netlink_dump+0x151/0x3b0 [...] """ >> Oh, this fell through the cracks. Commit bc8a0fac8677 was introduced in >> v6.9-rc1 so I should have sent this to v6.9 but it's too late now. I'll >> need to queue this for v6.10 via wireless tree. >> >> Adding the regression also to regzbot: >> >> #regzbot introduced: bc8a0fac8677 >> #regzbot title: ath11k: connection to 6 GHz AP fails > > Hi Kalle, with an upcoming patch this regression is expected to be fixed: > > https://lore.kernel.org/all/20240506214536.310434f55f76.I6aca291ee06265e3f63e0f9024ba19a850b53a33@changeid/#t > > So here the ath11k fix would not be needed any more once above patch got merged. > > But I don't have time to test this, so suggest keeping it pending. We could drop it once above analysis got verified. Thx for the update. If that's the way forward I wonder if that change should have Fixes: and Stable: tags to ensure it ends up in 6.9.y -- ideally rather sooner than later if this is a regression users run into. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page.
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c index 3202e36f9663..69ad38e0efe0 100644 --- a/drivers/net/wireless/ath/ath11k/mac.c +++ b/drivers/net/wireless/ath/ath11k/mac.c @@ -7988,8 +7988,6 @@ ath11k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw, struct ath11k_base *ab = ar->ab; struct ath11k_vif *arvif = ath11k_vif_to_arvif(vif); int ret; - struct cur_regulatory_info *reg_info; - enum ieee80211_ap_reg_power power_type; mutex_lock(&ar->conf_mutex); @@ -8000,17 +7998,6 @@ ath11k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw, if (ath11k_wmi_supports_6ghz_cc_ext(ar) && ctx->def.chan->band == NL80211_BAND_6GHZ && arvif->vdev_type == WMI_VDEV_TYPE_STA) { - reg_info = &ab->reg_info_store[ar->pdev_idx]; - power_type = vif->bss_conf.power_type; - - ath11k_dbg(ab, ATH11K_DBG_MAC, "chanctx power type %d\n", power_type); - - if (power_type == IEEE80211_REG_UNSET_AP) { - ret = -EINVAL; - goto out; - } - - ath11k_reg_handle_chan_list(ab, reg_info, power_type); arvif->chanctx = *ctx; ath11k_mac_parse_tx_pwr_env(ar, vif, ctx); } @@ -9626,6 +9613,8 @@ static int ath11k_mac_op_sta_state(struct ieee80211_hw *hw, struct ath11k *ar = hw->priv; struct ath11k_vif *arvif = ath11k_vif_to_arvif(vif); struct ath11k_sta *arsta = ath11k_sta_to_arsta(sta); + enum ieee80211_ap_reg_power power_type; + struct cur_regulatory_info *reg_info; struct ath11k_peer *peer; int ret = 0; @@ -9705,6 +9694,29 @@ static int ath11k_mac_op_sta_state(struct ieee80211_hw *hw, ath11k_warn(ar->ab, "Unable to authorize peer %pM vdev %d: %d\n", sta->addr, arvif->vdev_id, ret); } + + if (!ret && + ath11k_wmi_supports_6ghz_cc_ext(ar) && + arvif->vdev_type == WMI_VDEV_TYPE_STA && + arvif->chanctx.def.chan && + arvif->chanctx.def.chan->band == NL80211_BAND_6GHZ) { + reg_info = &ar->ab->reg_info_store[ar->pdev_idx]; + power_type = vif->bss_conf.power_type; + + if (power_type == IEEE80211_REG_UNSET_AP) { + ath11k_warn(ar->ab, "invalid power type %d\n", + power_type); + ret = -EINVAL; + } else { + ret = ath11k_reg_handle_chan_list(ar->ab, + reg_info, + power_type); + if (ret) + ath11k_warn(ar->ab, + "failed to handle chan list with power type %d\n", + power_type); + } + } } else if (old_state == IEEE80211_STA_AUTHORIZED && new_state == IEEE80211_STA_ASSOC) { spin_lock_bh(&ar->ab->base_lock);
With commit bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") ath11k fails to connect to 6 GHz AP. This is because currently ath11k checks AP's power type in ath11k_mac_op_assign_vif_chanctx() which would be called in AUTH stage. However with above commit power type is not available until ASSOC stage. As a result power type check fails and therefore connection fails. Fix this by moving power type check to ASSOC stage, also move regulatory rules update there because it depends on power type. Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.30 Fixes: bc8a0fac8677 ("wifi: mac80211: don't set bss_conf in parsing") Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com> --- v2: - add check on 'arvif->chanctx.def.chan' in ath11k_mac_op_sta_state() to avoid NULL pointer dereference. - add 'Fixes:' tag. drivers/net/wireless/ath/ath11k/mac.c | 38 ++++++++++++++++++--------- 1 file changed, 25 insertions(+), 13 deletions(-) base-commit: 1b61047b44218a00c7a7836eff4f8e037d5634d8