On 1/26/24 15:01, Johannes Berg wrote:
On Thu, 2024-01-25 at 18:34 +0530, Aditya Kumar Singh wrote:
Currently, during channel switch, deflink (or link_id 0)
The parenthetical is wrong: deflink isn't even a valid/used link *at
all* in MLD data structures. Speaking about "link_id 0" would be a valid
thing to do even for MLD, but speaking about "deflink" isn't.
Okay got it. Will address in next version.
+++ b/drivers/net/wireless/virtual/mac80211_hwsim.c
@@ -2285,7 +2285,7 @@ static void mac80211_hwsim_beacon_tx(void *arg, u8 *mac,
}
if (link_conf->csa_active && ieee80211_beacon_cntdwn_is_complete(vif))
- ieee80211_csa_finish(vif);
+ ieee80211_csa_finish(vif, link_id);
Might make sense to keep the actual logic changes out of this patch?
It's pretty big, and this is pretty hidden...
Sure so I will move this ieee80211_csa_finish() changes to a different
patch.
+++ b/net/mac80211/cfg.c
@@ -1588,6 +1588,8 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev,
link->csa_block_tx = false;
}
+ wiphy_work_cancel(wiphy, &link->csa_finalize_work);
You don't _really_ need that here (if it runs, it'll just find
csa_active==false), and I'd feel better about it if we actually had this
in ieee80211_link_stop() in some way, that's called from
ieee80211_tear_down_links() and then it's really obvious to see that
this is removed before freeing the link.
Yeah makes sense. Let me see what I can do here. Thanks for the inputs! :)
- if (sdata->vif.bss_conf.eht_puncturing != sdata->vif.bss_conf.csa_punct_bitmap) {
- sdata->vif.bss_conf.eht_puncturing =
- sdata->vif.bss_conf.csa_punct_bitmap;
+ if (link_conf->eht_puncturing != link_conf->csa_punct_bitmap) {
+ link_conf->eht_puncturing = link_conf->csa_punct_bitmap;
changed |= BSS_CHANGED_EHT_PUNCTURING;
}
Hm. I'm going to send some patches soon that remove the puncturing stuff
and move it into the chandef (as we discussed elsewhere), but just
noting that - doesn't need to concern you here, I think.
Oh okay! Let's see if at all it conflicts, will send a re-based once
those gets merged.
@@ -3875,16 +3894,23 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
if (sdata->wdev.cac_started)
return -EBUSY;
- if (cfg80211_chandef_identical(¶ms->chandef,
- &sdata->vif.bss_conf.chandef))
+ if (WARN_ON(link_id >= IEEE80211_MLD_MAX_NUM_LINKS))
+ return -EINVAL;
+
+ link_data = wiphy_dereference(wiphy, sdata->link[link_id]);
+ if (!link_data)
+ return -ENOLINK;
+
+ link_conf = link_data->conf;
+
+ if (cfg80211_chandef_identical(¶ms->chandef, &link_conf->chandef))
return -EINVAL;
Also another thing unrelated to your patch - I'm thinking about removing
that identical() check entirely - you might want to switch to the same
channel with quiet=1. At least for testing that'd be really useful, and
I don't think it really serves any purpose to forbid it.
Yeah, we can do. But is there any actual use case? Also, what if some
notorious user space application simply sends NL command without even
quiet=1? There should be some check I guess?
