diff mbox series

[v2,1/2] nl80211: set NL80211_SCAN_FLAG_COLOCATED_6GHZ in scan

Message ID 20220406151537.437024-1-ilan.peer@intel.com
State Superseded
Headers show
Series [v2,1/2] nl80211: set NL80211_SCAN_FLAG_COLOCATED_6GHZ in scan | expand

Commit Message

Ilan Peer April 6, 2022, 3:15 p.m. UTC
From: Tova Mussai <tova.mussai@intel.com>

Set NL80211_SCAN_FLAG_COLOCATED_6GHZ in the scan params to enable
scanning for co-located AP's found in 2.4/5 GHz bands when
not scanning passively. Do so only when collocated scanning
is not disabled by higher layer logic.

Signed-off-by: Tova Mussai <tova.mussai@intel.com>
Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Avraham Stern <avraham.stern@intel.com>
---
 src/drivers/driver.h              | 10 ++++++++++
 src/drivers/driver_nl80211_scan.c | 11 +++++++++++
 wpa_supplicant/scan.c             |  1 +
 3 files changed, 22 insertions(+)

Comments

Ben Greear April 6, 2022, 3:40 p.m. UTC | #1
On 4/6/22 8:15 AM, Ilan Peer wrote:
> From: Tova Mussai <tova.mussai@intel.com>
> 
> Set NL80211_SCAN_FLAG_COLOCATED_6GHZ in the scan params to enable
> scanning for co-located AP's found in 2.4/5 GHz bands when
> not scanning passively. Do so only when collocated scanning
> is not disabled by higher layer logic.

If I read the double-negatives correctly, this patch enables the new
flag by default, and at least in this patch, there is no way to disable
that.

Should there be a user-configurable in wpa_supplicant.conf to allow enabling/disabling
this feature?

Thanks,
Ben
Johannes Berg April 6, 2022, 3:47 p.m. UTC | #2
On Wed, 2022-04-06 at 08:40 -0700, Ben Greear wrote:
> On 4/6/22 8:15 AM, Ilan Peer wrote:
> > From: Tova Mussai <tova.mussai@intel.com>
> > 
> > Set NL80211_SCAN_FLAG_COLOCATED_6GHZ in the scan params to enable
> > scanning for co-located AP's found in 2.4/5 GHz bands when
> > not scanning passively. Do so only when collocated scanning
> > is not disabled by higher layer logic.
> 
> If I read the double-negatives correctly, this patch enables the new
> flag by default, and at least in this patch, there is no way to disable
> that.
> 
> Should there be a user-configurable in wpa_supplicant.conf to allow enabling/disabling
> this feature?
> 

And if not, why didn't we just always set it in the kernel? This still
confuses me a bit :)

johannes
Ilan Peer April 7, 2022, 6:39 a.m. UTC | #3
Hi,

> -----Original Message-----
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Wednesday, April 06, 2022 18:47
> To: greearb <greearb@candelatech.com>; Peer, Ilan <ilan.peer@intel.com>;
> hostap@lists.infradead.org
> Cc: Mussai, Tova <tova.mussai@intel.com>; Otcheretianski, Andrei
> <andrei.otcheretianski@intel.com>; Stern, Avraham
> <avraham.stern@intel.com>
> Subject: Re: [PATCH v2 1/2] nl80211: set
> NL80211_SCAN_FLAG_COLOCATED_6GHZ in scan
> 
> On Wed, 2022-04-06 at 08:40 -0700, Ben Greear wrote:
> > On 4/6/22 8:15 AM, Ilan Peer wrote:
> > > From: Tova Mussai <tova.mussai@intel.com>
> > >
> > > Set NL80211_SCAN_FLAG_COLOCATED_6GHZ in the scan params to
> enable
> > > scanning for co-located AP's found in 2.4/5 GHz bands when not
> > > scanning passively. Do so only when collocated scanning is not
> > > disabled by higher layer logic.
> >
> > If I read the double-negatives correctly, this patch enables the new
> > flag by default, and at least in this patch, there is no way to
> > disable that.
> >
> > Should there be a user-configurable in wpa_supplicant.conf to allow
> > enabling/disabling this feature?
> >
> 

I do not see a real reason for this other than for testing purposes. The IEEE802.11 specification direction for discovery on non PSC channels is such that these channels should be scanned only if there is some other indication that there are APs operating on these channels.

> And if not, why didn't we just always set it in the kernel? This still confuses
> me a bit :)
> 

There are cases that the non PSC channels need to be explicitly scanned passively, e.g., for some RRM beacon report use cases. The same might also apply to P2P scanning (not sure if the behavior of P2P full scan is defined on 6GHz).

Regards,

Ilan.
Ben Greear April 7, 2022, 2:16 p.m. UTC | #4
On 4/6/22 11:39 PM, Peer, Ilan wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Johannes Berg <johannes@sipsolutions.net>
>> Sent: Wednesday, April 06, 2022 18:47
>> To: greearb <greearb@candelatech.com>; Peer, Ilan <ilan.peer@intel.com>;
>> hostap@lists.infradead.org
>> Cc: Mussai, Tova <tova.mussai@intel.com>; Otcheretianski, Andrei
>> <andrei.otcheretianski@intel.com>; Stern, Avraham
>> <avraham.stern@intel.com>
>> Subject: Re: [PATCH v2 1/2] nl80211: set
>> NL80211_SCAN_FLAG_COLOCATED_6GHZ in scan
>>
>> On Wed, 2022-04-06 at 08:40 -0700, Ben Greear wrote:
>>> On 4/6/22 8:15 AM, Ilan Peer wrote:
>>>> From: Tova Mussai <tova.mussai@intel.com>
>>>>
>>>> Set NL80211_SCAN_FLAG_COLOCATED_6GHZ in the scan params to
>> enable
>>>> scanning for co-located AP's found in 2.4/5 GHz bands when not
>>>> scanning passively. Do so only when collocated scanning is not
>>>> disabled by higher layer logic.
>>>
>>> If I read the double-negatives correctly, this patch enables the new
>>> flag by default, and at least in this patch, there is no way to
>>> disable that.
>>>
>>> Should there be a user-configurable in wpa_supplicant.conf to allow
>>> enabling/disabling this feature?
>>>
>>
> 
> I do not see a real reason for this other than for testing purposes. The IEEE802.11 specification direction for discovery on non PSC channels is such that these channels should be scanned only if there is some other indication that there are APs operating on these channels.

I am all about testing purposes!

I think it should be allowed to be disabled in case this feature
causes other regressions, but fine to default it to enabled if you
think that is a good idea.

Thanks,
--Ben
Ilan Peer April 10, 2022, 6:51 a.m. UTC | #5
Hi,

> -----Original Message-----
> From: Ben Greear <greearb@candelatech.com>
> Sent: Thursday, April 07, 2022 17:16
> To: Peer, Ilan <ilan.peer@intel.com>; Johannes Berg
> <johannes@sipsolutions.net>; hostap@lists.infradead.org
> Cc: Mussai, Tova <tova.mussai@intel.com>; Otcheretianski, Andrei
> <andrei.otcheretianski@intel.com>; Stern, Avraham
> <avraham.stern@intel.com>
> Subject: Re: [PATCH v2 1/2] nl80211: set
> NL80211_SCAN_FLAG_COLOCATED_6GHZ in scan
> 
> On 4/6/22 11:39 PM, Peer, Ilan wrote:

> > I do not see a real reason for this other than for testing purposes. The
> IEEE802.11 specification direction for discovery on non PSC channels is such
> that these channels should be scanned only if there is some other indication
> that there are APs operating on these channels.
> 
> I am all about testing purposes!
> 
> I think it should be allowed to be disabled in case this feature causes other
> regressions, but fine to default it to enabled if you think that is a good idea.
> 

I've send a new patch set that also includes a configuration option to disable non collocated 6GHz scanning.

BTW, did you try testing the previous version?

Regards,

Ilan.
diff mbox series

Patch

diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index 111e7e4081..0e31d6e3fc 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -651,6 +651,16 @@  struct wpa_driver_scan_params {
 	 */
 	unsigned int p2p_include_6ghz:1;
 
+	/**
+	 * non_coloc_6ghz - force scanning non-PSC 6GHz channels
+	 *
+	 * If this is set, the driver should scan non-PSC channels from the
+	 * scan request even if no co-located AP was reported on these channels.
+	 * The default is to scan non-PSC channels only if a co-located AP was
+	 * reported on the channel.
+	 */
+	unsigned int non_coloc_6ghz:1;
+
 	/*
 	 * NOTE: Whenever adding new parameters here, please make sure
 	 * wpa_scan_clone_params() and wpa_scan_free_params() get updated with
diff --git a/src/drivers/driver_nl80211_scan.c b/src/drivers/driver_nl80211_scan.c
index 1316084805..31e3308081 100644
--- a/src/drivers/driver_nl80211_scan.c
+++ b/src/drivers/driver_nl80211_scan.c
@@ -203,6 +203,17 @@  nl80211_scan_common(struct i802_bss *bss, u8 cmd,
 				goto fail;
 		}
 		nla_nest_end(msg, ssids);
+
+		/*
+		 * If allowed, scan for 6GHz APs that are reported by other
+		 * APs. If the flag is not set and 6GHz channels are to be
+		 * scanned, they will be scanned passively.
+		 */
+		wpa_printf(MSG_DEBUG, "nl80211: non_coloc_6ghz=%u",
+			   params->non_coloc_6ghz);
+
+		if (!params->non_coloc_6ghz)
+			scan_flags |= NL80211_SCAN_FLAG_COLOCATED_6GHZ;
 	} else {
 		wpa_printf(MSG_DEBUG, "nl80211: Passive scan requested");
 	}
diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c
index b0094ca6ca..31b694713b 100644
--- a/wpa_supplicant/scan.c
+++ b/wpa_supplicant/scan.c
@@ -2872,6 +2872,7 @@  wpa_scan_clone_params(const struct wpa_driver_scan_params *src)
 	params->duration = src->duration;
 	params->duration_mandatory = src->duration_mandatory;
 	params->oce_scan = src->oce_scan;
+	params->non_coloc_6ghz = src->non_coloc_6ghz;
 
 	if (src->sched_scan_plans_num > 0) {
 		params->sched_scan_plans =