On Thu, 2025-05-22 at 16:10 +0530, Raj Kumar Bhagat wrote:
>
> I'm not sure If I fully understood the this comment.
>
> This patch assumes that multiple radios are grouped under a single wiphy.
> Each radio has its own list of frequencies it can scan, and there is no overlap
> in frequencies between any two radios within the same wiphy.
Yeah I guess I'm just overly paranoid due to lack of familiarity with
all the multi-radio things.
> If this assumption holds, then if one radio is operating on a DFS channel and a
> new scan request does not include any frequencies from that radio's list, the
> scan should be allowed—since the DFS radio wouldn't be involved in handling that
> scan request.
Agree.
> > > + 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)?
> >
>
> It seems, (chan_radio_idx < 0) should never be true because the chan is
> taken from the valid scan request. I should remove this check in next version?
I'm not sure, why did you add it? Maybe it should be a WARN_ON and abort
the whole function? It just doesn't seem right to _ignore_.
johannes
