by Luca Coelho

[permalink] [raw]

Subject: Re: [PATCH 01/12] iwlwifi: mvm: write queue_sync_state only for sync

Luca Coelho <[email protected]> wrote:

> From: Johannes Berg <[email protected]>
>
> We use mvm->queue_sync_state to wait for synchronous queue sync
> messages, but if an async one happens inbetween we shouldn't
> clear mvm->queue_sync_state after sending the async one, that
> can run concurrently (at least from the CPU POV) with another
> synchronous queue sync.
>
> Signed-off-by: Johannes Berg <[email protected]>
> Signed-off-by: Luca Coelho <[email protected]>

11 patches applied to iwlwifi-next.git, thanks.

ac2d58b34dc9 iwlwifi: mvm: write queue_sync_state only for sync
d96fbcbb5040 iwlwifi: mvm: clean up queue sync implementation
35d261a2d9fa iwlwifi: mvm: when associated with PMF, use protected NDP ranging negotiation
920484e82ea5 iwlwifi: add ax201 killer device
f05a69e2c7d7 iwlwifi: mvm: Add support for 6GHz passive scan
7143a6771d2c iwlwifi: mvm: enable PPAG in China
4eedace3b6d6 iwlwifi: add new so-gf device
c3e599ec1757 iwlwifi: move iwl_configure_rxq to be used by other op_modes
bc0411c5b1f8 iwlwifi: mvm: support BIOS enable/disable for 11ax in Ukraine
d4eb23d659d3 iwlwifi: mvm: refactor ACPI DSM evaluation function
86817a2ea413 iwlwifi: mvm: Use IWL_INFO in fw_reset_handshake()

2021-04-13 15:17:24

by Ilan Peer

[permalink] [raw]

Subject: RE: [PATCH 06/12] iwlwifi: mvm: Add support for 6GHz passive scan

Hi Ben,

> -----Original Message-----
> From: Ben Greear <[email protected]>
> Sent: Sunday, April 11, 2021 19:15
> To: Peer, Ilan <[email protected]>; Luca Coelho <[email protected]>;
> [email protected]
> Cc: [email protected]
> Subject: Re: [PATCH 06/12] iwlwifi: mvm: Add support for 6GHz passive scan
>
> On 4/11/21 5:14 AM, Peer, Ilan wrote:
> > Hi,
> >
> >>> This logic enables a special 'passive' scan which is not directly
> >>> intended for discovery of APs for connection etc. but for discovery
> >>> of APs with country information in the beacons/probe responses, so
> >>> the fw could use this information as an input that might allow it to
> >>> enable 6GHz
> >> channels (which are supported but are disabled). This special scan is
> >> intended for cases that the device does not have any other regulatory
> >> information that allows it to enable the 6GHz channels.
> >>> Once these channels are enabled, we use passive scan as needed.
> >>>
> >>> We generally try to avoid passive scan on all the 6GHz channels as
> >>> this is a long flow that takes at least 6 seconds (as there are such
> >>> 64
> >> channels) and with the discovery mechanisms defined for the 6GHz is
> >> not really needed.
> >>
> >> If the station comes up and does a 6E passive scan and does not find
> >> any AP, perhaps because 6Ghz AP was turned on 1 minute after the
> >> station tried to initially scan, this means that it will take 50
> >> minutes before it can have a chance to scan the AP and connect to the
> >> Internet? If station cannot connect after a relatively short time,
> >> then I think it should scan as widely as it can in order find some possible
> way to connect.
> >>
> >
> > The purpose of this heuristic was to handle a very specific corner
> > case where there are no APs on the 2GHz/5GHz bands and there are only
> > one or more non-collocated APs on the 6GHz band. Based on our
> > understanding, this is not a very likely situation and thus, due to
> > other consideration e.g., power KPIs etc., the above conditions were
> > defined. However, as you can see in the patch, there are options to tune
> the heuristic to be more aggressive, and if it would indeed be needed we can
> change the behavior such cases.
>
> Yes, and I can tweak the code myself if needed. But better if upstream
> driver is already nice as possible.
>

As I emphasized above, this entire 6GHz passive scan concept is only targeting a very specific
corner case that tries to handle the case that we have no information about the current MCC.
I've raised your points with our regulatory team and they further emphasized that for the common
use case of station operation, as this is expected to be a rare case, they prefer have this logic as
it is implemented.

> >> And why care about 'at least 4 channels'. If we know the AP channel,
> >> and can scan exactly there, then your concern about taking a long time is
> resolved.
> >>
> >
> > The assumption was that while a connection was not established a full
> > scan is expected, so that's why the above condition was set. However,
> > I'll take this with my colleagues and see if this condition can be
> > removed or defined differently.
>
> The complexity of the restrictions are going to silently break certain configs
> that a user can reasonable expect to work I think.

Based on the intended use of this flow, we do not think that this is going to break
user configurations.

>
> >
> >> How else can we tell the radio that 6E is allowed? I previously
> >> tried all sorts of things to enable 6E channels so that I could more
> >> easily set the radio to sniff one of those channels in monitor mode, and I
> had no luck.
> >>
> >
> > Are you asking specifically for iwlwifi devices? I'm not familiar with
> > a simple way to do so other the one described here, but I can check if you
> need it.
>
> Yes. ax210 in particular.
>

With the current regulatory settings that our NICs ships with, the regulatory team stated that
if the device finds enough APs with consistent country code in their beacons (also on legacy bands),
e.g., US country code, then the FW would be able to update the MCC, and if the MCC enables the
6GHz channels then they would be enabled. Thus, the special case of 6GHz passive scan would not be needed.

> >
> >> If all of the 6E channels are marked as passive, what harm is it to
> >> enable the channels in the regdom from the beginning?
> >>
> >
> > AFAIK, as the 6GHz regulatory is still evolving, we are not yet
> > allowed to do so. But once again, If you are interested I can further check
> this our regulatory team.
>
> Your patch enables passive scan of 6Ghz when no regulatory specifically
> allows it.
> I think just enabling the band as passive is the same thing, but significantly
> simplifies things. If there are regulatory reasons to not allow even passsive
> scanning on 6E, then your patch breaks that. If not, then just always allow 6E
> channels to be available in passive mode.
>

The decision was not to enable 6GHz passive scan if not needed as it has a high
penalty in terms on time and power KPIs. When the 6GHz channels are enabled,
our device follows the discovery mechanisms define in Draft IEEE802.11ax which
minimize the scan time and out of channel activity, which is the preferable way.

> The logic to optimize scanning time and channels belongs up in the supplicant
> and other higher level code that can take user input/config and make
> decisions using info that the driver will never have.
>

Agree on this point, but since current wpa_supplicant version and older ones have
no such support than in practice enabling all the 6GHz channels to allow passive
scanning by default would incur penalties in scan/connection time and power.

Hope this helps.

Regards,

Ilan.