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
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);
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.
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 <[email protected]>
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);
> [ 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
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
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
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 <[email protected]>
Looks good to me, I guess you tested it.
Acked-by: Johannes Berg <[email protected]>
> 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);
>
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 <[email protected]>
>
> Looks good to me, I guess you tested it.
Yes, works fine.
> Acked-by: Johannes Berg <[email protected]>
>
> > 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.