2008-10-25 09:10:05

by Shaddy Baddah

[permalink] [raw]
Subject: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

[ 271.325304] zd1211rw 4-3:1.0: zd_usb_init_hw()
[ 271.325336] usb 4-3: request_fw_file() fw name zd1211/zd1211_ub
[ 271.325350] firmware: requesting zd1211/zd1211_ub
[ 271.476106] usb 4-3: upload_firmware() firmware device id 0x4330 is equal to the actual device id
[ 271.476137] usb 4-3: request_fw_file() fw name zd1211/zd1211_uphr
[ 271.476150] firmware: requesting zd1211/zd1211_uphr
[ 271.569862] usb 4-3: upload_code() transfer size 4096
[ 271.578182] usb 4-3: upload_code() transfer size 1024
[ 271.580654] usb 4-3: upload_code() firmware confirm return value 0x01
[ 271.582132] zd1211rw 4-3:1.0: zd_usb_enable_int()
[ 271.582157] zd1211rw 4-3:1.0: zd_usb_enable_int() submit urb fffff8003d87d760
[ 271.582248] zd1211rw 4-3:1.0: zd_chip_init_hw()
[ 271.585008] zd1211rw 4-3:1.0: dump_cr() CR_AFTER_PNP 0x00000000
[ 271.588099] zd1211rw 4-3:1.0: dump_cr() CR_GPI_EN 0x00000000
[ 271.590355] zd1211rw 4-3:1.0: dump_cr() CR_INTERRUPT 0x00000000
[ 271.593683] zd1211rw 4-3:1.0: read_fw_regs_offset() fw_regs_base: 0xf7d4
[ 271.602272] zd1211rw 4-3:1.0: read_pod() E2P_POD 0x0000011d
[ 271.602304] zd1211rw 4-3:1.0: read_pod() RF RF2959_RF 0xd PA type 0x0 patch CCK 1 patch CR157 0 patch 6M 0 new PHY 0 link LED1 tx led 1
[ 271.602323] zd1211rw 4-3:1.0: hw_init()
[ 271.602334] zd1211rw 4-3:1.0: zd1211_hw_reset_phy()
[ 271.648438] zd1211rw 4-3:1.0: zd1211_hw_init_hmac()
[ 271.655178] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x0000007 bits 24
[ 271.656674] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x007dd43 bits 24
[ 271.658680] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x0080959 bits 24
[ 271.660681] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x00e6666 bits 24
[ 271.662678] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x0116a57 bits 24
[ 271.664679] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x017dd43 bits 24
[ 271.666676] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x01819f9 bits 24
[ 271.668677] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x01e6666 bits 24
[ 271.670676] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x0214554 bits 24
[ 271.672676] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x025e7fa bits 24
[ 271.674675] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x027fffa bits 24
[ 271.676674] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x0294128 bits 24
[ 271.678675] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x02c0000 bits 24
[ 271.680675] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x0300000 bits 24
[ 271.682673] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x0340000 bits 24
[ 271.684672] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x0381e0f bits 24
[ 271.686673] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x06c180f bits 24
[ 271.692137] zd1211rw 4-3:1.0: firmware version 4605
[ 271.751128] zd1211rw 4-3:1.0: dump_fw_registers() FW_FIRMWARE_VER 0x4605
[ 271.751150] zd1211rw 4-3:1.0: dump_fw_registers() FW_USB_SPEED 0x0001
[ 271.751165] zd1211rw 4-3:1.0: dump_fw_registers() FW_FIX_TX_RATE 0x0000
[ 271.751180] zd1211rw 4-3:1.0: dump_fw_registers() FW_LINK_STATUS 0x0000
[ 271.753131] zd1211rw 4-3:1.0: dump_cr() CR_AFTER_PNP 0x00000001
[ 271.755128] zd1211rw 4-3:1.0: dump_cr() CR_GPI_EN 0x00000000
[ 271.757129] zd1211rw 4-3:1.0: dump_cr() CR_INTERRUPT 0x00000000
[ 271.797122] zd1211rw 4-3:1.0: zd1211 chip 6891:a727 v4330 high 00-14-7c RF2959_RF pa0 g----
[ 271.898091] zd1211rw 4-3:1.0: zd_read_regdomain() regdomain: 0x10
[ 271.898769] zd1211rw 4-3:1.0: regdomain 0x10
[ 271.898892] zd1211rw 4-3:1.0: zd_usb_disable_int() urb fffff8003d87d760 killed
[ 271.898911] zd1211rw 4-3:1.0: zd_usb_enable_int()
[ 271.898927] zd1211rw 4-3:1.0: zd_usb_enable_int() submit urb fffff8003d87d760
[ 271.904612] zd1211rw 4-3:1.0: zd_usb_enable_rx()
[ 271.905164] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 271.998149] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 272.091852] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 272.185590] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 272.279318] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 272.373829] zd1211rw 4-3:1.0: housekeeping_enable()
[ 272.373863] zd1211rw 4-3:1.0: zd_write_mac_addr() mac addr 00:14:7c:66:b0:e7
[ 274.128969] zd1211rw 4-3:1.0: zd_op_bss_info_changed() changes: 6
[ 274.130992] zd1211rw 4-3:1.0: zd_chip_set_rts_cts_rate_locked() preamble=0
[ 274.183133] zd1211rw 4-3:1.0: zd_rf_set_channel() channel: 1
[ 274.183158] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x0181979 bits 24
[ 274.184989] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x01e6666 bits 24
[ 274.190470] ADDRCONF(NETDEV_UP): wlan0: link is not ready
[ 276.910035] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 277.003044] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 277.096765] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 277.190494] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 277.284219] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 277.378743] usb 4-3: rx_urb_complete() *** first fragment ***
[ 277.378770] usb 4-3: rx_urb_complete() *** second fragment ***
[ 281.926514] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 282.019509] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 282.113227] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 282.206956] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
[ 282.300681] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]


Attachments:
.config (55.99 kB)
dmesg.txt (5.58 kB)
Download all attachments
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

* Johannes Berg | 2008-10-25 14:05:11 [+0200]:

>On Sat, 2008-10-25 at 13:25 +0200, Sebastian Andrzej Siewior wrote:
>> * Johannes Berg | 2008-10-25 11:17:21 [+0200]:
>>
>> >> [ 277.190494] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
>> >
>> >my best bet is this:
>> >
>> > fc = *(__le16 *)buffer;
>> >
>> >could you try something with get_unaligned? Though if this is it,
>> >then ... We really need disassembly of the exact spot.
>> A few lines before that:
>> | buffer += ZD_PLCP_HEADER_SIZE;
>>
>> and ZD_PLCP_HEADER_SIZE is 5 so we end up unaligned, don't we?
>
>Depends where the buffer was before, and thought has gone into this
>alignment stuff here so it shouldn't. Might well be a new bug though.

On my x86_64 box the address of the buffer pointer right after the add
of ZD_PLCP_HEADER_SIZE is:
[ 28.462273] buffer: ffff88003e310005

so it looks to me like it was perfectly aligned before the add. Is it
possible that it is not a new bug and we need a get_unaligend helper
here?

>
>johannes

Sebastian

Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

* Michael Buesch | 2008-10-25 13:25:07 [+0200]:

>On Saturday 25 October 2008 13:21:47 Sebastian Andrzej Siewior wrote:
>> - fc = *(__le16 *)buffer;
>> + fc = get_unaligned_le32(buffer);
>
>I'd say this semantically changes the code.
argh,
sorry,

Subject: [PATCH] wireless/zd1211rw: use get_unaligned_le16 helper (v2)

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
drivers/net/wireless/zd1211rw/zd_mac.c | 5 +++--
drivers/net/wireless/zd1211rw/zd_usb.c | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c b/drivers/net/wireless/zd1211rw/zd_mac.c
index 4d7b98b..bf4b4a4 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -25,6 +25,7 @@
#include <linux/usb.h>
#include <linux/jiffies.h>
#include <net/ieee80211_radiotap.h>
+#include <asm/unaligned.h>

#include "zd_def.h"
#include "zd_chip.h"
@@ -662,7 +663,7 @@ int zd_mac_rx(struct ieee80211_hw *hw, const u8 *buffer, unsigned int length)
&& !mac->pass_ctrl)
return 0;

- fc = *(__le16 *)buffer;
+ fc = get_unaligned_le16(buffer);
need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);

skb = dev_alloc_skb(length + (need_padding ? 2 : 0));
@@ -758,7 +759,7 @@ void zd_process_intr(struct work_struct *work)
u16 int_status;
struct zd_mac *mac = container_of(work, struct zd_mac, process_intr);

- int_status = le16_to_cpu(*(__le16 *)(mac->intr_buffer+4));
+ int_status = get_unaligned_le16(mac->intr_buffer+4);
if (int_status & INT_CFG_NEXT_BCN) {
if (net_ratelimit())
dev_dbg_f(zd_mac_dev(mac), "INT_CFG_NEXT_BCN\n");
diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c b/drivers/net/wireless/zd1211rw/zd_usb.c
index a60ae86..0b27778 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zd1211rw/zd_usb.c
@@ -355,7 +355,7 @@ static inline void handle_regs_int(struct urb *urb)
ZD_ASSERT(in_interrupt());
spin_lock(&intr->lock);

- int_num = le16_to_cpu(*(__le16 *)(urb->transfer_buffer+2));
+ int_num = get_unaligned_le16(urb->transfer_buffer+2);
if (int_num == CR_INTERRUPT) {
struct zd_mac *mac = zd_hw_mac(zd_usb_to_hw(urb->context));
memcpy(&mac->intr_buffer, urb->transfer_buffer,
--
1.6.0.2


2008-10-25 12:05:21

by Johannes Berg

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On Sat, 2008-10-25 at 13:25 +0200, Sebastian Andrzej Siewior wrote:
> * Johannes Berg | 2008-10-25 11:17:21 [+0200]:
>
> >> [ 277.190494] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
> >
> >my best bet is this:
> >
> > fc = *(__le16 *)buffer;
> >
> >could you try something with get_unaligned? Though if this is it,
> >then ... We really need disassembly of the exact spot.
> A few lines before that:
> | buffer += ZD_PLCP_HEADER_SIZE;
>
> and ZD_PLCP_HEADER_SIZE is 5 so we end up unaligned, don't we?

Depends where the buffer was before, and thought has gone into this
alignment stuff here so it shouldn't. Might well be a new bug though.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-10-27 07:00:38

by Johannes Berg

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On Sun, 2008-10-26 at 23:00 +0100, Sebastian Andrzej Siewior wrote:

> On my x86_64 box the address of the buffer pointer right after the add
> of ZD_PLCP_HEADER_SIZE is:
> [ 28.462273] buffer: ffff88003e310005
>
> so it looks to me like it was perfectly aligned before the add. Is it
> possible that it is not a new bug and we need a get_unaligend helper
> here?

Possible, yeah, maybe before we only saw problems with the IP alignment
and somehow this shows up now. Not sure. Anyway, if that fixes it,
obviously we want such a patch :)

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-10-25 09:17:28

by Johannes Berg

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

> [ 277.190494] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]

my best bet is this:

fc = *(__le16 *)buffer;

could you try something with get_unaligned? Though if this is it,
then ... We really need disassembly of the exact spot.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

* Johannes Berg | 2008-10-25 11:17:21 [+0200]:

>> [ 277.190494] Kernel unaligned access at TPC[10129b68] zd_mac_rx+0x144/0x32c [zd1211rw]
>
>my best bet is this:
>
> fc = *(__le16 *)buffer;
>
>could you try something with get_unaligned? Though if this is it,
>then ... We really need disassembly of the exact spot.
A few lines before that:
| buffer += ZD_PLCP_HEADER_SIZE;

and ZD_PLCP_HEADER_SIZE is 5 so we end up unaligned, don't we?

>
>johannes

Sebastian

2008-10-25 11:31:46

by Michael Büsch

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On Saturday 25 October 2008 13:28:13 Sebastian Andrzej Siewior wrote:
> @@ -662,7 +663,7 @@ int zd_mac_rx(struct ieee80211_hw *hw, const u8 *buffer, unsigned int length)
> && !mac->pass_ctrl)
> return 0;
>
> - fc = *(__le16 *)buffer;
> + fc = get_unaligned_le16(buffer);

This still does semantically change the code.
fc is supposed to be LE and is used as LE in the following code.

> need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
>
> skb = dev_alloc_skb(length + (need_padding ? 2 : 0));

--
Greetings Michael.

2008-10-25 11:25:30

by Michael Büsch

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On Saturday 25 October 2008 13:21:47 Sebastian Andrzej Siewior wrote:
> - fc = *(__le16 *)buffer;
> + fc = get_unaligned_le32(buffer);

I'd say this semantically changes the code.

> need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
>
> skb = dev_alloc_skb(length + (need_padding ? 2 : 0));

--
Greetings Michael.

Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

* Michael Buesch | 2008-10-25 13:31:24 [+0200]:

>On Saturday 25 October 2008 13:28:13 Sebastian Andrzej Siewior wrote:
>> @@ -662,7 +663,7 @@ int zd_mac_rx(struct ieee80211_hw *hw, const u8 *buffer, unsigned int length)
>> && !mac->pass_ctrl)
>> return 0;
>>
>> - fc = *(__le16 *)buffer;
>> + fc = get_unaligned_le16(buffer);
>
>This still does semantically change the code.
>fc is supposed to be LE and is used as LE in the following code.
So you say we need a get_unaligned() here. Okay.
A few lines before that, in filter_ack() we dereferance the same
position. filter_ack() is used once and probably inlined so I guess
that's the address where the unaligned access occurs.

>> need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
>>
>> skb = dev_alloc_skb(length + (need_padding ? 2 : 0));



>--
>Greetings Michael.

Sebastian

Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

* Shaddy Baddah | 2008-10-25 19:54:19 [+1100]:

> [ 282.300681] Kernel unaligned access at TPC[10129b68]
> zd_mac_rx+0x144/0x32c [zd1211rw]
>
> periodically is reported even after the ifconfig command completes
> (seemingly successfully).
>
> Thanks in advance for your assistance,
Could you try this patch:

>From a46436536ab13926ff3f592cfa3c333e06a99a0a Mon Sep 17 00:00:00 2001
From: Sebastian Andrzej Siewior <[email protected]>
Date: Sat, 25 Oct 2008 13:15:14 +0200
Subject: [PATCH] wireless/zd1211rw: use get_unaligned_le16 helper

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
drivers/net/wireless/zd1211rw/zd_mac.c | 5 +++--
drivers/net/wireless/zd1211rw/zd_usb.c | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c b/drivers/net/wireless/zd1211rw/zd_mac.c
index 4d7b98b..bf4b4a4 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -25,6 +25,7 @@
#include <linux/usb.h>
#include <linux/jiffies.h>
#include <net/ieee80211_radiotap.h>
+#include <asm/unaligned.h>

#include "zd_def.h"
#include "zd_chip.h"
@@ -662,7 +663,7 @@ int zd_mac_rx(struct ieee80211_hw *hw, const u8 *buffer, unsigned int length)
&& !mac->pass_ctrl)
return 0;

- fc = *(__le16 *)buffer;
+ fc = get_unaligned_le32(buffer);
need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);

skb = dev_alloc_skb(length + (need_padding ? 2 : 0));
@@ -758,7 +759,7 @@ void zd_process_intr(struct work_struct *work)
u16 int_status;
struct zd_mac *mac = container_of(work, struct zd_mac, process_intr);

- int_status = le16_to_cpu(*(__le16 *)(mac->intr_buffer+4));
+ int_status = get_unaligned_le16(mac->intr_buffer+4);
if (int_status & INT_CFG_NEXT_BCN) {
if (net_ratelimit())
dev_dbg_f(zd_mac_dev(mac), "INT_CFG_NEXT_BCN\n");
diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c b/drivers/net/wireless/zd1211rw/zd_usb.c
index a60ae86..0b27778 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zd1211rw/zd_usb.c
@@ -355,7 +355,7 @@ static inline void handle_regs_int(struct urb *urb)
ZD_ASSERT(in_interrupt());
spin_lock(&intr->lock);

- int_num = le16_to_cpu(*(__le16 *)(urb->transfer_buffer+2));
+ int_num = get_unaligned_le16(urb->transfer_buffer+2);
if (int_num == CR_INTERRUPT) {
struct zd_mac *mac = zd_hw_mac(zd_usb_to_hw(urb->context));
memcpy(&mac->intr_buffer, urb->transfer_buffer,
--
1.6.0.2

2008-10-25 15:13:26

by Shaddy Baddah

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

Hi Sebastian,

Sebastian Andrzej Siewior wrote:
> * Michael Buesch | 2008-10-25 13:25:07 [+0200]:
>
>> On Saturday 25 October 2008 13:21:47 Sebastian Andrzej Siewior wrote:
>>> - fc = *(__le16 *)buffer;
>>> + fc = get_unaligned_le32(buffer);
>> I'd say this semantically changes the code.
> argh,
> sorry,
>
> Subject: [PATCH] wireless/zd1211rw: use get_unaligned_le16 helper (v2)
>
> Signed-off-by: Sebastian Andrzej Siewior <[email protected]>

After applying this patch, things seem better. I can associate to an
open access point, get an IP address and even ping.

However on association, I am still seeing:


[ 179.249516] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x01e6666 bits 24
[ 179.254524] wlan0: Initial auth_alg=0
[ 179.254549] wlan0: authenticate with AP XX:XX:XX:XX:XX:XX
[ 179.256215] Kernel unaligned access at TPC[10129b94]
zd_mac_rx+0x174/0x320 [zd1211rw]
[ 179.349135] Kernel unaligned access at TPC[10129b9c]
zd_mac_rx+0x17c/0x320 [zd1211rw]
[ 179.442852] Kernel unaligned access at TPC[10129ba0]
zd_mac_rx+0x180/0x320 [zd1211rw]
[ 179.536579] Kernel unaligned access at TPC[10129ba4]
zd_mac_rx+0x184/0x320 [zd1211rw]
[ 179.630306] Kernel unaligned access at TPC[10129ba8]
zd_mac_rx+0x188/0x320 [zd1211rw]
[ 179.727277] wlan0: RX authentication from XX:XX:XX:XX:XX:XX (alg=0
transaction=2 status=0)
[ 179.727302] wlan0: authenticated
[ 179.727318] wlan0: associate with AP XX:XX:XX:XX:XX:XX
[ 179.727440] wlan0: associate with AP XX:XX:XX:XX:XX:XX
[ 179.736350] wlan0: RX AssocResp from XX:XX:XX:XX:XX:XX (capab=0x421
status=0 aid=12)
[ 179.736380] wlan0: associated
[ 179.736421] wlan0: CTS protection enabled (BSSID=XX:XX:XX:XX:XX:XX)
[ 179.736440] wlan0: switched to short barker preamble
(BSSID=XX:XX:XX:XX:XX:XX)
[ 179.736521] zd1211rw 4-3:1.0: zd_op_bss_info_changed() changes: 7
[ 179.736666] wlan0: association frame received from XX:XX:XX:XX:XX:XX,
but not in associate state - ignored
[ 179.737404] ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
[ 179.807280] zd1211rw 4-3:1.0: zd_chip_set_rts_cts_rate_locked()
preamble=1

and when I start pinging the AP, this starts chiming in:

[ 240.139093] Kernel unaligned access at TPC[100f7f44]
sta_info_get+0x24/0x68 [mac80211]
[ 240.233255] Kernel unaligned access at TPC[100f7f48]
sta_info_get+0x28/0x68 [mac80211]
[ 240.328015] Kernel unaligned access at TPC[100f7f50]
sta_info_get+0x30/0x68 [mac80211]
[ 240.422771] Kernel unaligned access at TPC[100f7f44]
sta_info_get+0x24/0x68 [mac80211]
[ 240.517554] Kernel unaligned access at TPC[100f7f48]
sta_info_get+0x28/0x68 [mac80211]

I also cannot get a full scan with:

iwlist wlan0 scanning

I get about 4 APs (I know there's more) then a bus error. This may be
related to a similar problem that I was assisted with in my first round
of posts last year, with regards to the wireless-tools. Perhaps the
patches still haven't made it downstream?

Thanking you for help,
Shaddy


2008-11-08 13:11:33

by Johannes Berg

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On Sat, 2008-11-08 at 23:45 +1100, Shaddy Baddah wrote:

> > [ 179.249516] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x01e6666 bits 24
> > [ 179.254524] wlan0: Initial auth_alg=0
> > [ 179.254549] wlan0: authenticate with AP XX:XX:XX:XX:XX:XX
> > [ 179.256215] Kernel unaligned access at TPC[10129b94]
> > zd_mac_rx+0x174/0x320 [zd1211rw]
> > [ 179.349135] Kernel unaligned access at TPC[10129b9c]
> > zd_mac_rx+0x17c/0x320 [zd1211rw]
> > [ 179.442852] Kernel unaligned access at TPC[10129ba0]
> > zd_mac_rx+0x180/0x320 [zd1211rw]
> > [ 179.536579] Kernel unaligned access at TPC[10129ba4]
> > zd_mac_rx+0x184/0x320 [zd1211rw]
> > [ 179.630306] Kernel unaligned access at TPC[10129ba8]
> > zd_mac_rx+0x188/0x320 [zd1211rw]
>
> I'm keen on troubleshooting this myself. Could you please tell me how I
> can get a disassembly intermixed with source. An objdump -S ./zd1211rw.o
> is not doing the job. I'd rather not re-compile the whole kernel if
> possible, because it takes long. Should I rm the file, do a build, take
> the command line and substitute -g for any -O option?

I usually just _add_ -g3 to the command line I get with make V=1 M=...

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-11-29 09:50:13

by Johannes Berg

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On Sat, 2008-11-29 at 19:59 +1100, Shaddy Baddah wrote:

> net/mac80211/tx.c:1304: if (memcmp(odev->dev_addr, hdr->addr4,
> ETH_ALEN) != 0)
>
> I would have thought it would use compare_ether_addr() if we could
> safely assume alignment.

You're looking at the TX path, that's irrelevant :)

> In any case, say that alignment was always the
> intention... then can't we just use memcmp() as a hack (not being
> derogatory. Just that it would turn into a hack in deference to using
> compare_ether_addr()) consistent with hacks like the above? Even in the
> interim until a program to iron out alignment is set in place?

No, the IP stack assumes alignment. Besides, zd1211 actually enforces
the alignment before passing the skb off to mac80211.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-11-09 12:00:49

by Michael Büsch

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On Sunday 09 November 2008 04:06:14 Shaddy Baddah wrote:
> Looking into this, and the underlying problem is again
> compare_ether_addr(). Now, I'm sure that replacing this with a memcmp()
> is now treading on more toes than just zd1211rw users. And for this
> reason, I think a better solution is going to be required.

I think this requires a comment in the code why we do not use
compare_ether_addr() at this place. Something like:

/* We use memcmp() instead of compare_ether_addr(), because the
* packet might not be 4-byte aligned, yet. This is done later. */

--
Greetings Michael.

2008-11-09 08:56:28

by Johannes Berg

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

Hi,


> So, looking into this problem, I was able to come up with a patch
> (attached. Note, it also includes remnants of the patch recommended by
> Sebastian Andrzej Siewior in
> http://marc.info/?l=linux-wireless&m=122493409906326&w=2). The problem
> occurs within compare_ether_addr() and this immediately reminded me that
> the patches recommended to me way back last year
> (http://marc.info/?l=linux-wireless&m=119543627712471&w=2) also dealt
> with this problem.

I think you forgot to attach the patch.

> Looking into how it was solved then, all the compare_ether_addr() calls
> were replaced with memcmp() calls. This is what I have done with my
> patch as well... but this approach did not fill me with confidence. It
> seemed to me like a quick fix that didn't directly address the alignment
> problem. And IMO my subsequent findings confirm this.
>
> I now hit the following kernel errors:
>
> > [ 240.139093] Kernel unaligned access at TPC[100f7f44]
> > sta_info_get+0x24/0x68 [mac80211]
> > [ 240.233255] Kernel unaligned access at TPC[100f7f48]
> > sta_info_get+0x28/0x68 [mac80211]
> > [ 240.328015] Kernel unaligned access at TPC[100f7f50]
> > sta_info_get+0x30/0x68 [mac80211]
> > [ 240.422771] Kernel unaligned access at TPC[100f7f44]
> > sta_info_get+0x24/0x68 [mac80211]
> > [ 240.517554] Kernel unaligned access at TPC[100f7f48]
> > sta_info_get+0x28/0x68 [mac80211]
>
> Looking into this, and the underlying problem is again
> compare_ether_addr(). Now, I'm sure that replacing this with a memcmp()
> is now treading on more toes than just zd1211rw users. And for this
> reason, I think a better solution is going to be required.
>
> Does that sound right? I will persist in trying to understand the code
> to try and come up with a fix. But obviously, I hope that I can get help
> with this to make the former an education exercise, not a kernel
> maintenance exercise.

:)
Yes, I think you're right, it appears that the 802.11 header isn't
aligned on a 2-byte boundary, which is extremely strange. Can you, with
your patch applied, do something like

printk(KERN_DEBUG "%p\n", skb->data);

before this code in zd_mac.c:

memcpy(skb_put(skb, length), buffer, length);

ieee80211_rx_irqsafe(hw, skb, &stats);


johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-11-28 22:45:03

by Michael Büsch

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On Friday 28 November 2008 06:34:34 Shaddy Baddah wrote:
> case the same simple function replacement would not be OK. But I saw
> several places where memcmp() is preferred to compare_ether_addr(),
> which I assume indicates that mac80211 does not expect alignment of
> 80211 packets passed to it.

The whole networking stack expects 4byte alignment and the driver _must_
make sure it is this way. Either by padding a constant number of bytes at the front
of each allocated SKB, or (if the alignment differs) use dynamic checks
for the lowest few bits of the pointer passed to mac80211.


--
Greetings Michael.

2008-11-09 14:21:00

by Shaddy Baddah

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

Hi,

On 09/11/08 19:56, Johannes Berg wrote:
> Yes, I think you're right, it appears that the 802.11 header isn't
> aligned on a 2-byte boundary, which is extremely strange. Can you, with
> your patch applied, do something like
>
> printk(KERN_DEBUG "%p\n", skb->data);
>
> before this code in zd_mac.c:
>
> memcpy(skb_put(skb, length), buffer, length);
>
> ieee80211_rx_irqsafe(hw, skb, &stats);

I have done this... but I think that there is too much output to place
here. What I can tell you with confidence is that every value is 4-byte
aligned.

However, I modify filter_ack() in zd_mac.c so that the code looks like this:

for (skb = q->next; skb != (struct sk_buff *)q; skb = skb->next) {
struct ieee80211_hdr *tx_hdr;

tx_hdr = (struct ieee80211_hdr *)skb->data;
printk(KERN_DEBUG "%s:%u: skb %p skb->data %p tx_hdr->addr2 %p
rx_hdr->addr1 %p\n", __FILE__, __LINE__, skb, skb->data, tx_hdr->addr2,
rx_hdr->addr1);
//if (likely(!memcmp(tx_hdr->addr2, rx_hdr->addr1,
ETH_ALEN)))
if (likely(!compare_ether_addr(tx_hdr->addr2,
rx_hdr->addr1)))
{

(note, I re-instated the compare_ether_addr() so that I could be sure
that the unaligned access corresponded with whatever values I was seeing
in the print).

Here is the output of the first print from this location that I see in
syslog:

Nov 10 00:47:17 trad kernel: [ 8239.534950]
drivers/net/wireless/zd1211rw/zd_mac.c:639: skb fffff8003d0d8fc0
skb->data fffff8003d874cdb tx_hdr->addr2 fffff8003d874ce5 rx_hdr->addr1
fffff8003f3e0009
Nov 10 00:47:17 trad kernel: [ 8239.534986] Kernel unaligned access at
TPC[100f5bd0] zd_mac_rx+0x1b0/0x398 [zd1211rw]
Nov 10 00:47:17 trad kernel: [ 8239.637697] Kernel unaligned access at
TPC[100f5bd4] zd_mac_rx+0x1b4/0x398 [zd1211rw]
Nov 10 00:47:17 trad kernel: [ 8239.740208] Kernel unaligned access at
TPC[100f5bd8] zd_mac_rx+0x1b8/0x398 [zd1211rw]
Nov 10 00:47:17 trad kernel: [ 8239.842417] Kernel unaligned access at
TPC[100f5bdc] zd_mac_rx+0x1bc/0x398 [zd1211rw]
Nov 10 00:47:17 trad kernel: [ 8239.944583] Kernel unaligned access at
TPC[100f5be0] zd_mac_rx+0x1c0/0x398 [zd1211rw]

So you can see that skb->data is not even 2-byte aligned. And my debug
leads me to believe that the problem is this line in zd_mac_tx_to_dev():

skb_pull(skb, sizeof(struct zd_ctrlset));

A before and after of this line gives me this output:

Nov 10 00:47:17 trad kernel: [ 8239.534065]
drivers/net/wireless/zd1211rw/zd_mac.c:376: skb fffff8003d0d8fc0
skb->data fffff8003d874cd0
Nov 10 00:47:17 trad kernel: [ 8239.534092]
drivers/net/wireless/zd1211rw/zd_mac.c:390: skb fffff8003d0d8fc0
skb->data fffff8003d874cdb

Perhaps there needs to be some padding there?

Also, I've just had a look at the git version of zd_mac.c, and I notice
it is all change there again. I would feel uncomfortable having studied
the 2.6.26 code to have to start again. Is it expected that I use the
latest available via git?

Thanks in advance,
Shaddy


2008-11-29 12:56:56

by Shaddy Baddah

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

Hi,

On 29/11/08 20:50, Johannes Berg wrote:
> On Sat, 2008-11-29 at 19:59 +1100, Shaddy Baddah wrote:
>
>> net/mac80211/tx.c:1304: if (memcmp(odev->dev_addr, hdr->addr4,
>> ETH_ALEN) != 0)
>>
>> I would have thought it would use compare_ether_addr() if we could
>> safely assume alignment.
>
> You're looking at the TX path, that's irrelevant :)

Do you mean by that the code that handles TX does not expect alignment?

I suspect I still missing some important point. But here it goes.
Although the sta_info_get() function seems to be a more general purpose,
the unaligned access is happening on TX. I found the stack trace:

Nov 22 19:04:43 trad kernel: [ 1459.328317] [000000001039574c]
__ieee80211_tx_prepare+0x288/0x3e8 [mac80211]
Nov 22 19:04:43 trad kernel: [ 1459.420571] [0000000010398174]
ieee80211_master_start_xmit+0x378/0x4f4 [mac80211]
Nov 22 19:04:43 trad kernel: [ 1459.518716] [000000000061de88]
dev_hard_start_xmit+0x210/0x2a0
Nov 22 19:04:43 trad kernel: [ 1459.596342] [000000000062ff38]
__qdisc_run+0xe4/0x220
Nov 22 19:04:43 trad kernel: [ 1459.664429] [0000000000620c94]
dev_queue_xmit+0x3dc/0x5d0
Nov 22 19:04:43 trad kernel: [ 1459.737359] [000000001038ba10]
ieee80211_associated+0x10c/0x154 [mac80211]
Nov 22 19:04:43 trad kernel: [ 1459.827398] [000000001038dda0]
ieee80211_sta_work+0xd50/0xe48 [mac80211]
Nov 22 19:04:43 trad kernel: [ 1459.915701] [000000000045e584]
run_workqueue+0x98/0x130
Nov 22 19:04:43 trad kernel: [ 1459.985658] [000000000045ea30]
worker_thread+0xa4/0xbc
Nov 22 19:04:43 trad kernel: [ 1460.054609] [0000000000461d5c]
kthread+0x3c/0x70
Nov 22 19:04:43 trad kernel: [ 1460.117894] [0000000000426f84]
kernel_thread+0x30/0x48
Nov 22 19:04:43 trad kernel: [ 1460.186919] [0000000000461c18]
kthreadd+0xb0/0x124

> No, the IP stack assumes alignment. Besides, zd1211 actually enforces
> the alignment before passing the skb off to mac80211.

You mean on RX, right? I'm OK on RX I think. The memcmp() is fine in
filter_ack(), because it is on the raw packet that comes in from over
USB which will always be unaligned (because of PLCP header, from the
device itself). And it is before we actually memcpy() aligned into the
skb buffer to pass to mac80211.

Perhaps I'm second guessing a little too much here, so I'll ask a simple
question. Why do I get a 80211 packet passed into
__ieee80211_tx_prepare() which is at an odd address in the skb buffer? I
would have expected that they would have always been constructed aligned?

Thanks in advance,
Shaddy


2008-11-10 15:13:16

by Shaddy Baddah

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

Hi again,

On 10/11/08 01:20, Shaddy Baddah wrote:
]> So you can see that skb->data is not even 2-byte aligned. And my debug
> leads me to believe that the problem is this line in zd_mac_tx_to_dev():
>
> skb_pull(skb, sizeof(struct zd_ctrlset));
>
> A before and after of this line gives me this output:
>
> Nov 10 00:47:17 trad kernel: [ 8239.534065]
> drivers/net/wireless/zd1211rw/zd_mac.c:376: skb fffff8003d0d8fc0
> skb->data fffff8003d874cd0
> Nov 10 00:47:17 trad kernel: [ 8239.534092]
> drivers/net/wireless/zd1211rw/zd_mac.c:390: skb fffff8003d0d8fc0
> skb->data fffff8003d874cdb
>
> Perhaps there needs to be some padding there?

Ok, after a long hard slog at trying to understand all this, I'm going
to have to pause the effort. I am beginning to understand the mechanisms
in place... however, I am not totally sure about anything. My first
impression though is that before control is even passed to the driver
via zd_op_tx(), alignment issues have already hit. As far as I could
tell, at that stage skb->data can already be on an odd byte boundary.

Now, all this is very confusing to me... largely because I don't fully
understand what an SKB is. However, it would seem to me that the
sunlance driver would use this mechanism as well, and AFAICT, that
driver (even if it isn't a wireless driver) does not suffer from
alignment issues.

I have much digging left... but I would greatly appreciate the offering
of opinions as to whether this problem lies deeper in the kernel than
the zd1211rw driver itself. I get a sense that I'm being a little
disengaged... but that's totally cool. I understand the onus of at least
trying to contribute a fix myself.

> Also, I've just had a look at the git version of zd_mac.c, and I notice
> it is all change there again. I would feel uncomfortable having studied
> the 2.6.26 code to have to start again. Is it expected that I use the
> latest available via git?

Ignore that. I was looking at older code (via web git).
Thanks in advance,
Shaddy



2008-11-09 03:06:26

by Shaddy Baddah

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

Hi,

On 09/11/08 00:11, Johannes Berg wrote:
> On Sat, 2008-11-08 at 23:45 +1100, Shaddy Baddah wrote:
>
>>> [ 179.249516] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x01e6666 bits 24
>>> [ 179.254524] wlan0: Initial auth_alg=0
>>> [ 179.254549] wlan0: authenticate with AP XX:XX:XX:XX:XX:XX
>>> [ 179.256215] Kernel unaligned access at TPC[10129b94]
>>> zd_mac_rx+0x174/0x320 [zd1211rw]
>>> [ 179.349135] Kernel unaligned access at TPC[10129b9c]
>>> zd_mac_rx+0x17c/0x320 [zd1211rw]
>>> [ 179.442852] Kernel unaligned access at TPC[10129ba0]
>>> zd_mac_rx+0x180/0x320 [zd1211rw]
>>> [ 179.536579] Kernel unaligned access at TPC[10129ba4]
>>> zd_mac_rx+0x184/0x320 [zd1211rw]
>>> [ 179.630306] Kernel unaligned access at TPC[10129ba8]
>>> zd_mac_rx+0x188/0x320 [zd1211rw]
>> I'm keen on troubleshooting this myself. Could you please tell me how I
>> can get a disassembly intermixed with source. An objdump -S ./zd1211rw.o
>> is not doing the job. I'd rather not re-compile the whole kernel if
>> possible, because it takes long. Should I rm the file, do a build, take
>> the command line and substitute -g for any -O option?
>
> I usually just _add_ -g3 to the command line I get with make V=1 M=...

Johannes, thank you for the tip. It's helped me no end.

So, looking into this problem, I was able to come up with a patch
(attached. Note, it also includes remnants of the patch recommended by
Sebastian Andrzej Siewior in
http://marc.info/?l=linux-wireless&m=122493409906326&w=2). The problem
occurs within compare_ether_addr() and this immediately reminded me that
the patches recommended to me way back last year
(http://marc.info/?l=linux-wireless&m=119543627712471&w=2) also dealt
with this problem.

Looking into how it was solved then, all the compare_ether_addr() calls
were replaced with memcmp() calls. This is what I have done with my
patch as well... but this approach did not fill me with confidence. It
seemed to me like a quick fix that didn't directly address the alignment
problem. And IMO my subsequent findings confirm this.

I now hit the following kernel errors:

> [ 240.139093] Kernel unaligned access at TPC[100f7f44]
> sta_info_get+0x24/0x68 [mac80211]
> [ 240.233255] Kernel unaligned access at TPC[100f7f48]
> sta_info_get+0x28/0x68 [mac80211]
> [ 240.328015] Kernel unaligned access at TPC[100f7f50]
> sta_info_get+0x30/0x68 [mac80211]
> [ 240.422771] Kernel unaligned access at TPC[100f7f44]
> sta_info_get+0x24/0x68 [mac80211]
> [ 240.517554] Kernel unaligned access at TPC[100f7f48]
> sta_info_get+0x28/0x68 [mac80211]

Looking into this, and the underlying problem is again
compare_ether_addr(). Now, I'm sure that replacing this with a memcmp()
is now treading on more toes than just zd1211rw users. And for this
reason, I think a better solution is going to be required.

Does that sound right? I will persist in trying to understand the code
to try and come up with a fix. But obviously, I hope that I can get help
with this to make the former an education exercise, not a kernel
maintenance exercise.

TIA,
Shaddy


2008-11-09 03:09:25

by Shaddy Baddah

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On 09/11/08 14:06, Shaddy Baddah wrote:
> (attached. Note, it also includes remnants of the patch recommended by
> Sebastian Andrzej Siewior in
> http://marc.info/?l=linux-wireless&m=122493409906326&w=2). The problem
> occurs within compare_ether_addr() and this immediately reminded me that

This time, really attached. Sorry about that.

Regards,
Shaddy


Attachments:
zd_mac_rx_fix.diff (1.38 kB)

2008-11-29 08:59:11

by Shaddy Baddah

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On 29/11/08 09:44, Michael Buesch wrote:
> On Friday 28 November 2008 06:34:34 Shaddy Baddah wrote:
>> case the same simple function replacement would not be OK. But I saw
>> several places where memcmp() is preferred to compare_ether_addr(),
>> which I assume indicates that mac80211 does not expect alignment of
>> 80211 packets passed to it.
>
> The whole networking stack expects 4byte alignment and the driver _must_
> make sure it is this way. Either by padding a constant number of bytes at the front
> of each allocated SKB, or (if the alignment differs) use dynamic checks
> for the lowest few bits of the pointer passed to mac80211.

Right. But I'm not sure that in this case that the packet is being
passed to mac80211 by the driver. My recall of the stack traces I saw
were that the unalignment I was seeing in sta_info_get() were a
consequence of transmission attempts.

I must admit though, I could not source where the unaligned packet came
from, as I do not fully understand the different queuing mechanisms in
use and where and how enqueues occur. My limited efforts to understand
this gave me the impression that network stack itself, and not the
driver, was constructing these unaligned packets, for transmission.

The memcmp() that really persuaded me to believe that the mac80211 had
no alignment requirements was this one:

net/mac80211/tx.c:1304: if (memcmp(odev->dev_addr, hdr->addr4,
ETH_ALEN) != 0)

I would have thought it would use compare_ether_addr() if we could
safely assume alignment. In any case, say that alignment was always the
intention... then can't we just use memcmp() as a hack (not being
derogatory. Just that it would turn into a hack in deference to using
compare_ether_addr()) consistent with hacks like the above? Even in the
interim until a program to iron out alignment is set in place?

Regards,
Shaddy



2008-11-28 07:10:32

by David Miller

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

From: Shaddy Baddah <[email protected]>
Date: Fri, 28 Nov 2008 16:34:34 +1100

> It is interesting (at least to me) to note that a change between
> kernel versions 2.6.26 and 2.6.28-rc6 fixed something that allowed
> WPA2 to work on sparc64. At least according to my careful
> verification. Perhaps one day, I'll narrow that change down.

That would be the following set of changes:

commit 0f5cabba49021d36e9f76bd97d7fa0f4a408063f
Author: David S. Miller <[email protected]>
Date: Tue Jun 3 07:39:16 2008 -0700

wext: Create IW_REQUEST_FLAG_COMPAT and set it as needed.

Now low-level WEXT ioctl handlers can do compat handling
when necessary.

Signed-off-by: David S. Miller <[email protected]>

commit 169a3ec492ddb6b0a8203fccba2ddff077154e26
Author: David S. Miller <[email protected]>
Date: Fri Dec 21 03:55:13 2007 -0800

wext: Remove compat handling from fs/compat_ioctl.c

No longer used.

Signed-off-by: David S. Miller <[email protected]>

commit 87de87d5e47f94b4ea647a5bd1bc8dc1f7930db4
Author: David S. Miller <[email protected]>
Date: Tue Jun 3 09:14:03 2008 -0700

wext: Dispatch and handle compat ioctls entirely in net/wireless/wext.c

Next we can kill the hacks in fs/compat_ioctl.c and also
dispatch compat ioctls down into the driver and 80211 protocol
helper layers in order to handle iw_point objects embedded in
stream replies which need to be translated.

Signed-off-by: David S. Miller <[email protected]>

commit a67fa76d8be4e24e2d61cd76438a893d4c2886f7
Author: David S. Miller <[email protected]>
Date: Tue Jun 3 07:36:30 2008 -0700

wext: Pull top-level ioctl dispatch logic into helper function.

Signed-off-by: David S. Miller <[email protected]>

commit d2911255590d9ca561a481b9dbebcfcbbf38fa4e
Author: David S. Miller <[email protected]>
Date: Fri Dec 21 03:46:01 2007 -0800

wext: Pass iwreq pointer down into standard/private handlers.

They have no need to see the object as an ifreq.

Signed-off-by: David S. Miller <[email protected]>

commit ca1e8bb8e4e89e2769e2b39eb29fdcfc5c19cf89
Author: David S. Miller <[email protected]>
Date: Fri Dec 21 03:41:45 2007 -0800

wext: Parameterize the standard/private handlers.

The WEXT standard and private handlers to use are now
arguments to wireless_process_ioctl().

Signed-off-by: David S. Miller <[email protected]>

commit 67dd7608078b17f63f29ff2108fc5bf2407ddcec
Author: David S. Miller <[email protected]>
Date: Fri Dec 21 03:36:31 2007 -0800

wext: Pull ioctl permission checking out into helper function.

Signed-off-by: David S. Miller <[email protected]>

commit d88174e4d295f0880e5f9cb6d42f26b0367c8fd9
Author: David S. Miller <[email protected]>
Date: Fri Dec 21 03:33:46 2007 -0800

wext: Extract private call iw_point handling into seperate functions.

Signed-off-by: David S. Miller <[email protected]>

commit 84149b0fca08f9ec554dfc28dabc39839fdf8a06
Author: David S. Miller <[email protected]>
Date: Fri Dec 21 03:27:17 2007 -0800

wext: Extract standard call iw_point handling into seperate function.

Signed-off-by: David S. Miller <[email protected]>

commit 208887d4cc5a5c1eeb68bd170e21e32b1129cd94
Author: David S. Miller <[email protected]>
Date: Fri Dec 21 03:24:24 2007 -0800

wext: Make adjust_priv_size() take a "struct iw_point *".

Signed-off-by: David S. Miller <[email protected]>

commit 25519a2a769d42fc2733a8f119682272d99b1304
Author: David S. Miller <[email protected]>
Date: Fri Dec 21 03:22:38 2007 -0800

wext: Remove inline from get_priv_size() and adjust_priv_size().

The compiler inlines when appropriate.

Signed-off-by: David S. Miller <[email protected]>


2008-11-28 05:34:46

by Shaddy Baddah

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

Hi,

On 2008-11-11 02:13, Shaddy Baddah wrote:
> Ok, after a long hard slog at trying to understand all this, I'm going
> to have to pause the effort. I am beginning to understand the mechanisms
> in place... however, I am not totally sure about anything. My first

Devoting a little bit of time here and there to understanding the
unaligned accesses, I think I understand a whole lot better now.

First off, I only came to realise that the reason the module seemed to
continue operations despite reporting the unaligned access error was
because the exception handler actually emulates the intended operation
(albeit much slower). Good to know.

I then came to understand that we cannot expect alignment on RX in the
zd1211rw module. In fact, we might expect that 80211 packets are always
unaligned, because the 5 byte PLCP prefix in the SKB buffer will always
push it into an odd address (assuming the SKB buffer is allocated
aligned). In which case, as detailed in the
Documentation/unaligned-memory-access.txt, replacing
compare_ether_addr() with memcmp() is perfectly fine.

After solving that, I was still getting an unaligned access from a
compare_ether_addr() call in sta_info_get() from mac80211 module. My
dilemma was whether that module would expect alignment always, in which
case the same simple function replacement would not be OK. But I saw
several places where memcmp() is preferred to compare_ether_addr(),
which I assume indicates that mac80211 does not expect alignment of
80211 packets passed to it.

Applying Sebastian Andrzej Siewior wireless/zd1211rw: use
get_unaligned_le16 helper (v2) patch and my two changes quietens down
the kernel completely. I will submit these two patches in follow up emails.

It is interesting (at least to me) to note that a change between kernel
versions 2.6.26 and 2.6.28-rc6 fixed something that allowed WPA2 to work
on sparc64. At least according to my careful verification. Perhaps one
day, I'll narrow that change down.

Regards,
Shaddy


2008-11-09 18:31:18

by Johannes Berg

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On Sun, 2008-11-09 at 13:02 +0100, Michael Buesch wrote:
> On Sunday 09 November 2008 10:03:58 Johannes Berg wrote:
> > On Sun, 2008-11-09 at 09:56 +0100, Johannes Berg wrote:
> >
> > > I think you forgot to attach the patch.
> >
> > Sorry, I missed your second mail. (btw, if you generate patches with -p
> > in diff arguments it helps a bit reading them)
> >
> > Ok so it looks like the buffer you're getting is unaligned. This is
> > strange, but I'm too unfamiliar with the code. It seems that there's the
> > PLCP 5 byte header in front. Well, try the printk I asked for, because
> > at that point zd1211 should avoid "leaking" the alignment problem.
>
> zd1211rw has some code to align the packet to a 4-byte boundary.
> But the compare_ether_addr() and the other le16 accesses are done before
> that realignment.
> This these blow up if (ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc))

Well, yes, however, then his patch should have fixed the whole thing,
which it didn't.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-11-08 12:51:53

by Shaddy Baddah

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

Hi again,

On 26/10/08 02:13, Shaddy Baddah wrote:
> After applying this patch, things seem better. I can associate to an
> open access point, get an IP address and even ping.
>
> However on association, I am still seeing:
>
>
> [ 179.249516] zd1211rw 4-3:1.0: zd_usb_rfwrite() value 0x01e6666 bits 24
> [ 179.254524] wlan0: Initial auth_alg=0
> [ 179.254549] wlan0: authenticate with AP XX:XX:XX:XX:XX:XX
> [ 179.256215] Kernel unaligned access at TPC[10129b94]
> zd_mac_rx+0x174/0x320 [zd1211rw]
> [ 179.349135] Kernel unaligned access at TPC[10129b9c]
> zd_mac_rx+0x17c/0x320 [zd1211rw]
> [ 179.442852] Kernel unaligned access at TPC[10129ba0]
> zd_mac_rx+0x180/0x320 [zd1211rw]
> [ 179.536579] Kernel unaligned access at TPC[10129ba4]
> zd_mac_rx+0x184/0x320 [zd1211rw]
> [ 179.630306] Kernel unaligned access at TPC[10129ba8]
> zd_mac_rx+0x188/0x320 [zd1211rw]

I'm keen on troubleshooting this myself. Could you please tell me how I
can get a disassembly intermixed with source. An objdump -S ./zd1211rw.o
is not doing the job. I'd rather not re-compile the whole kernel if
possible, because it takes long. Should I rm the file, do a build, take
the command line and substitute -g for any -O option?

TIA,
Shaddy



2008-11-09 12:02:37

by Michael Büsch

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On Sunday 09 November 2008 10:03:58 Johannes Berg wrote:
> On Sun, 2008-11-09 at 09:56 +0100, Johannes Berg wrote:
>
> > I think you forgot to attach the patch.
>
> Sorry, I missed your second mail. (btw, if you generate patches with -p
> in diff arguments it helps a bit reading them)
>
> Ok so it looks like the buffer you're getting is unaligned. This is
> strange, but I'm too unfamiliar with the code. It seems that there's the
> PLCP 5 byte header in front. Well, try the printk I asked for, because
> at that point zd1211 should avoid "leaking" the alignment problem.

zd1211rw has some code to align the packet to a 4-byte boundary.
But the compare_ether_addr() and the other le16 accesses are done before
that realignment.
This these blow up if (ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc))

--
Greetings Michael.

2008-11-09 09:03:55

by Johannes Berg

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On Sun, 2008-11-09 at 09:56 +0100, Johannes Berg wrote:

> I think you forgot to attach the patch.

Sorry, I missed your second mail. (btw, if you generate patches with -p
in diff arguments it helps a bit reading them)

Ok so it looks like the buffer you're getting is unaligned. This is
strange, but I'm too unfamiliar with the code. It seems that there's the
PLCP 5 byte header in front. Well, try the printk I asked for, because
at that point zd1211 should avoid "leaking" the alignment problem.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-11-28 05:45:25

by Shaddy Baddah

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On 25/10/08 22:28, Sebastian Andrzej Siewior wrote:
> Subject: [PATCH] wireless/zd1211rw: use get_unaligned_le16 helper (v2)

I'm not too sure about the protocol for updating a patch to claim that
you've verified it, but I hope to get this patch applied to git by
verifying it worked for me on an sparc64 machine.

Subject: [PATCH] wireless/zd1211rw: use get_unaligned_le16 helper (v2)

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
Tested-by: Shaddy Baddah <[email protected]>
---
drivers/net/wireless/zd1211rw/zd_mac.c | 5 +++--
drivers/net/wireless/zd1211rw/zd_usb.c | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c
b/drivers/net/wireless/zd1211rw/zd_mac.c
index 4d7b98b..bf4b4a4 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -25,6 +25,7 @@
#include <linux/usb.h>
#include <linux/jiffies.h>
#include <net/ieee80211_radiotap.h>
+#include <asm/unaligned.h>

#include "zd_def.h"
#include "zd_chip.h"
@@ -662,7 +663,7 @@ int zd_mac_rx(struct ieee80211_hw *hw, const u8
*buffer, unsigned int length)
&& !mac->pass_ctrl)
return 0;

- fc = *(__le16 *)buffer;
+ fc = get_unaligned_le16(buffer);
need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);

skb = dev_alloc_skb(length + (need_padding ? 2 : 0));
@@ -758,7 +759,7 @@ void zd_process_intr(struct work_struct *work)
u16 int_status;
struct zd_mac *mac = container_of(work, struct zd_mac, process_intr);

- int_status = le16_to_cpu(*(__le16 *)(mac->intr_buffer+4));
+ int_status = get_unaligned_le16(mac->intr_buffer+4);
if (int_status & INT_CFG_NEXT_BCN) {
if (net_ratelimit())
dev_dbg_f(zd_mac_dev(mac), "INT_CFG_NEXT_BCN\n");
diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c
b/drivers/net/wireless/zd1211rw/zd_usb.c
index a60ae86..0b27778 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zd1211rw/zd_usb.c
@@ -355,7 +355,7 @@ static inline void handle_regs_int(struct urb *urb)
ZD_ASSERT(in_interrupt());
spin_lock(&intr->lock);

- int_num = le16_to_cpu(*(__le16 *)(urb->transfer_buffer+2));
+ int_num = get_unaligned_le16(urb->transfer_buffer+2);
if (int_num == CR_INTERRUPT) {
struct zd_mac *mac = zd_hw_mac(zd_usb_to_hw(urb->context));
memcpy(&mac->intr_buffer, urb->transfer_buffer,
-- 1.6.0.2



2008-11-28 06:47:19

by Harvey Harrison

[permalink] [raw]
Subject: Re: zd1211rw (2.6.26 sparc64): unaligned access (zd_mac_rx)

On Fri, 2008-11-28 at 16:45 +1100, Shaddy Baddah wrote:
> On 25/10/08 22:28, Sebastian Andrzej Siewior wrote:
> > Subject: [PATCH] wireless/zd1211rw: use get_unaligned_le16 helper (v2)
>
> I'm not too sure about the protocol for updating a patch to claim that
> you've verified it, but I hope to get this patch applied to git by
> verifying it worked for me on an sparc64 machine.
>
> Subject: [PATCH] wireless/zd1211rw: use get_unaligned_le16 helper (v2)
>
> Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
> Tested-by: Shaddy Baddah <[email protected]>
> ---
> drivers/net/wireless/zd1211rw/zd_mac.c | 5 +++--
> drivers/net/wireless/zd1211rw/zd_usb.c | 2 +-
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c
> b/drivers/net/wireless/zd1211rw/zd_mac.c
> index 4d7b98b..bf4b4a4 100644
> --- a/drivers/net/wireless/zd1211rw/zd_mac.c
> +++ b/drivers/net/wireless/zd1211rw/zd_mac.c
> @@ -25,6 +25,7 @@
> #include <linux/usb.h>
> #include <linux/jiffies.h>
> #include <net/ieee80211_radiotap.h>
> +#include <asm/unaligned.h>
>
> #include "zd_def.h"
> #include "zd_chip.h"
> @@ -662,7 +663,7 @@ int zd_mac_rx(struct ieee80211_hw *hw, const u8
> *buffer, unsigned int length)
> && !mac->pass_ctrl)
> return 0;
>
> - fc = *(__le16 *)buffer;
> + fc = get_unaligned_le16(buffer);
> need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
>

I think this should be:

fc = get_unaligned((__le16 *)buffer);

As ieee8011_is_data_qos wants a le16 and your use above will swap to cpu-order.

>
> skb = dev_alloc_skb(length + (need_padding ? 2 : 0));
> @@ -758,7 +759,7 @@ void zd_process_intr(struct work_struct *work)
> u16 int_status;
> struct zd_mac *mac = container_of(work, struct zd_mac, process_intr);
>
> - int_status = le16_to_cpu(*(__le16 *)(mac->intr_buffer+4));
> + int_status = get_unaligned_le16(mac->intr_buffer+4);

OK.

> if (int_status & INT_CFG_NEXT_BCN) {
> if (net_ratelimit())
> dev_dbg_f(zd_mac_dev(mac), "INT_CFG_NEXT_BCN\n");
> diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c
> b/drivers/net/wireless/zd1211rw/zd_usb.c
> index a60ae86..0b27778 100644
> --- a/drivers/net/wireless/zd1211rw/zd_usb.c
> +++ b/drivers/net/wireless/zd1211rw/zd_usb.c
> @@ -355,7 +355,7 @@ static inline void handle_regs_int(struct urb *urb)
> ZD_ASSERT(in_interrupt());
> spin_lock(&intr->lock);
>
> - int_num = le16_to_cpu(*(__le16 *)(urb->transfer_buffer+2));
> + int_num = get_unaligned_le16(urb->transfer_buffer+2);
> if (int_num == CR_INTERRUPT) {
> struct zd_mac *mac = zd_hw_mac(zd_usb_to_hw(urb->context));
> memcpy(&mac->intr_buffer, urb->transfer_buffer,

OK.

So the first hunk needs changing, but other than that, looks OK.

Harvey