6lowpan: handle only real link-local addresses

PATCH In review Alexander Aring 5 participants 9 messages Aug 14, 2013 – Aug 15, 2013 1 message not in the archive

Subsystems (3)

Networking Network drivers IEEE 802.15.4

lore

Share & tools

lore

Alexander Aring

author

2013-08-14 11:01 UTC #0 lore raw

PATCH v2 1/1 6lowpan: handle only real link-local addresses

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 <[email protected]>

Reviewed-by Werner Almesberger <[email protected]>

---
v2:
- Add "net-next" tag

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

net/ieee802154/6lowpan.h +5 -2

index 4b8f917..61f0ce9 100644
@@ -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,

Signature · 9 lines

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

Hannes Frederic Sowa

2013-08-14 11:08 UTC #1 lore raw

Quoted 6 lines · Alexander Aring

net/ieee802154/6lowpan.h +0 -0

index 4b8f917..61f0ce9 100644
@@ -83,8 +83,11 @@
 #define ipaddr_prefixcmp(addr1, addr2, length) \

net/ieee802154/6lowpan.h line 91

Quoted 10 lines

net/ieee802154/6lowpan.h +5 -2

 	(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

Alexander Aring

maintainer

2013-08-14 11:25 UTC #2 lore raw

Hi Hannes,

Quoted 9 lines · Hannes Frederic Sowa

Quoted 8 lines · Alexander Aring

net/ieee802154/6lowpan.h +0 -0

index 4b8f917..61f0ce9 100644
@@ -83,8 +83,11 @@
 #define ipaddr_prefixcmp(addr1, addr2, length) \
 	(memcmp(addr1, addr2, length >> 3) == 0)

Quoted 11 lines

net/ieee802154/6lowpan.h line 91

Quoted 8 lines

net/ieee802154/6lowpan.h +5 -2

-/* 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

Hannes Frederic Sowa

2013-08-14 11:38 UTC #3 lore raw

Quoted 23 lines · Alexander Aring

Hi Hannes,
Quoted 9 lines · Hannes Frederic Sowa
Quoted 8 lines · Alexander Aring
net/ieee802154/6lowpan.h +0 -0
index 4b8f917..61f0ce9 100644
@@ -83,8 +83,11 @@
 #define ipaddr_prefixcmp(addr1, addr2, length) \
 	(memcmp(addr1, addr2, length >> 3) == 0)
 
Quoted 11 lines
net/ieee802154/6lowpan.h line 91
Quoted 8 lines
net/ieee802154/6lowpan.h +5 -2
-/* 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?

Quoted 8 lines

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

David Miller

maintainer

2013-08-14 19:53 UTC #4 lore raw

In reply to Alexander Aring

From: Alexander Aring <[email protected]>
Date: Wed, 14 Aug 2013 13:01:07 +0200

Quoted 8 lines

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 <[email protected]>
Reviewed-by Werner Almesberger <[email protected]>

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.

Alexander Aring

maintainer

2013-08-14 21:13 UTC #5 lore raw

Hi David,

Quoted 7 lines · David Miller

From: Alexander Aring <[email protected]>
Date: Wed, 14 Aug 2013 13:01:07 +0200
Quoted 4 lines
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.

Quoted 7 lines

Quoted 4 lines
Also fix a comment issue "local link" -> "link-local"
Signed-off-by Alexander Aring <[email protected]>
Reviewed-by Werner Almesberger <[email protected]>
You still haven't addressed my biggest concern.

Sorry, I thought that was the missing "net-next" tag in the subject.

Quoted 2 lines

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.

Quoted 3 lines

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.

Quoted 2 lines

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

David Miller

maintainer

2013-08-15 00:18 UTC #6 lore raw

From: Alexander Aring <[email protected]>
Date: Wed, 14 Aug 2013 23:17:01 +0200

Quoted 1 line

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.

Alexander Aring

maintainer

2013-08-15 04:53 UTC #7 lore raw

Hi David,

Quoted 9 lines · David Miller

From: Alexander Aring <[email protected]>
Date: Wed, 14 Aug 2013 23:17:01 +0200
Quoted 1 line
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.

Quoted 3 lines

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.

Quoted 2 lines

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

A message that is not in the archive (sent off-list or not yet received): 20130815045701.GA3110-P4zwP3w+81Pu/[email protected]

David Miller

2013-08-15 07:38 UTC #9 lore raw

From: Alexander Aring <[email protected]>
Date: Thu, 15 Aug 2013 06:57:21 +0200

Quoted 8 lines

Hi David,
Quoted 5 lines · David Miller
From: Alexander Aring <[email protected]>
Date: Wed, 14 Aug 2013 23:17:01 +0200
Quoted 1 line
Ok, please let me know what should I do to submit it properly.

Quoted 7 lines

Quoted 4 lines
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