diff mbox

[next] brcmfmac: don't remove interface on link down firmware event

Message ID 1464557989-20003-1-git-send-email-zajec5@gmail.com
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

Rafał Miłecki May 29, 2016, 9:39 p.m. UTC 
There are two firmware events we handle similarly in brcmfmac:
BRCMF_E_LINK and BRCMF_E_IF. The difference from firmware point of view
is that the first one means BSS remains present in the firmware. Trying
to (re)create it (e.g. when adding new virtual interface) will result in
an error.

Current code treats both events in a similar way. It removes Linux
interface for each of them. It works OK with e.g. BCM43602. Its firmware
generates both events for each interface. It means we get BRCMF_E_LINK
and remove interface. That is soon followed by BRCMF_E_IF which means
BSS was also removed in a firmware. The only downside of this is a
harmless error like:
[  208.643180] brcmfmac: brcmf_fweh_call_event_handler: no interface object

Unfortunately BCM4366 firmware doesn't automatically remove BSS and so
it doesn't generate BRCMF_E_IF. In such case we incorrectly remove Linux
interface on BRCMF_E_LINK as BSS is still present in the firmware. It
results in an error when trying to re-create virtual interface, e.g.:
> iw phy phy1 interface add wlan1-1 type __ap
[ 3602.929199] brcmfmac: brcmf_ap_add_vif: timeout occurred
command failed: I/O error (-5)

With this patch we don't remove Linux interface while firmware keeps
BSS. Thanks to this we keep a consistent states of host driver and
device firmware.

Further improvement should be to mark BSS as disabled and remove
interface on BRCMF_E_LINK. Then we should add support for reusing
BSS-es.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 --
 1 file changed, 2 deletions(-)

Comments

kernel test robot May 29, 2016, 10:01 p.m. UTC | #1 
Hi,

[auto build test WARNING on next-20160527]

url:    https://github.com/0day-ci/linux/commits/Rafa-Mi-ecki/brcmfmac-don-t-remove-interface-on-link-down-firmware-event/20160530-054251
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All warnings (new ones prefixed by >>):

   drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c: In function 'brcmf_notify_connect_status_ap':
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:5339:19: warning: unused variable 'ifp' [-Wunused-variable]
     struct brcmf_if *ifp = netdev_priv(ndev);
                      ^

vim +/ifp +5339 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c

5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2011-10-05  5323  					conn_info->resp_ie_len,
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2011-10-05  5324  					completed ? WLAN_STATUS_SUCCESS :
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2011-10-05  5325  						    WLAN_STATUS_AUTH_TIMEOUT,
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2011-10-05  5326  					GFP_KERNEL);
16886735 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2012-12-05  5327  		brcmf_dbg(CONN, "Report connect result - connection %s\n",
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2011-10-05  5328  			  completed ? "succeeded" : "failed");
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2011-10-05  5329  	}
d96b801f drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2012-12-05  5330  	brcmf_dbg(TRACE, "Exit\n");
12f32370 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Peter Senna Tschudin 2014-05-31  5331  	return 0;
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2011-10-05  5332  }
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2011-10-05  5333  
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2011-10-05  5334  static s32
27a68fe3 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2012-09-27  5335  brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Hante Meuleman       2012-09-27  5336  			       struct net_device *ndev,
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Hante Meuleman       2012-09-27  5337  			       const struct brcmf_event_msg *e, void *data)
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Hante Meuleman       2012-09-27  5338  {
a44aa400 drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c    Hante Meuleman       2014-12-03 @5339  	struct brcmf_if *ifp = netdev_priv(ndev);
7ee29602 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Hante Meuleman       2013-02-06  5340  	static int generation;
5c36b99a drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2012-11-14  5341  	u32 event = e->event_code;
5c36b99a drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2012-11-14  5342  	u32 reason = e->reason;
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Hante Meuleman       2012-09-27  5343  	struct station_info sinfo;
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Hante Meuleman       2012-09-27  5344  
16886735 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2012-12-05  5345  	brcmf_dbg(CONN, "event %d, reason %d\n", event, reason);
5f4f9f11 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2013-02-08  5346  	if (event == BRCMF_E_LINK && reason == BRCMF_E_REASON_LINK_BSSCFG_DIS &&
5f4f9f11 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel     2013-02-08  5347  	    ndev != cfg_to_ndev(cfg)) {

:::::: The code at line 5339 was first introduced by commit
:::::: a44aa4001a86d46f936ca449e5d6c268446bfae2 brcmfmac: add multiple BSS support.

:::::: TO: Hante Meuleman <meuleman@broadcom.com>
:::::: CC: John W. Linville <linville@tuxdriver.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 38df1be..26ff0f4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5439,8 +5439,6 @@  brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
 	    ndev != cfg_to_ndev(cfg)) {
 		brcmf_dbg(CONN, "AP mode link down\n");
 		complete(&cfg->vif_disabled);
-		if (ifp->vif->mbss)
-			brcmf_remove_interface(ifp);
 		return 0;
 	}