Re: [PATCH RFC 1/2] brcmfmac: remove interface before notifying listener

On 18 June 2016 at 23:58, Rafał Miłecki [off-list ref] wrote:

On 18 June 2016 at 21:26, Arend van Spriel [off-list ref] wrote:

quoted

On 18-06-16 20:18, Rafał Miłecki wrote:

quoted

So far when receiving event about in-firmware-interface removal we were
notifying our listener and afterwards we were removing Linux interface.

[snip]

quoted

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index 9da7a4c..5fd1886 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c

@@ -18,6 +18,7 @@
 #include "brcmu_wifi.h"
 #include "brcmu_utils.h"

+#include "cfg80211.h"
 #include "core.h"
 #include "debug.h"
 #include "tracepoint.h"

@@ -180,10 +181,16 @@ static void brcmf_fweh_handle_if_event(struct brcmf_pub *drvr,
      if (ifp && ifevent->action == BRCMF_E_IF_CHANGE)
              brcmf_fws_reset_interface(ifp);

-     err = brcmf_fweh_call_event_handler(ifp, emsg->event_code, emsg, data);

The reason for doing this first is because we are passing the ifp, which
is netdev_priv(ifp->ndev). In brcmf_remove_interface() we only
unregister the netdev, which will end up (after scheduling) in
brcmf_free_netdev() thus freeing the ifp. By moving the event handler
function ifp may be stale already.

Good catch. What about making brcmf_fweh_call_event_handler work
without ifp? Would that be OK then?

Maybe I have even better idea. What about handling Linux interface
removal in the code waiting for BRCMF_E_IF_DEL? We already do
something like that in case of BRCMF_E_IF_ADD (it is brcmf_ap_add_vif
that calls brcmf_net_attach).