[PATCH] mac80211: stop tx before doing hw config and rate update

[PATCH] mac80211: stop tx before doing hw config and rate update

[Rajkumar Manoharan]

The assumption is that during the hw config, transmission was
already stopped by mac80211. But during channel type change,
the mac80211 continue to transmit frames. The driver like ath9k
does chip reset while doing channel set. This could leads to
buffer overflow at driver side. And also after configuring the channel
and before calling rate updation, the frames are continued to xmit
with older rates. This patch ensures that the frames are always
xmitted with updated rates and avoid buffer overflow.

Signed-off-by: Rajkumar Manoharan <rmanohar@qca.qualcomm.com>
---
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |   10 ++++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index c204cee..8db9f9a 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -670,6 +670,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_CHTYPE_CHANGE,
 };
 
 #ifdef CONFIG_MAC80211_LEDS
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 60a6f27..5b51390 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -232,6 +232,13 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 		WARN_ON(!ieee80211_set_channel_type(local, sdata, channel_type));
 	}
 
+	ieee80211_stop_queues_by_reason(&sdata->local->hw,
+			IEEE80211_QUEUE_STOP_REASON_CHTYPE_CHANGE);
+
+	/* flush out all packets */
+	synchronize_net();
+
+	drv_flush(local, false);
 	/* channel_type change automatically detected */
 	ieee80211_hw_config(local, 0);
 
@@ -245,6 +252,9 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 		rcu_read_unlock();
 	}
 
+	ieee80211_wake_queues_by_reason(&sdata->local->hw,
+			IEEE80211_QUEUE_STOP_REASON_CHTYPE_CHANGE);
+
 	ht_opmode = le16_to_cpu(hti->operation_mode);
 
 	/* if bss configuration changed store the new one */
-- 
1.7.6.1

Re: [PATCH] mac80211: stop tx before doing hw config and rate update

[Johannes Berg]

On Tue, 2011-08-30 at 09:11 +0530, Rajkumar Manoharan wrote:
> The assumption is that during the hw config, transmission was
> already stopped by mac80211. But during channel type change,
> the mac80211 continue to transmit frames. The driver like ath9k
> does chip reset while doing channel set. This could leads to
> buffer overflow at driver side. And also after configuring the channel
> and before calling rate updation, the frames are continued to xmit
> with older rates. This patch ensures that the frames are always
> xmitted with updated rates and avoid buffer overflow.
> 
> Signed-off-by: Rajkumar Manoharan <rmanohar@qca.qualcomm.com>
> ---
>  net/mac80211/ieee80211_i.h |    1 +
>  net/mac80211/mlme.c        |   10 ++++++++++
>  2 files changed, 11 insertions(+), 0 deletions(-)
> 

> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -232,6 +232,13 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
>  		WARN_ON(!ieee80211_set_channel_type(local, sdata, channel_type));
>  	}
>  
> +	ieee80211_stop_queues_by_reason(&sdata->local->hw,
> +			IEEE80211_QUEUE_STOP_REASON_CHTYPE_CHANGE);
> +
> +	/* flush out all packets */
> +	synchronize_net();
> +
> +	drv_flush(local, false);

I'm curious -- how do you run into the case where this doesn't work
correctly right now? AFAICT that can only happen on an AP that doesn't
advertise HT in its probe responses, which is a little strange?

This can't happen when the probe response has HT since with that the
enable_ht will happen during the initial connect in
ieee80211_assoc_success(), before starting the queues in
ieee80211_set_associated().

Therefore, this can only happen with the late HT-enabling in
ieee80211_rx_mgmt_beacon(). I don't even remember what kind of stupid AP
required that, but maybe we should put this workaround there, or make it
optional? There's no need to stop queues in the regular case when the
queues aren't even started yet.

Also, as we partially discussed on IRC the other day, I'm beginning to
think that we need to not just track different reasons for stopping the
device queues, but also different reasons for stopping interface queues.
This really needs to stop interface queues only, but as you pointed out
scanning also has cases where it stops interface queues which can cause
race conditions.

johannes