2016-08-10 20:37:47

by Wright Feng

[permalink] [raw]
Subject: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface

When stopping hostap on virtual interface, driver will set INFRA and AP
mode that may affect the functionality on primary interface. For example,
if we create and stop hostapd on virtual interface then association
cannot work on primary interface because INFRA mode has been set to IBSS.
Hence we shut down AP and set IBSS mode only on primary interface.

Signed-off-by: Wright Feng <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 2628d5e..0687ab9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -4716,6 +4716,8 @@ exit:

static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
{
+ struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
+ struct net_device *primary_ndev = cfg_to_ndev(cfg);
struct brcmf_if *ifp = netdev_priv(ndev);
s32 err;
struct brcmf_fil_bss_enable_le bss_enable;
@@ -4723,7 +4725,8 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)

brcmf_dbg(TRACE, "Enter\n");

- if (ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) {
+ if ((ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) &&
+ (ndev == primary_ndev)) {
/* Due to most likely deauths outstanding we sleep */
/* first to make sure they get processed by fw. */
msleep(400);
--
1.9.1


This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.


2016-08-10 19:53:21

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface

On 10-8-2016 10:01, Wright Feng wrote:
> When stopping hostap on virtual interface, driver will set INFRA and AP
> mode that may affect the functionality on primary interface. For example,
> if we create and stop hostapd on virtual interface then association
> cannot work on primary interface because INFRA mode has been set to IBSS.
> Hence we shut down AP and set IBSS mode only on primary interface.

What is actually the use-case here. Can you elaborate? BRCMF_C_DOWN
command turns out to be effectively bring the whole stack down and not
just the supplied interface. I suppose you are hitting that issue here
as well, right?

Regards,
Arend

> Signed-off-by: Wright Feng <[email protected]>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 2628d5e..0687ab9 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -4716,6 +4716,8 @@ exit:
>
> static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
> {
> + struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
> + struct net_device *primary_ndev = cfg_to_ndev(cfg);
> struct brcmf_if *ifp = netdev_priv(ndev);
> s32 err;
> struct brcmf_fil_bss_enable_le bss_enable;
> @@ -4723,7 +4725,8 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
>
> brcmf_dbg(TRACE, "Enter\n");
>
> - if (ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) {
> + if ((ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) &&
> + (ndev == primary_ndev)) {
> /* Due to most likely deauths outstanding we sleep */
> /* first to make sure they get processed by fw. */
> msleep(400);
> --
> 1.9.1
>
>
> This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.
>

2016-08-11 05:07:20

by Wright Feng

[permalink] [raw]
Subject: RE: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface



On 10-8-2016 12:15, Arend Van Spriel wrote:
> On 10-8-2016 11:44, Wright Feng wrote:
> > Hi Arend,
> >
> > Thanks for the reply.
> >
> > On 10-8-2016 10:26, Arend Van Spriel wrote:
> >> On 10-8-2016 10:01, Wright Feng wrote:
> >>> When stopping hostap on virtual interface, driver will set INFRA and
> >>> AP mode that may affect the functionality on primary interface. For
> >>> example, if we create and stop hostapd on virtual interface then
> >>> association cannot work on primary interface because INFRA mode has
> >> been set to IBSS.
> >>> Hence we shut down AP and set IBSS mode only on primary interface.
> >>
> >> What is actually the use-case here. Can you elaborate? BRCMF_C_DOWN
> >> command turns out to be effectively bring the whole stack down and
> >> not just the supplied interface. I suppose you are hitting that issue here as
> well, right?
> > We want to use AP mode to let client connecting in AP+STA mode with
> 43438 wi-fi chip.
> > After that, the AP mode will be stopped, and wpa_supplicant cannot
> associate to the access point.
> > I describe the steps in detail as below.
> > 1. Create virtual interface and set mode to __ap 2. start
> > wpa_supplicant on primary interface and connect to wireless router.
> > 3. start hostap daemon on virtual interface and let client connecting.
> > 4. stop hostap daemon
> > 5. wpa_supplicant cannot associate to access point normally.
> >
> > Like you said, the issue may be hit by BRCMF_C_DOWN and same as
> > BRCMF_C_SET_INFRA BRCMF_C_SET_INFRA is not just for the supplied
> interface either. The default bss will be changed in firmware and let firmware
> uses IBSS mode to join.
> > About BRCMF_C_DOWN, driver will set BRCMF_C_UP command to bring
> device back up so it can be used again.
> > However, there is no way to set INFRA mode back except starting AP mode
> again.
>
> Ok.
>
> >>
> >> Regards,
> >> Arend
> >>
> >>> Signed-off-by: Wright Feng <[email protected]>
> >>> ---
> >>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5
> >> ++++-
> >>> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git
> >>> a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> >>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> >>> index 2628d5e..0687ab9 100644
> >>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> >>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> >>> @@ -4716,6 +4716,8 @@ exit:
> >>>
> >>> static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct
> >>> net_device *ndev) {
> >>> + struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
> >>> + struct net_device *primary_ndev = cfg_to_ndev(cfg);
> >>> struct brcmf_if *ifp = netdev_priv(ndev);
> >>> s32 err;
> >>> struct brcmf_fil_bss_enable_le bss_enable; @@ -4723,7
> >>> +4725,8 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy,
> >>> struct net_device *ndev)
> >>>
> >>> brcmf_dbg(TRACE, "Enter\n");
> >>>
> >>> - if (ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) {
> >>> + if ((ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) &&
> >>> + (ndev == primary_ndev)) {
> >>> /* Due to most likely deauths outstanding we sleep */
> >>> /* first to make sure they get processed by fw. */
> >>> msleep(400);
> >>> --
> >>> 1.9.1
> >>>
> >>>
> >>> This message and any attachments may contain Cypress (or its
> >>> subsidiaries)
> >> confidential information. If it has been received in error, please
> >> advise the sender and immediately delete this message.
> >>>
> >
> > This message and any attachments may contain Cypress (or its subsidiaries)
> confidential information. If it has been received in error, please advise the
> sender and immediately delete this message.
>
> Is there any way for you to get rid of this foot note. It may keep Kalle from
> taking this patch. Other option is to take this patch through our internal tree.
No problem. I will remove the footnote from the PATCH v2.
>
> Regards,
> Arend

This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.

2016-08-10 19:12:15

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface

On 10-8-2016 11:44, Wright Feng wrote:
> Hi Arend,
>
> Thanks for the reply.
>
> On 10-8-2016 10:26, Arend Van Spriel wrote:
>> On 10-8-2016 10:01, Wright Feng wrote:
>>> When stopping hostap on virtual interface, driver will set INFRA and
>>> AP mode that may affect the functionality on primary interface. For
>>> example, if we create and stop hostapd on virtual interface then
>>> association cannot work on primary interface because INFRA mode has
>> been set to IBSS.
>>> Hence we shut down AP and set IBSS mode only on primary interface.
>>
>> What is actually the use-case here. Can you elaborate? BRCMF_C_DOWN
>> command turns out to be effectively bring the whole stack down and not just
>> the supplied interface. I suppose you are hitting that issue here as well, right?
> We want to use AP mode to let client connecting in AP+STA mode with 43438 wi-fi chip.
> After that, the AP mode will be stopped, and wpa_supplicant cannot associate to the access point.
> I describe the steps in detail as below.
> 1. Create virtual interface and set mode to __ap
> 2. start wpa_supplicant on primary interface and connect to wireless router.
> 3. start hostap daemon on virtual interface and let client connecting.
> 4. stop hostap daemon
> 5. wpa_supplicant cannot associate to access point normally.
>
> Like you said, the issue may be hit by BRCMF_C_DOWN and same as BRCMF_C_SET_INFRA
> BRCMF_C_SET_INFRA is not just for the supplied interface either. The default bss will be changed in firmware and let firmware uses IBSS mode to join.
> About BRCMF_C_DOWN, driver will set BRCMF_C_UP command to bring device back up so it can be used again.
> However, there is no way to set INFRA mode back except starting AP mode again.

Ok.

>>
>> Regards,
>> Arend
>>
>>> Signed-off-by: Wright Feng <[email protected]>
>>> ---
>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5
>> ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git
>>> a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> index 2628d5e..0687ab9 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> @@ -4716,6 +4716,8 @@ exit:
>>>
>>> static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct
>>> net_device *ndev) {
>>> + struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
>>> + struct net_device *primary_ndev = cfg_to_ndev(cfg);
>>> struct brcmf_if *ifp = netdev_priv(ndev);
>>> s32 err;
>>> struct brcmf_fil_bss_enable_le bss_enable; @@ -4723,7 +4725,8
>>> @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct
>>> net_device *ndev)
>>>
>>> brcmf_dbg(TRACE, "Enter\n");
>>>
>>> - if (ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) {
>>> + if ((ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) &&
>>> + (ndev == primary_ndev)) {
>>> /* Due to most likely deauths outstanding we sleep */
>>> /* first to make sure they get processed by fw. */
>>> msleep(400);
>>> --
>>> 1.9.1
>>>
>>>
>>> This message and any attachments may contain Cypress (or its subsidiaries)
>> confidential information. If it has been received in error, please advise the
>> sender and immediately delete this message.
>>>
>
> This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.

Is there any way for you to get rid of this foot note. It may keep Kalle
from taking this patch. Other option is to take this patch through our
internal tree.

Regards,
Arend

2016-08-10 22:46:17

by Wright Feng

[permalink] [raw]
Subject: RE: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface

Hi Arend,

Thanks for the reply.

On 10-8-2016 10:26, Arend Van Spriel wrote:
> On 10-8-2016 10:01, Wright Feng wrote:
> > When stopping hostap on virtual interface, driver will set INFRA and
> > AP mode that may affect the functionality on primary interface. For
> > example, if we create and stop hostapd on virtual interface then
> > association cannot work on primary interface because INFRA mode has
> been set to IBSS.
> > Hence we shut down AP and set IBSS mode only on primary interface.
>
> What is actually the use-case here. Can you elaborate? BRCMF_C_DOWN
> command turns out to be effectively bring the whole stack down and not just
> the supplied interface. I suppose you are hitting that issue here as well, right?
We want to use AP mode to let client connecting in AP+STA mode with 43438 wi-fi chip.
After that, the AP mode will be stopped, and wpa_supplicant cannot associate to the access point.
I describe the steps in detail as below.
1. Create virtual interface and set mode to __ap
2. start wpa_supplicant on primary interface and connect to wireless router.
3. start hostap daemon on virtual interface and let client connecting.
4. stop hostap daemon
5. wpa_supplicant cannot associate to access point normally.

Like you said, the issue may be hit by BRCMF_C_DOWN and same as BRCMF_C_SET_INFRA
BRCMF_C_SET_INFRA is not just for the supplied interface either. The default bss will be changed in firmware and let firmware uses IBSS mode to join.
About BRCMF_C_DOWN, driver will set BRCMF_C_UP command to bring device back up so it can be used again.
However, there is no way to set INFRA mode back except starting AP mode again.
>
> Regards,
> Arend
>
> > Signed-off-by: Wright Feng <[email protected]>
> > ---
> > drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5
> ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git
> > a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > index 2628d5e..0687ab9 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > @@ -4716,6 +4716,8 @@ exit:
> >
> > static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct
> > net_device *ndev) {
> > + struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
> > + struct net_device *primary_ndev = cfg_to_ndev(cfg);
> > struct brcmf_if *ifp = netdev_priv(ndev);
> > s32 err;
> > struct brcmf_fil_bss_enable_le bss_enable; @@ -4723,7 +4725,8
> > @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct
> > net_device *ndev)
> >
> > brcmf_dbg(TRACE, "Enter\n");
> >
> > - if (ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) {
> > + if ((ifp->vif->wdev.iftype == NL80211_IFTYPE_AP) &&
> > + (ndev == primary_ndev)) {
> > /* Due to most likely deauths outstanding we sleep */
> > /* first to make sure they get processed by fw. */
> > msleep(400);
> > --
> > 1.9.1
> >
> >
> > This message and any attachments may contain Cypress (or its subsidiaries)
> confidential information. If it has been received in error, please advise the
> sender and immediately delete this message.
> >

This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.

2016-08-15 08:24:54

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface

Wright Feng <[email protected]> writes:

>> -----Original Message-----
>> From: Kalle Valo [mailto:[email protected]]
>> Sent: Monday, August 15, 2016 4:05 PM
>> To: Arend Van Spriel <[email protected]>
>> Cc: Wright Feng <[email protected]>; brcm80211-dev-
>> [email protected]; [email protected];
>> [email protected]; [email protected]; Chi-Hsien Lin
>> <[email protected]>; [email protected]
>> Subject: Re: [PATCH] brcmfmac: shut down AP and set IBSS mode only on
>> primary interface
>>
>> Arend Van Spriel <[email protected]> writes:
>>
>> >> This message and any attachments may contain Cypress (or its
>> >> subsidiaries) confidential information. If it has been received in
>> >> error, please advise the sender and immediately delete this message.
>> >
>> > Is there any way for you to get rid of this foot note. It may keep
>> > Kalle from taking this patch.
>>
>> Correct. I'll automatically drop patches with these kind of notices.
>
> I've removed the foot note and sent the [PATCH v2] to you on 8/11.
> Is that okay to you?

>From a quick look seems to be ok. But I'll do proper review later, need
to go through pile of mails after coming back from vacation.

--
Kalle Valo

2016-08-15 08:43:54

by Wright Feng

[permalink] [raw]
Subject: RE: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface

Hi Kalle and Arend,

> -----Original Message-----
> From: Kalle Valo [mailto:[email protected]]
> Sent: Monday, August 15, 2016 4:05 PM
> To: Arend Van Spriel <[email protected]>
> Cc: Wright Feng <[email protected]>; brcm80211-dev-
> [email protected]; [email protected];
> [email protected]; [email protected]; Chi-Hsien Lin
> <[email protected]>; [email protected]
> Subject: Re: [PATCH] brcmfmac: shut down AP and set IBSS mode only on
> primary interface
>
> Arend Van Spriel <[email protected]> writes:
>
> >> This message and any attachments may contain Cypress (or its
> >> subsidiaries) confidential information. If it has been received in
> >> error, please advise the sender and immediately delete this message.
> >
> > Is there any way for you to get rid of this foot note. It may keep
> > Kalle from taking this patch.
>
> Correct. I'll automatically drop patches with these kind of notices.
I've removed the foot note and sent the [PATCH v2] to you on 8/11.
Is that okay to you?
>
> --
> Kalle Valo

This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.

2016-08-15 08:05:14

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface

Arend Van Spriel <[email protected]> writes:

>> This message and any attachments may contain Cypress (or its
>> subsidiaries) confidential information. If it has been received in
>> error, please advise the sender and immediately delete this message.
>
> Is there any way for you to get rid of this foot note. It may keep Kalle
> from taking this patch.

Correct. I'll automatically drop patches with these kind of notices.

--
Kalle Valo