2022-02-24 18:42:50

by James Prestwood

[permalink] [raw]
Subject: [PATCH] brcmfmac: include missing AP scan feature

This driver does not advertise this feature yet scanning with on an
AP interface appears to work just fine.
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 ++
1 file changed, 2 insertions(+)

I've submitted this patch mainly to start a discussion about it. I
find it hard to believe that ALL brcmfmac devices support AP scanning
in which case this feature needs to be limited to those devices
only. Trouble is there is no FW feature for AP scanning AFAIK.

In any case I think this driver needs to sort out if it supports this
feature or not, and advertise as such rather than leaving userspace
in the dark.

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index fb727778312c..b6a50e65dbf6 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -7729,6 +7729,8 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
#endif
}

+ wiphy->features |= NL80211_FEATURE_AP_SCAN;
+
return cfg;

detach:
--
2.34.1


2022-02-26 01:30:32

by Alvin Šipraga

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: include missing AP scan feature

Hi James,

James Prestwood <[email protected]> writes:

> This driver does not advertise this feature yet scanning with on an
> AP interface appears to work just fine.
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> I've submitted this patch mainly to start a discussion about it. I
> find it hard to believe that ALL brcmfmac devices support AP scanning
> in which case this feature needs to be limited to those devices
> only. Trouble is there is no FW feature for AP scanning AFAIK.
>
> In any case I think this driver needs to sort out if it supports this
> feature or not, and advertise as such rather than leaving userspace
> in the dark.

By the way, what are the typical use-cases for AP scanning?

I know that hostapd does a passive scan on the AP interface on the
assumption that the driver/firmware will gather channel survey data, but
that's not a universally applicable assumption. Not all implementations
will do that.

Kind regards,
Alvin

>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index fb727778312c..b6a50e65dbf6 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -7729,6 +7729,8 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
> #endif
> }
>
> + wiphy->features |= NL80211_FEATURE_AP_SCAN;
> +
> return cfg;
>
> detach:

2022-02-26 02:01:39

by James Prestwood

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: include missing AP scan feature

Hi Alvin,

On Fri, 2022-02-25 at 22:55 +0000, Alvin Šipraga wrote:
> Hi James,
>
> James Prestwood <[email protected]> writes:
>
> > This driver does not advertise this feature yet scanning with on an
> > AP interface appears to work just fine.
> > ---
> >  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > I've submitted this patch mainly to start a discussion about it. I
> > find it hard to believe that ALL brcmfmac devices support AP
> > scanning
> > in which case this feature needs to be limited to those devices
> > only. Trouble is there is no FW feature for AP scanning AFAIK.
> >
> > In any case I think this driver needs to sort out if it supports
> > this
> > feature or not, and advertise as such rather than leaving userspace
> > in the dark.
>
> By the way, what are the typical use-cases for AP scanning?
>
> I know that hostapd does a passive scan on the AP interface on the
> assumption that the driver/firmware will gather channel survey data,
> but
> that's not a universally applicable assumption. Not all
> implementations
> will do that.

We have someone wanting it for onboarding/configuring a new headless
device. Where on boot, if it is unconfigured, it starts an AP and waits
for a client to configure it.

A client would connect, have the device scan and present available
networks. The client then selects a network and provides credentials.
The new device can then switch back to station mode and connect.

This is a relatively common practice I've seen with IoT devices.

Other than this I cant see much else of a use case besides, like you
mentioned, gathering survey data to choose a low load channel (ACS its
called I think?)

Sadly this onboarding use case is quite perfect for DPP, but since
Apple came up with their own protocol DPP won't work for any products
that want cross compatibility...

>
> Kind regards,
> Alvin
>
> >
> > diff --git
> > a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > index fb727778312c..b6a50e65dbf6 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > @@ -7729,6 +7729,8 @@ struct brcmf_cfg80211_info
> > *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
> >  #endif
> >         }
> >  
> > +       wiphy->features |= NL80211_FEATURE_AP_SCAN;
> > +
> >         return cfg;
> >  
> >  detach:


2022-02-26 12:44:36

by Alvin Šipraga

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: include missing AP scan feature

Hi James,

James Prestwood <[email protected]> writes:

> Hi Alvin,
>
> On Fri, 2022-02-25 at 22:55 +0000, Alvin Šipraga wrote:
>> Hi James,
>>
>> James Prestwood <[email protected]> writes:
>>
>> > This driver does not advertise this feature yet scanning with on an
>> > AP interface appears to work just fine.
>> > ---
>> >  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > I've submitted this patch mainly to start a discussion about it. I
>> > find it hard to believe that ALL brcmfmac devices support AP
>> > scanning
>> > in which case this feature needs to be limited to those devices
>> > only. Trouble is there is no FW feature for AP scanning AFAIK.
>> >
>> > In any case I think this driver needs to sort out if it supports
>> > this
>> > feature or not, and advertise as such rather than leaving userspace
>> > in the dark.
>>
>> By the way, what are the typical use-cases for AP scanning?
>>
>> I know that hostapd does a passive scan on the AP interface on the
>> assumption that the driver/firmware will gather channel survey data,
>> but
>> that's not a universally applicable assumption. Not all
>> implementations
>> will do that.
>
> We have someone wanting it for onboarding/configuring a new headless
> device. Where on boot, if it is unconfigured, it starts an AP and waits
> for a client to configure it.
>
> A client would connect, have the device scan and present available
> networks. The client then selects a network and provides credentials.
> The new device can then switch back to station mode and connect.
>
> This is a relatively common practice I've seen with IoT devices.

Ah OK! Actually we do pretty much the same here at B&O with
brcmfmac. But rather than starting the AP on the primary interface (the
one default created by the driver), we add a separate AP interface with
the equivalent of the following command:

# iw dev wlan0 add uap0 type __ap

Here wlan0 is the primary interface, and uap0 is what I call my AP.

In that case you can start the AP on uap0, but still do scanning on
wlan0 (which remains in station mode). Don't quote me on it, but I think
this is the canonical approach with this driver. Is it something you
have considered?

>
> Other than this I cant see much else of a use case besides, like you
> mentioned, gathering survey data to choose a low load channel (ACS its
> called I think?)

Yes, see hostap/src/ap/acs.c.

See also
https://lore.kernel.org/linux-wireless/[email protected]/
for some grumblings of mine on this API.

>
> Sadly this onboarding use case is quite perfect for DPP, but since
> Apple came up with their own protocol DPP won't work for any products
> that want cross compatibility...

Yes, this is a real shame. And I highly doubt that Apple will abandon
their own protocol.

>
>>
>> Kind regards,
>> Alvin
>>
>> >
>> > diff --git
>> > a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> > index fb727778312c..b6a50e65dbf6 100644
>> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> > @@ -7729,6 +7729,8 @@ struct brcmf_cfg80211_info
>> > *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
>> >  #endif
>> >         }
>> >  
>> > +       wiphy->features |= NL80211_FEATURE_AP_SCAN;
>> > +
>> >         return cfg;
>> >  
>> >  detach:

2022-02-28 17:59:16

by James Prestwood

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: include missing AP scan feature

Hi Alvin,

On Sat, 2022-02-26 at 11:27 +0000, Alvin Šipraga wrote:
> Hi James,
>
> James Prestwood <[email protected]> writes:
>
> > Hi Alvin,
> >
> > On Fri, 2022-02-25 at 22:55 +0000, Alvin Šipraga wrote:
> > > Hi James,
> > >
> > > James Prestwood <[email protected]> writes:
> > >
> > > > This driver does not advertise this feature yet scanning with
> > > > on an
> > > > AP interface appears to work just fine.
> > > > ---
> > > >  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c |
> > > > 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > I've submitted this patch mainly to start a discussion about
> > > > it. I
> > > > find it hard to believe that ALL brcmfmac devices support AP
> > > > scanning
> > > > in which case this feature needs to be limited to those devices
> > > > only. Trouble is there is no FW feature for AP scanning AFAIK.
> > > >
> > > > In any case I think this driver needs to sort out if it
> > > > supports
> > > > this
> > > > feature or not, and advertise as such rather than leaving
> > > > userspace
> > > > in the dark.
> > >
> > > By the way, what are the typical use-cases for AP scanning?
> > >
> > > I know that hostapd does a passive scan on the AP interface on
> > > the
> > > assumption that the driver/firmware will gather channel survey
> > > data,
> > > but
> > > that's not a universally applicable assumption. Not all
> > > implementations
> > > will do that.
> >
> > We have someone wanting it for onboarding/configuring a new
> > headless
> > device. Where on boot, if it is unconfigured, it starts an AP and
> > waits
> > for a client to configure it.
> >
> > A client would connect, have the device scan and present available
> > networks. The client then selects a network and provides
> > credentials.
> > The new device can then switch back to station mode and connect.
> >
> > This is a relatively common practice I've seen with IoT devices.
>
> Ah OK! Actually we do pretty much the same here at B&O with
> brcmfmac. But rather than starting the AP on the primary interface
> (the
> one default created by the driver), we add a separate AP interface
> with
> the equivalent of the following command:
>
> # iw dev wlan0 add uap0 type __ap
>
> Here wlan0 is the primary interface, and uap0 is what I call my AP.
>
> In that case you can start the AP on uap0, but still do scanning on
> wlan0 (which remains in station mode). Don't quote me on it, but I
> think
> this is the canonical approach with this driver. Is it something you
> have considered?

Thanks, this does seem to work on brcmfmac. I had tried this on other
hardware without any luck. I mentioned this to the user but since the
AP scanning feature has been implemented they may want to still use
that, but who knows. I think finding out if brcmfmac is intended to
scan on the AP interface would still be good to know.

> >
> > Other than this I cant see much else of a use case besides, like
> > you
> > mentioned, gathering survey data to choose a low load channel (ACS
> > its
> > called I think?)
>
> Yes, see hostap/src/ap/acs.c.
>
> See also
> https://lore.kernel.org/linux-wireless/[email protected]/
> for some grumblings of mine on this API.
>
> >
> > Sadly this onboarding use case is quite perfect for DPP, but since
> > Apple came up with their own protocol DPP won't work for any
> > products
> > that want cross compatibility...
>
> Yes, this is a real shame. And I highly doubt that Apple will abandon
> their own protocol.
>
> >
> > >
> > > Kind regards,
> > > Alvin
> > >
> > > >
> > > > diff --git
> > > > a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > > > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > > > index fb727778312c..b6a50e65dbf6 100644
> > > > ---
> > > > a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > > > +++
> > > > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > > > @@ -7729,6 +7729,8 @@ struct brcmf_cfg80211_info
> > > > *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
> > > >  #endif
> > > >         }
> > > >  
> > > > +       wiphy->features |= NL80211_FEATURE_AP_SCAN;
> > > > +
> > > >         return cfg;
> > > >  
> > > >  detach:


2022-03-02 23:38:54

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: include missing AP scan feature

On 2/28/2022 6:17 PM, James Prestwood wrote:
> Hi Alvin,
>
> On Sat, 2022-02-26 at 11:27 +0000, Alvin Šipraga wrote:
>> Hi James,
>>
>> James Prestwood <[email protected]> writes:
>>
>>> Hi Alvin,
>>>
>>> On Fri, 2022-02-25 at 22:55 +0000, Alvin Šipraga wrote:
>>>> Hi James,
>>>>
>>>> James Prestwood <[email protected]> writes:
>>>>
>>>>> This driver does not advertise this feature yet scanning with
>>>>> on an
>>>>> AP interface appears to work just fine.
>>>>> ---
>>>>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c |
>>>>> 2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> I've submitted this patch mainly to start a discussion about
>>>>> it. I
>>>>> find it hard to believe that ALL brcmfmac devices support AP
>>>>> scanning
>>>>> in which case this feature needs to be limited to those devices
>>>>> only. Trouble is there is no FW feature for AP scanning AFAIK.
>>>>>
>>>>> In any case I think this driver needs to sort out if it
>>>>> supports
>>>>> this
>>>>> feature or not, and advertise as such rather than leaving
>>>>> userspace
>>>>> in the dark.
>>>>
>>>> By the way, what are the typical use-cases for AP scanning?
>>>>
>>>> I know that hostapd does a passive scan on the AP interface on
>>>> the
>>>> assumption that the driver/firmware will gather channel survey
>>>> data,
>>>> but
>>>> that's not a universally applicable assumption. Not all
>>>> implementations
>>>> will do that.
>>>
>>> We have someone wanting it for onboarding/configuring a new
>>> headless
>>> device. Where on boot, if it is unconfigured, it starts an AP and
>>> waits
>>> for a client to configure it.
>>>
>>> A client would connect, have the device scan and present available
>>> networks. The client then selects a network and provides
>>> credentials.
>>> The new device can then switch back to station mode and connect.
>>>
>>> This is a relatively common practice I've seen with IoT devices.
>>
>> Ah OK! Actually we do pretty much the same here at B&O with
>> brcmfmac. But rather than starting the AP on the primary interface
>> (the
>> one default created by the driver), we add a separate AP interface
>> with
>> the equivalent of the following command:
>>
>> # iw dev wlan0 add uap0 type __ap
>>
>> Here wlan0 is the primary interface, and uap0 is what I call my AP.
>>
>> In that case you can start the AP on uap0, but still do scanning on
>> wlan0 (which remains in station mode). Don't quote me on it, but I
>> think
>> this is the canonical approach with this driver. Is it something you
>> have considered?
>
> Thanks, this does seem to work on brcmfmac. I had tried this on other
> hardware without any luck. I mentioned this to the user but since the
> AP scanning feature has been implemented they may want to still use
> that, but who knows. I think finding out if brcmfmac is intended to
> scan on the AP interface would still be good to know.

There is no easy answer to that. It really depends on the
device/firmware. To be honest I don't know if the older chips can
support it. Need to check that.

What device are you specifically looking at?

Regards,
Arend


Attachments:
smime.p7s (4.12 kB)
S/MIME Cryptographic Signature

2022-03-03 00:27:37

by James Prestwood

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: include missing AP scan feature

Hi Arend,

On Wed, 2022-03-02 at 10:24 +0100, Arend van Spriel wrote:
> On 2/28/2022 6:17 PM, James Prestwood wrote:
> > Hi Alvin,
> >
> > On Sat, 2022-02-26 at 11:27 +0000, Alvin Šipraga wrote:
> > > Hi James,
> > >
> > > James Prestwood <[email protected]> writes:
> > >
> > > > Hi Alvin,
> > > >
> > > > On Fri, 2022-02-25 at 22:55 +0000, Alvin Šipraga wrote:
> > > > > Hi James,
> > > > >
> > > > > James Prestwood <[email protected]> writes:
> > > > >
> > > > > > This driver does not advertise this feature yet scanning
> > > > > > with
> > > > > > on an
> > > > > > AP interface appears to work just fine.
> > > > > > ---
> > > > > >   drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211
> > > > > > .c |
> > > > > > 2 ++
> > > > > >   1 file changed, 2 insertions(+)
> > > > > >
> > > > > > I've submitted this patch mainly to start a discussion
> > > > > > about
> > > > > > it. I
> > > > > > find it hard to believe that ALL brcmfmac devices support
> > > > > > AP
> > > > > > scanning
> > > > > > in which case this feature needs to be limited to those
> > > > > > devices
> > > > > > only. Trouble is there is no FW feature for AP scanning
> > > > > > AFAIK.
> > > > > >
> > > > > > In any case I think this driver needs to sort out if it
> > > > > > supports
> > > > > > this
> > > > > > feature or not, and advertise as such rather than leaving
> > > > > > userspace
> > > > > > in the dark.
> > > > >
> > > > > By the way, what are the typical use-cases for AP scanning?
> > > > >
> > > > > I know that hostapd does a passive scan on the AP interface
> > > > > on
> > > > > the
> > > > > assumption that the driver/firmware will gather channel
> > > > > survey
> > > > > data,
> > > > > but
> > > > > that's not a universally applicable assumption. Not all
> > > > > implementations
> > > > > will do that.
> > > >
> > > > We have someone wanting it for onboarding/configuring a new
> > > > headless
> > > > device. Where on boot, if it is unconfigured, it starts an AP
> > > > and
> > > > waits
> > > > for a client to configure it.
> > > >
> > > > A client would connect, have the device scan and present
> > > > available
> > > > networks. The client then selects a network and provides
> > > > credentials.
> > > > The new device can then switch back to station mode and
> > > > connect.
> > > >
> > > > This is a relatively common practice I've seen with IoT
> > > > devices.
> > >
> > > Ah OK! Actually we do pretty much the same here at B&O with
> > > brcmfmac. But rather than starting the AP on the primary
> > > interface
> > > (the
> > > one default created by the driver), we add a separate AP
> > > interface
> > > with
> > > the equivalent of the following command:
> > >
> > > # iw dev wlan0 add uap0 type __ap
> > >
> > > Here wlan0 is the primary interface, and uap0 is what I call my
> > > AP.
> > >
> > > In that case you can start the AP on uap0, but still do scanning
> > > on
> > > wlan0 (which remains in station mode). Don't quote me on it, but
> > > I
> > > think
> > > this is the canonical approach with this driver. Is it something
> > > you
> > > have considered?
> >
> > Thanks, this does seem to work on brcmfmac. I had tried this on
> > other
> > hardware without any luck. I mentioned this to the user but since
> > the
> > AP scanning feature has been implemented they may want to still use
> > that, but who knows. I think finding out if brcmfmac is intended to
> > scan on the AP interface would still be good to know.
>
> There is no easy answer to that. It really depends on the
> device/firmware. To be honest I don't know if the older chips can
> support it. Need to check that.

Thats what I figured.

>
> What device are you specifically looking at?

I've tried on a Raspberry Pi 3, which is a BCM4345 I believe.

But I'm not too concerned about knowing for this one device. We really
would like a general way to know, i.e. the feature flag. If this is
possible to set based on some FW values that would be great to do.
Otherwise, if its not possible, saying "No it doesn't support it" is
also fine with me, and we can continue on assuming brcmfmac as a whole
doesn't support it.

What is confusing (to me) is the lack of the feature flag but the
driver still accepts scans on AP interfaces, as well as accepting the
NL80211_SCAN_FLAG_AP. I would expect CMD_TRIGGER scan to fail with "Not
Supported".

Thanks,
James

>
> Regards,
> Arend