| Message ID | 1357668645-5101-2-git-send-email-seth.forshee@canonical.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, 2013-01-08 at 12:10 -0600, Seth Forshee wrote: > +++ b/net/mac80211/offchannel.c > @@ -136,8 +136,23 @@ void ieee80211_offchannel_stop_vifs(struct ieee80211_local *local, > netif_tx_stop_all_queues(sdata->dev); > if (offchannel_ps_enable && > (sdata->vif.type == NL80211_IFTYPE_STATION) && > - sdata->u.mgd.associated) > + sdata->u.mgd.associated) { > + /* > + * Need to flush frames in driver queues > + * before sending nullfunc. Otherwise > + * devices which support QoS may send the > + * nullfunc before these queued frames, and > + * those frames may not have PM set. > + * > + * XXX: Would be nice to not flush for each > + * vif, however I don't see that there's any > + * protection to prevent frames being handed > + * to the driver before stopping the netdev > + * queue. > + */ > + drv_flush(local, false); > ieee80211_offchannel_ps_enable(sdata); Could we split the loop, and send the frames in a second loop, to combine the flushes into a single one? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 17, 2013 at 12:34:14AM +0100, Johannes Berg wrote: > On Tue, 2013-01-08 at 12:10 -0600, Seth Forshee wrote: > > > +++ b/net/mac80211/offchannel.c > > @@ -136,8 +136,23 @@ void ieee80211_offchannel_stop_vifs(struct ieee80211_local *local, > > netif_tx_stop_all_queues(sdata->dev); > > if (offchannel_ps_enable && > > (sdata->vif.type == NL80211_IFTYPE_STATION) && > > - sdata->u.mgd.associated) > > + sdata->u.mgd.associated) { > > + /* > > + * Need to flush frames in driver queues > > + * before sending nullfunc. Otherwise > > + * devices which support QoS may send the > > + * nullfunc before these queued frames, and > > + * those frames may not have PM set. > > + * > > + * XXX: Would be nice to not flush for each > > + * vif, however I don't see that there's any > > + * protection to prevent frames being handed > > + * to the driver before stopping the netdev > > + * queue. > > + */ > > + drv_flush(local, false); > > ieee80211_offchannel_ps_enable(sdata); > > Could we split the loop, and send the frames in a second loop, to > combine the flushes into a single one? I started thinking about just that earlier today. Seems like it ought to be possible. Seth -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c index a5379ae..ccb91a2 100644 --- a/net/mac80211/offchannel.c +++ b/net/mac80211/offchannel.c @@ -136,8 +136,23 @@ void ieee80211_offchannel_stop_vifs(struct ieee80211_local *local, netif_tx_stop_all_queues(sdata->dev); if (offchannel_ps_enable && (sdata->vif.type == NL80211_IFTYPE_STATION) && - sdata->u.mgd.associated) + sdata->u.mgd.associated) { + /* + * Need to flush frames in driver queues + * before sending nullfunc. Otherwise + * devices which support QoS may send the + * nullfunc before these queued frames, and + * those frames may not have PM set. + * + * XXX: Would be nice to not flush for each + * vif, however I don't see that there's any + * protection to prevent frames being handed + * to the driver before stopping the netdev + * queue. + */ + drv_flush(local, false); ieee80211_offchannel_ps_enable(sdata); + } } } mutex_unlock(&local->iflist_mtx); diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c index 8ed83dc..a875f74 100644 --- a/net/mac80211/scan.c +++ b/net/mac80211/scan.c @@ -355,6 +355,14 @@ static int ieee80211_start_sw_scan(struct ieee80211_local *local) ieee80211_offchannel_stop_vifs(local, true); + /* + * Flush hw queues to ensure that the nullfunc to enable powersave + * gets sent before going off-channel. + * + * XXX: Delay for drivers not supporting flush? + */ + drv_flush(local, false); + ieee80211_configure_filter(local); /* We need to set power level at maximum rate for scanning. */
We've got a couple of races when enabling powersave with an AP for off-channel operation. The first is fairly simple. If we go off-channel prior to the nullfunc frame to enable PS is actually transmitted then it may not be received by the AP. Add a flush after enabling off-channel PS to prevent this from happening. The second race is a bit more subtle. If the driver supports QoS and has frames queued when the nullfunc frame is queued, those frames may get transmitted after the nullfunc frame. If PM is not set then the AP is being told that we've exited PS before we go off-channel and may try to deliver frames. To prevent this, add a flush after stopping the netdev queues but before passing the nullfunc frame to the driver. Signed-off-by: Seth Forshee <seth.forshee@canonical.com> --- net/mac80211/offchannel.c | 17 ++++++++++++++++- net/mac80211/scan.c | 8 ++++++++ 2 files changed, 24 insertions(+), 1 deletion(-)