Hi Johannes,
> From: Johannes Berg [mailto:[email protected]]
> Sent: Friday, October 21, 2016 12:16 AM
> To: Amitkumar Karwar
> Cc: Kalle Valo; Brian Norris; Nishant Sarmukadam; Cathy Luo; linux-
> [email protected]; Ganapathi Bhat
> Subject: Re: cfg80211: race problem between suspend and disconnect event
>
> Hi,
>
> > Mwifiex driver rejects del_key() requests from cfg80211 during
> > suspend. They came very late when driver's cfg80211_suspend handler is
> > already executed and driver is in the middle of SDIO's suspend
> > handler.
>
> Interesting. Rejecting those calls is probably perfectly reasonable, and
> in fact it's not clear to me why we even try to delete the keys after
> we've disconnected - any driver implementation should have removed them
> already anyway? You probably don't actually care about the key removal
> either?
Thanks for your reply.
Yes. In our case, *802_11_DEAUTHENTICATE command downloaded to firmware takes
care of flushing the keys.
I can see below code in cfg80211's disconnect handling. It seems to be there
for long time.
---------------------
/*
* Delete all the keys ... pairwise keys can't really
* exist any more anyway, but default keys might.
*/
if (rdev->ops->del_key)
for (i = 0; i < 6; i++)
rdev_del_key(rdev, dev, i, false, NULL);
-----------------------
>
> That said though, there's also the critical protocol stop and the
> set_qos_map(NULL) call, which removes the QoS mapping. It doesn't look
> like you support this right now in your driver, but in any case it'd be
> pretty strange to have that happen after or during suspend.
>
> > Please let us know if you have any suggestions to resolves this with
> > cfg80211/driver change.
>
> For cfg80211 we could do something like this:
>
> --- a/net/wireless/sysfs.c
> +++ b/net/wireless/sysfs.c
> @@ -104,13 +104,16 @@ static int wiphy_suspend(struct device *dev)
>
> rtnl_lock();
> if (rdev->wiphy.registered) {
> - if (!rdev->wiphy.wowlan_config)
> + if (!rdev->wiphy.wowlan_config) {
> cfg80211_leave_all(rdev);
> + cfg80211_process_rdev_events(rdev);
> + }
> if (rdev->ops->suspend)
> ret = rdev_suspend(rdev, rdev->wiphy.wowlan_config);
> if (ret == 1) {
> /* Driver refuse to configure wowlan */
> cfg80211_leave_all(rdev);
> + cfg80211_process_rdev_events(rdev);
> ret = rdev_suspend(rdev, NULL);
> }
> }
>
>
> However, that assumes that you actually cfg80211_disconnected()
> synchronously while being asked to disconnect, which doesn't seem to be
> true from looking at mwifiex, if HostCmd_CMD_802_11_DEAUTHENTICATE
> command can be sent to the firmware then you wait for EVENT_LINK_LOST,
> EVENT_DEAUTHENTICATED or EVENT_DISASSOCIATED to come back from the
> firmware, so this cfg80211 change won't help.
>
I think, your cfg80211 change will help. We do ensure that
cfg80211_disconnected() is called before exiting mwifiex_cfg80211_disconnect().
Sending HostCmd_CMD_802_11_DEAUTHENTICATE command to firmware is a blocking
call. cfg80211_disconnected() is called while handling that command's response.
mwifiex_ret_802_11_deauthenticate()->mwifiex_reset_connect_state()->cfg80211_disconnected()
>
> So somehow you'd have to synchronize with the firmware as well, to
> process all those things before suspend, I guess?
>
> We could then export cfg80211_process_rdev_events() as
> cfg80211_process_wiphy_events() or so, so that you can call that at an
> appropriate place from your suspend handler, after having synchronized
> with the firmware?
This would not be needed.
Regards,
Amitkumar
