Re: ZD1211RW unaligned accesses...

2007-12-01 Thread Ulrich Kunitz 
John W. Linville wrote:

 So, did the patch below fix the problem?  Should I apply it?
 
 John

John,

the patch would have worked, but I have sent a second one to the
list, which is based on Herbert's and has an assert to be able to test
the patch on x86.

You should be notify that the mac80211 driver, doesn't suffer
from the problem and Daniel has already provided a patch to
replace zd1211rw by the mac80211 driver. Daniel's patch must of
course break by the new patch.

-- 
Uli Kunitz
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ZD1211RW unaligned accesses...

2007-11-30 Thread Shaddy Baddah 

Hi again,

Herbert Xu wrote:

On Thu, Nov 29, 2007 at 04:45:33PM -0500, John W. Linville wrote:

So, did the patch below fix the problem?  Should I apply it?


I'm keen to find out the result too :)

Chances are it does make progress however we may still have the
general wireless/IP stack alignment issue that we are still discussing.



OK... so I've applied patches left right and centre. As there have been 
a few, I'll in-line them all at the bottom of this email.


The result is that there are no more unaligned access messages at all. 
However, I still can only scan one (occasionally two) AP, using iwlist 
eth2 scanning command before a bus error. Jean, I missed your emails 
regarding compiling the wireless-tools, I will try these and see if they 
help.


Perhaps related to the scanning problems, I cannot setup any wireless 
links, with Open access points, WEP access points, anything at all. I am 
losing direction on what information to supply here-in, but am willing 
to take suggestions.


Thanks for all your help,
Shaddy

Patches applied follow:

diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c 
b/drivers/net/wireless/zd1211rw/zd_mac.c

index a903645..d06b05b 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -1166,15 +1166,16 @@ static void do_rx(unsigned long mac_ptr)
 int zd_mac_rx_irq(struct zd_mac *mac, const u8 *buffer, unsigned int 
length)

 {
struct sk_buff *skb;
+   unsigned int hlen = ALIGN(sizeof(struct zd_rt_hdr), 16);

-   skb = dev_alloc_skb(sizeof(struct zd_rt_hdr) + length);
+   skb = dev_alloc_skb(hlen + length);
if (!skb) {
struct ieee80211_device *ieee = zd_mac_to_ieee80211(mac);
dev_warn(zd_mac_dev(mac), Could not allocate skb.\n);
ieee-stats.rx_dropped++;
return -ENOMEM;
}
-   skb_reserve(skb, sizeof(struct zd_rt_hdr));
+   skb_reserve(skb, hlen - ZD_PLCP_HEADER_SIZE);
memcpy(__skb_put(skb, length), buffer, length);
skb_queue_tail(mac-rx_queue, skb);
tasklet_schedule(mac-rx_tasklet);

diff --git a/net/ieee80211/ieee80211_tx.c b/net/ieee80211/ieee80211_tx.c
index a4c3c51..6d06f13 100644
--- a/net/ieee80211/ieee80211_tx.c
+++ b/net/ieee80211/ieee80211_tx.c
@@ -144,7 +144,8 @@ static int ieee80211_copy_snap(u8 * data, u16 h_proto)
snap-oui[1] = oui[1];
snap-oui[2] = oui[2];

-   *(u16 *) (data + SNAP_SIZE) = htons(h_proto);
+   h_proto = htons(h_proto);
+   memcpy(data + SNAP_SIZE, h_proto, sizeof(u16));

return SNAP_SIZE + sizeof(u16);
 }

Index: linux-2.6.24-rc3-git1/drivers/net/wireless/zd1211rw/zd_mac.c
===
--- linux-2.6.24-rc3-git1.orig/drivers/net/wireless/zd1211rw/zd_mac.c
+++ linux-2.6.24-rc3-git1/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -974,14 +974,14 @@ static int is_data_packet_for_us(struct
switch (ieee-iw_mode) {
case IW_MODE_ADHOC:
if ((fc  (IEEE80211_FCTL_TODS|IEEE80211_FCTL_FROMDS)) != 0 ||
-   compare_ether_addr(hdr-addr3, ieee-bssid) != 0)
+   memcmp(hdr-addr3, ieee-bssid, ETH_ALEN) != 0)
return 0;
break;
case IW_MODE_AUTO:
case IW_MODE_INFRA:
if ((fc  (IEEE80211_FCTL_TODS|IEEE80211_FCTL_FROMDS)) !=
IEEE80211_FCTL_FROMDS ||
-   compare_ether_addr(hdr-addr2, ieee-bssid) != 0)
+   memcmp(hdr-addr2, ieee-bssid, ETH_ALEN) != 0)
return 0;
break;
default:
@@ -989,9 +989,9 @@ static int is_data_packet_for_us(struct
return 0;
}

-   return compare_ether_addr(hdr-addr1, netdev-dev_addr) == 0 ||
+   return memcmp(hdr-addr1, netdev-dev_addr, ETH_ALEN) == 0 ||
   (is_multicast_ether_addr(hdr-addr1) 
-   compare_ether_addr(hdr-addr3, netdev-dev_addr) != 0) ||
+   memcmp(hdr-addr3, netdev-dev_addr, ETH_ALEN) != 0) ||
   (netdev-flags  IFF_PROMISC);
 }

@@ -1047,7 +1047,7 @@ static void update_qual_rssi(struct zd_m
hdr = (struct ieee80211_hdr_3addr *)buffer;
if (length  offsetof(struct ieee80211_hdr_3addr, addr3))
return;
-   if (compare_ether_addr(hdr-addr2, zd_mac_to_ieee80211(mac)-bssid) != 
0)
+   if (memcmp(hdr-addr2, zd_mac_to_ieee80211(mac)-bssid, ETH_ALEN) != 0)
return;

spin_lock_irqsave(mac-lock, flags);


--- everything.orig/drivers/net/wireless/zd1211rw/Makefile	2007-11-23 
11:36:30.652094075 +0100
+++ everything/drivers/net/wireless/zd1211rw/Makefile	2007-11-23 
11:36:57.112090711 +0100

@@ -1,5 +1,7 @@
 obj-$(CONFIG_ZD1211RW) += zd1211rw.o

+EXTRA_CFLAGS += -fno-inline-functions-called-once
+
 zd1211rw-objs := zd_chip.o zd_ieee80211.o \
zd_mac.o zd_netdev.o \
zd_rf_al2230.o

Re: ZD1211RW unaligned accesses...

2007-11-30 Thread Johannes Berg 

 Chances are it does make progress however we may still have the
 general wireless/IP stack alignment issue that we are still discussing.

Yeah, it's still on my list. I'll make a patch to WARN_ON unaligned data
in a packet... although this is a bit complicated. ath5k actually had a
bug with this where they simply ignored the hardware-added padding in
QoS frames initially which would result in corrupted data...

johannes


signature.asc
Description: This is a digitally signed message part

Re: ZD1211RW unaligned accesses...

2007-11-30 Thread Herbert Xu 
On Fri, Nov 30, 2007 at 06:34:56PM +1100, Shaddy Baddah wrote:
 
 OK... so I've applied patches left right and centre. As there have been 
 a few, I'll in-line them all at the bottom of this email.
 
 The result is that there are no more unaligned access messages at all. 

Good stuff! I was sort of worried that you might end up getting unaligned
traps in the IP stack but it's good to see that you didn't.  Of course
it's still something that we need to fix up at some point for other
configurations.

John, you can apply my patch now.

 However, I still can only scan one (occasionally two) AP, using iwlist 
 eth2 scanning command before a bus error. Jean, I missed your emails 
 regarding compiling the wireless-tools, I will try these and see if they 
 help.

That's kind of expected because we haven't completely fixed up the
user-kernel wireless interface for the 32-in-64 case.  But rest assured
we will be working on that.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ZD1211RW unaligned accesses...

2007-11-29 Thread John W. Linville 
So, did the patch below fix the problem?  Should I apply it?

John

On Sat, Nov 24, 2007 at 11:02:16PM +0800, Herbert Xu wrote:
 On Wed, Nov 21, 2007 at 01:00:44PM +, Shaddy Baddah wrote:
 
  It hasn't seemed to. I patched the source (confirming the patched lines 
  are in), compiled, installed and rebooted to effect the changes. My 
  zd1211rw modules timestamp indicates that I have an updated module:
 
 Thanks for your quick response and sorry for my late answer :)
 
 I think Dave's patch is definietly on the right track but there
 are subsequent unaligned accesses of a similar kind which is
 why it still appears to be broken if you look at the kernel
 messages.
 
 But there is definitely progress because those addresses are now
 bigger (0x394/0x39c/0x3a8 vs. 0x2** earlier).
 
 So please try the following patch (instead of the original one)
 which should fix all the unailgned accesses in do_rx.
 
 Cheers,
 -- 
 Visit Openswan at http://www.openswan.org/
 Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
 Home Page: http://gondor.apana.org.au/~herbert/
 PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
 --
 diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c 
 b/drivers/net/wireless/zd1211rw/zd_mac.c
 index a903645..d06b05b 100644
 --- a/drivers/net/wireless/zd1211rw/zd_mac.c
 +++ b/drivers/net/wireless/zd1211rw/zd_mac.c
 @@ -1166,15 +1166,16 @@ static void do_rx(unsigned long mac_ptr)
  int zd_mac_rx_irq(struct zd_mac *mac, const u8 *buffer, unsigned int length)
  {
   struct sk_buff *skb;
 + unsigned int hlen = ALIGN(sizeof(struct zd_rt_hdr), 16);
  
 - skb = dev_alloc_skb(sizeof(struct zd_rt_hdr) + length);
 + skb = dev_alloc_skb(hlen + length);
   if (!skb) {
   struct ieee80211_device *ieee = zd_mac_to_ieee80211(mac);
   dev_warn(zd_mac_dev(mac), Could not allocate skb.\n);
   ieee-stats.rx_dropped++;
   return -ENOMEM;
   }
 - skb_reserve(skb, sizeof(struct zd_rt_hdr));
 + skb_reserve(skb, hlen - ZD_PLCP_HEADER_SIZE);
   memcpy(__skb_put(skb, length), buffer, length);
   skb_queue_tail(mac-rx_queue, skb);
   tasklet_schedule(mac-rx_tasklet);
 -
 To unsubscribe from this list: send the line unsubscribe linux-wireless in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
John W. Linville
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ZD1211RW unaligned accesses...

2007-11-29 Thread Herbert Xu 
On Thu, Nov 29, 2007 at 04:45:33PM -0500, John W. Linville wrote:
 So, did the patch below fix the problem?  Should I apply it?

I'm keen to find out the result too :)

Chances are it does make progress however we may still have the
general wireless/IP stack alignment issue that we are still discussing.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ZD1211RW unaligned accesses...

2007-11-24 Thread Herbert Xu 
On Wed, Nov 21, 2007 at 01:00:44PM +, Shaddy Baddah wrote:

 It hasn't seemed to. I patched the source (confirming the patched lines 
 are in), compiled, installed and rebooted to effect the changes. My 
 zd1211rw modules timestamp indicates that I have an updated module:

Thanks for your quick response and sorry for my late answer :)

I think Dave's patch is definietly on the right track but there
are subsequent unaligned accesses of a similar kind which is
why it still appears to be broken if you look at the kernel
messages.

But there is definitely progress because those addresses are now
bigger (0x394/0x39c/0x3a8 vs. 0x2** earlier).

So please try the following patch (instead of the original one)
which should fix all the unailgned accesses in do_rx.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c 
b/drivers/net/wireless/zd1211rw/zd_mac.c
index a903645..d06b05b 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -1166,15 +1166,16 @@ static void do_rx(unsigned long mac_ptr)
 int zd_mac_rx_irq(struct zd_mac *mac, const u8 *buffer, unsigned int length)
 {
struct sk_buff *skb;
+   unsigned int hlen = ALIGN(sizeof(struct zd_rt_hdr), 16);
 
-   skb = dev_alloc_skb(sizeof(struct zd_rt_hdr) + length);
+   skb = dev_alloc_skb(hlen + length);
if (!skb) {
struct ieee80211_device *ieee = zd_mac_to_ieee80211(mac);
dev_warn(zd_mac_dev(mac), Could not allocate skb.\n);
ieee-stats.rx_dropped++;
return -ENOMEM;
}
-   skb_reserve(skb, sizeof(struct zd_rt_hdr));
+   skb_reserve(skb, hlen - ZD_PLCP_HEADER_SIZE);
memcpy(__skb_put(skb, length), buffer, length);
skb_queue_tail(mac-rx_queue, skb);
tasklet_schedule(mac-rx_tasklet);
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ZD1211RW unaligned accesses...

2007-11-24 Thread Ulrich Kunitz 
Herbert Xu wrote:

 So please try the following patch (instead of the original one)
 which should fix all the unailgned accesses in do_rx.
 
 Cheers,
 -- 
 Visit Openswan at http://www.openswan.org/
 Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
 Home Page: http://gondor.apana.org.au/~herbert/
 PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
 --
 diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c 
 b/drivers/net/wireless/zd1211rw/zd_mac.c
 index a903645..d06b05b 100644
 --- a/drivers/net/wireless/zd1211rw/zd_mac.c
 +++ b/drivers/net/wireless/zd1211rw/zd_mac.c
 @@ -1166,15 +1166,16 @@ static void do_rx(unsigned long mac_ptr)
  int zd_mac_rx_irq(struct zd_mac *mac, const u8 *buffer, unsigned int length)
  {
   struct sk_buff *skb;
 + unsigned int hlen = ALIGN(sizeof(struct zd_rt_hdr), 16);
  
 - skb = dev_alloc_skb(sizeof(struct zd_rt_hdr) + length);
 + skb = dev_alloc_skb(hlen + length);
   if (!skb) {
   struct ieee80211_device *ieee = zd_mac_to_ieee80211(mac);
   dev_warn(zd_mac_dev(mac), Could not allocate skb.\n);
   ieee-stats.rx_dropped++;
   return -ENOMEM;
   }
 - skb_reserve(skb, sizeof(struct zd_rt_hdr));
 + skb_reserve(skb, hlen - ZD_PLCP_HEADER_SIZE);
   memcpy(__skb_put(skb, length), buffer, length);
   skb_queue_tail(mac-rx_queue, skb);
   tasklet_schedule(mac-rx_tasklet);

ACK. This patch should solve it and is better than my patch.

-- 
Uli Kunitz
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ZD1211RW unaligned accesses...

2007-11-23 Thread Johannes Berg 
 The problem is
 drivers/net/wireless/zd1211/zd_mac.c:update_qual_rssi().
 Specifically the compare_ether_addr() call

I don't believe this is true. Shaddy seems to back that up by the patch
not helping.

 Wireless folks, I would suggest we do some auditing of the
 compare_ether_addr() calls and for the ones that are operating
 on these potentially unaligned structs we change it to either
 a straight memcmp() or some new routine which will more reflect
 the issue (say something like compare_ether_addr_unaligned()
 or ieee80211_compare_ether_addr()).

All MAC addresses in 802.11 headers are at least aligned on 16-bit
boundaries. Hence, if the some MAC address like the BSSID here was
unaligned we'd also have the IP header unaligned causing a lot more
trouble than this.

Shaddy, please rebuild zd1211 with the patch below, it should make the
compiler not inline all those static functions allowing us to pinpoint
much better where the problem occurs. You will probably need to delete
all *.o files in the zd1211rw/ directory to the them rebuilt after the
Makefile change.

--- everything.orig/drivers/net/wireless/zd1211rw/Makefile  2007-11-23 
11:36:30.652094075 +0100
+++ everything/drivers/net/wireless/zd1211rw/Makefile   2007-11-23 
11:36:57.112090711 +0100
@@ -1,5 +1,7 @@
 obj-$(CONFIG_ZD1211RW) += zd1211rw.o
 
+EXTRA_CFLAGS += -fno-inline-functions-called-once
+
 zd1211rw-objs := zd_chip.o zd_ieee80211.o \
zd_mac.o zd_netdev.o \
zd_rf_al2230.o zd_rf_rf2959.o \


-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ZD1211RW unaligned accesses...

2007-11-21 Thread Shaddy Baddah 

Hi David,

David Miller wrote:

Shaddy I attach a hack patch that you can use which should get
rid of the warnings.


It hasn't seemed to. I patched the source (confirming the patched lines 
are in), compiled, installed and rebooted to effect the changes. My 
zd1211rw modules timestamp indicates that I have an updated module:


$ ls -l /lib/modules/2.6.22/kernel/drivers/net/wireless/zd1211rw/zd1211rw.ko
-rw-r--r-- 1 root root 84536 2007-11-21 23:18 
/lib/modules/2.6.22/kernel/drivers/net/wireless/zd1211rw/zd1211rw.ko


lsmod confirms the module is loaded. After activating the interface 
(without configuring it yet):


$ ifconfig eth2 up

I start getting the messages over and over on the console:

Kernel unaligned access at TPC[100ee624] do_rx+0x394/0x5ec [zd1211rw]
Kernel unaligned access at TPC[100ee62c] do_rx+0x39c/0x5ec [zd1211rw]
Kernel unaligned access at TPC[100ee638] do_rx+0x3a8/0x5ec [zd1211rw]

Sorry that this has not been successful this time, but thanks for your 
help. I will be trying to follow-up on some of the other questions put 
to me.


Regards,
Shaddy
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

ZD1211RW unaligned accesses...

2007-11-20 Thread David Miller 

The problem is drivers/net/wireless/zd1211/zd_mac.c:update_qual_rssi().
Specifically the compare_ether_addr() call.  Now, ieee80211_hdr_3addr
is marked with attribute((unaligned)) but compare_ether_addr() does
not know that and does u16 * dereferences in the optimized
comparison.

Shaddy I attach a hack patch that you can use which should get
rid of the warnings.

Wireless folks, I would suggest we do some auditing of the
compare_ether_addr() calls and for the ones that are operating
on these potentially unaligned structs we change it to either
a straight memcmp() or some new routine which will more reflect
the issue (say something like compare_ether_addr_unaligned()
or ieee80211_compare_ether_addr()).

diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c 
b/drivers/net/wireless/zd1211rw/zd_mac.c
index a903645..4999869 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -1047,8 +1047,13 @@ static void update_qual_rssi(struct zd_mac *mac,
hdr = (struct ieee80211_hdr_3addr *)buffer;
if (length  offsetof(struct ieee80211_hdr_3addr, addr3))
return;
+#if 1
+   if (memcmp(hdr-addr2, zd_mac_to_ieee80211(mac)-bssid, ETH_ALEN))
+   return;
+#else
if (compare_ether_addr(hdr-addr2, zd_mac_to_ieee80211(mac)-bssid) != 
0)
return;
+#endif
 
spin_lock_irqsave(mac-lock, flags);
i = mac-stats_count % ZD_MAC_STATS_BUFFER_SIZE;
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html