On 19-9-2016 16:48, Dan Williams wrote:
> On Fri, 2016-09-16 at 11:58 +0200, Arend Van Spriel wrote:
>> On 15-9-2016 16:42, Dan Williams wrote:
>>>
>>> Hi,
>>>
>>> While refining NetworkManager's MAC address randomization behavior
>>> we
>>> came across two issues with brcmfmac:
>>>
>>> 1) when changing the MAC address, the driver schedules work for the
>>> new
>>> change and returns success, but doesn't actually change the MAC
>>> until
>>> the work is scheduled. Because it returns 0 from the
>>> ndo_set_mac_address hook the net core will generate a
>>> NETDEV_CHANGEADDR
>>> event and rtnetlink will send out an RTM_NEWLINK with the old MAC
>>> address. No event for the new address will be sent. So it's
>>> pretty
>>> hard to figure out when the address actually changed, and when its
>>> safe
>>> to associate, without polling the device's MAC address. Ugly.
>> And apparently unnecessary. I recalled we had this as the
>> ndo_set_mac_address callback could be called in atomic context. So we
>> are using a worker because we are grabbing a mutex upon sending the
>> control info to the device. Looking into the core network code it
>> seems
>> the callback is not called in atomic context so it seems we can get
>> rid
>> of the worker here. I made a patch
>>
>>>
>>> 2) when closing the device (eg, set !IFF_UP) the driver
>>> unconditionally
>>> blocks for 500ms in __brcmf_cfg80211_down():
>>>
>>> if (check_vif_up(ifp->vif)) {
>>> brcmf_link_down(ifp->vif, WLAN_REASON_UNSPECIFIED);
>>>
>>> /* Make sure WPA_Supplicant receives all the event
>>> generated due to DISASSOC call to the fw to keep
>>> the state fw and WPA_Supplicant state consistent
>>> */
>>> brcmf_delay(500);
>>> }
>> This is actually a bogus delay as we are under an RTNL lock here so I
>> think the events will not go out until after the delay has finished.
>> I
>> did submit a patch long ago removing this delay, but the change was
>> not
>> accepted. Let me revisit that.
>>
>>>
>>> Should I dump these into kernel bugzilla, or is there some internal
>>> bug
>>> tracker they could get stuffed into?
>> Kernel bugzilla is fine although I check it rather infrequently.
>
> Thanks for taking another look at these. Should I still file in
> bugzilla, or are the patches going through the process already?
For the mac address delay I let this [1] one fly today. A pity
git-send-email does not add the 'Reported-by:' email to the Cc:
The other one is a bit more tricky. The 500ms delay can be removed, but
I need to fix a related scenario. Working on it.
Regards,
Arend
[1] 1474283399-14385-6-git-send-email-arend.vanspriel@xxxxxxxxxxxx
