[PATCH 0/3] mac80211: some queue management fixes

[PATCH 0/3] mac80211: some queue management fixes

[Johannes Berg]

I let these run through some testing, and it seems fine. The actual
error conditions are hard to test though -- particularly for the last
patch, most likely nobody ever saw that.

johannes

[PATCH 1/3] mac80211: don't fiddle with netdev queues in MLME code

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

The netdev queues should always represent the state that
the driver gave them, so fiddling with them isn't really
appropriate in the mlme code. Also, since we stop queues
for flushing now, this really isn't necessary any more.

As the scan/offchannel code has also been modified to no
longer do this a while ago, remove the outdated smp_mb()
and comments about it.

While at it, also add a pair of braces that was missing.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/mlme.c | 26 ++------------------------
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index e12fedc..8a71796 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1443,13 +1443,11 @@ void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
 
 	if ((local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) &&
 	    !(ifmgd->flags & IEEE80211_STA_NULLFUNC_ACKED)) {
-		netif_tx_stop_all_queues(sdata->dev);
-
-		if (drv_tx_frames_pending(local))
+		if (drv_tx_frames_pending(local)) {
 			mod_timer(&local->dynamic_ps_timer, jiffies +
 				  msecs_to_jiffies(
 				  local->hw.conf.dynamic_ps_timeout));
-		else {
+		} else {
 			ieee80211_send_nullfunc(local, sdata, 1);
 			/* Flush to get the tx status of nullfunc frame */
 			ieee80211_flush_queues(local, sdata);
@@ -1463,9 +1461,6 @@ void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
 		local->hw.conf.flags |= IEEE80211_CONF_PS;
 		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
 	}
-
-	if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK)
-		netif_tx_wake_all_queues(sdata->dev);
 }
 
 void ieee80211_dynamic_ps_timer(unsigned long data)
@@ -1725,7 +1720,6 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata,
 	ieee80211_recalc_smps(sdata);
 	ieee80211_recalc_ps_vif(sdata);
 
-	netif_tx_start_all_queues(sdata->dev);
 	netif_carrier_on(sdata->dev);
 }
 
@@ -1748,22 +1742,6 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 	ieee80211_stop_poll(sdata);
 
 	ifmgd->associated = NULL;
-
-	/*
-	 * we need to commit the associated = NULL change because the
-	 * scan code uses that to determine whether this iface should
-	 * go to/wake up from powersave or not -- and could otherwise
-	 * wake the queues erroneously.
-	 */
-	smp_mb();
-
-	/*
-	 * Thus, we can only afterwards stop the queues -- to account
-	 * for the case where another CPU is finishing a scan at this
-	 * time -- we don't want the scan code to enable queues.
-	 */
-
-	netif_tx_stop_all_queues(sdata->dev);
 	netif_carrier_off(sdata->dev);
 
 	/*
-- 
1.8.0

[PATCH 2/3] mac80211: replace some dead code by a warning

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Given the (nested) switch statements, this code can't
be reached, so make it warn instead of manipulating
the carrier state which seems purposeful.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/iface.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 2bdbf14..273d851 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -581,7 +581,8 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
 		case NL80211_IFTYPE_P2P_DEVICE:
 			break;
 		default:
-			netif_carrier_on(dev);
+			/* not reached */
+			WARN_ON(1);
 		}
 
 		/*
-- 
1.8.0

[PATCH 3/3] mac80211: don't start new netdev queues if driver stopped

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

If a new netdev (e.g. an AP VLAN) is created while the driver
has queues stopped, the new netdev queues will be started even
though they shouldn't. This will lead to frames accumulating
on the internal mac80211 pending queues instead of properly
being held on the netdev queues.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/iface.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 273d851..79f3758 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -639,8 +639,28 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
 
 	ieee80211_recalc_ps(local, -1);
 
-	if (dev)
-		netif_tx_start_all_queues(dev);
+	if (dev) {
+		unsigned long flags;
+		int n_acs = IEEE80211_NUM_ACS;
+		int ac;
+
+		if (local->hw.queues < IEEE80211_NUM_ACS)
+			n_acs = 1;
+
+		spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
+		if (sdata->vif.cab_queue == IEEE80211_INVAL_HW_QUEUE ||
+		    (local->queue_stop_reasons[sdata->vif.cab_queue] == 0 &&
+		     skb_queue_empty(&local->pending[sdata->vif.cab_queue]))) {
+			for (ac = 0; ac < n_acs; ac++) {
+				int ac_queue = sdata->vif.hw_queue[ac];
+
+				if (local->queue_stop_reasons[ac_queue] == 0 &&
+				    skb_queue_empty(&local->pending[ac_queue]))
+					netif_start_subqueue(dev, ac);
+			}
+		}
+		spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
+	}
 
 	return 0;
  err_del_interface:
-- 
1.8.0

Re: [PATCH 0/3] mac80211: some queue management fixes

[Johannes Berg]

On Thu, 2013-03-28 at 16:46 +0100, Johannes Berg wrote:
> I let these run through some testing, and it seems fine. The actual
> error conditions are hard to test though -- particularly for the last
> patch, most likely nobody ever saw that.

All applied.

johannes