Thread (17 messages) 17 messages, 2 authors, 2017-02-15

Re: [PATCH 5/5] 6lowpan: Use netdev addr_len to determine lladdr len

From: Alexander Aring <hidden>
Date: 2017-02-15 10:21:01

Hi,

On 02/15/2017 09:24 AM, Luiz Augusto von Dentz wrote:
Hi Alex,

On Wed, Feb 15, 2017 at 9:44 AM, Alexander Aring [off-list ref] wrote:
quoted
Hi,

On 02/09/2017 03:55 PM, Luiz Augusto von Dentz wrote:
quoted
From: Luiz Augusto von Dentz <redacted>

This allow technologies such as Bluetooth to use its native lladdr which
is eui48 instead of eui64 which was expected by functions like
lowpan_header_decompress and lowpan_header_compress.

Signed-off-by: Luiz Augusto von Dentz <redacted>
---
 net/6lowpan/iphc.c      | 17 +++++++++++++++--
 net/bluetooth/6lowpan.c | 43 ++++++++++---------------------------------
 2 files changed, 25 insertions(+), 35 deletions(-)
diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index fb5f6fa..ee88feb 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -827,8 +827,21 @@ static u8 lowpan_compress_ctx_addr(u8 **hc_ptr, const struct net_device *dev,
              }
              break;
      default:
-             /* check for SAM/DAM = 11 */
-             memcpy(&tmp.s6_addr[8], lladdr, EUI64_ADDR_LEN);
+             switch (dev->addr_len) {
+             case ETH_ALEN:
+                     memcpy(&tmp.s6_addr[8], lladdr, 3);
+                     tmp.s6_addr[11] = 0xFF;
+                     tmp.s6_addr[12] = 0xFE;
+                     memcpy(&tmp.s6_addr[13], lladdr + 3, 3);
+                     break;
+             case EUI64_ADDR_LEN:
+                     memcpy(&tmp.s6_addr[8], lladdr, EUI64_ADDR_LEN);
+                     break;
+             default:
+                     dam = LOWPAN_IPHC_DAM_11;
+                     goto out;
+             }
+
              /* second bit-flip (Universe/Local) is done according RFC2464 */
              tmp.s6_addr[8] ^= 0x02;
move this handling in per link-layer layer decision, see below.
quoted
              /* context information are always used */
PLEASE... and this is one of my rant!

PLEASE LOOK WHAT I HAVE DONE IN THIS FILE IN MY RFC PATCH SERIES YOU
NEED TO FIX MORE THAN JUST CONTEXT BASED COMPRESSION.

e.g. stateless un/compression
I haven't changed that and probably it needs a separate patch in that
case since Im only, and I really mean only, fixing the address length
and because your patches end up changing a lot more things it is
pretty hard to extract the exact parts that fixes this.
Yes, then please check everything which makes use of L3 address handling in
combination with the L2 address. This will be simple broken if you
remove the FF:FE pattern which was wrong before.

In my patch-series I suppose I hit all parts and also testing it.
quoted
Now I for this, I want to see a link-layer evaluation not address length based.
Please do so also for the other patch which fixing some behaviour in ipv6.
The reason is here that uncompress/compress addresses are defined by RFC
6LoWPAN adapation.

There is already a link-layer case, so please add BTLE LLTYPE there! Not
move your handling in default, add some WARN_ONCE there...
Here I have to pretty blunt, it is a mistake to put link layer logic
inside the 6lowpan module, first it creates a dependencies of these
technologies and second it slow down the development because we end up
with multiple trees having to synchronize which each other.
This is only because the bluetooth 6LoWPAN implementation was wrong at
the initial commit.

Synchronize each other? No, your changes doesn't require any 802.15.4
changes. You just need to fix it right and doing it not more broken than
before... which this patch does by fixing only the half of un/compress
addresses. Then some cases simple doesn't work. It's even worse, you
will grab 8 bytes from a pointer which is 6 bytes now.

So now, to you link-layer logic... This is how 6LoWPAN works,
compression/uncompression will reconstruct L3 addresses from information
from L2 header...

What we could do is to provide one callback to make a SLAAC address to a
given prefix, then call it in SLAAC generation (ipv6) or here in IPHC.
I think this is one of my 1001 cleanups for 6LoWPAN which I have in my
mind. Remove the switch-case stuff and make callback to provide this
handling which depends on L2 6LOWPAN implementation.

So we have no link-layer handling there anymore, but it's moved to a
callback which contains the 6LoWPAN BTLE module.
Also I have not seen a single mention of link local IP that is not
using 6 or 8 bytes link layer format, and in fact if there exist one
then we should probably have the caller handle the link local ip
format, but then again I haven't seen any indication that these needs
changing in fact for 15.4 16 bit address you can generate a valid 8
bytes address before calling 6lowpan API, which is probably what we
should do to avoid depending on 15.4 code inside 6lowpan.
yes there is _currently_ not other 6LoWPAN implementation which has 8 or
6 bytes.

What do you think about a callback? Then these 2 byte 802.15.4 address
magic will be handled by net/ieee802154/6lowpan/core.c and we also can
call the callback in IPv6 code to make SLAAC address.

I think this is the right way to do it.

- Alex
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help