| Message ID | 20250513-bug_fix_mlo_scan-v1-0-94235bb42fbe@oss.qualcomm.com |
|---|---|
| Headers | show |
| Series | wifi: mac80211: some bug fixes in MLO scan handling | expand |
On Tue, 2025-05-13 at 09:26 +0530, Aditya Kumar Singh wrote: > > - if (sdata->deflink.u.ap.beacon && > + if ((sdata->deflink.u.ap.beacon || > + ieee80211_num_beaconing_links(sdata)) && > Do we even still need the deflink check? Seems like num_beaconing_links() _should_ return 1 anyway even though it currently doesn't, and we need to fix that? Also seems the VLAN carrier handling is broken. johannes
On 5/13/2025 12:47 PM, Johannes Berg wrote: > On Tue, 2025-05-13 at 09:26 +0530, Aditya Kumar Singh wrote: >> >> - if (sdata->deflink.u.ap.beacon && >> + if ((sdata->deflink.u.ap.beacon || >> + ieee80211_num_beaconing_links(sdata)) && >> > > Do we even still need the deflink check? Seems like > num_beaconing_links() _should_ return 1 anyway even though it currently > doesn't, and we need to fix that? If the ieee80211_num_beaconing_links() is modified then deflink check is not required. Do you want to me to send a clean up for that function first or would take this and later the clean up part? > > Also seems the VLAN carrier handling is broken. With this patch? Or in general you are saying? HWSIM test cases seems to be working fine for me with this series applied. May be there is no test case to make it evident?
On 5/13/2025 3:51 PM, Johannes Berg wrote: > On Tue, 2025-05-13 at 15:47 +0530, Aditya Kumar Singh wrote: >> On 5/13/2025 12:47 PM, Johannes Berg wrote: >>> On Tue, 2025-05-13 at 09:26 +0530, Aditya Kumar Singh wrote: >>>> >>>> - if (sdata->deflink.u.ap.beacon && >>>> + if ((sdata->deflink.u.ap.beacon || >>>> + ieee80211_num_beaconing_links(sdata)) && >>>> >>> >>> Do we even still need the deflink check? Seems like >>> num_beaconing_links() _should_ return 1 anyway even though it currently >>> doesn't, and we need to fix that? >> >> If the ieee80211_num_beaconing_links() is modified then deflink check is >> not required. Do you want to me to send a clean up for that function >> first or would take this and later the clean up part? > > Given that you're targeting wireless-next for all, I guess I'd prefer > you clean up ieee80211_num_beaconing_links() first? But no strong > preferences. Okay sure let me first send a clean up. So, ieee80211_num_beaconing_links() should return 1 for non-MLO as well? And callers should test for == 1 instead of <= 1. > >>> Also seems the VLAN carrier handling is broken. >> With this patch? Or in general you are saying? HWSIM test cases seems to >> be working fine for me with this series applied. May be there is no test >> case to make it evident? > > Oh, I meant in general. > > So here I looked at callers of ieee80211_num_beaconing_links(), and many > of them seemed wrong because it doesn't handle non-MLO? But now that I > look again, I'm actually wrong, it simply always returns 0 for non-MLO, > but the comparisons are for <=1 which makes that ... OK but unexpected I > guess. > > > But still - also unrelated to this patch - the VLAN handling here seems > wrong? > Yes VLAN handling with MLO is not handled with the change which brought this beaconing links API. > if (ieee80211_num_beaconing_links(sdata) <= 1) > netif_carrier_on(dev); > > list_for_each_entry(vlan, &sdata->u.ap.vlans, u.vlan.list) > netif_carrier_on(vlan->dev); > > Shouldn't that loop be inside the ieee80211_num_beaconing_links() if? > Also on the netif_carrier_off() side? At least someone was fixing VLAN > vs. MLO recently, so maybe that needs fixes too. > Yes. MLO VLAN changes should handle this part. Since at this moment, code does not claim to support MLO VLAN fully. So change introducing the support should ideally handle it.
On 5/13/2025 4:18 PM, Johannes Berg wrote: > On Tue, 2025-05-13 at 16:02 +0530, Aditya Kumar Singh wrote: >> >> Okay sure let me first send a clean up. So, >> ieee80211_num_beaconing_links() should return 1 for non-MLO as well? >> > > That would seem logical to me? For this and many other things non-MLO is > mostly equivalent to just having a single link (apart from the address > translation.) Yeah in a way true only. > >> And callers should test for == 1 instead of <= 1. > > Not even sure that matters enough to need to change? yeah can be left as it is. Sure then I will change the function alone to return 1 for non-MLO case as well. Thanks for the inputs :)
On 5/13/2025 4:26 PM, Aditya Kumar Singh wrote: > On 5/13/2025 4:18 PM, Johannes Berg wrote: >> On Tue, 2025-05-13 at 16:02 +0530, Aditya Kumar Singh wrote: >>> >>> Okay sure let me first send a clean up. So, >>> ieee80211_num_beaconing_links() should return 1 for non-MLO as well? >>> >> >> That would seem logical to me? For this and many other things non-MLO is >> mostly equivalent to just having a single link (apart from the address >> translation.) > > Yeah in a way true only. > >> >>> And callers should test for == 1 instead of <= 1. >> >> Not even sure that matters enough to need to change? > yeah can be left as it is. Sure then I will change the function alone to > return 1 for non-MLO case as well. > > Thanks for the inputs :) > Done. Here is the clean up patch for review - Link: https://lore.kernel.org/all/20250515-fix_num_beaconing_links-v1-1-4a39e2704314@oss.qualcomm.com/
This series addresses issues related to MLO handling in probe response acceptance and scan request validation. * Validate SCAN_FLAG_AP in scan request during MLO: Enforce the requirement for the NL80211_SCAN_FLAG_AP flag in scan requests when an AP interface is beaconing. Apply this restriction to ML interfaces by using the existing helper ieee80211_num_beaconing_links() to check if any link is beaconing. * Accept probe response on link address as well: Ensure unicast probe response frames are accepted if the destination address matches any of the link addresses when a random MAC address is not requested. This change corrects the behavior where probe response frames are dropped incorrectly for MLO interfaces. Both the changes are as such independent from each other. Since both are related to MLO scanning, currently kept in same series. --- Aditya Kumar Singh (2): wifi: mac80211: validate SCAN_FLAG_AP in scan request during MLO wifi: mac80211: accept probe response on link address as well net/mac80211/cfg.c | 3 ++- net/mac80211/scan.c | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) --- base-commit: 63a9a727d373fa5b8ce509eef50dbc45e0f745b9 change-id: 20250402-bug_fix_mlo_scan-fe57f57b1c86