brcmfmac: remove setting IBSS mode when stopping AP

PATCH In review Wright Feng 3 participants 5 messages May 18, 2017 – May 22, 2017 Broadcom BRCM80211 IEEE802.11 wireless drivers

lore

Share & tools

lore

Wright Feng

author

2017-05-18 10:32 UTC #0 lore raw

PATCH 1/1 brcmfmac: remove setting IBSS mode when stopping AP

From: Chi-hsien Lin <[email protected]>

Setting IBSS mode when stopping AP may affect functionality of INFRA mode on another interface. For instance, if user creates and stops hostap daemon on virtual interface, then association cannot work on primary interface because default BSS has been set to IBSS mode in firmware side. The IBSS mode should be set when cfg80211 changes the interface.

Reviewed-by Wright Feng <[email protected]>

Signed-off-by Chi-hsien Lin <[email protected]>

Signed-off-by Wright Feng <[email protected]>

---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 ---
 1 file changed, 3 deletions(-)

drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +0 -3

index 944b83c..ce2a5b6 100644
@@ -4658,9 +4658,6 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
 		err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
 		if (err < 0)
 			brcmf_err("setting AP mode failed %d\n", err);
-		err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_INFRA, 0);
-		if (err < 0)
-			brcmf_err("setting INFRA mode failed %d\n", err);
 		if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MBSS))
 			brcmf_fil_iovar_int_set(ifp, "mbss", 0);
 		brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_REGULATORY,

Signature · 1 line

2.1.0

Kalle Valo

2017-05-19 04:50 UTC #1 lore raw

Quoted 21 lines · Wright Feng

From: Chi-hsien Lin <[email protected]>
Setting IBSS mode when stopping AP may affect functionality of INFRA mode on another interface. For instance, if user creates and stops hostap daemon on virtual interface, then association cannot work on primary interface because default BSS has been set to IBSS mode in firmware side. The IBSS mode should be set when cfg80211 changes the interface.
Reviewed-by Wright Feng <[email protected]>
Signed-off-by Chi-hsien Lin <[email protected]>
Signed-off-by Wright Feng <[email protected]>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 ---
 1 file changed, 3 deletions(-)
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +0 -0
index 944b83c..ce2a5b6 100644

drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev) · line 4663

Quoted 6 lines

drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +0 -3

@@ -4658,9 +4658,6 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
 		err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
 		if (err < 0)
 			brcmf_err("setting AP mode failed %d\n", err);
-		err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_INFRA, 0);
-		if (err < 0)
-			brcmf_err("setting INFRA mode failed %d\n", err);

The title talks about setting IBSS mode but the code looks to be about setting INFRA mode. Is this correct?

Signature · 1 line

Kalle Valo

Arend van Spriel

maintainer

2017-05-19 07:50 UTC #2 lore raw

Quoted 18 lines · Kalle Valo

Quoted 16 lines · Wright Feng
From: Chi-hsien Lin <[email protected]>
Setting IBSS mode when stopping AP may affect functionality of INFRA mode on another interface. For instance, if user creates and stops hostap daemon on virtual interface, then association cannot work on primary interface because default BSS has been set to IBSS mode in firmware side. The IBSS mode should be set when cfg80211 changes the interface.
Reviewed-by Wright Feng <[email protected]>
Signed-off-by Chi-hsien Lin <[email protected]>
Signed-off-by Wright Feng <[email protected]>
---
  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 ---
  1 file changed, 3 deletions(-)

Quoted 14 lines

drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev) · line 4663
Quoted 11 lines
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +0 -3
index 944b83c..ce2a5b6 100644
@@ -4658,9 +4658,6 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
  		err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
  		if (err < 0)
  			brcmf_err("setting AP mode failed %d\n", err);
-		err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_INFRA, 0);
-		if (err < 0)
-			brcmf_err("setting INFRA mode failed %d\n", err);
The title talks about setting IBSS mode but the code looks to be about setting INFRA mode. Is this correct?

Both are correct :-p Have to admit this is confusing. The command SET_INFRA with zero value means "disable INFRA; thus enable IBSS".

I would rephrase the commit message indeed explaining what the code currently does, ie.:

Upon stopping an AP interface the driver disable INFRA mode effectively setting the interface in IBSS mode. However, this may affect other interfaces running in INFRA mode. For instance, ....

Regards,
Arend

Kalle Valo

2017-05-19 07:54 UTC #3 lore raw

Quoted 34 lines · Arend van Spriel

Quoted 18 lines · Kalle Valo
Quoted 16 lines · Wright Feng
From: Chi-hsien Lin <[email protected]>
Setting IBSS mode when stopping AP may affect functionality of INFRA mode on another interface. For instance, if user creates and stops hostap daemon on virtual interface, then association cannot work on primary interface because default BSS has been set to IBSS mode in firmware side. The IBSS mode should be set when cfg80211 changes the interface.
Reviewed-by Wright Feng <[email protected]>
Signed-off-by Chi-hsien Lin <[email protected]>
Signed-off-by Wright Feng <[email protected]>
---
  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 ---
  1 file changed, 3 deletions(-)
Quoted 14 lines
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev) · line 4663
Quoted 11 lines
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +0 -3
index 944b83c..ce2a5b6 100644
@@ -4658,9 +4658,6 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
  		err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
  		if (err < 0)
  			brcmf_err("setting AP mode failed %d\n", err);
-		err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_INFRA, 0);
-		if (err < 0)
-			brcmf_err("setting INFRA mode failed %d\n", err);
The title talks about setting IBSS mode but the code looks to be about setting INFRA mode. Is this correct?

Quoted 2 lines

Both are correct :-p Have to admit this is confusing. The command SET_INFRA with zero value means "disable INFRA; thus enable IBSS".

Ah, that IS confusing :)

Quoted 6 lines

I would rephrase the commit message indeed explaining what the code currently does, ie.:
Upon stopping an AP interface the driver disable INFRA mode effectively setting the interface in IBSS mode. However, this may affect other interfaces running in INFRA mode. For instance, ....

I can fix that during commit. Thanks.

Signature · 1 line

Kalle Valo

Kalle Valo

2017-05-22 15:21 UTC #4 lore raw

In reply to Wright Feng

Quoted 10 lines · Wright Feng

From: Chi-hsien Lin <[email protected]>
Upon stopping an AP interface the driver disable INFRA mode effectively setting the interface in IBSS mode. However, this may affect other interfaces running in INFRA mode. For instance, if user creates and stops hostap daemon on virtual interface, then association cannot work on primary interface because default BSS has been set to IBSS mode in firmware side. The IBSS mode should be set when cfg80211 changes the interface.

Quoted 4 lines

Reviewed-by Wright Feng <[email protected]>
Signed-off-by Chi-hsien Lin <[email protected]>
[[email protected]: rephased commit log based on discussion]
Signed-off-by Wright Feng <[email protected]>

Patch applied to wireless-drivers-next.git, thanks.

9029679f66d9 brcmfmac: remove setting IBSS mode when stopping AP

Signature · 3 lines

https://patchwork.kernel.org/patch/9733205/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches