[PATCH wireless-next 0/3] wifi: cfg80211/mac80211: Handle simultaneous scan and DFS operations on different radios

[PATCH wireless-next 0/3] wifi: cfg80211/mac80211: Handle simultaneous scan and DFS operations on different radios

[Raj Kumar Bhagat]

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

[PATCH wireless-next 1/3] wifi: cfg80211: Add utility API to get radio index from channel

[Raj Kumar Bhagat]

From: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>

Add utility API cfg80211_get_radio_idx_by_chan() to retrieve the radio
index corresponding to a given channel in a multi-radio wiphy.

This utility function can be used when we want to check the radio-specific
data for a channel in a multi-radio wiphy. For example, it can help
determine the radio index required to handle a scan request. This index
can then be used to decide whether the scan can proceed without
interfering with ongoing DFS operations on another radio.

Signed-off-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>
Co-developed-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
---
 include/net/cfg80211.h | 11 +++++++++++
 net/wireless/util.c    | 24 ++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d1848dc8ec99df36592517b46d58223be9bee7e5..7719a90ab4d75045983ac8a63360fd90472cbb7d 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -9372,6 +9372,17 @@ int cfg80211_iter_combinations(struct wiphy *wiphy,
 			       void (*iter)(const struct ieee80211_iface_combination *c,
 					    void *data),
 			       void *data);
+/**
+ * cfg80211_get_radio_idx_by_chan - get the radio index by the channel
+ *
+ * @wiphy: the wiphy
+ * @chan: channel for which the supported radio index is required
+ *
+ * Return: radio index on success or a negative error code
+ */
+int cfg80211_get_radio_idx_by_chan(struct wiphy *wiphy,
+				   const struct ieee80211_channel *chan);
+
 
 /**
  * cfg80211_stop_iface - trigger interface disconnection
diff --git a/net/wireless/util.c b/net/wireless/util.c
index ed868c0f7ca8ed7a9e85e70c10a738f592eac1d6..8ba1d0b268fc350064b2b8adcfeae121c3dcab70 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -2516,6 +2516,30 @@ int cfg80211_check_combinations(struct wiphy *wiphy,
 }
 EXPORT_SYMBOL(cfg80211_check_combinations);
 
+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;
+		}
+	}
+
+	return -ENOENT;
+}
+EXPORT_SYMBOL(cfg80211_get_radio_idx_by_chan);
+
 int ieee80211_get_ratemask(struct ieee80211_supported_band *sband,
 			   const u8 *rates, unsigned int n_rates,
 			   u32 *mask)

-- 
2.34.1

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

[Raj Kumar Bhagat]

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.

Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
---
 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 ++++++++++++++++++++++++++++
 5 files changed, 77 insertions(+), 12 deletions(-)

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 3aaf5abf1acc13008a0472672c826d495c80407c..94c83f58e23d0666af881e5e0c981cca17231bfd 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -644,15 +644,39 @@ ieee80211_find_chanctx(struct ieee80211_local *local,
 	return NULL;
 }
 
-bool ieee80211_is_radar_required(struct ieee80211_local *local)
+bool ieee80211_is_radar_required(struct ieee80211_local *local,
+				 struct cfg80211_scan_request *req)
 {
+	struct wiphy *wiphy = local->hw.wiphy;
 	struct ieee80211_link_data *link;
+	struct ieee80211_channel *chan;
+	int radio_idx;
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
+	if (!req)
+		return false;
+
 	for_each_sdata_link(local, link) {
-		if (link->radar_required)
-			return true;
+		if (link->radar_required) {
+			if (wiphy->n_radio < 2)
+				return true;
+
+			chan = link->conf->chanreq.oper.chan;
+			radio_idx = cfg80211_get_radio_idx_by_chan(wiphy, chan);
+			/*
+			 * The radio index (radio_idx) is expected to be valid,
+			 * as it's derived from a channel tied to a link. If
+			 * it's invalid (i.e., negative), return true to avoid
+			 * potential issues with radar-sensitive operations.
+			 */
+			if (radio_idx < 0)
+				return true;
+
+			if (ieee80211_is_radio_idx_in_scan_req(wiphy, req,
+							       radio_idx))
+				return true;
+		}
 	}
 
 	return false;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 30809f0b35f73e77b05a6f802011a64900e1532f..a5050e3025b795a1c2a24f1a370ef7e05c2fde6a 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -2712,8 +2712,12 @@ void ieee80211_recalc_chanctx_min_def(struct ieee80211_local *local,
 				      struct ieee80211_chanctx *ctx,
 				      struct ieee80211_link_data *rsvd_for,
 				      bool check_reserved);
-bool ieee80211_is_radar_required(struct ieee80211_local *local);
+bool ieee80211_is_radar_required(struct ieee80211_local *local,
+				 struct cfg80211_scan_request *req);
 
+bool ieee80211_is_radio_idx_in_scan_req(struct wiphy *wiphy,
+					struct cfg80211_scan_request *scan_req,
+					int radio_idx);
 void ieee80211_dfs_cac_timer_work(struct wiphy *wiphy, struct wiphy_work *work);
 void ieee80211_dfs_cac_cancel(struct ieee80211_local *local,
 			      struct ieee80211_chanctx *chanctx);
diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index 2b9abc27462eb5a41f47523d1653c7f27137588d..686d9f6e9b527acaa3500e8d99d5ca513bb1025e 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -567,6 +567,7 @@ static int ieee80211_start_roc_work(struct ieee80211_local *local,
 {
 	struct ieee80211_roc_work *roc, *tmp;
 	bool queued = false, combine_started = true;
+	struct cfg80211_scan_request *req;
 	int ret;
 
 	lockdep_assert_wiphy(local->hw.wiphy);
@@ -612,9 +613,11 @@ static int ieee80211_start_roc_work(struct ieee80211_local *local,
 		roc->mgmt_tx_cookie = *cookie;
 	}
 
+	req = wiphy_dereference(local->hw.wiphy, local->scan_req);
+
 	/* if there's no need to queue, handle it immediately */
 	if (list_empty(&local->roc_list) &&
-	    !local->scanning && !ieee80211_is_radar_required(local)) {
+	    !local->scanning && !ieee80211_is_radar_required(local, req)) {
 		/* if not HW assist, just queue & schedule work */
 		if (!local->ops->remain_on_channel) {
 			list_add_tail(&roc->list, &local->roc_list);
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index cb707907188585d6874bf290874bdb0ca33bb399..6947a7ede51c990af97ed875017c3e4d351f1cd9 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -571,7 +571,8 @@ static int ieee80211_start_sw_scan(struct ieee80211_local *local,
 	return 0;
 }
 
-static bool __ieee80211_can_leave_ch(struct ieee80211_sub_if_data *sdata)
+static bool __ieee80211_can_leave_ch(struct ieee80211_sub_if_data *sdata,
+				     struct cfg80211_scan_request *req)
 {
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_sub_if_data *sdata_iter;
@@ -579,7 +580,7 @@ static bool __ieee80211_can_leave_ch(struct ieee80211_sub_if_data *sdata)
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
-	if (!ieee80211_is_radar_required(local))
+	if (!ieee80211_is_radar_required(local, req))
 		return true;
 
 	if (!regulatory_pre_cac_allowed(local->hw.wiphy))
@@ -595,9 +596,10 @@ static bool __ieee80211_can_leave_ch(struct ieee80211_sub_if_data *sdata)
 }
 
 static bool ieee80211_can_scan(struct ieee80211_local *local,
-			       struct ieee80211_sub_if_data *sdata)
+			       struct ieee80211_sub_if_data *sdata,
+			       struct cfg80211_scan_request *req)
 {
-	if (!__ieee80211_can_leave_ch(sdata))
+	if (!__ieee80211_can_leave_ch(sdata, req))
 		return false;
 
 	if (!list_empty(&local->roc_list))
@@ -612,15 +614,19 @@ static bool ieee80211_can_scan(struct ieee80211_local *local,
 
 void ieee80211_run_deferred_scan(struct ieee80211_local *local)
 {
+	struct cfg80211_scan_request *req;
+
 	lockdep_assert_wiphy(local->hw.wiphy);
 
 	if (!local->scan_req || local->scanning)
 		return;
 
+	req = wiphy_dereference(local->hw.wiphy, local->scan_req);
 	if (!ieee80211_can_scan(local,
 				rcu_dereference_protected(
 					local->scan_sdata,
-					lockdep_is_held(&local->hw.wiphy->mtx))))
+					lockdep_is_held(&local->hw.wiphy->mtx)),
+				req))
 		return;
 
 	wiphy_delayed_work_queue(local->hw.wiphy, &local->scan_work,
@@ -717,10 +723,10 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
 	    !(sdata->vif.active_links & BIT(req->tsf_report_link_id)))
 		return -EINVAL;
 
-	if (!__ieee80211_can_leave_ch(sdata))
+	if (!__ieee80211_can_leave_ch(sdata, req))
 		return -EBUSY;
 
-	if (!ieee80211_can_scan(local, sdata)) {
+	if (!ieee80211_can_scan(local, sdata, req)) {
 		/* wait for the work to finish/time out */
 		rcu_assign_pointer(local->scan_req, req);
 		rcu_assign_pointer(local->scan_sdata, sdata);
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 27d414efa3fd4bda40d2a37b14da6a4aa6bf0a02..0c43216fb7a1b0941349be760c331045d2cf9101 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -3953,6 +3953,34 @@ static u8 ieee80211_chanctx_radar_detect(struct ieee80211_local *local,
 	return radar_detect;
 }
 
+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;
+
+	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;
+
+		if (chan_radio_idx == radio_idx)
+			return true;
+	}
+
+	return false;
+}
+
 static u32
 __ieee80211_get_radio_mask(struct ieee80211_sub_if_data *sdata)
 {

-- 
2.34.1

[PATCH wireless-next 3/3] wifi: mac80211: Allow DFS/CSA on a radio if scan is ongoing on another radio

[Raj Kumar Bhagat]

From: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>

Currently, in multi-radio wiphy cases, if a scan is ongoing on one radio,
-EBUSY is returned when DFS or a channel switch is initiated on another
radio. Because of this, an MLD AP with one radio (link) in an ongoing scan
cannot initiate DFS or a channel switch on another radio (link).

In multi-radio wiphy cases, multiple radios are grouped under a single
wiphy. Hence, if a scan is ongoing on one underlying radio and DFS or a
channel switch is requested on a different underlying radio of the same
wiphy, these operations can be allowed simultaneously.

Add logic to check the underlying radio used for the ongoing scan. If the
radio on which DFS or a channel switch is requested is not being used for
the scan, allow the operation; otherwise, return -EBUSY.

Signed-off-by: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>
Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
---
 net/mac80211/cfg.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 2cd8731d8275b2f67c1b1305ec0bafc368a4498a..b76598abfb76708468432b3ec082955d4820e4a3 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3549,6 +3549,68 @@ static int ieee80211_set_bitrate_mask(struct wiphy *wiphy,
 	return 0;
 }
 
+static bool
+__ieee80211_is_scan_ongoing(struct wiphy *wiphy,
+			    struct ieee80211_local *local,
+			    struct cfg80211_chan_def *chandef)
+{
+	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;
+
+	req_radio_idx = cfg80211_get_radio_idx_by_chan(wiphy, chandef->chan);
+	if (req_radio_idx < 0)
+		return true;
+
+	if (local->scanning) {
+		scan_req = wiphy_dereference(wiphy, local->scan_req);
+		/*
+		 * Scan is going on but info is not there. Should not happen
+		 * but if it does, let's not take risk and assume we can't use
+		 * the hw hence return true
+		 */
+		if (WARN_ON_ONCE(!scan_req))
+			return true;
+
+		return ieee80211_is_radio_idx_in_scan_req(wiphy, scan_req,
+							  req_radio_idx);
+	}
+
+	if (!list_empty(&local->roc_list)) {
+		list_for_each_entry(roc, &local->roc_list, list) {
+			chan_radio_idx =
+				cfg80211_get_radio_idx_by_chan(wiphy,
+							       roc->chan);
+			/*
+			 * The roc work is added but chan_radio_idx is invalid.
+			 * Should not happen but if it does, let's not take
+			 * risk and return true.
+			 */
+			if (chan_radio_idx < 0)
+				return true;
+
+			if (chan_radio_idx == req_radio_idx)
+				return true;
+		}
+	}
+
+	return false;
+}
+
 static int ieee80211_start_radar_detection(struct wiphy *wiphy,
 					   struct net_device *dev,
 					   struct cfg80211_chan_def *chandef,
@@ -3562,7 +3624,7 @@ static int ieee80211_start_radar_detection(struct wiphy *wiphy,
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
-	if (!list_empty(&local->roc_list) || local->scanning)
+	if (__ieee80211_is_scan_ongoing(wiphy, local, chandef))
 		return -EBUSY;
 
 	link_data = sdata_dereference(sdata->link[link_id], sdata);
@@ -4054,7 +4116,7 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
-	if (!list_empty(&local->roc_list) || local->scanning)
+	if (__ieee80211_is_scan_ongoing(wiphy, local, &params->chandef))
 		return -EBUSY;
 
 	if (sdata->wdev.links[link_id].cac_started)

-- 
2.34.1

Re: [PATCH wireless-next 1/3] wifi: cfg80211: Add utility API to get radio index from channel

[Johannes Berg]

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

Re: [PATCH wireless-next 1/3] wifi: cfg80211: Add utility API to get radio index from channel

[Johannes Berg]

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

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

[Johannes Berg]

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

Re: [PATCH wireless-next 3/3] wifi: mac80211: Allow DFS/CSA on a radio if scan is ongoing on another radio

[Johannes Berg]

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

Re: [PATCH wireless-next 1/3] wifi: cfg80211: Add utility API to get radio index from channel

[Raj Kumar Bhagat]

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.

Re: [PATCH wireless-next 3/3] wifi: mac80211: Allow DFS/CSA on a radio if scan is ongoing on another radio

[Raj Kumar Bhagat]

On 5/16/2025 1:35 PM, Johannes Berg wrote:
> 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?
> 

Next version will rename this function to "ieee80211_is_scan_ongoing()".

>> +{
>> +	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?
> 

Thanks for suggestion, will simplify the above logic in next version.

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

Sure, will do in next version.

> 
> Also patch-order wise, it seems this one really should go before the
> 2nd?
> 

Sure will update the patch order as suggested.

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

[Raj Kumar Bhagat]

On 5/16/2025 1:31 PM, Johannes Berg wrote:
> 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 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.

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.

> 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 :)
> 

sure, this function should not be called with NULL scan_req and invalid
radio_idx. Better will remove the check: if (!scan_req).

> 
>> +	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?

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

[Johannes Berg]

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

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

[Raj Kumar Bhagat]

On 5/22/2025 4:53 PM, Johannes Berg wrote:
>>>> +	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_.

I initially added the check as a precautionary measure.

In the next version, I’ll replace it with a WARN_ON() to flag the unexpected
condition. As a conservative approach, will return true in that case, assuming
the scan request might use the specified radio_idx to safely abort the function.