[PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices

[PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices

[Alexander Aring]

The function addrconf_ifid_eui64 will copy eui into dev->dev_addr.
We need first to manipulate eui[0] before we call memcpy afterwards.

Broken since commit:
5e98a36ed4bf6ea396170e3af0dd4fcbe51d772f

Since this commit I got many trouble with the ieee802154 stack.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/ipv6/addrconf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index fa36a67..4460237 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1731,8 +1731,8 @@ static int addrconf_ifid_eui64(u8 *eui, struct net_device *dev)
 {
 	if (dev->addr_len != IEEE802154_ADDR_LEN)
 		return -1;
-	memcpy(eui, dev->dev_addr, 8);
 	eui[0] ^= 2;
+	memcpy(eui, dev->dev_addr, 8);
 	return 0;
 }
 
-- 
1.8.1.5

Re: [PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices

[Alexander Aring]

Hi,

I don't know if this is a right solution for that, because other
functions like addrconf_ifid_infiniband to manipulate eui after memcpy,
too.

But this patch solves my problems with ieee802154 stack.

Any other ideas?

Regards
Alex

Re: [PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices

[Ben Hutchings]

On Thu, 2013-03-07 at 21:13 +0100, Alexander Aring wrote:
> The function addrconf_ifid_eui64 will copy eui into dev->dev_addr.
> We need first to manipulate eui[0] before we call memcpy afterwards.

memcpy() does not work that way.

> Broken since commit:
> 5e98a36ed4bf6ea396170e3af0dd4fcbe51d772f
> 
> Since this commit I got many trouble with the ieee802154 stack.

Your change is effectively reverting that commit.

Ben.

> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>  net/ipv6/addrconf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index fa36a67..4460237 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -1731,8 +1731,8 @@ static int addrconf_ifid_eui64(u8 *eui, struct net_device *dev)
>  {
>  	if (dev->addr_len != IEEE802154_ADDR_LEN)
>  		return -1;
> -	memcpy(eui, dev->dev_addr, 8);
>  	eui[0] ^= 2;
> +	memcpy(eui, dev->dev_addr, 8);
>  	return 0;
>  }
>  

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices

[Eric Dumazet]

On Thu, 2013-03-07 at 22:41 +0000, Ben Hutchings wrote:
> On Thu, 2013-03-07 at 21:13 +0100, Alexander Aring wrote:
> > The function addrconf_ifid_eui64 will copy eui into dev->dev_addr.
> > We need first to manipulate eui[0] before we call memcpy afterwards.
> 
> memcpy() does not work that way.
> 
> > Broken since commit:
> > 5e98a36ed4bf6ea396170e3af0dd4fcbe51d772f
> > 
> > Since this commit I got many trouble with the ieee802154 stack.
> 
> Your change is effectively reverting that commit.


Reverting the commit seems the solution ;)

Re: [PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 07 Mar 2013 14:57:05 -0800

> On Thu, 2013-03-07 at 22:41 +0000, Ben Hutchings wrote:
>> On Thu, 2013-03-07 at 21:13 +0100, Alexander Aring wrote:
>> > The function addrconf_ifid_eui64 will copy eui into dev->dev_addr.
>> > We need first to manipulate eui[0] before we call memcpy afterwards.
>> 
>> memcpy() does not work that way.
>> 
>> > Broken since commit:
>> > 5e98a36ed4bf6ea396170e3af0dd4fcbe51d772f
>> > 
>> > Since this commit I got many trouble with the ieee802154 stack.
>> 
>> Your change is effectively reverting that commit.
> 
> Reverting the commit seems the solution ;)

So Yoshifuji HIDEAKI and Alexander need to work out what is happening
here.

Re: [PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices

[YOSHIFUJI Hideaki]

Hi,

Alexander Aring wrote:
> Hi,
> 
> I don't know if this is a right solution for that, because other
> functions like addrconf_ifid_infiniband to manipulate eui after memcpy,
> too.
> 
> But this patch solves my problems with ieee802154 stack.

What kind of problems do you have?

--yoshfuji

Re: [PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices

[Alexander Aring]

On Thu, Mar 07, 2013 at 06:17:51PM -0500, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 07 Mar 2013 14:57:05 -0800
> 
> > On Thu, 2013-03-07 at 22:41 +0000, Ben Hutchings wrote:
> >> On Thu, 2013-03-07 at 21:13 +0100, Alexander Aring wrote:
> >> > The function addrconf_ifid_eui64 will copy eui into dev->dev_addr.
> >> > We need first to manipulate eui[0] before we call memcpy afterwards.
> >> 
> >> memcpy() does not work that way.
> >> 
> >> > Broken since commit:
> >> > 5e98a36ed4bf6ea396170e3af0dd4fcbe51d772f
> >> > 
> >> > Since this commit I got many trouble with the ieee802154 stack.
> >> 
> >> Your change is effectively reverting that commit.
> > 
> > Reverting the commit seems the solution ;)
> 
> So Yoshifuji HIDEAKI and Alexander need to work out what is happening
> here.

Ok, I will try that. Maybe somebody with a another 802.15.4 device with
6lowpan on linux-zigbee-devel mailinglist can confirm that?

I have a at86rf231 device.

Alex

Re: [PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices

[Alexander Aring]

On Thu, Mar 07, 2013 at 10:41:28PM +0000, Ben Hutchings wrote:
> On Thu, 2013-03-07 at 21:13 +0100, Alexander Aring wrote:
> > The function addrconf_ifid_eui64 will copy eui into dev->dev_addr.
> > We need first to manipulate eui[0] before we call memcpy afterwards.
> 
> memcpy() does not work that way.
> 
oh yeah, sry. It was too late yesterday. Change dest and src address of a common
function, sry. :-)

Alex

------------------------------------------------------------------------------
Symantec Endpoint Protection 12 positioned as A LEADER in The Forrester  
Wave(TM): Endpoint Security, Q1 2013 and "remains a good choice" in the  
endpoint security space. For insight on selecting the right partner to 
tackle endpoint security challenges, access the full report. 
http://p.sf.net/sfu/symantec-dev2dev

Re: [PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices

[Alexander Aring]

Hi,

On Fri, Mar 08, 2013 at 11:21:38AM +0900, YOSHIFUJI Hideaki wrote:
> Hi,
> 
> Alexander Aring wrote:
> > Hi,
> > 
> > I don't know if this is a right solution for that, because other
> > functions like addrconf_ifid_infiniband to manipulate eui after memcpy,
> > too.
> > 
> > But this patch solves my problems with ieee802154 stack.
> 
> What kind of problems do you have?
>

I compile with #define DEBUG enabled in net/ieee802154/6lowpan.c

The problem is the address compression in 6lowpan.

First device has address fe80::a000:0:0:1/64. Second device has address
fe80::a000:0:0:2/64. I use only a ping6 to check connection.

Debug Information on the receiving side:
Without your patch which is working.

iphc0 = 7a, iphc1 = 11
NH flag is set, next header carried inline: 3a
source address stateless compression
(lowpan_uncompress_addr) linklocal address: 
a0 00 00 00 00 00 00 02
uncompressing 2 + 8 => 
fe 80 00 00 00 00 00 00 a0 00 00 00 00 00 00 02
dest: stateless compression
(lowpan_uncompress_addr) linklocal address: 
a0 00 00 00 00 00 00 01
uncompressing 2 + 8 => 
fe 80 00 00 00 00 00 00 a0 00 00 00 00 00 00 01
skb headroom size = 40, data length = 64
IPv6 header dump:
	version = 6
	length  = 64
	nexthdr = 0x3a
	hop_lim = 64


With your patch, which isn't working anymore:

iphc0 = 7b, iphc1 = 3b
NH flag is set, next header carried inline: 3a
source address stateless compression
(lowpan_uncompress_addr) linklocal address: 
02 85 00 3a 20 00 00 00
uncompressing 2 + 0 => 
fe 80 00 00 00 00 00 00 00 85 00 3a 20 00 00 00
dest: non context-based mcast compression
uncompressing 2 + 1 => 
ff 02 00 00 00 00 00 00 00 00 00 00 00 00 00 02
skb headroom size = 25, data length = 24
IPv6 header dump:
	version = 6
	length  = 24
	nexthdr = 0x3a
	hop_lim = 255

And for every ipv6 packet I got:
ICMPv6 checksum failed [fe80:0000:0000:0000:0001:ff00:0001:8700 > ff02:0000:0000:0000:0000:0001:ff00:0001]

I seems that the reconstructed destination address and header length are
not right.

I will try to figure out this problem. Hope this information helps you to
know more about my problems.

Regards
Alex

Re: [PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices

[YOSHIFUJI Hideaki]

Alexander Aring wrote:
> Hi,
> 
> On Fri, Mar 08, 2013 at 11:21:38AM +0900, YOSHIFUJI Hideaki wrote:
>> Hi,
>>
>> Alexander Aring wrote:
>>> Hi,
>>>
>>> I don't know if this is a right solution for that, because other
>>> functions like addrconf_ifid_infiniband to manipulate eui after memcpy,
>>> too.
>>>
>>> But this patch solves my problems with ieee802154 stack.
>>
>> What kind of problems do you have?
>>
> 
> I compile with #define DEBUG enabled in net/ieee802154/6lowpan.c
> 
> The problem is the address compression in 6lowpan.
> 
> First device has address fe80::a000:0:0:1/64. Second device has address
> fe80::a000:0:0:2/64. I use only a ping6 to check connection.
> 
> Debug Information on the receiving side:
> Without your patch which is working.
> 
> iphc0 = 7a, iphc1 = 11
> NH flag is set, next header carried inline: 3a
> source address stateless compression
> (lowpan_uncompress_addr) linklocal address: 
> a0 00 00 00 00 00 00 02
> uncompressing 2 + 8 => 
> fe 80 00 00 00 00 00 00 a0 00 00 00 00 00 00 02
> dest: stateless compression
> (lowpan_uncompress_addr) linklocal address: 
> a0 00 00 00 00 00 00 01
> uncompressing 2 + 8 => 
> fe 80 00 00 00 00 00 00 a0 00 00 00 00 00 00 01
> skb headroom size = 40, data length = 64
> IPv6 header dump:
> 	version = 6
> 	length  = 64
> 	nexthdr = 0x3a
> 	hop_lim = 64
> 
> 
> With your patch, which isn't working anymore:
> 
> iphc0 = 7b, iphc1 = 3b
> NH flag is set, next header carried inline: 3a
> source address stateless compression
> (lowpan_uncompress_addr) linklocal address: 
> 02 85 00 3a 20 00 00 00
> uncompressing 2 + 0 => 
> fe 80 00 00 00 00 00 00 00 85 00 3a 20 00 00 00
> dest: non context-based mcast compression
> uncompressing 2 + 1 => 
> ff 02 00 00 00 00 00 00 00 00 00 00 00 00 00 02
> skb headroom size = 25, data length = 24
> IPv6 header dump:
> 	version = 6
> 	length  = 24
> 	nexthdr = 0x3a
> 	hop_lim = 255
> 
> And for every ipv6 packet I got:
> ICMPv6 checksum failed [fe80:0000:0000:0000:0001:ff00:0001:8700 > ff02:0000:0000:0000:0000:0001:ff00:0001]
> 
> I seems that the reconstructed destination address and header length are
> not right.

That's wired.  It is expected to work whatever valid source/destination
addresses are used, right?

--yoshfuji

Re: [PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices

[YOSHIFUJI Hideaki]

YOSHIFUJI Hideaki wrote:
> Alexander Aring wrote:
:
>> With your patch, which isn't working anymore:
>>
>> iphc0 = 7b, iphc1 = 3b
>> NH flag is set, next header carried inline: 3a
>> source address stateless compression
>> (lowpan_uncompress_addr) linklocal address: 
>> 02 85 00 3a 20 00 00 00
>> uncompressing 2 + 0 => 
>> fe 80 00 00 00 00 00 00 00 85 00 3a 20 00 00 00
>> dest: non context-based mcast compression
>> uncompressing 2 + 1 => 
>> ff 02 00 00 00 00 00 00 00 00 00 00 00 00 00 02
>> skb headroom size = 25, data length = 24
>> IPv6 header dump:
>> 	version = 6
>> 	length  = 24
>> 	nexthdr = 0x3a
>> 	hop_lim = 255
>>
>> And for every ipv6 packet I got:
>> ICMPv6 checksum failed [fe80:0000:0000:0000:0001:ff00:0001:8700 > ff02:0000:0000:0000:0000:0001:ff00:0001]
>>
>> I seems that the reconstructed destination address and header length are
>> not right.
> 
> That's wired.  It is expected to work whatever valid source/destination
> addesses are used, right?

It seems the last argument for lowpan_uncompress_address() (lladdr,
show as "linklocal address") is incorrect.
Note that it should say link-"layer" address, BTW.

--yoshfuji

Re: [PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices

[Alexander Aring]

Hi,

I try to figure out this problem but I think it will take some days.
I will send a fix then.

Thanks for your help.

Alex