[PATCH] mac80211: detect driver tx bugs

[PATCH] mac80211: detect driver tx bugs

[Johannes Berg]

When a driver rejects a frame in it's ->tx() callback, it must also
stop queues, otherwise mac80211 can go into a loop here. Detect this
situation and abort the loop after five retries, warning about the
driver bug.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
This fixes resume with connected device with b43 for me, the warning
triggers.

 net/mac80211/tx.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

--- everything.orig/net/mac80211/tx.c	2008-06-15 14:19:10.000000000 +0200
+++ everything/net/mac80211/tx.c	2008-06-15 14:21:06.000000000 +0200
@@ -1091,7 +1091,7 @@ static int ieee80211_tx(struct net_devic
 	struct ieee80211_tx_data tx;
 	ieee80211_tx_result res = TX_DROP, res_prepare;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
-	int ret, i;
+	int ret, i, retries = 0;
 	u16 queue;
 
 	queue = skb_get_queue_mapping(skb);
@@ -1189,6 +1189,13 @@ retry:
 		 */
 		if (!__netif_subqueue_stopped(local->mdev, queue)) {
 			clear_bit(queue, local->queues_pending);
+			retries++;
+			/*
+			 * Driver bug, it's rejecting packets but
+			 * not stopping queues.
+			 */
+			if (WARN_ON(retries > 5))
+				goto drop;
 			goto retry;
 		}
 		store->skb = skb;

[PATCH v2] mac80211: detect driver tx bugs

[Johannes Berg]

When a driver rejects a frame in it's ->tx() callback, it must also
stop queues, otherwise mac80211 can go into a loop here. Detect this
situation and abort the loop after five retries, warning about the
driver bug.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
v2: Michael suggested to use WARN_ON_ONCE instead

 net/mac80211/tx.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

--- everything.orig/net/mac80211/tx.c	2008-06-15 14:19:10.000000000 +0200
+++ everything/net/mac80211/tx.c	2008-06-15 14:53:52.000000000 +0200
@@ -1091,7 +1091,7 @@ static int ieee80211_tx(struct net_devic
 	struct ieee80211_tx_data tx;
 	ieee80211_tx_result res = TX_DROP, res_prepare;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
-	int ret, i;
+	int ret, i, retries = 0;
 	u16 queue;
 
 	queue = skb_get_queue_mapping(skb);
@@ -1189,6 +1189,13 @@ retry:
 		 */
 		if (!__netif_subqueue_stopped(local->mdev, queue)) {
 			clear_bit(queue, local->queues_pending);
+			retries++;
+			/*
+			 * Driver bug, it's rejecting packets but
+			 * not stopping queues.
+			 */
+			if (WARN_ON_ONCE(retries > 5))
+				goto drop;
 			goto retry;
 		}
 		store->skb = skb;

Re: [PATCH] mac80211: detect driver tx bugs

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 2758 bytes --]

On Sun, 2008-06-15 at 14:47 +0200, Johannes Berg wrote:
> When a driver rejects a frame in it's ->tx() callback, it must also
> stop queues, otherwise mac80211 can go into a loop here. Detect this
> situation and abort the loop after five retries, warning about the
> driver bug.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> ---
> This fixes resume with connected device with b43 for me, the warning
> triggers.

For reference:

[  177.789427] b43-phy0 debug: Resuming...
[  177.816496] ------------[ cut here ]------------
[  177.816663] Badness at net/mac80211/tx.c:1197
[  177.816813] NIP: f24fadfc LR: f24fae18 CTR: c0324458
[  177.816983] REGS: eec27ce0 TRAP: 0700   Not tainted  (2.6.26-rc6-wl-10526-ga0d3022-dirty)
[  177.817248] MSR: 00029032 <EE,ME,IR,DR>  CR: 28000084  XER: 20000000
[  177.817590] TASK = ef964e60[1050] 'b43' THREAD: eec26000
[  177.817766] GPR00: 00000001 eec27d90 ef964e60 00000001 00009032 eec27d98 00000000 00000001 
[  177.818200] GPR08: f2548b3c eec2a480 00000000 00000000 00000001 00000000 017b7354 017b70c8 
[  177.818633] GPR16: 41400000 00240e64 00000000 c03e03e0 ffffffff c06315bc eec28000 eec26000 
[  177.819066] GPR24: eec27d98 00000000 00000420 00000001 eedc18c0 efbd8220 efbd8788 00000006 
[  177.819514] NIP [f24fadfc] ieee80211_master_start_xmit+0x334/0x504 [mac80211]
[  177.819843] LR [f24fae18] ieee80211_master_start_xmit+0x350/0x504 [mac80211]
[  177.820984] Call Trace:
[  177.821908] [eec27d90] [f24fae18] ieee80211_master_start_xmit+0x350/0x504 [mac80211] (unreliable)
[  177.823110] [eec27df0] [c027b3f4] dev_hard_start_xmit+0x320/0x3a4
[  177.824184] [eec27e30] [c028e3d4] __qdisc_run+0x248/0x27c
[  177.825238] [eec27e70] [c027b8bc] dev_queue_xmit+0x2e8/0x3b4
[  177.826300] [eec27e90] [f24ecb00] ieee80211_sta_tx+0x74/0x88 [mac80211]
[  177.827414] [eec27eb0] [f24ef200] ieee80211_associated+0x154/0x1e8 [mac80211]
[  177.828541] [eec27ef0] [f24f1054] ieee80211_sta_work+0x57c/0x700 [mac80211]
[  177.829662] [eec27f60] [c0043278] run_workqueue+0xc0/0x164
[  177.830723] [eec27f90] [c0043634] worker_thread+0xb0/0x134
[  177.831782] [eec27fd0] [c00479e4] kthread+0x50/0x88
[  177.832824] [eec27ff0] [c0012500] kernel_thread+0x44/0x60
[  177.833889] Instruction dump:
[  177.834855] 7d29d214 80090004 700b0001 408200c0 7c00f028 7c00d878 7c00f12d 40a2fff4 
[  177.836106] 3bff0001 2f9f0005 7c000026 5400f7fe <0f000000> 2f800000 409e00dc 7fa3eb78 
[  177.966351] b43-phy0: Loading firmware version 410.2160 (2007-05-26 15:32:10)
[  178.176634] b43-phy0 debug: Chip initialized
[  178.180244] b43-phy0 debug: 30-bit DMA initialized
[  178.180734] b43-phy0 debug: Wireless interface started
[  178.180744] b43-phy0 debug: Device resumed.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2] mac80211: detect driver tx bugs

[Rafael J. Wysocki]

On Sunday, 15 of June 2008, Johannes Berg wrote:
> When a driver rejects a frame in it's ->tx() callback, it must also
> stop queues, otherwise mac80211 can go into a loop here. Detect this
> situation and abort the loop after five retries, warning about the
> driver bug.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

I'm unable to test this patch at the moment, because suspend is broken with
linux-next on my box.  Tomorrow I'll try to find out why.

Thanks,
Rafael


> ---
> v2: Michael suggested to use WARN_ON_ONCE instead
> 
>  net/mac80211/tx.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> --- everything.orig/net/mac80211/tx.c	2008-06-15 14:19:10.000000000 +0200
> +++ everything/net/mac80211/tx.c	2008-06-15 14:53:52.000000000 +0200
> @@ -1091,7 +1091,7 @@ static int ieee80211_tx(struct net_devic
>  	struct ieee80211_tx_data tx;
>  	ieee80211_tx_result res = TX_DROP, res_prepare;
>  	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> -	int ret, i;
> +	int ret, i, retries = 0;
>  	u16 queue;
>  
>  	queue = skb_get_queue_mapping(skb);
> @@ -1189,6 +1189,13 @@ retry:
>  		 */
>  		if (!__netif_subqueue_stopped(local->mdev, queue)) {
>  			clear_bit(queue, local->queues_pending);
> +			retries++;
> +			/*
> +			 * Driver bug, it's rejecting packets but
> +			 * not stopping queues.
> +			 */
> +			if (WARN_ON_ONCE(retries > 5))
> +				goto drop;
>  			goto retry;
>  		}
>  		store->skb = skb;
> 

Re: [PATCH v2] mac80211: detect driver tx bugs

[Rafael J. Wysocki]

On Sunday, 15 of June 2008, Johannes Berg wrote:
> When a driver rejects a frame in it's ->tx() callback, it must also
> stop queues, otherwise mac80211 can go into a loop here. Detect this
> situation and abort the loop after five retries, warning about the
> driver bug.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

The patch forks for me nicely. :-)

I'm now able to hibernate the box without turning off the b43 wireless.

Thanks a lot,
Rafael


> ---
> v2: Michael suggested to use WARN_ON_ONCE instead
> 
>  net/mac80211/tx.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> --- everything.orig/net/mac80211/tx.c	2008-06-15 14:19:10.000000000 +0200
> +++ everything/net/mac80211/tx.c	2008-06-15 14:53:52.000000000 +0200
> @@ -1091,7 +1091,7 @@ static int ieee80211_tx(struct net_devic
>  	struct ieee80211_tx_data tx;
>  	ieee80211_tx_result res = TX_DROP, res_prepare;
>  	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> -	int ret, i;
> +	int ret, i, retries = 0;
>  	u16 queue;
>  
>  	queue = skb_get_queue_mapping(skb);
> @@ -1189,6 +1189,13 @@ retry:
>  		 */
>  		if (!__netif_subqueue_stopped(local->mdev, queue)) {
>  			clear_bit(queue, local->queues_pending);
> +			retries++;
> +			/*
> +			 * Driver bug, it's rejecting packets but
> +			 * not stopping queues.
> +			 */
> +			if (WARN_ON_ONCE(retries > 5))
> +				goto drop;
>  			goto retry;
>  		}
>  		store->skb = skb;
> 

[PATCH 2.6.26] mac80211: detect driver tx bugs

[Johannes Berg]

When a driver rejects a frame in it's ->tx() callback, it must also
stop queues, otherwise mac80211 can go into a loop here. Detect this
situation and abort the loop after five retries, warning about the
driver bug.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
This is a port of the patch to 2.6.26 which seems to suffer from the
same problem and could well be the cause of all the mac80211 softlockup
warnings on kerneloops.org.

Dave, with John on vacation until the 22nd, would you push this to Linu=
s
with the next net update instead of it going through John's tree first?

Ideally we'd fix the driver bugs, but that'd need an audit and this at
least lets the system continue and work, a b43 bugfix was posted too to
just drop frames if they come in at inopportune times.

 net/mac80211/tx.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

--- a=EF=BB=BF/net/mac80211/tx.c	2008-06-17 08:45:25.096294488 +0200
+++ b/=EF=BB=BFnet/mac80211/tx.c	2008-06-17 08:47:02.106291721 +0200
@@ -1132,7 +1132,7 @@ static int ieee80211_tx(struct net_devic
 	ieee80211_tx_handler *handler;
 	struct ieee80211_tx_data tx;
 	ieee80211_tx_result res =3D TX_DROP, res_prepare;
-	int ret, i;
+	int ret, i, retries =3D 0;
=20
 	WARN_ON(__ieee80211_queue_pending(local, control->queue));
=20
@@ -1216,6 +1216,13 @@ retry:
 		if (!__ieee80211_queue_stopped(local, control->queue)) {
 			clear_bit(IEEE80211_LINK_STATE_PENDING,
 				  &local->state[control->queue]);
+			retries++;
+			/*
+			 * Driver bug, it's rejecting packets but
+			 * not stopping queues.
+			 */
+			if (WARN_ON_ONCE(retries > 5))
+				goto drop;
 			goto retry;
 		}
 		memcpy(&store->control, control,


--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2.6.26 (resend)] mac80211: detect driver tx bugs

[John W. Linville]

On Wed, Jun 18, 2008 at 12:30:45PM +0200, Johannes Berg wrote:
> When a driver rejects a frame in it's ->tx() callback, it must also
> stop queues, otherwise mac80211 can go into a loop here. Detect this
> situation and abort the loop after five retries, warning about the
> driver bug.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> ---
> This is a port of the patch to 2.6.26 which seems to suffer from the
> same problem and could well be the cause of some of the mac80211 softlockup
> warnings on kerneloops.org.
> 
> Dave, with John on vacation until the 22nd, would you push this to Linus
> with the next net update instead of it going through John's tree first?

This is fine with me.  Otherwise I'll try to get to it in the next
day or so.

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH 2.6.26 (resend)] mac80211: detect driver tx bugs

[David Miller]

From: "John W. Linville" <linville@tuxdriver.com>
Date: Wed, 18 Jun 2008 16:37:14 -0400

> On Wed, Jun 18, 2008 at 12:30:45PM +0200, Johannes Berg wrote:
> > When a driver rejects a frame in it's ->tx() callback, it must also
> > stop queues, otherwise mac80211 can go into a loop here. Detect this
> > situation and abort the loop after five retries, warning about the
> > driver bug.
> > 
> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> > ---
> > This is a port of the patch to 2.6.26 which seems to suffer from the
> > same problem and could well be the cause of some of the mac80211 softlockup
> > warnings on kerneloops.org.
> > 
> > Dave, with John on vacation until the 22nd, would you push this to Linus
> > with the next net update instead of it going through John's tree first?
> 
> This is fine with me.  Otherwise I'll try to get to it in the next
> day or so.

I'll toss it into my tree, don't worry about it.

Re: [PATCH 2.6.26 (resend)] mac80211: detect driver tx bugs

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Wed, 18 Jun 2008 12:30:45 +0200

> When a driver rejects a frame in it's ->tx() callback, it must also
> stop queues, otherwise mac80211 can go into a loop here. Detect this
> situation and abort the loop after five retries, warning about the
> driver bug.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> ---
> This is a port of the patch to 2.6.26 which seems to suffer from the
> same problem and could well be the cause of some of the mac80211 softlockup
> warnings on kerneloops.org.
> 
> Dave, with John on vacation until the 22nd, would you push this to Linus
> with the next net update instead of it going through John's tree first?

I've applied this to net-2.6, thanks.

I'm going to give Linus a little bit more time to take in the
pull request I sent last night before I push this to kernel.org,
so don't panic if it doesn't show up for a bit :)