[PATCH 1/2] mac80211: synchronize scan off/on-channel and PS states

[PATCH 1/2] mac80211: synchronize scan off/on-channel and PS states

[Stanislaw Gruszka]

Since:

commit b23b025fe246f3acc2988eb6d400df34c27cb8ae
Author: Ben Greear <greearb@candelatech.com>
Date:   Fri Feb 4 11:54:17 2011 -0800

    mac80211: Optimize scans on current operating channel.

we do not disable PS while going back to operational channel (on
ieee80211_scan_state_suspend) and deffer that until scan finish.
But since we are allowed to send frames, we can send a frame to AP
without PM bit set, so disable PS on AP side. Then when we switch
to off-channel (in ieee80211_scan_state_resume) we do not enable PS.
Hence we are off-channel with PS disabled, frames are not buffered
by AP.

To fix remove offchannel_ps_disable argument and always enable PS when
going off-channel and disable it when going on-channel, like it was
before.

Cc: stable@vger.kernel.org # 2.6.39+
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 net/mac80211/ieee80211_i.h |    6 ++----
 net/mac80211/offchannel.c  |   19 +++++++------------
 net/mac80211/scan.c        |   15 +++++----------
 3 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 42d0d02..63c5d9f 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1357,10 +1357,8 @@ int ieee80211_request_sched_scan_stop(struct ieee80211_sub_if_data *sdata);
 void ieee80211_sched_scan_stopped_work(struct work_struct *work);
 
 /* off-channel helpers */
-void ieee80211_offchannel_stop_vifs(struct ieee80211_local *local,
-				    bool offchannel_ps_enable);
-void ieee80211_offchannel_return(struct ieee80211_local *local,
-				 bool offchannel_ps_disable);
+void ieee80211_offchannel_stop_vifs(struct ieee80211_local *local);
+void ieee80211_offchannel_return(struct ieee80211_local *local);
 void ieee80211_roc_setup(struct ieee80211_local *local);
 void ieee80211_start_next_roc(struct ieee80211_local *local);
 void ieee80211_roc_purge(struct ieee80211_sub_if_data *sdata);
diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index a5379ae..a3ad4c3 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -102,8 +102,7 @@ static void ieee80211_offchannel_ps_disable(struct ieee80211_sub_if_data *sdata)
 	ieee80211_sta_reset_conn_monitor(sdata);
 }
 
-void ieee80211_offchannel_stop_vifs(struct ieee80211_local *local,
-				    bool offchannel_ps_enable)
+void ieee80211_offchannel_stop_vifs(struct ieee80211_local *local)
 {
 	struct ieee80211_sub_if_data *sdata;
 
@@ -134,8 +133,7 @@ void ieee80211_offchannel_stop_vifs(struct ieee80211_local *local,
 
 		if (sdata->vif.type != NL80211_IFTYPE_MONITOR) {
 			netif_tx_stop_all_queues(sdata->dev);
-			if (offchannel_ps_enable &&
-			    (sdata->vif.type == NL80211_IFTYPE_STATION) &&
+			if (sdata->vif.type == NL80211_IFTYPE_STATION &&
 			    sdata->u.mgd.associated)
 				ieee80211_offchannel_ps_enable(sdata);
 		}
@@ -143,8 +141,7 @@ void ieee80211_offchannel_stop_vifs(struct ieee80211_local *local,
 	mutex_unlock(&local->iflist_mtx);
 }
 
-void ieee80211_offchannel_return(struct ieee80211_local *local,
-				 bool offchannel_ps_disable)
+void ieee80211_offchannel_return(struct ieee80211_local *local)
 {
 	struct ieee80211_sub_if_data *sdata;
 
@@ -163,11 +160,9 @@ void ieee80211_offchannel_return(struct ieee80211_local *local,
 			continue;
 
 		/* Tell AP we're back */
-		if (offchannel_ps_disable &&
-		    sdata->vif.type == NL80211_IFTYPE_STATION) {
-			if (sdata->u.mgd.associated)
-				ieee80211_offchannel_ps_disable(sdata);
-		}
+		if (sdata->vif.type == NL80211_IFTYPE_STATION &&
+		    sdata->u.mgd.associated)
+			ieee80211_offchannel_ps_disable(sdata);
 
 		if (sdata->vif.type != NL80211_IFTYPE_MONITOR) {
 			/*
@@ -385,7 +380,7 @@ void ieee80211_sw_roc_work(struct work_struct *work)
 			local->tmp_channel = NULL;
 			ieee80211_hw_config(local, 0);
 
-			ieee80211_offchannel_return(local, true);
+			ieee80211_offchannel_return(local);
 		}
 
 		ieee80211_recalc_idle(local);
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 8ed83dc..2ac129a 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -304,7 +304,7 @@ static void __ieee80211_scan_completed(struct ieee80211_hw *hw, bool aborted,
 	if (!was_hw_scan) {
 		ieee80211_configure_filter(local);
 		drv_sw_scan_complete(local);
-		ieee80211_offchannel_return(local, true);
+		ieee80211_offchannel_return(local);
 	}
 
 	ieee80211_recalc_idle(local);
@@ -353,7 +353,7 @@ static int ieee80211_start_sw_scan(struct ieee80211_local *local)
 	local->next_scan_state = SCAN_DECISION;
 	local->scan_channel_idx = 0;
 
-	ieee80211_offchannel_stop_vifs(local, true);
+	ieee80211_offchannel_stop_vifs(local);
 
 	ieee80211_configure_filter(local);
 
@@ -690,12 +690,8 @@ static void ieee80211_scan_state_suspend(struct ieee80211_local *local,
 	local->scan_channel = NULL;
 	ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
 
-	/*
-	 * Re-enable vifs and beaconing.  Leave PS
-	 * in off-channel state..will put that back
-	 * on-channel at the end of scanning.
-	 */
-	ieee80211_offchannel_return(local, false);
+	/* disable PS */
+	ieee80211_offchannel_return(local);
 
 	*next_delay = HZ / 5;
 	/* afterwards, resume scan & go to next channel */
@@ -705,8 +701,7 @@ static void ieee80211_scan_state_suspend(struct ieee80211_local *local,
 static void ieee80211_scan_state_resume(struct ieee80211_local *local,
 					unsigned long *next_delay)
 {
-	/* PS already is in off-channel mode */
-	ieee80211_offchannel_stop_vifs(local, false);
+	ieee80211_offchannel_stop_vifs(local);
 
 	if (local->ops->flush) {
 		drv_flush(local, false);
-- 
1.7.1

[PATCH 2/2] mac80211: properly stop/wake queues before/after off-channel

[Stanislaw Gruszka]

Do not assume driver will not stop some queue during off-channel.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/offchannel.c  |   19 +++++--------------
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 63c5d9f..4302494 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -830,6 +830,7 @@ enum queue_stop_reason {
 	IEEE80211_QUEUE_STOP_REASON_AGGREGATION,
 	IEEE80211_QUEUE_STOP_REASON_SUSPEND,
 	IEEE80211_QUEUE_STOP_REASON_SKB_ADD,
+	IEEE80211_QUEUE_STOP_REASON_OFFCHANNEL,
 };
 
 #ifdef CONFIG_MAC80211_LEDS
diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index a3ad4c3..0fdbe75 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -132,7 +132,8 @@ void ieee80211_offchannel_stop_vifs(struct ieee80211_local *local)
 				sdata, BSS_CHANGED_BEACON_ENABLED);
 
 		if (sdata->vif.type != NL80211_IFTYPE_MONITOR) {
-			netif_tx_stop_all_queues(sdata->dev);
+			ieee80211_stop_queues_by_reason(&local->hw,
+				IEEE80211_QUEUE_STOP_REASON_OFFCHANNEL);
 			if (sdata->vif.type == NL80211_IFTYPE_STATION &&
 			    sdata->u.mgd.associated)
 				ieee80211_offchannel_ps_enable(sdata);
@@ -164,19 +165,9 @@ void ieee80211_offchannel_return(struct ieee80211_local *local)
 		    sdata->u.mgd.associated)
 			ieee80211_offchannel_ps_disable(sdata);
 
-		if (sdata->vif.type != NL80211_IFTYPE_MONITOR) {
-			/*
-			 * This may wake up queues even though the driver
-			 * currently has them stopped. This is not very
-			 * likely, since the driver won't have gotten any
-			 * (or hardly any) new packets while we weren't
-			 * on the right channel, and even if it happens
-			 * it will at most lead to queueing up one more
-			 * packet per queue in mac80211 rather than on
-			 * the interface qdisc.
-			 */
-			netif_tx_wake_all_queues(sdata->dev);
-		}
+		if (sdata->vif.type != NL80211_IFTYPE_MONITOR)
+			ieee80211_wake_queues_by_reason(&local->hw,
+				IEEE80211_QUEUE_STOP_REASON_OFFCHANNEL);
 
 		if (sdata->vif.type == NL80211_IFTYPE_AP ||
 		    sdata->vif.type == NL80211_IFTYPE_ADHOC ||
-- 
1.7.1

Re: [PATCH 2/2] mac80211: properly stop/wake queues before/after off-channel

[Johannes Berg]

On Thu, 2012-12-20 at 14:41 +0100, Stanislaw Gruszka wrote:

>  		if (sdata->vif.type != NL80211_IFTYPE_MONITOR) {
> -			netif_tx_stop_all_queues(sdata->dev);
> +			ieee80211_stop_queues_by_reason(&local->hw,
> +				IEEE80211_QUEUE_STOP_REASON_OFFCHANNEL);

That won't work ... if you do this you can't send probe requests or
offchannel frames any more.

What we should be doing is refcount the netif queues as well to avoid
them starting because while offchannel the driver re-enables.

johannes

Re: [PATCH 1/2] mac80211: synchronize scan off/on-channel and PS states

[Seth Forshee]

On Thu, Dec 20, 2012 at 02:41:18PM +0100, Stanislaw Gruszka wrote:
> Since:
> 
> commit b23b025fe246f3acc2988eb6d400df34c27cb8ae
> Author: Ben Greear <greearb@candelatech.com>
> Date:   Fri Feb 4 11:54:17 2011 -0800
> 
>     mac80211: Optimize scans on current operating channel.
> 
> we do not disable PS while going back to operational channel (on
> ieee80211_scan_state_suspend) and deffer that until scan finish.
> But since we are allowed to send frames, we can send a frame to AP
> without PM bit set, so disable PS on AP side. Then when we switch
> to off-channel (in ieee80211_scan_state_resume) we do not enable PS.
> Hence we are off-channel with PS disabled, frames are not buffered
> by AP.
> 
> To fix remove offchannel_ps_disable argument and always enable PS when
> going off-channel and disable it when going on-channel, like it was
> before.
> 
> Cc: stable@vger.kernel.org # 2.6.39+
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>

This patch makes a huge difference in performance and packet loss during
software scans in my testing with iperf.

Tested-by: Seth Forshee <seth.forshee@canonical.com>

Re: [PATCH 1/2] mac80211: synchronize scan off/on-channel and PS states

[Johannes Berg]

On Mon, 2013-01-14 at 10:49 -0600, Seth Forshee wrote:
> On Thu, Dec 20, 2012 at 02:41:18PM +0100, Stanislaw Gruszka wrote:
> > Since:
> > 
> > commit b23b025fe246f3acc2988eb6d400df34c27cb8ae
> > Author: Ben Greear <greearb@candelatech.com>
> > Date:   Fri Feb 4 11:54:17 2011 -0800
> > 
> >     mac80211: Optimize scans on current operating channel.
> > 
> > we do not disable PS while going back to operational channel (on
> > ieee80211_scan_state_suspend) and deffer that until scan finish.
> > But since we are allowed to send frames, we can send a frame to AP
> > without PM bit set, so disable PS on AP side. Then when we switch
> > to off-channel (in ieee80211_scan_state_resume) we do not enable PS.
> > Hence we are off-channel with PS disabled, frames are not buffered
> > by AP.
> > 
> > To fix remove offchannel_ps_disable argument and always enable PS when
> > going off-channel and disable it when going on-channel, like it was
> > before.
> > 
> > Cc: stable@vger.kernel.org # 2.6.39+
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> 
> This patch makes a huge difference in performance and packet loss during
> software scans in my testing with iperf.
> 
> Tested-by: Seth Forshee <seth.forshee@canonical.com>

Applied, sorry about the delay.

johannes