Hi Johannes, > From: Johannes Berg [mailto:johannes@xxxxxxxxxxxxxxxx] > Sent: Friday, October 21, 2016 12:16 AM > To: Amitkumar Karwar > Cc: Kalle Valo; Brian Norris; Nishant Sarmukadam; Cathy Luo; linux- > wireless@xxxxxxxxxxxxxxx; 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
