[PATCH] mac80211: fix crash with single-queue drivers

[PATCH] mac80211: fix crash with single-queue drivers

[Johannes Berg]

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

Larry (and some others I think) reported that with
single-queue drivers mac80211 crashes when waking
the queues. This happens because we allocate just
a single queue for each virtual interface in case
the driver doesn't have at least 4 queues, but the
code stopping/waking the virtual interface queues
wasn't taking this into account.

Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/util.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 26d2103..8f34d2f 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -268,6 +268,10 @@ EXPORT_SYMBOL(ieee80211_ctstoself_duration);
 void ieee80211_propagate_queue_wake(struct ieee80211_local *local, int queue)
 {
 	struct ieee80211_sub_if_data *sdata;
+	int n_acs = IEEE80211_NUM_ACS;
+
+	if (local->hw.queues < IEEE80211_NUM_ACS)
+		n_acs = 1;
 
 	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
 		int ac;
@@ -282,7 +286,7 @@ void ieee80211_propagate_queue_wake(struct ieee80211_local *local, int queue)
 		    local->queue_stop_reasons[sdata->vif.cab_queue] != 0)
 			continue;
 
-		for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
+		for (ac = 0; ac < n_acs; ac++) {
 			int ac_queue = sdata->vif.hw_queue[ac];
 
 			if (ac_queue == queue ||
@@ -344,6 +348,7 @@ static void __ieee80211_stop_queue(struct ieee80211_hw *hw, int queue,
 {
 	struct ieee80211_local *local = hw_to_local(hw);
 	struct ieee80211_sub_if_data *sdata;
+	int n_acs = IEEE80211_NUM_ACS;
 
 	trace_stop_queue(local, queue, reason);
 
@@ -355,6 +360,9 @@ static void __ieee80211_stop_queue(struct ieee80211_hw *hw, int queue,
 
 	__set_bit(reason, &local->queue_stop_reasons[queue]);
 
+	if (local->hw.queues < IEEE80211_NUM_ACS)
+		n_acs = 1;
+
 	rcu_read_lock();
 	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
 		int ac;
@@ -362,7 +370,7 @@ static void __ieee80211_stop_queue(struct ieee80211_hw *hw, int queue,
 		if (!sdata->dev)
 			continue;
 
-		for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
+		for (ac = 0; ac < n_acs; ac++) {
 			if (sdata->vif.hw_queue[ac] == queue ||
 			    sdata->vif.cab_queue == queue)
 				netif_stop_subqueue(sdata->dev, ac);
-- 
1.7.10

Re: [PATCH] mac80211: fix crash with single-queue drivers

[Johannes Berg]

On Wed, 2012-07-04 at 12:53 +0200, Johannes Berg wrote:


> @@ -362,7 +370,7 @@ static void __ieee80211_stop_queue(struct ieee80211_hw *hw, int queue,
>  		if (!sdata->dev)
>  			continue;
>  
> -		for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
> +		for (ac = 0; ac < n_acs; ac++) {

Ok, I based this on the wrong tree, but should be easy to resolve. I've
applied a fixed version, but will wait for Larry to report test results.

johannes

Re: [PATCH] mac80211: fix crash with single-queue drivers

[Larry Finger]

On 07/04/2012 05:53 AM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Larry (and some others I think) reported that with
> single-queue drivers mac80211 crashes when waking
> the queues. This happens because we allocate just
> a single queue for each virtual interface in case
> the driver doesn't have at least 4 queues, but the
> code stopping/waking the virtual interface queues
> wasn't taking this into account.
>
> Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>   net/mac80211/util.c |   12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)

Tested-by: Larry Finger <Larry.Finger@lwfinger.net>

This patch fixes the problem for drivers that always report a single queue such 
as at76c503-usb. There is still a problem for b43 with open-source firmware 
where the number of queues changes from 4 to 1. As that only affects b43, do you 
agree that the problem should be fixed there?

Larry

Re: [PATCH] mac80211: fix crash with single-queue drivers

[Johannes Berg]

On Fri, 2012-07-06 at 08:15 -0500, Larry Finger wrote:
> On 07/04/2012 05:53 AM, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > Larry (and some others I think) reported that with
> > single-queue drivers mac80211 crashes when waking
> > the queues. This happens because we allocate just
> > a single queue for each virtual interface in case
> > the driver doesn't have at least 4 queues, but the
> > code stopping/waking the virtual interface queues
> > wasn't taking this into account.
> >
> > Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> > ---
> >   net/mac80211/util.c |   12 ++++++++++--
> >   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> Tested-by: Larry Finger <Larry.Finger@lwfinger.net>
> 
> This patch fixes the problem for drivers that always report a single queue such 
> as at76c503-usb.

Thanks, I'll merge this.

>  There is still a problem for b43 with open-source firmware 
> where the number of queues changes from 4 to 1. As that only affects b43, do you 
> agree that the problem should be fixed there?

Yes, b43 confuses me a bit. We were looking at this a few weeks ago, but
I couldn't figure out how to solve it.

I think for b43 the solution should be to check if it's proprietary or
free firmware before registering with mac80211, then to mac80211 it's
really just either 1 or 4, not changing.

johannes

Re: [PATCH] mac80211: fix crash with single-queue drivers

[Larry Finger]

On 07/06/2012 08:26 AM, Johannes Berg wrote:
> On Fri, 2012-07-06 at 08:15 -0500, Larry Finger wrote:
>> On 07/04/2012 05:53 AM, Johannes Berg wrote:
>>> From: Johannes Berg <johannes.berg@intel.com>
>>>
>>> Larry (and some others I think) reported that with
>>> single-queue drivers mac80211 crashes when waking
>>> the queues. This happens because we allocate just
>>> a single queue for each virtual interface in case
>>> the driver doesn't have at least 4 queues, but the
>>> code stopping/waking the virtual interface queues
>>> wasn't taking this into account.
>>>
>>> Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
>>> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>>> ---
>>>    net/mac80211/util.c |   12 ++++++++++--
>>>    1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> Tested-by: Larry Finger <Larry.Finger@lwfinger.net>
>>
>> This patch fixes the problem for drivers that always report a single queue such
>> as at76c503-usb.
>
> Thanks, I'll merge this.
>
>>   There is still a problem for b43 with open-source firmware
>> where the number of queues changes from 4 to 1. As that only affects b43, do you
>> agree that the problem should be fixed there?
>
> Yes, b43 confuses me a bit. We were looking at this a few weeks ago, but
> I couldn't figure out how to solve it.
>
> I think for b43 the solution should be to check if it's proprietary or
> free firmware before registering with mac80211, then to mac80211 it's
> really just either 1 or 4, not changing.

I agree with that solution, and I will try to implement that model.

Larry

Re: [PATCH] mac80211: fix crash with single-queue drivers

[Johannes Berg]

On Fri, 2012-07-06 at 09:12 -0500, Larry Finger wrote:
> On 07/06/2012 08:26 AM, Johannes Berg wrote:
> > On Fri, 2012-07-06 at 08:15 -0500, Larry Finger wrote:
> >> On 07/04/2012 05:53 AM, Johannes Berg wrote:
> >>> From: Johannes Berg <johannes.berg@intel.com>
> >>>
> >>> Larry (and some others I think) reported that with
> >>> single-queue drivers mac80211 crashes when waking
> >>> the queues. This happens because we allocate just
> >>> a single queue for each virtual interface in case
> >>> the driver doesn't have at least 4 queues, but the
> >>> code stopping/waking the virtual interface queues
> >>> wasn't taking this into account.
> >>>
> >>> Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
> >>> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> >>> ---
> >>>    net/mac80211/util.c |   12 ++++++++++--
> >>>    1 file changed, 10 insertions(+), 2 deletions(-)
> >>
> >> Tested-by: Larry Finger <Larry.Finger@lwfinger.net>
> >>
> >> This patch fixes the problem for drivers that always report a single queue such
> >> as at76c503-usb.
> >
> > Thanks, I'll merge this.
> >
> >>   There is still a problem for b43 with open-source firmware
> >> where the number of queues changes from 4 to 1. As that only affects b43, do you
> >> agree that the problem should be fixed there?
> >
> > Yes, b43 confuses me a bit. We were looking at this a few weeks ago, but
> > I couldn't figure out how to solve it.
> >
> > I think for b43 the solution should be to check if it's proprietary or
> > free firmware before registering with mac80211, then to mac80211 it's
> > really just either 1 or 4, not changing.
> 
> I agree with that solution, and I will try to implement that model.

My think my last attempt was this:
http://p.sipsolutions.net/e45e57bbc9509e69.txt

Andre reported it didn't work though.

johannes

Re: [PATCH] mac80211: fix crash with single-queue drivers

[Stanislaw Gruszka]

On Wed, Jul 04, 2012 at 12:53:45PM +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> Larry (and some others I think) reported that with
> single-queue drivers mac80211 crashes when waking
> the queues. This happens because we allocate just
> a single queue for each virtual interface in case
> the driver doesn't have at least 4 queues, but the
> code stopping/waking the virtual interface queues
> wasn't taking this into account.
> 
> Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

John, this patch is needed in 3.5, it fix this bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=44411
You can cherry-pick that patch from wireless-next commit:
a6f38ac3cc853189705006cc1e0f17ce8467a1df
If it does not make 3.5 release it has to be posted to
stable.

Stanislaw