Re: [PATCH wireless-next 2/3] wifi: mac80211: Allow scan on a radio while operating on DFS on another radio

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2025-05-14 at 16:58 +0530, Raj Kumar Bhagat wrote:
> Currently, in multi-radio wiphy cases, if one radio is operating on a DFS
> channel, -EBUSY is returned even when a scan is requested on a different
> radio. Because of this, an MLD AP with one radio (link) on a DFS channel
> and Automatic Channel Selection (ACS) on another radio (link) cannot be
> brought up.
> 
> In multi-radio wiphy cases, multiple radios are grouped under a single
> wiphy. Hence, if a radio is operating on a DFS channel and a scan is
> requested on a different radio of the same wiphy, the scan can be allowed
> simultaneously without impacting the DFS operations.
> 
> Add logic to check the underlying radio used for the requested scan. If the
> radio on which DFS is already running is not being used, allow the scan
> operation; otherwise, return -EBUSY.

So while I agree in principle, I think this needs to be more carefully
constructed because it relies on an unstated (?) assumption that each
radio is going to ever be used for scanning on a certain band. That
seems to make sense, and a radio will certainly only ever be able to
_operate_ on the frequencies listed for it (due to chanctx etc.), but is
it really true that it will never be able to operate at all on other
frequencies?

I'm not sure I find the notion of e.g. having a 5 and 6 GHz radio that
are used for operating on those bands, but being able to scan 5 GHz
channels using the 6 GHz radio completely unimaginable? Maybe it is
though and I'm just overly paranoid?

We could also just leave that up to the drivers, of course, but then I
think we should _state_ this assumption somewhere in the docs/header
file(s)?

> +bool ieee80211_is_radio_idx_in_scan_req(struct wiphy *wiphy,
> +					struct cfg80211_scan_request *scan_req,
> +					int radio_idx)
> +{
> +	struct ieee80211_channel *chan;
> +	int i, chan_radio_idx;
> +
> +	if (!scan_req)
> +		return false;

That seems overly paranoid, or maybe it should be WARN_ON()? I mean,
asking something about a scan request and then not giving one is just
the wrong thing to do in the first place, no?

And if you're going to be paranoid then this probably shouldn't be
called with an invalid/negative radio_idx either :)


> +	for (i = 0; i < scan_req->n_channels; i++) {
> +		chan = scan_req->channels[i];
> +		chan_radio_idx = cfg80211_get_radio_idx_by_chan(wiphy, chan);
> +		/*
> +		 * Skip channels with an invalid radio index and continue
> +		 * checking. If any channel in the scan request matches the
> +		 * given radio index, return true.
> +		 */
> +		if (chan_radio_idx < 0)
> +			continue;

This seems ... wrong? If there's a channel in the scan request that
didn't map to _any_ radio then how are we even scanning there? And the
comment seems even stranger, why would we _want_ to ignore it (which it
conveniently doesn't answer)?

johannes