| Message ID | 20250514-mlo-dfs-acs-v1-0-74e42a5583c6@quicinc.com |
|---|---|
| Headers | show |
| Series | wifi: cfg80211/mac80211: Handle simultaneous scan and DFS operations on different radios | expand |
On Wed, 2025-05-14 at 16:58 +0530, Raj Kumar Bhagat wrote: > > +int cfg80211_get_radio_idx_by_chan(struct wiphy *wiphy, > + const struct ieee80211_channel *chan) > +{ > + const struct wiphy_radio *radio; > + int i, j; > + u32 freq; > + > + if (!chan) > + return -EINVAL; > + > + freq = ieee80211_channel_to_khz(chan); > + for (i = 0; i < wiphy->n_radio; i++) { > + radio = &wiphy->radio[i]; > + for (j = 0; j < radio->n_freq_range; j++) { > + if (freq >= radio->freq_range[j].start_freq && > + freq <= radio->freq_range[j].end_freq) > + return i; > I believe we also discussed this in the past elsewhere, but I don't think the the >= and <= can simultaneously be wrong. If the frequency ranges for radios are adjacent, then the intervals here need to be half open. I _think_ it should be < instead of <=, and therefore a half-open interval of "[start, end[" (or "[start, end)" depending on your preferred notation.) johannes
On Fri, 2025-05-16 at 09:51 +0200, Johannes Berg wrote: > On Wed, 2025-05-14 at 16:58 +0530, Raj Kumar Bhagat wrote: > > > > +int cfg80211_get_radio_idx_by_chan(struct wiphy *wiphy, > > + const struct ieee80211_channel *chan) > > +{ > > + const struct wiphy_radio *radio; > > + int i, j; > > + u32 freq; > > + > > + if (!chan) > > + return -EINVAL; > > + > > + freq = ieee80211_channel_to_khz(chan); > > + for (i = 0; i < wiphy->n_radio; i++) { > > + radio = &wiphy->radio[i]; > > + for (j = 0; j < radio->n_freq_range; j++) { > > + if (freq >= radio->freq_range[j].start_freq && > > + freq <= radio->freq_range[j].end_freq) > > + return i; > > > > I believe we also discussed this in the past elsewhere, but I don't > think the the >= and <= can simultaneously be wrong. If the frequency "simultaneously be correct". I meant "correct", not "wrong"... johannes
On Wed, 2025-05-14 at 16:58 +0530, Raj Kumar Bhagat wrote: > > +static bool > +__ieee80211_is_scan_ongoing(struct wiphy *wiphy, > + struct ieee80211_local *local, > + struct cfg80211_chan_def *chandef) Any particular reason or the __ name? We usually have that for internal locking-related things, but here doesn't matter, and there's no non-__ version either? > +{ > + struct cfg80211_scan_request *scan_req; > + int chan_radio_idx, req_radio_idx; > + struct ieee80211_roc_work *roc; > + bool ret = false; > + > + if (!list_empty(&local->roc_list) || local->scanning) > + ret = true; > + > + if (wiphy->n_radio < 2) > + return ret; > + > + /* > + * Multiple HWs are grouped under same wiphy. If not scanning then > + * return now itself > + */ > + if (!ret) > + return ret; I don't fully understand this logic, and certainly not the comment. You can certainly "return false" here anyway or something. And initialize ret = list_empty || scanning or something, the whole thing is hard to follow? > + if (!list_empty(&local->roc_list)) { > + list_for_each_entry(roc, &local->roc_list, list) { There's no point in checking first before iterating, it's perfectly fine to iterate an empty list and do nothing while doing so ... Also patch-order wise, it seems this one really should go before the 2nd? johannes
On 5/16/2025 1:21 PM, Johannes Berg wrote: > On Wed, 2025-05-14 at 16:58 +0530, Raj Kumar Bhagat wrote: >> >> +int cfg80211_get_radio_idx_by_chan(struct wiphy *wiphy, >> + const struct ieee80211_channel *chan) >> +{ >> + const struct wiphy_radio *radio; >> + int i, j; >> + u32 freq; >> + >> + if (!chan) >> + return -EINVAL; >> + >> + freq = ieee80211_channel_to_khz(chan); >> + for (i = 0; i < wiphy->n_radio; i++) { >> + radio = &wiphy->radio[i]; >> + for (j = 0; j < radio->n_freq_range; j++) { >> + if (freq >= radio->freq_range[j].start_freq && >> + freq <= radio->freq_range[j].end_freq) >> + return i; >> > > I believe we also discussed this in the past elsewhere, but I don't > think the the >= and <= can simultaneously be wrong. If the frequency > ranges for radios are adjacent, then the intervals here need to be half > open. I _think_ it should be < instead of <=, and therefore a half-open > interval of "[start, end[" (or "[start, end)" depending on your > preferred notation.) > Sure, will use half-open interval ([start, end[) in next version.
This patch series addresses the intersection of scan and DFS operations with multi-radio wiphy case. The changes allow for more flexible handling of scan and DFS operations when both operations are on different radios within a same wiphy. --- Aditya Kumar Singh (1): wifi: mac80211: Allow DFS/CSA on a radio if scan is ongoing on another radio Raj Kumar Bhagat (1): wifi: mac80211: Allow scan on a radio while operating on DFS on another radio Vasanthakumar Thiagarajan (1): wifi: cfg80211: Add utility API to get radio index from channel include/net/cfg80211.h | 11 ++++++++ net/mac80211/cfg.c | 66 ++++++++++++++++++++++++++++++++++++++++++++-- net/mac80211/chan.c | 30 ++++++++++++++++++--- net/mac80211/ieee80211_i.h | 6 ++++- net/mac80211/offchannel.c | 5 +++- net/mac80211/scan.c | 20 +++++++++----- net/mac80211/util.c | 28 ++++++++++++++++++++ net/wireless/util.c | 24 +++++++++++++++++ 8 files changed, 176 insertions(+), 14 deletions(-) --- base-commit: 63a9a727d373fa5b8ce509eef50dbc45e0f745b9 change-id: 20250514-mlo-dfs-acs-33d7d9eea437