Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device

Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device

[Arend van Spriel]

On 02/26/2014 12:10 PM, Johannes Berg wrote:
> On Wed, 2014-02-26 at 11:43 +0100, Arend van Spriel wrote:
> 
>> Actually, this was not the reason for the doing the msleep(). The
>> problem was that wpa_supplicant received RTM_DELLINK, before getting the
>> NL08211_DISCONNECT. This resulted in RTM_DELLINK, immediately followed
>> by RTM_ADDLINK, followed by NL80211_DISCONNECT.
> 
> I'm not sure this isn't still possible? I don't know when DELLINK is
> sent.

Given the behaviour I would say before the netdev notifier, which may be
considered wrong. It depends whether the notifier was intended for the
kind of thing that cfg80211 is doing, ie. send a netlink event to
user-space?

Gr. AvS

Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device

[Johannes Berg]

On Wed, 2014-02-26 at 12:34 +0100, Arend van Spriel wrote:

> > I'm not sure this isn't still possible? I don't know when DELLINK is
> > sent.
> 
> Given the behaviour I would say before the netdev notifier, which may be
> considered wrong. It depends whether the notifier was intended for the
> kind of thing that cfg80211 is doing, ie. send a netlink event to
> user-space?

Well, I checked:

                call_netdevice_notifiers(NETDEV_UNREGISTER, dev);

                if (!dev->rtnl_link_ops ||
                    dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
                        rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);

so ... :)

johannes

Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device

[Arend van Spriel]

On 02/26/2014 12:48 PM, Johannes Berg wrote:
> On Wed, 2014-02-26 at 12:34 +0100, Arend van Spriel wrote:
> 
>>> I'm not sure this isn't still possible? I don't know when DELLINK is
>>> sent.
>>
>> Given the behaviour I would say before the netdev notifier, which may be
>> considered wrong. It depends whether the notifier was intended for the
>> kind of thing that cfg80211 is doing, ie. send a netlink event to
>> user-space?
> 
> Well, I checked:
> 
>                 call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
> 
>                 if (!dev->rtnl_link_ops ||
>                     dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
>                         rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
> 
> so ... :)

Feeling sad :-( but thanks for checking. Clearly need to look into this
some more. John, can you please drop this one.

Gr. AvS

Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device

[Johannes Berg]

On Wed, 2014-02-26 at 13:18 +0100, Arend van Spriel wrote:

> Feeling sad :-( but thanks for checking. Clearly need to look into this
> some more. John, can you please drop this one.

I think you probably still want to drop the other sleep?

johannes

Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device

[Arend van Spriel]

On 02/26/2014 01:22 PM, Johannes Berg wrote:
> On Wed, 2014-02-26 at 13:18 +0100, Arend van Spriel wrote:
> 
>> Feeling sad :-( but thanks for checking. Clearly need to look into this
>> some more. John, can you please drop this one.
> 
> I think you probably still want to drop the other sleep?

Yeah. That one is pretty useless anyway given that it is under
rtnl_lock. Just want to check that it works when I remove the sleep
between cfg80211_disconnected() and unregister_netdev() as well.

Gr. AvS