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.
--
Aditya
