[PATCHv2 net-next] 6lowpan: handle only real link-local addresses

[PATCHv2 net-next] 6lowpan: handle only real link-local addresses

[Alexander Aring]

A link-local address isn't fe80::/10 it's fe80::/64
see http://tools.ietf.org/html/rfc4291#section-2.5.6
for more details.

Also fix a comment issue "local link" -> "link-local"

Signed-off-by: Alexander Aring <alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Werner Almesberger <werner-SEdMjqphH88wryQfseakQg@public.gmane.org>
---
v2:
 - Add "net-next" tag

 net/ieee802154/6lowpan.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/ieee802154/6lowpan.h b/net/ieee802154/6lowpan.h
index 4b8f917..61f0ce9 100644
--- a/net/ieee802154/6lowpan.h
+++ b/net/ieee802154/6lowpan.h
@@ -83,8 +83,11 @@
 #define ipaddr_prefixcmp(addr1, addr2, length) \
 	(memcmp(addr1, addr2, length >> 3) == 0)
 
-/* local link, i.e. FE80::/10 */
-#define is_addr_link_local(a) (((a)->s6_addr16[0]) == htons(0xFE80))
+/* link-local, i.e. FE80::/64 */
+#define is_addr_link_local(a) (((a)->s6_addr16[0]) == htons(0xFE80) &&	\
+		(((a)->s6_addr16[1]) == 0) &&	\
+		(((a)->s6_addr16[2]) == 0) &&	\
+		(((a)->s6_addr16[3]) == 0))
 
 /*
  * check whether we can compress the IID to 16 bits,
-- 
1.8.3.3


------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk

Re: [PATCHv2 net-next] 6lowpan: handle only real link-local addresses

[Hannes Frederic Sowa]

On Wed, Aug 14, 2013 at 01:01:07PM +0200, Alexander Aring wrote:
> diff --git a/net/ieee802154/6lowpan.h b/net/ieee802154/6lowpan.h
> index 4b8f917..61f0ce9 100644
> --- a/net/ieee802154/6lowpan.h
> +++ b/net/ieee802154/6lowpan.h
> @@ -83,8 +83,11 @@
>  #define ipaddr_prefixcmp(addr1, addr2, length) \
>  	(memcmp(addr1, addr2, length >> 3) == 0)
>  
> -/* local link, i.e. FE80::/10 */
> -#define is_addr_link_local(a) (((a)->s6_addr16[0]) == htons(0xFE80))
> +/* link-local, i.e. FE80::/64 */
> +#define is_addr_link_local(a) (((a)->s6_addr16[0]) == htons(0xFE80) &&	\
> +		(((a)->s6_addr16[1]) == 0) &&	\
> +		(((a)->s6_addr16[2]) == 0) &&	\
> +		(((a)->s6_addr16[3]) == 0))
>  

Can't you use ipv6_addr_type(a)&IPV6_ADDR_LINKLOCAL?

Greetings,

  Hannes

Re: [PATCHv2 net-next] 6lowpan: handle only real link-local addresses

[Alexander Aring]

Hi Hannes,

On Wed, Aug 14, 2013 at 01:08:21PM +0200, Hannes Frederic Sowa wrote:
> On Wed, Aug 14, 2013 at 01:01:07PM +0200, Alexander Aring wrote:
> > diff --git a/net/ieee802154/6lowpan.h b/net/ieee802154/6lowpan.h
> > index 4b8f917..61f0ce9 100644
> > --- a/net/ieee802154/6lowpan.h
> > +++ b/net/ieee802154/6lowpan.h
> > @@ -83,8 +83,11 @@
> >  #define ipaddr_prefixcmp(addr1, addr2, length) \
> >  	(memcmp(addr1, addr2, length >> 3) == 0)
> >  
> > -/* local link, i.e. FE80::/10 */
> > -#define is_addr_link_local(a) (((a)->s6_addr16[0]) == htons(0xFE80))
> > +/* link-local, i.e. FE80::/64 */
> > +#define is_addr_link_local(a) (((a)->s6_addr16[0]) == htons(0xFE80) &&	\
> > +		(((a)->s6_addr16[1]) == 0) &&	\
> > +		(((a)->s6_addr16[2]) == 0) &&	\
> > +		(((a)->s6_addr16[3]) == 0))
> >  
> 
> Can't you use ipv6_addr_type(a)&IPV6_ADDR_LINKLOCAL?
>
Of course, thanks for this hint I will take this on the list to change
it for checking for a multicast address, too.


I looked in the implementation of ipv6_addr_type and some little thing
confusing me.
A local-link address is FE80::/64, but the __ipv6_addr_type function
checks for FE80::/32.

-- snip
...
__be32 st;
st = addr->s6_addr32[0];
...
if ((st & htonl(0xFFC00000)) == htonl(0xFE800000))
	return ...
-- end snip


Is there some (addr->s6_addr32[1] == 0) check which I don't see?

Regards
Alex

Re: [PATCHv2 net-next] 6lowpan: handle only real link-local addresses

[Hannes Frederic Sowa]

On Wed, Aug 14, 2013 at 01:29:04PM +0200, Alexander Aring wrote:
> Hi Hannes,
> 
> On Wed, Aug 14, 2013 at 01:08:21PM +0200, Hannes Frederic Sowa wrote:
> > On Wed, Aug 14, 2013 at 01:01:07PM +0200, Alexander Aring wrote:
> > > diff --git a/net/ieee802154/6lowpan.h b/net/ieee802154/6lowpan.h
> > > index 4b8f917..61f0ce9 100644
> > > --- a/net/ieee802154/6lowpan.h
> > > +++ b/net/ieee802154/6lowpan.h
> > > @@ -83,8 +83,11 @@
> > >  #define ipaddr_prefixcmp(addr1, addr2, length) \
> > >  	(memcmp(addr1, addr2, length >> 3) == 0)
> > >  
> > > -/* local link, i.e. FE80::/10 */
> > > -#define is_addr_link_local(a) (((a)->s6_addr16[0]) == htons(0xFE80))
> > > +/* link-local, i.e. FE80::/64 */
> > > +#define is_addr_link_local(a) (((a)->s6_addr16[0]) == htons(0xFE80) &&	\
> > > +		(((a)->s6_addr16[1]) == 0) &&	\
> > > +		(((a)->s6_addr16[2]) == 0) &&	\
> > > +		(((a)->s6_addr16[3]) == 0))
> > >  
> > 
> > Can't you use ipv6_addr_type(a)&IPV6_ADDR_LINKLOCAL?
> >
> Of course, thanks for this hint I will take this on the list to change
> it for checking for a multicast address, too.
> 
> 
> I looked in the implementation of ipv6_addr_type and some little thing
> confusing me.
> A local-link address is FE80::/64, but the __ipv6_addr_type function
> checks for FE80::/32.

This should be a test of fe80::/10 as specified in
http://tools.ietf.org/html/rfc4291#section-2.5.6. Look at the parentheses
between (st & htonl()) == htonl(..)

Yes, we don't seem to care about the zeros padded in the address. As specified
by IANA, these addresses would still count as link-local:
https://www.iana.org/assignments/ipv6-address-space/ipv6-address-space.xhtml

Greetings,

  Hannes

Re: [PATCHv2 net-next] 6lowpan: handle only real link-local addresses

[David Miller]

From: Alexander Aring <alex.aring@gmail.com>
Date: Wed, 14 Aug 2013 13:01:07 +0200

> A link-local address isn't fe80::/10 it's fe80::/64
> see http://tools.ietf.org/html/rfc4291#section-2.5.6
> for more details.
> 
> Also fix a comment issue "local link" -> "link-local"
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> Reviewed-by: Werner Almesberger <werner@almesberger.net>

You still haven't addressed my biggest concern.

What order do I apply this patch relative to the other 6?

Don't make me guess, instead place this patch into the necessary
place within the series of 6 patches.

I'm not applying any of this until you can submit it properly.

Thanks.

Re: [PATCHv2 net-next] 6lowpan: handle only real link-local addresses

[Alexander Aring]

Hi David,

On Wed, Aug 14, 2013 at 12:53:46PM -0700, David Miller wrote:
> From: Alexander Aring <alex.aring@gmail.com>
> Date: Wed, 14 Aug 2013 13:01:07 +0200
> 
> > A link-local address isn't fe80::/10 it's fe80::/64
> > see http://tools.ietf.org/html/rfc4291#section-2.5.6
> > for more details.
> > 
> > Also fix a comment issue "local link" -> "link-local"
> > 
> > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > Reviewed-by: Werner Almesberger <werner@almesberger.net>
> 
> You still haven't addressed my biggest concern.
> 
Sorry, I thought that was the missing "net-next" tag in the subject.

> What order do I apply this patch relative to the other 6?
> 
This patch is a fix for the compression of an address. To make the right
compression on a link-local address.

So this patch has nothing to do for the address uncompression fix
(the patch serie should fix the uncompression of addresses). This is a
bigger patch serie because we make a major rewrite of this function.

The order doesn't matter for this patch. Necessary is that the patch
series will apply in the order 1 to 6. So you can apply this patch
before or afterwards the patch-series.

> Don't make me guess, instead place this patch into the necessary
> place within the series of 6 patches.
> 
I can understand that. This patches only fixes some address behaviour
which is in the current implementation wrong. But there are several
other problems with the current 6LoWPAN implementation.
I don't know which is the right place for these patches, because it is
a big change and the 6LoWPAN implementation is broken anyway.

> I'm not applying any of this until you can submit it properly.
> 
Ok, please let me know what should I do to submit it properly.

Thanks

Regards
Alex

Re: [PATCHv2 net-next] 6lowpan: handle only real link-local addresses

[David Miller]

From: Alexander Aring <alex.aring@gmail.com>
Date: Wed, 14 Aug 2013 23:17:01 +0200

> Ok, please let me know what should I do to submit it properly.

What you don't understand is that just because patches aren't
related doesn't mean that their order of application doesn't
matter.

So you put all the patches into a full series so that there is no
ambiguity as to what order the patches are to be applied.

Do not ever seperate patches when you are submitting changes to the
same exact files at one time.

Re: [PATCHv2 net-next] 6lowpan: handle only real link-local addresses

[Alexander Aring]

Hi David,

On Wed, Aug 14, 2013 at 05:18:25PM -0700, David Miller wrote:
> From: Alexander Aring <alex.aring@gmail.com>
> Date: Wed, 14 Aug 2013 23:17:01 +0200
> 
> > Ok, please let me know what should I do to submit it properly.
> 
> What you don't understand is that just because patches aren't
> related doesn't mean that their order of application doesn't
> matter.
> 
Ok. I got the information on an other mailinglist that I should
not mix "features" and "fixes" in a single series. Then I just
try to seperate...

My first patch-serie included all patches in a single series.

> So you put all the patches into a full series so that there is no
> ambiguity as to what order the patches are to be applied.
> 
Ok.

> Do not ever seperate patches when you are submitting changes to the
> same exact files at one time.
Ok, I will remember it.

Thanks and apologize for the inconvenience

Regards
Alex

Re: [PATCHv2 net-next] 6lowpan: handle only real link-local addresses

[David Miller]

From: Alexander Aring <alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Thu, 15 Aug 2013 06:57:21 +0200

> Hi David,
> 
> On Wed, Aug 14, 2013 at 05:18:25PM -0700, David Miller wrote:
>> From: Alexander Aring <alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Date: Wed, 14 Aug 2013 23:17:01 +0200
>> 
>> > Ok, please let me know what should I do to submit it properly.
>> 
>> What you don't understand is that just because patches aren't
>> related doesn't mean that their order of application doesn't
>> matter.
>> 
> Ok. I got the information on an other mailinglist that I should
> not mix "features" and "fixes" in a single series. Then I just
> try to seperate...

If it's a fix it's destined for the 'net' tree not 'net-next'.

And in that case you first submit only the bug fix, then you
politely ask me after it's been included to merge 'net' into
'net-next' so you can then rebase your net-next patches and
submit them seperately.

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk