Strange mac80211 oops

Strange mac80211 oops

[Michael Buesch]

I just had a strange exception using the zd1211rw driver on mac80211
from latest wireless-2.6 tree.
I'm not sure what happened.

[   98.415423] ------------[ cut here ]------------
[   98.415627] Badness at e221c924 [verbose debug info unavailable]
[   98.415843] NIP: e221c924 LR: e221c904 CTR: 00000000
[   98.416030] REGS: c056dcc0 TRAP: 0700   Not tainted  (2.6.24-rc5-wl26)
[   98.416262] MSR: 00029032 <EE,ME,IR,DR>  CR: 44000024  XER: 20000000
[   98.416648] TASK = c052d5d0[0] 'swapper' THREAD: c056c000
[   98.416844] GPR00: 00000001 c056dd70 c052d5d0 0000001a 00000000 00000000 c056dddc 00000000 
[   98.416877] GPR08: 00000000 e2240000 00000a88 00000018 00000000 00000000 00000000 00000000 
[   98.416886] GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00d85b70 00d87474 
[   98.416895] GPR24: df8409c0 005b7000 c056de20 de564220 df190210 df8409c0 de564220 00000008 
[   98.416905] NIP [e221c924] __ieee80211_rx+0x48c/0xd44 [mac80211]
[   98.417204] LR [e221c904] __ieee80211_rx+0x46c/0xd44 [mac80211]
[   98.417458] Call Trace:
[   98.417562] [c056dd70] [e221d16c] __ieee80211_rx+0xcd4/0xd44 [mac80211] (unreliable)
[   98.417929] [c056de10] [e220d30c] ieee80211_tasklet_handler+0x74/0xf4 [mac80211]
[   98.418239] [c056de70] [c00344a0] tasklet_action+0x84/0xe0
[   98.418504] [c056de80] [c0034560] __do_softirq+0x64/0xd4
[   98.418755] [c056dea0] [c0006788] do_softirq+0x40/0x58
[   98.419003] [c056deb0] [c0034158] irq_exit+0x44/0x94
[   98.419244] [c056dec0] [c0006c78] do_IRQ+0xb0/0xcc
[   98.419478] [c056ded0] [c0014584] ret_from_except+0x0/0x14
[   98.419740] --- Exception: 501 at cpu_idle+0x94/0x100
[   98.419962]     LR = cpu_idle+0x94/0x100
[   98.420112] [c056df90] [c000a05c] cpu_idle+0x48/0x100 (unreliable)
[   98.420421] [c056dfa0] [c0004454] rest_init+0x74/0xa0
[   98.420664] [c056dfc0] [c04fa9b0] start_kernel+0x26c/0x280
[   98.420927] [c056dff0] [00003860] 0x3860
[   98.421135] Instruction dump:
[   98.421281] 7c030378 541f073a b0010048 48003d19 801800a0 7c001a14 70090003 41a2002c 
[   98.421800] 3d20e224 80098eb8 7c000034 5400d97e <0f000000> 2f800000 41be0010 38000001 

Re: Strange mac80211 oops

[Johannes Berg]

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


> [   98.415423] ------------[ cut here ]------------
> [   98.415627] Badness at e221c924 [verbose debug info unavailable]

> [   98.416905] NIP [e221c924] __ieee80211_rx+0x48c/0xd44 [mac80211]

ieee80211_rx_monitor() is inlined into __ieee80211_rx() and that +0x48c
is quite a high number, so I'm guessing it's this:

        /*
         * Drivers are required to align the payload data to a four-byte
         * boundary, so the last two bits of the address where it starts
         * may not be set. The header is required to be directly before
         * the payload data, padding like atheros hardware adds which is
         * inbetween the 802.11 header and the payload is not supported,
         * the driver is required to move the 802.11 header further back
         * in that case.
         */
        hdrlen = ieee80211_get_hdrlen(rx.fc);
        WARN_ON_ONCE(((unsigned long)(skb->data + hdrlen)) & 3);

johannes

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

Re: Strange mac80211 oops

[Johannes Berg]

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


On Mon, 2007-12-24 at 22:45 +0100, Johannes Berg wrote:
> > [   98.415423] ------------[ cut here ]------------
> > [   98.415627] Badness at e221c924 [verbose debug info unavailable]
> 
> > [   98.416905] NIP [e221c924] __ieee80211_rx+0x48c/0xd44 [mac80211]
> 
> ieee80211_rx_monitor() is inlined into __ieee80211_rx() and that +0x48c
> is quite a high number, so I'm guessing it's this:
> 
>         /*
>          * Drivers are required to align the payload data to a four-byte
>          * boundary, so the last two bits of the address where it starts
>          * may not be set. The header is required to be directly before
>          * the payload data, padding like atheros hardware adds which is
>          * inbetween the 802.11 header and the payload is not supported,
>          * the driver is required to move the 802.11 header further back
>          * in that case.
>          */
>         hdrlen = ieee80211_get_hdrlen(rx.fc);
>         WARN_ON_ONCE(((unsigned long)(skb->data + hdrlen)) & 3);

Yup, that's what it is, Michael sent me the assembly, __ieee80211_rx
starts at 0x1990 and we find at 0x1990+0x48c == 0x1e1c


    1df8:       48 00 00 01     bl      1df8 <__ieee80211_rx+0x468>
                        1df8: R_PPC_REL24       ieee80211_get_hdrlen
    1dfc:       80 18 00 a0     lwz     r0,160(r24)
    1e00:       7c 00 1a 14     add     r0,r0,r3
    1e04:       70 09 00 03     andi.   r9,r0,3
    1e08:       41 a2 00 2c     beq+    1e34 <__ieee80211_rx+0x4a4>
    1e0c:       3d 20 00 00     lis     r9,0
                        1e0e: R_PPC_ADDR16_HA   .sbss
    1e10:       80 09 00 00     lwz     r0,0(r9)
                        1e12: R_PPC_ADDR16_LO   .sbss
    1e14:       7c 00 00 34     cntlzw  r0,r0
    1e18:       54 00 d9 7e     rlwinm  r0,r0,27,5,31
    1e1c:       0f 00 00 00     twnei   r0,0

which is exactly the WARN_ON_ONCE above.

johannes

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

Re: Strange mac80211 oops

[Michael Buesch]

On Monday 24 December 2007 22:57:03 Johannes Berg wrote:
> On Mon, 2007-12-24 at 22:45 +0100, Johannes Berg wrote:
> > > [   98.415423] ------------[ cut here ]------------
> > > [   98.415627] Badness at e221c924 [verbose debug info unavailable]
> > 
> > > [   98.416905] NIP [e221c924] __ieee80211_rx+0x48c/0xd44 [mac80211]
> > 
> > ieee80211_rx_monitor() is inlined into __ieee80211_rx() and that +0x48c
> > is quite a high number, so I'm guessing it's this:
> > 
> >         /*
> >          * Drivers are required to align the payload data to a four-byte
> >          * boundary, so the last two bits of the address where it starts
> >          * may not be set. The header is required to be directly before
> >          * the payload data, padding like atheros hardware adds which is
> >          * inbetween the 802.11 header and the payload is not supported,
> >          * the driver is required to move the 802.11 header further back
> >          * in that case.
> >          */
> >         hdrlen = ieee80211_get_hdrlen(rx.fc);
> >         WARN_ON_ONCE(((unsigned long)(skb->data + hdrlen)) & 3);
> 
> Yup, that's what it is, Michael sent me the assembly, __ieee80211_rx
> starts at 0x1990 and we find at 0x1990+0x48c == 0x1e1c
> 
> 
>     1df8:       48 00 00 01     bl      1df8 <__ieee80211_rx+0x468>
>                         1df8: R_PPC_REL24       ieee80211_get_hdrlen
>     1dfc:       80 18 00 a0     lwz     r0,160(r24)
>     1e00:       7c 00 1a 14     add     r0,r0,r3
>     1e04:       70 09 00 03     andi.   r9,r0,3
>     1e08:       41 a2 00 2c     beq+    1e34 <__ieee80211_rx+0x4a4>
>     1e0c:       3d 20 00 00     lis     r9,0
>                         1e0e: R_PPC_ADDR16_HA   .sbss
>     1e10:       80 09 00 00     lwz     r0,0(r9)
>                         1e12: R_PPC_ADDR16_LO   .sbss
>     1e14:       7c 00 00 34     cntlzw  r0,r0
>     1e18:       54 00 d9 7e     rlwinm  r0,r0,27,5,31
>     1e1c:       0f 00 00 00     twnei   r0,0
> 
> which is exactly the WARN_ON_ONCE above.

So zd1211rw-mac80211 is pushing some unaligned data up the RX path, hm.

-- 
Greetings Michael.

Re: Strange mac80211 oops

[Daniel Drake]

Michael Buesch wrote:
> So zd1211rw-mac80211 is pushing some unaligned data up the RX path, hm.

Johannes suggests that this may be due to QoS or WDS frames, which have 
headers that are 26 or 30 bytes. I don't recall seeing any logic in 
zd1211rw to correctly realign these.

Daniel

Re: Strange mac80211 oops

[Johannes Berg]

This might help. Totally untested.

---
 drivers/net/wireless/zd1211rw/zd_mac.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

--- everything.orig/drivers/net/wireless/zd1211rw/zd_mac.c	2007-12-29 14:42:18.292833767 +0100
+++ everything/drivers/net/wireless/zd1211rw/zd_mac.c	2007-12-29 14:47:34.022831923 +0100
@@ -612,6 +612,8 @@ int zd_mac_rx(struct ieee80211_hw *hw, c
 	int bad_frame = 0;
 	int i;
 	u8 rate;
+	u16 fc;
+	int is_qos, is_4addr;
 
 	if (length < ZD_PLCP_HEADER_SIZE + 10 /* IEEE80211_1ADDR_LEN */ +
 	             FCS_LEN + sizeof(struct rx_status))
@@ -671,6 +673,16 @@ int zd_mac_rx(struct ieee80211_hw *hw, c
 	skb = dev_alloc_skb(length);
 	if (skb == NULL)
 		return -ENOMEM;
+
+	fc = le16_to_cpu(*((__le16 *) buffer));
+
+	is_qos = !!(fc & IEEE80211_STYPE_QOS_DATA);
+	is_4addr = (fc & (IEEE80211_FCTL_TODS|IEEE80211_FCTL_FROMDS)) ==
+			(IEEE80211_FCTL_TODS|IEEE80211_FCTL_FROMDS);
+
+	if (is_qos ^ is_4addr)
+		skb_reserve(skb, 2);
+
 	memcpy(skb_put(skb, length), buffer, length);
 
 	ieee80211_rx_irqsafe(hw, skb, &stats);

Re: Strange mac80211 oops

[Michael Buesch]

This patch fixes RX packet alignment issues in the zd1211rw driver.
This is based on a patch by Johannes Berg.

Signed-off-by: Michael Buesch <mb@bu3sch.de>

Index: wireless-2.6/drivers/net/wireless/zd1211rw/zd_mac.c
===================================================================
--- wireless-2.6.orig/drivers/net/wireless/zd1211rw/zd_mac.c	2007-12-29 17:14:41.000000000 +0100
+++ wireless-2.6/drivers/net/wireless/zd1211rw/zd_mac.c	2007-12-29 17:15:00.000000000 +0100
@@ -623,6 +623,8 @@ int zd_mac_rx(struct ieee80211_hw *hw, c
 	const struct rx_status *status;
 	struct sk_buff *skb;
 	int bad_frame = 0;
+	u16 fc;
+	bool is_qos, is_4addr, need_padding;
 
 	if (length < ZD_PLCP_HEADER_SIZE + 10 /* IEEE80211_1ADDR_LEN */ +
 	             FCS_LEN + sizeof(struct rx_status))
@@ -674,9 +676,22 @@ int zd_mac_rx(struct ieee80211_hw *hw, c
 			&& !mac->pass_ctrl)
 		return 0;
 
-	skb = dev_alloc_skb(length);
+	fc = le16_to_cpu(*((__le16 *) buffer));
+
+	is_qos = ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA) &&
+		 ((fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_QOS_DATA);
+	is_4addr = (fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) ==
+		   (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS);
+	need_padding = is_qos ^ is_4addr;
+
+	skb = dev_alloc_skb(length + (need_padding ? 2 : 0));
 	if (skb == NULL)
 		return -ENOMEM;
+	if (need_padding) {
+		/* Make sure the the payload data is 4 byte aligned. */
+		skb_reserve(skb, 2);
+	}
+
 	memcpy(skb_put(skb, length), buffer, length);
 
 	ieee80211_rx_irqsafe(hw, skb, &stats);

Re: Strange mac80211 oops

[Johannes Berg]

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


On Sat, 2007-12-29 at 17:24 +0100, Michael Buesch wrote:
> This patch fixes RX packet alignment issues in the zd1211rw driver.
> This is based on a patch by Johannes Berg.
> 
> Signed-off-by: Michael Buesch <mb@bu3sch.de>

Looks good to me, I guess you tested it.

Acked-by: Johannes Berg <johannes@sipsolutions.net>

> Index: wireless-2.6/drivers/net/wireless/zd1211rw/zd_mac.c
> ===================================================================
> --- wireless-2.6.orig/drivers/net/wireless/zd1211rw/zd_mac.c	2007-12-29 17:14:41.000000000 +0100
> +++ wireless-2.6/drivers/net/wireless/zd1211rw/zd_mac.c	2007-12-29 17:15:00.000000000 +0100
> @@ -623,6 +623,8 @@ int zd_mac_rx(struct ieee80211_hw *hw, c
>  	const struct rx_status *status;
>  	struct sk_buff *skb;
>  	int bad_frame = 0;
> +	u16 fc;
> +	bool is_qos, is_4addr, need_padding;
>  
>  	if (length < ZD_PLCP_HEADER_SIZE + 10 /* IEEE80211_1ADDR_LEN */ +
>  	             FCS_LEN + sizeof(struct rx_status))
> @@ -674,9 +676,22 @@ int zd_mac_rx(struct ieee80211_hw *hw, c
>  			&& !mac->pass_ctrl)
>  		return 0;
>  
> -	skb = dev_alloc_skb(length);
> +	fc = le16_to_cpu(*((__le16 *) buffer));
> +
> +	is_qos = ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA) &&
> +		 ((fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_QOS_DATA);
> +	is_4addr = (fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) ==
> +		   (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS);
> +	need_padding = is_qos ^ is_4addr;
> +
> +	skb = dev_alloc_skb(length + (need_padding ? 2 : 0));
>  	if (skb == NULL)
>  		return -ENOMEM;
> +	if (need_padding) {
> +		/* Make sure the the payload data is 4 byte aligned. */
> +		skb_reserve(skb, 2);
> +	}
> +
>  	memcpy(skb_put(skb, length), buffer, length);
>  
>  	ieee80211_rx_irqsafe(hw, skb, &stats);
> 

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

Re: Strange mac80211 oops

[Michael Buesch]

On Wednesday 02 January 2008 09:48:51 Johannes Berg wrote:
> 
> On Sat, 2007-12-29 at 17:24 +0100, Michael Buesch wrote:
> > This patch fixes RX packet alignment issues in the zd1211rw driver.
> > This is based on a patch by Johannes Berg.
> > 
> > Signed-off-by: Michael Buesch <mb@bu3sch.de>
> 
> Looks good to me, I guess you tested it.

Yes, works fine.

> Acked-by: Johannes Berg <johannes@sipsolutions.net>
> 
> > Index: wireless-2.6/drivers/net/wireless/zd1211rw/zd_mac.c
> > ===================================================================
> > --- wireless-2.6.orig/drivers/net/wireless/zd1211rw/zd_mac.c	2007-12-29 17:14:41.000000000 +0100
> > +++ wireless-2.6/drivers/net/wireless/zd1211rw/zd_mac.c	2007-12-29 17:15:00.000000000 +0100
> > @@ -623,6 +623,8 @@ int zd_mac_rx(struct ieee80211_hw *hw, c
> >  	const struct rx_status *status;
> >  	struct sk_buff *skb;
> >  	int bad_frame = 0;
> > +	u16 fc;
> > +	bool is_qos, is_4addr, need_padding;
> >  
> >  	if (length < ZD_PLCP_HEADER_SIZE + 10 /* IEEE80211_1ADDR_LEN */ +
> >  	             FCS_LEN + sizeof(struct rx_status))
> > @@ -674,9 +676,22 @@ int zd_mac_rx(struct ieee80211_hw *hw, c
> >  			&& !mac->pass_ctrl)
> >  		return 0;
> >  
> > -	skb = dev_alloc_skb(length);
> > +	fc = le16_to_cpu(*((__le16 *) buffer));
> > +
> > +	is_qos = ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA) &&
> > +		 ((fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_QOS_DATA);
> > +	is_4addr = (fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) ==
> > +		   (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS);
> > +	need_padding = is_qos ^ is_4addr;
> > +
> > +	skb = dev_alloc_skb(length + (need_padding ? 2 : 0));
> >  	if (skb == NULL)
> >  		return -ENOMEM;
> > +	if (need_padding) {
> > +		/* Make sure the the payload data is 4 byte aligned. */
> > +		skb_reserve(skb, 2);
> > +	}
> > +
> >  	memcpy(skb_put(skb, length), buffer, length);
> >  
> >  	ieee80211_rx_irqsafe(hw, skb, &stats);
> > 
> 



-- 
Greetings Michael.