| Message ID | 20221220043823.20382-1-quic_mpubbise@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: ath11k: Optimize 6 GHz scan time | expand |
Hi Manikanta, > Currently, time taken to scan all supported channels on WCN6750 > is ~8 seconds and connection time is almost 10 seconds. WCN6750 > supports three Wi-Fi bands (i.e., 2.4/5/6 GHz) and the numbers of > channels for scan come around ~100 channels (default case). > Since the chip doesn't have support for DBS (Dual Band Simultaneous), > scans cannot be parallelized resulting in longer scan times. > > Among the 100 odd channels, ~60 channels are in 6 GHz band. Therefore, > optimizing the scan for 6 GHz channels will bring down the overall > scan time. > > WCN6750 firmware has support to scan a 6 GHz channel based on co-located > AP information i.e., RNR IE which is found in the legacy 2.4/5 GHz scan > results. When a scan request with all supported channel list is enqueued > to the firmware, then based on WMI_SCAN_CHAN_FLAG_SCAN_ONLY_IF_RNR_FOUND > scan channel flag, firmware will scan only those 6 GHz channels for which > RNR IEs are found in the legacy scan results. > > In the proposed design, based on NL80211_SCAN_FLAG_COLOCATED_6GHZ scan > flag, driver will set the WMI_SCAN_CHAN_FLAG_SCAN_ONLY_IF_RNR_FOUND flag > for non-PSC channels. Since there is high probability to find 6 GHz APs > on PSC channels, these channels are always scanned. Only non-PSC channels > are selectively scanned based on cached RNR information from the legacy > scan results. > > If NL80211_SCAN_FLAG_COLOCATED_6GHZ is not set in the scan flags, > then scan will happen on all supported channels (default behavior). is this really a good idea? The interpretation on what scan results will be reported would be preferable the same no matter what hardware is present. Why would ath11k now have a different behavior? And more important, why is this something driver or even Linux kernel specific. Let userspace select the frequencies to scan. Looks like that iwd and wpa_supplicant set this flag regardless which means to me that a driver should respect the requested frequencies to be scanned. Anyhow, if you worry about time-to-connect, then fix userspace to be smart with scanning. I am also confused on how a savings of 1.5 seconds out of 8 seconds is significant. It still means you spent 6+ seconds in the 2.4 GHz and 5 GHz bands. I assume that you spent most time in 5 GHz right now. I highly doubt that a 6+ seconds plus 2 seconds connection time is anywhere acceptable. Have you tried using iwd and see what the connection time actually is after initial connection. Regards Marcel
On 12/20/2022 6:36 PM, Marcel Holtmann wrote: > Hi Manikanta, > >> Currently, time taken to scan all supported channels on WCN6750 >> is ~8 seconds and connection time is almost 10 seconds. WCN6750 >> supports three Wi-Fi bands (i.e., 2.4/5/6 GHz) and the numbers of >> channels for scan come around ~100 channels (default case). >> Since the chip doesn't have support for DBS (Dual Band Simultaneous), >> scans cannot be parallelized resulting in longer scan times. >> >> Among the 100 odd channels, ~60 channels are in 6 GHz band. Therefore, >> optimizing the scan for 6 GHz channels will bring down the overall >> scan time. >> >> WCN6750 firmware has support to scan a 6 GHz channel based on co-located >> AP information i.e., RNR IE which is found in the legacy 2.4/5 GHz scan >> results. When a scan request with all supported channel list is enqueued >> to the firmware, then based on WMI_SCAN_CHAN_FLAG_SCAN_ONLY_IF_RNR_FOUND >> scan channel flag, firmware will scan only those 6 GHz channels for which >> RNR IEs are found in the legacy scan results. >> >> In the proposed design, based on NL80211_SCAN_FLAG_COLOCATED_6GHZ scan >> flag, driver will set the WMI_SCAN_CHAN_FLAG_SCAN_ONLY_IF_RNR_FOUND flag >> for non-PSC channels. Since there is high probability to find 6 GHz APs >> on PSC channels, these channels are always scanned. Only non-PSC channels >> are selectively scanned based on cached RNR information from the legacy >> scan results. >> >> If NL80211_SCAN_FLAG_COLOCATED_6GHZ is not set in the scan flags, >> then scan will happen on all supported channels (default behavior). > > is this really a good idea? The interpretation on what scan results will > be reported would be preferable the same no matter what hardware is > present. Why would ath11k now have a different behavior? > > And more important, why is this something driver or even Linux kernel > specific. Let userspace select the frequencies to scan. > By the way, userspace itself selects the frequencies to scan, not the driver. If we see the split scan implementation in cfg80211, this is the how it is implemented. If NL80211_SCAN_FLAG_COLOCATED_6GHZ is set, it selects all PSC channels and those non-PSC channels where RNR IE information is found in the legacy scan results. If this flag is not set, all channels in 6 GHz are included in the scan freq list. It is upto userspace to decide what it wants. > Looks like that iwd and wpa_supplicant set this flag regardless which > means to me that a driver should respect the requested frequencies to > be scanned. > > Anyhow, if you worry about time-to-connect, then fix userspace to be > smart with scanning. I am also confused on how a savings of 1.5 seconds > out of 8 seconds is significant. Most of the times, we see ~2 seconds of savings. 2 seconds of scan time improvement out of 8 seconds is about 25% improvement and this is indeed significant. 8 seconds of time I'm talking here is for passive scan when the regdomain is set to WORLD. If correct regdomain is set and channel flags are proper, then the time it takes to complete scan & connection is much lesser with this change. It is around 4 seconds. It still means you spent 6+ seconds > in the 2.4 GHz and 5 GHz bands. I assume that you spent most time in > 5 GHz right now. > > I highly doubt that a 6+ seconds plus 2 seconds connection time is > anywhere acceptable As I said before, this is the worst case scenario. Active scan and connection on legacy band takes less than 4 seconds in normal usecase. Have you tried using iwd and see what the > connection time actually is after initial connection. > Not really, I'll try it out. Thanks for the suggestion. Thanks, Manikanta
Hi Manikanta, > By the way, userspace itself selects the frequencies to scan, not the > driver. > > If we see the split scan implementation in cfg80211, this is the how > it > is implemented. If NL80211_SCAN_FLAG_COLOCATED_6GHZ is set, it > selects > all PSC channels and those non-PSC channels where RNR IE information > is > found in the legacy scan results. If this flag is not set, all > channels > in 6 GHz are included in the scan freq list. It is upto userspace to > decide what it wants. This isn't your problem, but it needs to be said: The nl80211 docs need and update to reflect this behavior (or remove the PSC logic). IMO this is really weird that the kernel selects PSC's based on the co-located flag. The docs don't describe this behavior and the flag's name is misleading (its not SCAN_FLAG_COLOCATED_AND_PSC_6GHZ) :) If userspace doesn't specify a channel list then the kernel can do whatever it wants, but otherwise it should stick to the requested channels and scan only for co-located APs if the flag was set, not PSC's too. This basically adds ~3x the time to IWD's quick scan's for 6GHz capable cards regardless if there are any co-located AP's. ugh.
On 12/29/2022 2:52 AM, James Prestwood wrote: > Hi Manikanta, >> By the way, userspace itself selects the frequencies to scan, not the >> driver. >> >> If we see the split scan implementation in cfg80211, this is the how >> it >> is implemented. If NL80211_SCAN_FLAG_COLOCATED_6GHZ is set, it >> selects >> all PSC channels and those non-PSC channels where RNR IE information >> is >> found in the legacy scan results. If this flag is not set, all >> channels >> in 6 GHz are included in the scan freq list. It is upto userspace to >> decide what it wants. > > > This isn't your problem, but it needs to be said: > > The nl80211 docs need and update to reflect this behavior (or remove > the PSC logic). IMO this is really weird that the kernel selects PSC's > based on the co-located flag. The docs don't describe this behavior and > the flag's name is misleading (its not > SCAN_FLAG_COLOCATED_AND_PSC_6GHZ) :) > Sorry for the late reply, I was on vacation. What you said make sense. The existing flag should not add PSC channels according to the flag description. We can add another flag something like you pointed out SCAN_FLAG_COLOCATED_AND_PSC_6GHZ and include PSC channels if this flag is set. What do you say? > If userspace doesn't specify a channel list then the kernel can do > whatever it wants, but otherwise it should stick to the requested > channels and scan only for co-located APs if the flag was set, not > PSC's too. > > This basically adds ~3x the time to IWD's quick scan's for 6GHz capable > cards regardless if there are any co-located AP's. ugh. True. Thanks, Manikanta
On Tue, 2023-01-10 at 10:49 +0530, Manikanta Pubbisetty wrote: > On 12/29/2022 2:52 AM, James Prestwood wrote: > > Hi Manikanta, > > > By the way, userspace itself selects the frequencies to scan, not > > > the > > > driver. > > > > > > If we see the split scan implementation in cfg80211, this is the > > > how > > > it > > > is implemented. If NL80211_SCAN_FLAG_COLOCATED_6GHZ is set, it > > > selects > > > all PSC channels and those non-PSC channels where RNR IE > > > information > > > is > > > found in the legacy scan results. If this flag is not set, all > > > channels > > > in 6 GHz are included in the scan freq list. It is upto userspace > > > to > > > decide what it wants. > > > > > > This isn't your problem, but it needs to be said: > > > > The nl80211 docs need and update to reflect this behavior (or > > remove > > the PSC logic). IMO this is really weird that the kernel selects > > PSC's > > based on the co-located flag. The docs don't describe this behavior > > and > > the flag's name is misleading (its not > > SCAN_FLAG_COLOCATED_AND_PSC_6GHZ) :) > > > > Sorry for the late reply, I was on vacation. > > What you said make sense. The existing flag should not add PSC > channels > according to the flag description. > > We can add another flag something like you pointed out > SCAN_FLAG_COLOCATED_AND_PSC_6GHZ and include PSC channels if this > flag > is set. What do you say? I'm no authority here, just wanted to point this out. This is something that would need to be in mac80211 though, not just a specific driver. It would be up to the maintainers and would require changing the behavior of the existing flag, which then changes behavior in wpa_supplicant/hostapd. So its somewhat intrusive. But personally I'd be for it. And just require userspace include PSC's like any other channels if they need those. Thanks, James
On 1/10/2023 10:35 PM, James Prestwood wrote: > On Tue, 2023-01-10 at 10:49 +0530, Manikanta Pubbisetty wrote: >> On 12/29/2022 2:52 AM, James Prestwood wrote: >>> Hi Manikanta, >>>> By the way, userspace itself selects the frequencies to scan, not >>>> the >>>> driver. >>>> >>>> If we see the split scan implementation in cfg80211, this is the >>>> how >>>> it >>>> is implemented. If NL80211_SCAN_FLAG_COLOCATED_6GHZ is set, it >>>> selects >>>> all PSC channels and those non-PSC channels where RNR IE >>>> information >>>> is >>>> found in the legacy scan results. If this flag is not set, all >>>> channels >>>> in 6 GHz are included in the scan freq list. It is upto userspace >>>> to >>>> decide what it wants. >>> >>> >>> This isn't your problem, but it needs to be said: >>> >>> The nl80211 docs need and update to reflect this behavior (or >>> remove >>> the PSC logic). IMO this is really weird that the kernel selects >>> PSC's >>> based on the co-located flag. The docs don't describe this behavior >>> and >>> the flag's name is misleading (its not >>> SCAN_FLAG_COLOCATED_AND_PSC_6GHZ) :) >>> >> >> Sorry for the late reply, I was on vacation. >> >> What you said make sense. The existing flag should not add PSC >> channels >> according to the flag description. >> >> We can add another flag something like you pointed out >> SCAN_FLAG_COLOCATED_AND_PSC_6GHZ and include PSC channels if this >> flag >> is set. What do you say? > > I'm no authority here, just wanted to point this out. This is something > that would need to be in mac80211 though, not just a specific driver. > It would be up to the maintainers and would require changing the > behavior of the existing flag, which then changes behavior in > wpa_supplicant/hostapd. So its somewhat intrusive. > > But personally I'd be for it. And just require userspace include PSC's > like any other channels if they need those. > Hi Johannes, What is your opinion on the changes being proposed to the 6 GHz scan in cfg80211 that is being discussed in this thread? Thanks, Manikanta
On Fri, 2023-02-24 at 15:38 +0530, Manikanta Pubbisetty wrote: > On 1/10/2023 10:35 PM, James Prestwood wrote: > > On Tue, 2023-01-10 at 10:49 +0530, Manikanta Pubbisetty wrote: > > > On 12/29/2022 2:52 AM, James Prestwood wrote: > > > > Hi Manikanta, > > > > > By the way, userspace itself selects the frequencies to scan, not > > > > > the > > > > > driver. > > > > > > > > > > If we see the split scan implementation in cfg80211, this is the > > > > > how > > > > > it > > > > > is implemented. If NL80211_SCAN_FLAG_COLOCATED_6GHZ is set, it > > > > > selects > > > > > all PSC channels and those non-PSC channels where RNR IE > > > > > information > > > > > is > > > > > found in the legacy scan results. If this flag is not set, all > > > > > channels > > > > > in 6 GHz are included in the scan freq list. It is upto userspace > > > > > to > > > > > decide what it wants. > > > > > > > > > > > > This isn't your problem, but it needs to be said: > > > > > > > > The nl80211 docs need and update to reflect this behavior (or > > > > remove > > > > the PSC logic). IMO this is really weird that the kernel selects > > > > PSC's > > > > based on the co-located flag. The docs don't describe this behavior > > > > and > > > > the flag's name is misleading (its not > > > > SCAN_FLAG_COLOCATED_AND_PSC_6GHZ) :) > > > > > > > > > > Sorry for the late reply, I was on vacation. > > > > > > What you said make sense. The existing flag should not add PSC > > > channels > > > according to the flag description. > > > > > > We can add another flag something like you pointed out > > > SCAN_FLAG_COLOCATED_AND_PSC_6GHZ and include PSC channels if this > > > flag > > > is set. What do you say? > > > > I'm no authority here, just wanted to point this out. This is something > > that would need to be in mac80211 though, not just a specific driver. > > It would be up to the maintainers and would require changing the > > behavior of the existing flag, which then changes behavior in > > wpa_supplicant/hostapd. So its somewhat intrusive. > > > > But personally I'd be for it. And just require userspace include PSC's > > like any other channels if they need those. > > > > Hi Johannes, > > What is your opinion on the changes being proposed to the 6 GHz scan in > cfg80211 that is being discussed in this thread? > I don't think we can/should change the semantics of an existing flag now, but we can certainly update the documentation to match the implementation, and add more flags to make it more flexible. johannes
On 2/24/2023 3:45 PM, Johannes Berg wrote: > On Fri, 2023-02-24 at 15:38 +0530, Manikanta Pubbisetty wrote: >> On 1/10/2023 10:35 PM, James Prestwood wrote: >>> On Tue, 2023-01-10 at 10:49 +0530, Manikanta Pubbisetty wrote: >>>> On 12/29/2022 2:52 AM, James Prestwood wrote: >>>>> Hi Manikanta, >>>>>> By the way, userspace itself selects the frequencies to scan, not >>>>>> the >>>>>> driver. >>>>>> >>>>>> If we see the split scan implementation in cfg80211, this is the >>>>>> how >>>>>> it >>>>>> is implemented. If NL80211_SCAN_FLAG_COLOCATED_6GHZ is set, it >>>>>> selects >>>>>> all PSC channels and those non-PSC channels where RNR IE >>>>>> information >>>>>> is >>>>>> found in the legacy scan results. If this flag is not set, all >>>>>> channels >>>>>> in 6 GHz are included in the scan freq list. It is upto userspace >>>>>> to >>>>>> decide what it wants. >>>>> >>>>> >>>>> This isn't your problem, but it needs to be said: >>>>> >>>>> The nl80211 docs need and update to reflect this behavior (or >>>>> remove >>>>> the PSC logic). IMO this is really weird that the kernel selects >>>>> PSC's >>>>> based on the co-located flag. The docs don't describe this behavior >>>>> and >>>>> the flag's name is misleading (its not >>>>> SCAN_FLAG_COLOCATED_AND_PSC_6GHZ) :) >>>>> >>>> >>>> Sorry for the late reply, I was on vacation. >>>> >>>> What you said make sense. The existing flag should not add PSC >>>> channels >>>> according to the flag description. >>>> >>>> We can add another flag something like you pointed out >>>> SCAN_FLAG_COLOCATED_AND_PSC_6GHZ and include PSC channels if this >>>> flag >>>> is set. What do you say? >>> >>> I'm no authority here, just wanted to point this out. This is something >>> that would need to be in mac80211 though, not just a specific driver. >>> It would be up to the maintainers and would require changing the >>> behavior of the existing flag, which then changes behavior in >>> wpa_supplicant/hostapd. So its somewhat intrusive. >>> >>> But personally I'd be for it. And just require userspace include PSC's >>> like any other channels if they need those. >>> >> >> Hi Johannes, >> >> What is your opinion on the changes being proposed to the 6 GHz scan in >> cfg80211 that is being discussed in this thread? >> > > I don't think we can/should change the semantics of an existing flag > now, but we can certainly update the documentation to match the > implementation, and add more flags to make it more flexible. > > johannes Sure, makes sense. I'll make the changes and send them out for review. Thanks, Manikanta
Hi, > -----Original Message----- > From: Manikanta Pubbisetty <quic_mpubbise@quicinc.com> > Sent: Monday, February 27, 2023 06:24 > To: Johannes Berg <johannes@sipsolutions.net>; James Prestwood > <prestwoj@gmail.com>; Marcel Holtmann <marcel@holtmann.org> > Cc: ath11k@lists.infradead.org; linux-wireless@vger.kernel.org; Peer, Ilan > <ilan.peer@intel.com> > Subject: Re: [PATCH] wifi: ath11k: Optimize 6 GHz scan time > > On 2/24/2023 3:45 PM, Johannes Berg wrote: > > On Fri, 2023-02-24 at 15:38 +0530, Manikanta Pubbisetty wrote: > >> On 1/10/2023 10:35 PM, James Prestwood wrote: > >>> On Tue, 2023-01-10 at 10:49 +0530, Manikanta Pubbisetty wrote: > >>>> On 12/29/2022 2:52 AM, James Prestwood wrote: > >>>>> Hi Manikanta, > >>>>>> By the way, userspace itself selects the frequencies to scan, not > >>>>>> the driver. > >>>>>> > >>>>>> If we see the split scan implementation in cfg80211, this is the > >>>>>> how it is implemented. If NL80211_SCAN_FLAG_COLOCATED_6GHZ > is > >>>>>> set, it selects all PSC channels and those non-PSC channels where > >>>>>> RNR IE information is found in the legacy scan results. If this > >>>>>> flag is not set, all channels in 6 GHz are included in the scan > >>>>>> freq list. It is upto userspace to decide what it wants. > >>>>> > >>>>> > >>>>> This isn't your problem, but it needs to be said: > >>>>> > >>>>> The nl80211 docs need and update to reflect this behavior (or > >>>>> remove the PSC logic). IMO this is really weird that the kernel > >>>>> selects PSC's based on the co-located flag. The docs don't > >>>>> describe this behavior and the flag's name is misleading (its not > >>>>> SCAN_FLAG_COLOCATED_AND_PSC_6GHZ) :) > >>>>> > >>>> > >>>> Sorry for the late reply, I was on vacation. > >>>> > >>>> What you said make sense. The existing flag should not add PSC > >>>> channels according to the flag description. > >>>> > >>>> We can add another flag something like you pointed out > >>>> SCAN_FLAG_COLOCATED_AND_PSC_6GHZ and include PSC channels if > this > >>>> flag is set. What do you say? > >>> > >>> I'm no authority here, just wanted to point this out. This is > >>> something that would need to be in mac80211 though, not just a specific > driver. > >>> It would be up to the maintainers and would require changing the > >>> behavior of the existing flag, which then changes behavior in > >>> wpa_supplicant/hostapd. So its somewhat intrusive. > >>> > >>> But personally I'd be for it. And just require userspace include > >>> PSC's like any other channels if they need those. > >>> > >> > >> Hi Johannes, > >> > >> What is your opinion on the changes being proposed to the 6 GHz scan > >> in > >> cfg80211 that is being discussed in this thread? > >> > > > > I don't think we can/should change the semantics of an existing flag > > now, but we can certainly update the documentation to match the > > implementation, and add more flags to make it more flexible. > > > > johannes > > Sure, makes sense. I'll make the changes and send them out for review. > Sorry for joining this thread late. I agree that the documentation of the NL80211_SCAN_FLAG_COLOCATED_6GHZ flag can be updated, but FWIW, I want to clarify the intention behind this flag: First, the logic would always scan only the channels requested by user space, so if the PSC channels are not included they would not be scanned. Second, the intention of the NL80211_SCAN_FLAG_COLOCATED_6GHZ flag was to indicate to the kernel that before going to scan the 6GHz channels it should first scan the 2GHz/5GHz bands such that it would retrieve information about the collocated APs, so eventually it would only scan the given PSC channels and the channels on which collocated APs are found (in which case direct probing of these APs is expected). While the PSC channels could have been scanned together with the 2GHz/5GHz channels, since collocated APs might also reside on PSC channels, we wanted to avoid scanning PSC channels twice, so this is why they are scanned only after the scan on legacy bands. Hope this helps, Ilan.
On 2/27/2023 1:55 PM, Peer, Ilan wrote: > Hi, > >> -----Original Message----- >> From: Manikanta Pubbisetty <quic_mpubbise@quicinc.com> >> Sent: Monday, February 27, 2023 06:24 >> To: Johannes Berg <johannes@sipsolutions.net>; James Prestwood >> <prestwoj@gmail.com>; Marcel Holtmann <marcel@holtmann.org> >> Cc: ath11k@lists.infradead.org; linux-wireless@vger.kernel.org; Peer, Ilan >> <ilan.peer@intel.com> >> Subject: Re: [PATCH] wifi: ath11k: Optimize 6 GHz scan time >> >> On 2/24/2023 3:45 PM, Johannes Berg wrote: >>> On Fri, 2023-02-24 at 15:38 +0530, Manikanta Pubbisetty wrote: >>>> On 1/10/2023 10:35 PM, James Prestwood wrote: >>>>> On Tue, 2023-01-10 at 10:49 +0530, Manikanta Pubbisetty wrote: >>>>>> On 12/29/2022 2:52 AM, James Prestwood wrote: >>>>>>> Hi Manikanta, >>>>>>>> By the way, userspace itself selects the frequencies to scan, not >>>>>>>> the driver. >>>>>>>> >>>>>>>> If we see the split scan implementation in cfg80211, this is the >>>>>>>> how it is implemented. If NL80211_SCAN_FLAG_COLOCATED_6GHZ >> is >>>>>>>> set, it selects all PSC channels and those non-PSC channels where >>>>>>>> RNR IE information is found in the legacy scan results. If this >>>>>>>> flag is not set, all channels in 6 GHz are included in the scan >>>>>>>> freq list. It is upto userspace to decide what it wants. >>>>>>> >>>>>>> >>>>>>> This isn't your problem, but it needs to be said: >>>>>>> >>>>>>> The nl80211 docs need and update to reflect this behavior (or >>>>>>> remove the PSC logic). IMO this is really weird that the kernel >>>>>>> selects PSC's based on the co-located flag. The docs don't >>>>>>> describe this behavior and the flag's name is misleading (its not >>>>>>> SCAN_FLAG_COLOCATED_AND_PSC_6GHZ) :) >>>>>>> >>>>>> >>>>>> Sorry for the late reply, I was on vacation. >>>>>> >>>>>> What you said make sense. The existing flag should not add PSC >>>>>> channels according to the flag description. >>>>>> >>>>>> We can add another flag something like you pointed out >>>>>> SCAN_FLAG_COLOCATED_AND_PSC_6GHZ and include PSC channels if >> this >>>>>> flag is set. What do you say? >>>>> >>>>> I'm no authority here, just wanted to point this out. This is >>>>> something that would need to be in mac80211 though, not just a specific >> driver. >>>>> It would be up to the maintainers and would require changing the >>>>> behavior of the existing flag, which then changes behavior in >>>>> wpa_supplicant/hostapd. So its somewhat intrusive. >>>>> >>>>> But personally I'd be for it. And just require userspace include >>>>> PSC's like any other channels if they need those. >>>>> >>>> >>>> Hi Johannes, >>>> >>>> What is your opinion on the changes being proposed to the 6 GHz scan >>>> in >>>> cfg80211 that is being discussed in this thread? >>>> >>> >>> I don't think we can/should change the semantics of an existing flag >>> now, but we can certainly update the documentation to match the >>> implementation, and add more flags to make it more flexible. >>> >>> johannes >> >> Sure, makes sense. I'll make the changes and send them out for review. >> > > Sorry for joining this thread late. I agree that the documentation of the NL80211_SCAN_FLAG_COLOCATED_6GHZ > flag can be updated, but FWIW, I want to clarify the intention behind this flag: > > First, the logic would always scan only the channels requested by user space, so if the PSC channels are > not included they would not be scanned. > Agreed. > Second, the intention of the NL80211_SCAN_FLAG_COLOCATED_6GHZ flag was to indicate to the kernel that > before going to scan the 6GHz channels it should first scan the 2GHz/5GHz bands such that it would retrieve information > about the collocated APs, so eventually it would only scan the given PSC channels and the channels on which collocated APs > are found (in which case direct probing of these APs is expected). > Understood, currently the documentation of the NL80211_SCAN_FLAG_COLOCATED_6GHZ flag doesn't state this. It simply scans all the PSC channels the user space has asked for whether they are co-located or not. As you mentioned earlier, we need to fix the documentation part. > While the PSC channels could have been scanned together with the 2GHz/5GHz channels, since collocated APs might also reside > on PSC channels, we wanted to avoid scanning PSC channels twice, so this is why they are scanned only after the scan on legacy > bands. > > Hope this helps, > Understood, thanks a lot for the clarification. Manikanta
On 2/27/2023 9:53 AM, Manikanta Pubbisetty wrote: > On 2/24/2023 3:45 PM, Johannes Berg wrote: >> On Fri, 2023-02-24 at 15:38 +0530, Manikanta Pubbisetty wrote: >>> On 1/10/2023 10:35 PM, James Prestwood wrote: >>>> On Tue, 2023-01-10 at 10:49 +0530, Manikanta Pubbisetty wrote: >>>>> On 12/29/2022 2:52 AM, James Prestwood wrote: >>>>>> Hi Manikanta, >>>>>>> By the way, userspace itself selects the frequencies to scan, not >>>>>>> the >>>>>>> driver. >>>>>>> >>>>>>> If we see the split scan implementation in cfg80211, this is the >>>>>>> how >>>>>>> it >>>>>>> is implemented. If NL80211_SCAN_FLAG_COLOCATED_6GHZ is set, it >>>>>>> selects >>>>>>> all PSC channels and those non-PSC channels where RNR IE >>>>>>> information >>>>>>> is >>>>>>> found in the legacy scan results. If this flag is not set, all >>>>>>> channels >>>>>>> in 6 GHz are included in the scan freq list. It is upto userspace >>>>>>> to >>>>>>> decide what it wants. >>>>>> >>>>>> >>>>>> This isn't your problem, but it needs to be said: >>>>>> >>>>>> The nl80211 docs need and update to reflect this behavior (or >>>>>> remove >>>>>> the PSC logic). IMO this is really weird that the kernel selects >>>>>> PSC's >>>>>> based on the co-located flag. The docs don't describe this behavior >>>>>> and >>>>>> the flag's name is misleading (its not >>>>>> SCAN_FLAG_COLOCATED_AND_PSC_6GHZ) :) >>>>>> >>>>> >>>>> Sorry for the late reply, I was on vacation. >>>>> >>>>> What you said make sense. The existing flag should not add PSC >>>>> channels >>>>> according to the flag description. >>>>> >>>>> We can add another flag something like you pointed out >>>>> SCAN_FLAG_COLOCATED_AND_PSC_6GHZ and include PSC channels if this >>>>> flag >>>>> is set. What do you say? >>>> >>>> I'm no authority here, just wanted to point this out. This is something >>>> that would need to be in mac80211 though, not just a specific driver. >>>> It would be up to the maintainers and would require changing the >>>> behavior of the existing flag, which then changes behavior in >>>> wpa_supplicant/hostapd. So its somewhat intrusive. >>>> >>>> But personally I'd be for it. And just require userspace include PSC's >>>> like any other channels if they need those. >>>> >>> >>> Hi Johannes, >>> >>> What is your opinion on the changes being proposed to the 6 GHz scan in >>> cfg80211 that is being discussed in this thread? >>> >> >> I don't think we can/should change the semantics of an existing flag >> now, but we can certainly update the documentation to match the >> implementation, and add more flags to make it more flexible. >> >> johannes > > Sure, makes sense. I'll make the changes and send them out for review. > Sent out a patch to update the documentation for review. https://patchwork.kernel.org/project/linux-wireless/patch/20230308104556.9399-1-quic_mpubbise@quicinc.com/ Thanks, Manikanta
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c index b198edba76eb..0332e91c2c1c 100644 --- a/drivers/net/wireless/ath/ath11k/mac.c +++ b/drivers/net/wireless/ath/ath11k/mac.c @@ -3681,8 +3681,29 @@ static int ath11k_mac_op_hw_scan(struct ieee80211_hw *hw, goto exit; } - for (i = 0; i < arg->num_chan; i++) - arg->chan_list[i] = req->channels[i]->center_freq; + for (i = 0; i < arg->num_chan; i++) { + if (test_bit(WMI_TLV_SERVICE_SCAN_CONFIG_PER_CHANNEL, + ar->ab->wmi_ab.svc_map)) { + arg->chan_list[i] = + u32_encode_bits(req->channels[i]->center_freq, + WMI_SCAN_CONF_PER_CH_CHANNEL_MASK); + + /* If NL80211_SCAN_FLAG_COLOCATED_6GHZ is set in scan + * flags, then scan all PSC channels in 6 GHz band and + * those non-PSC channels where RNR IE is found during + * the legacy 2.4/5 GHz scan. + * If NL80211_SCAN_FLAG_COLOCATED_6GHZ is not set, + * then all channels in 6 GHz will be scanned. + */ + if (req->channels[i]->band == NL80211_BAND_6GHZ && + req->flags & NL80211_SCAN_FLAG_COLOCATED_6GHZ && + !cfg80211_channel_is_psc(req->channels[i])) + arg->chan_list[i] |= + WMI_SCAN_CH_FLAG_SCAN_ONLY_IF_RNR_FOUND; + } else { + arg->chan_list[i] = req->channels[i]->center_freq; + } + } } if (req->flags & NL80211_SCAN_FLAG_RANDOM_ADDR) { diff --git a/drivers/net/wireless/ath/ath11k/wmi.h b/drivers/net/wireless/ath/ath11k/wmi.h index 8f2c07d70a4a..90497a93598b 100644 --- a/drivers/net/wireless/ath/ath11k/wmi.h +++ b/drivers/net/wireless/ath/ath11k/wmi.h @@ -2095,6 +2095,7 @@ enum wmi_tlv_service { /* The second 128 bits */ WMI_MAX_EXT_SERVICE = 256, + WMI_TLV_SERVICE_SCAN_CONFIG_PER_CHANNEL = 265, WMI_TLV_SERVICE_BIOS_SAR_SUPPORT = 326, /* The third 128 bits */ @@ -3223,6 +3224,9 @@ struct wmi_start_scan_cmd { #define WMI_SCAN_DWELL_MODE_MASK 0x00E00000 #define WMI_SCAN_DWELL_MODE_SHIFT 21 +#define WMI_SCAN_CONF_PER_CH_CHANNEL_MASK GENMASK(19, 0) +#define WMI_SCAN_CH_FLAG_SCAN_ONLY_IF_RNR_FOUND BIT(20) + enum { WMI_SCAN_DWELL_MODE_DEFAULT = 0, WMI_SCAN_DWELL_MODE_CONSERVATIVE = 1,
Currently, time taken to scan all supported channels on WCN6750 is ~8 seconds and connection time is almost 10 seconds. WCN6750 supports three Wi-Fi bands (i.e., 2.4/5/6 GHz) and the numbers of channels for scan come around ~100 channels (default case). Since the chip doesn't have support for DBS (Dual Band Simultaneous), scans cannot be parallelized resulting in longer scan times. Among the 100 odd channels, ~60 channels are in 6 GHz band. Therefore, optimizing the scan for 6 GHz channels will bring down the overall scan time. WCN6750 firmware has support to scan a 6 GHz channel based on co-located AP information i.e., RNR IE which is found in the legacy 2.4/5 GHz scan results. When a scan request with all supported channel list is enqueued to the firmware, then based on WMI_SCAN_CHAN_FLAG_SCAN_ONLY_IF_RNR_FOUND scan channel flag, firmware will scan only those 6 GHz channels for which RNR IEs are found in the legacy scan results. In the proposed design, based on NL80211_SCAN_FLAG_COLOCATED_6GHZ scan flag, driver will set the WMI_SCAN_CHAN_FLAG_SCAN_ONLY_IF_RNR_FOUND flag for non-PSC channels. Since there is high probability to find 6 GHz APs on PSC channels, these channels are always scanned. Only non-PSC channels are selectively scanned based on cached RNR information from the legacy scan results. If NL80211_SCAN_FLAG_COLOCATED_6GHZ is not set in the scan flags, then scan will happen on all supported channels (default behavior). With these optimizations, scan time is improved by 1.5-1.8 seconds on WCN6750. Similar savings have been observed on WCN6855. Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1 Tested-on: WCN6855 hw2.1 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.16 Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@quicinc.com> --- drivers/net/wireless/ath/ath11k/mac.c | 25 +++++++++++++++++++++++-- drivers/net/wireless/ath/ath11k/wmi.h | 4 ++++ 2 files changed, 27 insertions(+), 2 deletions(-) base-commit: 58e4b9df840cad439a4c878f81bc105cac2197a7