[sparc64] 2.6.18 unaligned accesses in eth1394

[sparc64] 2.6.18 unaligned accesses in eth1394

[Will Simoneau]

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

Here's a pair of unaligned accesses I found playing with the eth1394 driver:

Kernel unaligned access at TPC[102c8190] ether1394_tx+0xf8/0x600 [eth1394]
Kernel unaligned access at TPC[10162c8c] ether1394_data_handler+0x914/0x1000 [eth1394]

The first one I seem to be able to fix by adding a get_unaligned() at
lines 1679-1680 of eth1394.c around eth->h_dest; the second one seems to
be triggered by this code:

(gdb) list *ether1394_data_handler+0x914
0xc94 is in ether1394_data_handler (drivers/ieee1394/eth1394.c:1264).
1259        priv->stats.rx_dropped++;
1260        dev_kfree_skb_any(skb);
1261        goto bad_proto;
1262     }
1263  
1264     if (netif_rx(skb) == NET_RX_DROP) {
1265        priv->stats.rx_errors++;
1266        priv->stats.rx_dropped++;
1267        goto bad_proto;
1268     }

Unaligned accesses caused by netif_rx seem to have happened before on
mips long ago (according to google). I suspect nobody has ever tried
eth1394 on sparc64 ;-)

eth1394 seems to work fine otherwise, I can plug a firewire hdd and a
laptop with firewire into the sparc and everything works OK. I get
~13MB/sec network throughput from laptop -> sparc (the sparc's CPU is
hitting 100% system time), I suspect it would be a lot faster with the
alignment problem fixed.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [sparc64] 2.6.18 unaligned accesses in eth1394

[David Miller]

From: Will Simoneau <simoneau@ele.uri.edu>
Date: Thu, 5 Oct 2006 17:15:44 -0400

> Here's a pair of unaligned accesses I found playing with the eth1394 driver:
> 
> Kernel unaligned access at TPC[102c8190] ether1394_tx+0xf8/0x600 [eth1394]
> Kernel unaligned access at TPC[10162c8c] ether1394_data_handler+0x914/0x1000 [eth1394]

Thanks a lot for reporting this.  I have a similar report from
Harald Welte involving IPV6 to look at as well.

I'll try to get these all fixed up next week.

Thanks again.

Re: [sparc64] 2.6.18 unaligned accesses in eth1394

[David Miller]

From: Will Simoneau <simoneau@ele.uri.edu>
Date: Thu, 5 Oct 2006 17:15:44 -0400

> The first one I seem to be able to fix by adding a get_unaligned() at
> lines 1679-1680 of eth1394.c around eth->h_dest; the second one seems to
> be triggered by this code:

Right.

> (gdb) list *ether1394_data_handler+0x914
> 0xc94 is in ether1394_data_handler (drivers/ieee1394/eth1394.c:1264).
> 1259        priv->stats.rx_dropped++;
> 1260        dev_kfree_skb_any(skb);
> 1261        goto bad_proto;
> 1262     }
> 1263  
> 1264     if (netif_rx(skb) == NET_RX_DROP) {
> 1265        priv->stats.rx_errors++;
> 1266        priv->stats.rx_dropped++;
> 1267        goto bad_proto;
> 1268     }

Actually, I think this one is in eth1394_parse_encap().

Can you test this patch and tell me if it makes both unaligned
access problems go away?  Thanks.

diff --git a/drivers/ieee1394/eth1394.c b/drivers/ieee1394/eth1394.c
index 8a7b8fa..78abb9b 100644
--- a/drivers/ieee1394/eth1394.c
+++ b/drivers/ieee1394/eth1394.c
@@ -64,6 +64,7 @@ #include <linux/bitops.h>
 #include <linux/ethtool.h>
 #include <asm/uaccess.h>
 #include <asm/delay.h>
+#include <asm/unaligned.h>
 #include <net/arp.h>
 
 #include "config_roms.h"
@@ -894,6 +895,7 @@ static inline u16 ether1394_parse_encap(
 		u16 maxpayload;
 		struct eth1394_node_ref *node;
 		struct eth1394_node_info *node_info;
+		__be64 guid;
 
 		/* Sanity check. MacOSX seems to be sending us 131 in this
 		 * field (atleast on my Panther G5). Not sure why. */
@@ -902,8 +904,9 @@ static inline u16 ether1394_parse_encap(
 
 		maxpayload = min(eth1394_speedto_maxpayload[sspd], (u16)(1 << (max_rec + 1)));
 
+		guid = get_unaligned(&arp1394->s_uniq_id);
 		node = eth1394_find_node_guid(&priv->ip_node_list,
-					      be64_to_cpu(arp1394->s_uniq_id));
+					      be64_to_cpu(guid));
 		if (!node) {
 			return 0;
 		}
@@ -1675,8 +1678,9 @@ #endif
 		if (max_payload < dg_size + hdr_type_len[ETH1394_HDR_LF_UF])
 			priv->bc_dgl++;
 	} else {
+		__be64 guid = get_unaligned((u64 *)eth->h_dest);
 		node = eth1394_find_node_guid(&priv->ip_node_list,
-					      be64_to_cpu(*(u64*)eth->h_dest));
+					      be64_to_cpu(guid));
 		if (!node) {
 			ret = -EAGAIN;
 			goto fail;

Re: [sparc64] 2.6.18 unaligned accesses in eth1394

[Will Simoneau]

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

On 18:36 Mon 09 Oct     , David Miller wrote:
> From: Will Simoneau <simoneau@ele.uri.edu>
> Date: Thu, 5 Oct 2006 17:15:44 -0400
> 
> > The first one I seem to be able to fix by adding a get_unaligned() at
> > lines 1679-1680 of eth1394.c around eth->h_dest; the second one seems to
> > be triggered by this code:
> 
> Right.
> 
> > (gdb) list *ether1394_data_handler+0x914
> > 0xc94 is in ether1394_data_handler (drivers/ieee1394/eth1394.c:1264).
> > 1259        priv->stats.rx_dropped++;
> > 1260        dev_kfree_skb_any(skb);
> > 1261        goto bad_proto;
> > 1262     }
> > 1263  
> > 1264     if (netif_rx(skb) == NET_RX_DROP) {
> > 1265        priv->stats.rx_errors++;
> > 1266        priv->stats.rx_dropped++;
> > 1267        goto bad_proto;
> > 1268     }
> 
> Actually, I think this one is in eth1394_parse_encap().
> 
> Can you test this patch and tell me if it makes both unaligned
> access problems go away?  Thanks.
> 
> diff --git a/drivers/ieee1394/eth1394.c b/drivers/ieee1394/eth1394.c
> index 8a7b8fa..78abb9b 100644
> --- a/drivers/ieee1394/eth1394.c
> +++ b/drivers/ieee1394/eth1394.c
> @@ -64,6 +64,7 @@ #include <linux/bitops.h>
>  #include <linux/ethtool.h>
>  #include <asm/uaccess.h>
>  #include <asm/delay.h>
> +#include <asm/unaligned.h>
>  #include <net/arp.h>
>  
>  #include "config_roms.h"
> @@ -894,6 +895,7 @@ static inline u16 ether1394_parse_encap(
>  		u16 maxpayload;
>  		struct eth1394_node_ref *node;
>  		struct eth1394_node_info *node_info;
> +		__be64 guid;
>  
>  		/* Sanity check. MacOSX seems to be sending us 131 in this
>  		 * field (atleast on my Panther G5). Not sure why. */
> @@ -902,8 +904,9 @@ static inline u16 ether1394_parse_encap(
>  
>  		maxpayload = min(eth1394_speedto_maxpayload[sspd], (u16)(1 << (max_rec + 1)));
>  
> +		guid = get_unaligned(&arp1394->s_uniq_id);
>  		node = eth1394_find_node_guid(&priv->ip_node_list,
> -					      be64_to_cpu(arp1394->s_uniq_id));
> +					      be64_to_cpu(guid));
>  		if (!node) {
>  			return 0;
>  		}
> @@ -1675,8 +1678,9 @@ #endif
>  		if (max_payload < dg_size + hdr_type_len[ETH1394_HDR_LF_UF])
>  			priv->bc_dgl++;
>  	} else {
> +		__be64 guid = get_unaligned((u64 *)eth->h_dest);
>  		node = eth1394_find_node_guid(&priv->ip_node_list,
> -					      be64_to_cpu(*(u64*)eth->h_dest));
> +					      be64_to_cpu(guid));
>  		if (!node) {
>  			ret = -EAGAIN;
>  			goto fail;


I still get:

Kernel unaligned access at TPC[10162164] ether1394_reset_priv+0x2c/0xe0 [eth1394]
Kernel unaligned access at TPC[10163148] ether1394_data_handler+0xdd0/0x1060 [eth1394]

The second one triggers on every packet received, the first only triggers once in a while.

If you want more gdb info or a disassembly just ask.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [sparc64] 2.6.18 unaligned accesses in eth1394

[David Miller]

From: Will Simoneau <simoneau@ele.uri.edu>
Date: Tue, 10 Oct 2006 09:29:43 -0400

> I still get:
> 
> Kernel unaligned access at TPC[10162164] ether1394_reset_priv+0x2c/0xe0 [eth1394]
> Kernel unaligned access at TPC[10163148] ether1394_data_handler+0xdd0/0x1060 [eth1394]
> 
> The second one triggers on every packet received, the first only triggers once in a while.
> 
> If you want more gdb info or a disassembly just ask.

Hmmm, can you do me a favor?  Build ieee1394 and eth1394 statically
into your kernel, reproduce, and post the kernel log messages
and the vmlinux image somewhere where I can fetch them.

I should be able to fix this once I have that.

Thanks a lot.

Re: [sparc64] 2.6.18 unaligned accesses in eth1394

[Stefan Richter]

On 10/11/2006 12:17 AM, David Miller wrote:
> From: Will Simoneau <simoneau@ele.uri.edu>
> Date: Tue, 10 Oct 2006 09:29:43 -0400
> 
>> I still get:
>> 
>> Kernel unaligned access at TPC[10162164] ether1394_reset_priv+0x2c/0xe0 [eth1394]
>> Kernel unaligned access at TPC[10163148] ether1394_data_handler+0xdd0/0x1060 [eth1394]
>> 
>> The second one triggers on every packet received, the first only triggers once in a while.
>> 
>> If you want more gdb info or a disassembly just ask.
> 
> Hmmm, can you do me a favor?  Build ieee1394 and eth1394 statically
> into your kernel, reproduce, and post the kernel log messages
> and the vmlinux image somewhere where I can fetch them.
> 
> I should be able to fix this once I have that.
> 
> Thanks a lot.

Please keep linux1394-devel informed too. (Now added to Cc list.)
-- 
Stefan Richter
-=====-=-==- =-=- -=-==
http://arcgraph.de/sr/

Re: [sparc64] 2.6.18 unaligned accesses in eth1394

[David Miller]

From: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Wed, 11 Oct 2006 14:33:22 +0200

> Please keep linux1394-devel informed too. (Now added to Cc list.)

No problem.

Here is the current patch I have Will testing.  It should fix
all the unaligned u64 pointer dereferences.

Do you guys mind if I push this into my next batch of 2.6.19-rcX
networking bug fixes, assuming that testing goes alright for
Will?

Thanks.

diff --git a/drivers/ieee1394/eth1394.c b/drivers/ieee1394/eth1394.c
index 8a7b8fa..31e5cc4 100644
--- a/drivers/ieee1394/eth1394.c
+++ b/drivers/ieee1394/eth1394.c
@@ -64,6 +64,7 @@ #include <linux/bitops.h>
 #include <linux/ethtool.h>
 #include <asm/uaccess.h>
 #include <asm/delay.h>
+#include <asm/unaligned.h>
 #include <net/arp.h>
 
 #include "config_roms.h"
@@ -491,7 +492,7 @@ static void ether1394_reset_priv (struct
 	int i;
 	struct eth1394_priv *priv = netdev_priv(dev);
 	struct hpsb_host *host = priv->host;
-	u64 guid = *((u64*)&(host->csr.rom->bus_info_data[3]));
+	u64 guid = get_unaligned((u64*)&(host->csr.rom->bus_info_data[3]));
 	u16 maxpayload = 1 << (host->csr.max_rec + 1);
 	int max_speed = IEEE1394_SPEED_MAX;
 
@@ -514,8 +515,8 @@ static void ether1394_reset_priv (struct
 				      ETHER1394_GASP_OVERHEAD)));
 
 		/* Set our hardware address while we're at it */
-		*(u64*)dev->dev_addr = guid;
-		*(u64*)dev->broadcast = ~0x0ULL;
+		memcpy(dev->dev_addr, &guid, sizeof(u64));
+		memset(dev->broadcast, 0xff, sizeof(u64));
 	}
 
 	spin_unlock_irqrestore (&priv->lock, flags);
@@ -894,6 +895,7 @@ static inline u16 ether1394_parse_encap(
 		u16 maxpayload;
 		struct eth1394_node_ref *node;
 		struct eth1394_node_info *node_info;
+		__be64 guid;
 
 		/* Sanity check. MacOSX seems to be sending us 131 in this
 		 * field (atleast on my Panther G5). Not sure why. */
@@ -902,8 +904,9 @@ static inline u16 ether1394_parse_encap(
 
 		maxpayload = min(eth1394_speedto_maxpayload[sspd], (u16)(1 << (max_rec + 1)));
 
+		guid = get_unaligned(&arp1394->s_uniq_id);
 		node = eth1394_find_node_guid(&priv->ip_node_list,
-					      be64_to_cpu(arp1394->s_uniq_id));
+					      be64_to_cpu(guid));
 		if (!node) {
 			return 0;
 		}
@@ -931,10 +934,9 @@ static inline u16 ether1394_parse_encap(
 		arp_ptr += arp->ar_pln;		/* skip over sender IP addr */
 
 		if (arp->ar_op == htons(ARPOP_REQUEST))
-			/* just set ARP req target unique ID to 0 */
-			*((u64*)arp_ptr) = 0;
+			memset(arp_ptr, 0, sizeof(u64));
 		else
-			*((u64*)arp_ptr) = *((u64*)dev->dev_addr);
+			memcpy(arp_ptr, dev->dev_addr, sizeof(u64));
 	}
 
 	/* Now add the ethernet header. */
@@ -1675,8 +1677,10 @@ #endif
 		if (max_payload < dg_size + hdr_type_len[ETH1394_HDR_LF_UF])
 			priv->bc_dgl++;
 	} else {
+		__be64 guid = get_unaligned((u64 *)eth->h_dest);
+
 		node = eth1394_find_node_guid(&priv->ip_node_list,
-					      be64_to_cpu(*(u64*)eth->h_dest));
+					      be64_to_cpu(guid));
 		if (!node) {
 			ret = -EAGAIN;
 			goto fail;

Re: [sparc64] 2.6.18 unaligned accesses in eth1394

[Stefan Richter]

David Miller wrote:
> Here is the current patch I have Will testing.  It should fix
> all the unaligned u64 pointer dereferences.
> 
> Do you guys mind if I push this into my next batch of 2.6.19-rcX
> networking bug fixes, assuming that testing goes alright for
> Will?

Please do so. Thanks | ACK.
-- 
Stefan Richter
-=====-=-==- =-=- ===--
http://arcgraph.de/sr/

Re: [sparc64] 2.6.18 unaligned accesses in eth1394

[Will Simoneau]

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

On 15:17 Tue 10 Oct     , David Miller wrote:
> From: Will Simoneau <simoneau@ele.uri.edu>
> Date: Tue, 10 Oct 2006 09:29:43 -0400
> 
> > I still get:
> > 
> > Kernel unaligned access at TPC[10162164] ether1394_reset_priv+0x2c/0xe0 [eth1394]
> > Kernel unaligned access at TPC[10163148] ether1394_data_handler+0xdd0/0x1060 [eth1394]
> > 
> > The second one triggers on every packet received, the first only triggers once in a while.
> > 
> > If you want more gdb info or a disassembly just ask.
> 
> Hmmm, can you do me a favor?  Build ieee1394 and eth1394 statically
> into your kernel, reproduce, and post the kernel log messages
> and the vmlinux image somewhere where I can fetch them.
> 
> I should be able to fix this once I have that.
> 
> Thanks a lot.

This will be difficult as the machine in question isn't one I want to
reboot too often. The running kernel and modules are compiled with debug
info, though; if I send you the relevant .o files will that help?

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [sparc64] 2.6.18 unaligned accesses in eth1394

[David Miller]

From: Will Simoneau <simoneau@ele.uri.edu>
Date: Thu, 12 Oct 2006 10:37:26 -0400

> This will be difficult as the machine in question isn't one I want to
> reboot too often. The running kernel and modules are compiled with debug
> info, though; if I send you the relevant .o files will that help?

The reason I asked for it in the image is because I want to tell gdb
"x/10i 0xZZZZZ" using the exact program counters in the dumps and get
exactly the instruction that is faulting.

That is very difficult with modules since they can get loaded at any
address, and the gdb listings you provided with those symbol-relative
addresses were non-sensible, and that's why I can't match the true
cause up right now.

So whilst I can try with a module image, you'd have to do things like
tell me exactly what is the base address of the loaded kernel module
at the time of the message trigger and other non-trivial stuff like
that to get me the debugging info I want to fix this.

Thanks.