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.
> +++ 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...
> +++ 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.
> - 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.
> @@ -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.
johannes
