2012-07-04 10:53:50

by Johannes Berg

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

From: Johannes Berg <[email protected]>

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 <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
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



2012-07-04 11:01:57

by Johannes Berg

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

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


2012-07-06 13:26:22

by Johannes Berg

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

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 <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Johannes Berg <[email protected]>
> > ---
> > net/mac80211/util.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
>
> Tested-by: Larry Finger <[email protected]>
>
> 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


2012-07-06 14:14:29

by Johannes Berg

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

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 <[email protected]>
> >>>
> >>> 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 <[email protected]>
> >>> Signed-off-by: Johannes Berg <[email protected]>
> >>> ---
> >>> net/mac80211/util.c | 12 ++++++++++--
> >>> 1 file changed, 10 insertions(+), 2 deletions(-)
> >>
> >> Tested-by: Larry Finger <[email protected]>
> >>
> >> 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


2012-07-06 14:12:06

by Larry Finger

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

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 <[email protected]>
>>>
>>> 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 <[email protected]>
>>> Signed-off-by: Johannes Berg <[email protected]>
>>> ---
>>> net/mac80211/util.c | 12 ++++++++++--
>>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> Tested-by: Larry Finger <[email protected]>
>>
>> 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


2012-07-17 10:58:39

by Stanislaw Gruszka

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

On Wed, Jul 04, 2012 at 12:53:45PM +0200, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>

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

2012-07-06 13:15:59

by Larry Finger

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

On 07/04/2012 05:53 AM, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> net/mac80211/util.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)

Tested-by: Larry Finger <[email protected]>

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