On Mon, 2025-06-09 at 12:33 +0530, Maharaja Kennadyrajan wrote:
>
> +++ b/net/mac80211/mlme.c
> @@ -2439,6 +2439,19 @@ static void ieee80211_csa_switch_work(struct wiphy *wiphy,
> }
> }
>
> + /* It is not necessary to reset these timers if any link does not
nit: I think we should start/continue using normal "non-networking"
comment style. I even removed the special style from the networking
style guide, and it was never a good idea (IMHO) to start with ... but I
know we've not been super consistent either way.
Anyway that wasn't why I was commenting, rather:
> + for_each_link_data(sdata, link) {
> + if (!(link->conf && link->conf->csa_active))
That condition, and other code later, really seemed confusing to me, and
when I carefully thought about it I realised that you were afraid that
link->conf would be NULL? However, it can't, that just doesn't make any
sense.
Now then I thought _further_ about it and sent/applied this patch:
https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless.git/commit/?id=d87c3ca0f8f1ca4c25f2ed819e954952f4d8d709
but that's not even relevant for your changes since you have full wiphy
lock held, not just RCU protection.
So please let's not be overly paranoid with this, and remove the
unnecessary "link->conf != NULL" checks.
> +static bool
> +ieee80211_is_csa_in_progress(struct ieee80211_sub_if_data *sdata)
> +{
> + /* In MLO, check the CSA flags 'active' and 'waiting_bcn' for all
> + * the links.
> + */
> + unsigned int link_id;
> +
> + guard(rcu)();
> + for (link_id = 0; link_id < ARRAY_SIZE(sdata->link);
> + link_id++) {
maybe add a blank line there, the guard is pretty hidden this way
> + struct ieee80211_link_data *link =
> + rcu_dereference(sdata->link[link_id]);
> +
> + if (!link)
> + continue;
> +
> + if (!(link->conf && link->conf->csa_active &&
> + !link->u.mgd.csa.waiting_bcn))
> + return false;
here, technically, it _is_ just with RCU, and I guess we can wait until
I merge the other patch, but it's pretty unlikely to get into any issues
even without it.
johannes
