| Message ID | 20220224181809.2808754-1-prestwoj@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | brcmfmac: include missing AP scan feature | expand |
Hi James, James Prestwood <prestwoj@gmail.com> 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:
Hi Alvin, On Fri, 2022-02-25 at 22:55 +0000, Alvin Šipraga wrote: > Hi James, > > James Prestwood <prestwoj@gmail.com> 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:
Hi James, James Prestwood <prestwoj@gmail.com> writes: > Hi Alvin, > > On Fri, 2022-02-25 at 22:55 +0000, Alvin Šipraga wrote: >> Hi James, >> >> James Prestwood <prestwoj@gmail.com> 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/96652669-0cad-8cdb-fbe1-4df0f7161102@bang-olufsen.dk/ 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:
Hi Alvin, On Sat, 2022-02-26 at 11:27 +0000, Alvin Šipraga wrote: > Hi James, > > James Prestwood <prestwoj@gmail.com> writes: > > > Hi Alvin, > > > > On Fri, 2022-02-25 at 22:55 +0000, Alvin Šipraga wrote: > > > Hi James, > > > > > > James Prestwood <prestwoj@gmail.com> 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/96652669-0cad-8cdb-fbe1-4df0f7161102@bang-olufsen.dk/ > 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:
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 <prestwoj@gmail.com> writes: >> >>> Hi Alvin, >>> >>> On Fri, 2022-02-25 at 22:55 +0000, Alvin Šipraga wrote: >>>> Hi James, >>>> >>>> James Prestwood <prestwoj@gmail.com> 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
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 <prestwoj@gmail.com> writes: > > > > > > > Hi Alvin, > > > > > > > > On Fri, 2022-02-25 at 22:55 +0000, Alvin Šipraga wrote: > > > > > Hi James, > > > > > > > > > > James Prestwood <prestwoj@gmail.com> 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
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: