gnrc_ndp: fixed ND Option handling for 6LoWPAN #4446

Yonezawa-T2 · 2015-12-09T10:05:39Z

The form of Source/Target Link-layer Address option of Neighbor Discovery protocol is defined by RFC 4944 Section 8 for 6LoWPAN. Link-layer addresses may be used by 6LoWPAN to restore IPv6 addresses, so that wrong handling may results in check sum error of ICMPv6.

Tested on OS X with modifications of #4443, #4444, and #4445.

miri64 · 2015-12-09T11:33:38Z

sys/net/gnrc/network_layer/ndp/internal/gnrc_ndp_internal.c

+    if (ipv6_iface->flags & GNRC_IPV6_NETIF_FLAGS_SIXLOWPAN) {
+        // https://tools.ietf.org/html/rfc4944#section-8
+        if (sl2a_opt->len == 1) {
+            sl2a_len = 2;


Since we have also 1-bit devices in mind to use with 6Lo, this might not be true

1-bit addresses?

1-byte addresses ^^ sorry.

miri64 · 2015-12-09T11:40:37Z

After reviewing the problem: It runs a little bit deeper. Basically what I assumed (and was trying to communicate to the driver developers at that time) is that the driver always takes addresses that are shorter than the required ones, and pads them with 0 at the end. Sadly it seems that this never was included into any of the link-layer implementations. The problem with the current solution is, that it expects the device either to be 24-bit MAC address compatible or an IEEE 802.15.4 device (16-bit and 64-bit addresses). Since 6Lo by now is target to devices that support different addresses (and the cc11xx [8-bit addresses] was historically the device we implemented 6Lo over first), this leaves all this devices in an uncertain (probably unsupported) state. Do we have a solution for that (apart from the already proposed less strict address acceptance by drivers)? @haukepetersen @kaspar030 @OlegHahm @thomaseichinger @jfischer-phytec-iot @jremmert-phytec-iot

miri64 · 2015-12-14T14:57:43Z

If configured correctly NETOPT_ADDR_LEN should give you the same values and would be much more general than the proposed solution.

Yonezawa-T2 · 2015-12-15T01:44:01Z

@authmillenon What if the sender has 64-bit address and the receiver has 16-bit address? XBee (or other IEEE 802.15.4 devices) allows this.

Yonezawa-T2 · 2015-12-15T02:08:53Z

Interpretation of Source/Target Link-layer Address option is delegated for each standards of IPv6-over-X (https://tools.ietf.org/html/rfc4861#section-4.6.1). 48 bits for IPv6 over Ethernet (https://tools.ietf.org/html/rfc2464#section-6), 16 or 64 bits depending on the length parameter for IPv6 over IEEE 802.15.4 (https://tools.ietf.org/html/rfc4944#section-8). We have to check the type of the link-layer to determine its address length. If GNRC_IPV6_NETIF_FLAGS_SIXLOWPAN flag is insufficient, we need to add GNRC_IPV6_NETIF_FLAGS_CC11XX flag and GNRC_IPV6_NETIF_FLAGS_IEEE_802_15_4 flag.

miri64 · 2015-12-15T09:52:55Z

Maybe it would suffice to have a default L2 address length member to the interface (1 byte should be enough for this, we can still squeeze this in somewhere). Since IEEE 802.15.4 is the only format I know of we can have the following exception: if default L2 address length == 2 and sl2a_opt->len == 2 then set address length to 8.

Yonezawa-T2 · 2015-12-15T10:47:48Z

Updated to use sl2a_opt->len iff src_l2addr_len is 2 or 8.

miri64 · 2015-12-15T10:55:40Z

sys/net/gnrc/network_layer/sixlowpan/nd/gnrc_sixlowpan_nd.c

+                     * regardless of link-layer address.
+                     * https://tools.ietf.org/html/rfc6775#section-4.1 */
+                    if ((nc_entry = gnrc_ipv6_nc_add(iface, &ipv6->src,
+                                                     &ar_opt->eui64, sizeof(eui64_t),


Errm this entry will be used for address translation, too (so with this patch short addresses won't be used as destination addresses). The EUI-64 to identify the node in address registration is set in https://github.com/RIOT-OS/RIOT/pull/4446/files#diff-9b0aedc5fd4543061c862e245fe43c75R359 and checked in https://github.com/RIOT-OS/RIOT/pull/4446/files#diff-9b0aedc5fd4543061c862e245fe43c75R337

miri64 · 2015-12-15T11:05:15Z

Apart from the confusion about the hardware address (used for address resolution) and the EUI-64 (used for neighbor identification in address registration) I like this solution much better (even better then the one proposed by myself :-))

OlegHahm · 2015-12-15T15:23:28Z

sys/net/gnrc/network_layer/ndp/internal/gnrc_ndp_internal.c

+            sl2a_len = 2;
+        } else if (sl2a_opt->len == 2) {
+            sl2a_len = 8;
+        } else {


This needs some style fixes (/* .. */ instead of // and newline before else).

Yonezawa-T2 · 2015-12-16T02:40:34Z

Updated. I will do git rebase -i to squash those changes when it is ready to merge.

miri64 · 2016-01-04T15:32:51Z

sys/net/gnrc/network_layer/sixlowpan/nd/gnrc_sixlowpan_nd.c

@@ -347,7 +347,8 @@ uint8_t gnrc_sixlowpan_nd_opt_ar_handle(kernel_pid_t iface, ipv6_hdr_t *ipv6,
                /* TODO: multihop DAD behavior */
                uint16_t reg_ltime;
                if (nc_entry == NULL) {
-                    if ((nc_entry = gnrc_ipv6_nc_add(iface, &ipv6->src, sl2a, sl2a_len,
+                    if ((nc_entry = gnrc_ipv6_nc_add(iface, &ipv6->src,
+                                                     sl2a, sl2a_len,


What's this change about?

miri64 · 2016-01-04T15:34:08Z

Apart from unnecessary formatting changes, this PR is ready to be merged

cgundogan · 2016-01-04T15:36:27Z

@Yonezawa-T2 could you also provide a backport to the release branch after addressing the minor comments of @authmillenon ?

cgundogan · 2016-01-05T10:56:23Z

@Yonezawa-T2 thanks again for the work that you contributed here. We want to create a new release candidate in the next few (2-3) hours and having your fixes in this release would be most beneficial. If you think that you can address the comments of @authmillenon within a short period of time then we will gladly wait a few hours longer in order to get these fixes merged and backported.

The forms of the Source/Target Link-layer Address option for 6LoWPAN are defined in RFC 4944 Section 8: https://tools.ietf.org/html/rfc4944#section-8 The address is 16 bit if length is 1, 64 bit if length is 2.

Yonezawa-T2 · 2016-01-06T00:43:02Z

Sorry for my late response. I was offline.

Reverted unnecessary style changes, then squashed all commits.

miri64 · 2016-01-06T00:45:37Z

ACK and go when Travis is happy

miri64 · 2016-01-06T00:46:58Z

(please provide a backport to the 2015.12-branch)

Yonezawa-T2 · 2016-01-06T01:09:01Z

Created #4581 for 2015.12-branch

…or_6lowpan gnrc_ndp: fixed ND Option handling for 6LoWPAN

miri64 · 2016-01-06T13:16:03Z

Thanks for your help and patience. :-)

OlegHahm added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking NSTF labels Dec 9, 2015

OlegHahm added this to the Release 2015.12 milestone Dec 9, 2015

OlegHahm assigned miri64 Dec 9, 2015

OlegHahm added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Dec 9, 2015

This was referenced Dec 9, 2015

fixes several bugs on GNRC network stack #4447

Merged

examples: add example of border router with XBee on native platform. #4448

Closed

miri64 reviewed Dec 9, 2015
View reviewed changes

miri64 added the Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. label Dec 9, 2015

Yonezawa-T2 mentioned this pull request Dec 10, 2015

xbee: fixed several bugs #4445

Merged

miri64 reviewed Dec 15, 2015
View reviewed changes

OlegHahm reviewed Dec 15, 2015
View reviewed changes

miri64 reviewed Jan 4, 2016
View reviewed changes

gnrc_ndp: fixed ND Option handling for 6LoWPAN

639f7dc

The forms of the Source/Target Link-layer Address option for 6LoWPAN are defined in RFC 4944 Section 8: https://tools.ietf.org/html/rfc4944#section-8 The address is 16 bit if length is 1, 64 bit if length is 2.

Yonezawa-T2 force-pushed the neighbor_discovery_option_for_6lowpan branch from d6cba96 to 639f7dc Compare January 6, 2016 00:39

miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 6, 2016

Yonezawa-T2 mentioned this pull request Jan 6, 2016

gnrc_ndp: fixed ND Option handling for 6LoWPAN #4581

Merged

miri64 removed the Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. label Jan 6, 2016

cgundogan removed the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Jan 6, 2016

miri64 added a commit that referenced this pull request Jan 6, 2016

Merge pull request #4446 from Yonezawa-T2/neighbor_discovery_option_f…

a4a8e83

…or_6lowpan gnrc_ndp: fixed ND Option handling for 6LoWPAN

miri64 merged commit a4a8e83 into RIOT-OS:master Jan 6, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gnrc_ndp: fixed ND Option handling for 6LoWPAN #4446

gnrc_ndp: fixed ND Option handling for 6LoWPAN #4446

Yonezawa-T2 commented Dec 9, 2015

miri64 Dec 9, 2015

OlegHahm Dec 9, 2015

miri64 Dec 9, 2015

miri64 commented Dec 9, 2015

miri64 commented Dec 14, 2015

Yonezawa-T2 commented Dec 15, 2015

Yonezawa-T2 commented Dec 15, 2015

miri64 commented Dec 15, 2015

Yonezawa-T2 commented Dec 15, 2015

miri64 Dec 15, 2015

miri64 commented Dec 15, 2015

OlegHahm Dec 15, 2015

Yonezawa-T2 commented Dec 16, 2015

miri64 Jan 4, 2016

miri64 commented Jan 4, 2016

cgundogan commented Jan 4, 2016

cgundogan commented Jan 5, 2016

Yonezawa-T2 commented Jan 6, 2016

miri64 commented Jan 6, 2016

miri64 commented Jan 6, 2016

Yonezawa-T2 commented Jan 6, 2016

miri64 commented Jan 6, 2016

gnrc_ndp: fixed ND Option handling for 6LoWPAN #4446

gnrc_ndp: fixed ND Option handling for 6LoWPAN #4446

Conversation

Yonezawa-T2 commented Dec 9, 2015

miri64 Dec 9, 2015

Choose a reason for hiding this comment

OlegHahm Dec 9, 2015

Choose a reason for hiding this comment

miri64 Dec 9, 2015

Choose a reason for hiding this comment

miri64 commented Dec 9, 2015

miri64 commented Dec 14, 2015

Yonezawa-T2 commented Dec 15, 2015

Yonezawa-T2 commented Dec 15, 2015

miri64 commented Dec 15, 2015

Yonezawa-T2 commented Dec 15, 2015

miri64 Dec 15, 2015

Choose a reason for hiding this comment

miri64 commented Dec 15, 2015

OlegHahm Dec 15, 2015

Choose a reason for hiding this comment

Yonezawa-T2 commented Dec 16, 2015

miri64 Jan 4, 2016

Choose a reason for hiding this comment

miri64 commented Jan 4, 2016

cgundogan commented Jan 4, 2016

cgundogan commented Jan 5, 2016

Yonezawa-T2 commented Jan 6, 2016

miri64 commented Jan 6, 2016

miri64 commented Jan 6, 2016

Yonezawa-T2 commented Jan 6, 2016

miri64 commented Jan 6, 2016