ng_ndp: initial import of address resolution by miri64 · Pull Request #2910 · RIOT-OS/RIOT

miri64 · 2015-05-03T13:28:57Z

This imports address resolution and neighbor unreachable discovery for IPv6.

Depends on ~~#2908~~ (merged), ~~#2909~~ (merged) and ~~#2964~~ (merged).

miri64 · 2015-05-10T15:49:10Z

Rebased to current master and current dependencies

miri64 · 2015-05-10T23:35:36Z

Added #2964 as dependency

miri64 · 2015-05-11T01:48:13Z

New fixes.

miri64 · 2015-05-12T17:35:43Z

Rebased to current dependencies

miri64 · 2015-05-12T17:36:47Z

(no longer WIP, but still untested)

miri64 · 2015-05-13T00:18:02Z

Tested and fixed bugs I found. Please test (hope there are devices to test this with...).

miri64 · 2015-05-13T01:18:53Z

(hope there are devices to test this with...).

Problems I ran into so far with current devices:

xbee: don't have one (need two to test properly)
ng_at86rf2xx: ng_at86rf2xx: Source address of IPv6 packets get overwritten #2961
ng_nativenet: works like a charm, but Linux failed to answer (I think this is a configuration problem since I still can't get two ng_nativenet nodes get to communicate either)

The way I did it:

I merged sys: net: add netdev device driver using dev_eth low-level ethernet driver API #2776 and ipv6_netif: initialize device dependent values #2944 into this one (there will be merge-conflicts, I hope these get merged soon, so I can resolve them here 😉)
I added a call to ng_ipv6_netif_init_by_dev() just before the shell started in the main() of tests/driver_netdev_eth from sys: net: add netdev device driver using dev_eth low-level ethernet driver API #2776
I compiled tests/driver_netdev_eth from sys: net: add netdev device driver using dev_eth low-level ethernet driver API #2776 with USEMODULE += ng_icmpv6_echo
I startet the application and typed ping6 1 fe80::1
- with wireshark you should now see NG_NDP_MAX_MC_NBR_SOL_NUMOF (which is normally 3) Neighbor Solicitations to ff:02::1:ff00:1 (fe80::1's solicited nodes group) send in an interval of ~NG_NDP_RETRANS_TIMER ms (currently configured to be 1000000 via the RFC),

Great now I know at least, that the initial probing works, but how do I test the receiving side:

the answer is scapy: when I hit [Enter] on ping6 1 fe80::1 I rapidly change to an already started scapy session (needs to be started with root previleges) and hit the following command (which I prepared too of course, since it needs to be in reply of one of the Neighbor solicitations) to send a corresponding Neighbor Advertisement:

sendp(Ether(src="<the tap's MAC address>", dst="<RIOT's MAC>") / \
      IPv6(src="fe80::1", dst="RIOT's IPv6 address") / \
      ICMPv6ND_NA(tgt="fe80::1",O=0,R=0,S=1) / \
      ICMPv6NDOptDstLLAddr(lladdr="<the tap's MAC address>"), \
      iface="<the name of the tap>")

Both RIOT's MAC and IPv6 address can be determined with ifconfig.

There will be only Neighbor Solicitations as long as no Neighbor Advertisement was received, so don't be shocked if you see less than 3 NS ;-)
If the NA was received correctly you should see with ncache that the address fe80::1 is indeed REACHABLE.
If you wait 2.5 to 7.5 sec (the device chooses a random value in this range) the address should go STALE.
Sending another packet to fe80::1 will now trigger the address to enter DELAY state and to trigger PROBE state NG_NDP_FIRST_PROBE_DELAY seconds (5 seconds) afterwards. Check it with another ping6 1 fe80::1 and ncache (you might not be able to even see the DELAY state, since the ping6 application is waiting 5 seconds for a reply).
In PROBE state the node starts to send out Neighbor Solicitations again. This time they are addressed to fe80::1 and there will be NG_NDP_MAX_UC_NBR_SOL_NUMOF of them (which is normally 3 too).
If the Neighbor Solicitations stay unanswered the entry will be removed from the Neighbor cache (the node is to be determined unreachable). Check with ncache.

miri64 · 2015-05-13T01:26:40Z

If you want to test the receiving of Neighbor Solicitations you can do that with scapy, too:

srp(Ether(src="<the tap's MAC address>", dst="<RIOT's MAC>") / \
    IPv6(src="fe80::1", dst="ff02::1") / \
    ICMPv6ND_NS(tgt="fe80::1") / \
    ICMPv6NDOptSrcLLAddr(lladdr="<the tap's MAC address>"), \
    iface="<the name of the tap>")

Instead of ff02::1 you can also go more correctly and use the solicited-nodes address corresponding to RIOT's IPv6 address (for simplicity sake I just let it at all-nodes). Any legal multicast address the interface is joined to is accepted, the solicited-nodes address is only the preferred one.

You will observe a Neighbor Advertisement being replied. The neighbor cache entry is first set to STALE by the receiving of the Neighbor Solicitation then, due to sending the unicast Neighbor Advertisement to fe80::1 directly put to DELAY and later reaches the PROBE phase again, leading again to 3 unicast NS.

scapy however will show the following, since it got the expected NA:

Begin emission:
Finished to send 1 packets.
*
Received 1 packets, got 1 answers, remaining 0 packets
(<Results: TCP:0 UDP:0 ICMP:0 Other:1>, <Unanswered: TCP:0 UDP:0 ICMP:0 Other:0>)

OlegHahm · 2015-05-13T05:20:03Z

ng_nativenet: works like a charm, but Linux failed to answer (I think this is a configuration problem since I still can't get two ng_nativenet nodes get to communicate either)

What do you mean? Works like charm, but does neither work with Linux nor between two RIOT native instances?

miri64 · 2015-05-13T08:42:06Z

What do you mean? Works like charm, but does neither work with Linux nor between two RIOT native instances?

See my [edit: test] descriptions below ;-) As I said: I think it's more a configuring problem of the TAP, than the implementation, since the packets seem to be correct.

OlegHahm · 2015-05-13T08:46:21Z

It's seems to be a race: contiki-os/contiki#1063 (comment) 😉

miri64 · 2015-05-13T09:07:13Z

Unfair! They have a headstart :D

OlegHahm · 2015-05-16T19:39:12Z

sys/net/network_layer/ng_ndp/ng_ndp.c

This looks ugly, but I don't have a better idea for now. Maybe introduce an is_empty() function.

Just for this one use-case? Seems inefficient to me... Especially since the code would basically look the same, just with an extra && !ng_ipv6_nc_is_empty() in l320, if I see this correctly.

Well, yes... But you agree that this double check looks weird, right?

Yes, but is_empty() is not the semantic we look for here. The router list is more like a circular list which is costly compared to the current array-list implementation, so …

Yes, but if both calls return NULL the list is empty, right?

Yes… but this is not how this is to be used here: the first ng_ipv6_nc_get_next_router() tries to round-robin the routers. Problem: since the router list (a subset of the neighbor cache) is an array list, the function returns NULL for both cases that the list is empty or the list has reached its end. The second call than checks if the list was empty or not and additionally retrieves a router if it was not empty. This means, unless I am mistaken: checking if the list was empty additionally would generate extra overhead.

Yes, you're right, it would probably introduce more overhead.

OlegHahm · 2015-05-16T20:39:39Z

sys/net/network_layer/ng_ndp/ng_ndp.c

if (pkt != NULL) ... here, too.

OlegHahm · 2015-05-16T21:04:22Z

If comments are adressed: ACK

miri64 · 2015-05-16T21:10:01Z

Rebased to current master and squashed

miri64 · 2015-05-16T21:10:19Z

(and addressed rest of the comments)

miri64 · 2015-05-17T07:17:47Z

Just a little fix so the unittests would still compile

kaspar030 · 2015-05-17T07:22:49Z

For routing we need a working routing protocol and router discovery first ;-).

What happened to static routing? If I add 2001:db8::1/64 to an interface, there should be some static routing entry, and RIOT should use ndp to find the link local address. No need for a routing protocol or router discovery here?

miri64 · 2015-05-17T07:31:26Z

@kaspar030 if you compile with the FIB and you add an entry to the FIB it will be able to route to this entry, so yes: static routing is possible.

ng_ndp: initial import of address resolution

PeterKietzmann · 2015-05-20T09:46:23Z

sys/net/network_layer/ng_ndp/ng_ndp.c

addr_str is not defined, i.e. array is missing

2nd argument needs to be a pointer. Here and some lines below

miri64 assigned OlegHahm May 3, 2015

miri64 added this to the Network Stack Task Force milestone May 3, 2015

miri64 force-pushed the ng_ndp/feat/addr-res branch 2 times, most recently from 78a52fb to 4f54b07 Compare May 10, 2015 15:46

miri64 mentioned this pull request May 10, 2015

[WIP] ng_ndp: Initial import of router discovery #2612

Closed

miri64 force-pushed the ng_ndp/feat/addr-res branch from 4f54b07 to 4759e0e Compare May 10, 2015 17:46

miri64 mentioned this pull request May 10, 2015

Network stack refactoring: TODO list #2278

Closed

36 tasks

miri64 force-pushed the ng_ndp/feat/addr-res branch from 4759e0e to dbf9a24 Compare May 10, 2015 23:34

miri64 force-pushed the ng_ndp/feat/addr-res branch from dbf9a24 to 15fb07e Compare May 11, 2015 01:48

miri64 force-pushed the ng_ndp/feat/addr-res branch 3 times, most recently from 0b6af41 to 56c3d09 Compare May 12, 2015 17:34

miri64 removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label May 12, 2015

miri64 force-pushed the ng_ndp/feat/addr-res branch from 31bc6bf to afa5c7a Compare May 13, 2015 00:52

OlegHahm reviewed May 16, 2015
View reviewed changes

miri64 added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label May 16, 2015

OlegHahm reviewed May 16, 2015
View reviewed changes

sys/net/network_layer/ng_ndp/ng_ndp.c Outdated

Copy link
Copy Markdown

Member

OlegHahm May 16, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (pkt != NULL) ... here, too.

miri64 force-pushed the ng_ndp/feat/addr-res branch from 9f583e5 to 6df82b9 Compare May 16, 2015 21:09

OlegHahm removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label May 16, 2015

miri64 added 2 commits May 17, 2015 09:15

ng_ndp: initial import of the address resolution

ee11a2a

shell: add examplery ng_ipv6_nc_still_reachable()

db33a10

miri64 force-pushed the ng_ndp/feat/addr-res branch from 6df82b9 to db33a10 Compare May 17, 2015 07:15

miri64 added a commit that referenced this pull request May 17, 2015

Merge pull request #2910 from authmillenon/ng_ndp/feat/addr-res

4f1d7ad

ng_ndp: initial import of address resolution

miri64 merged commit 4f1d7ad into RIOT-OS:master May 17, 2015

miri64 deleted the ng_ndp/feat/addr-res branch May 17, 2015 08:45

PeterKietzmann reviewed May 20, 2015
View reviewed changes

This was referenced May 23, 2015

NDP: implement missing functionality #3052

Open

gnrc_ndp_router: Initial import of router behavior of router discovery #3049

Merged

miri64 added the Area: network Area: Networking label Sep 30, 2018

Conversation

miri64 commented May 3, 2015

Uh oh!

miri64 commented May 10, 2015

Uh oh!

miri64 commented May 10, 2015

Uh oh!

miri64 commented May 11, 2015

Uh oh!

miri64 commented May 12, 2015

Uh oh!

miri64 commented May 12, 2015

Uh oh!

miri64 commented May 13, 2015

Uh oh!

miri64 commented May 13, 2015

Uh oh!

miri64 commented May 13, 2015

Uh oh!

OlegHahm commented May 13, 2015

Uh oh!

miri64 commented May 13, 2015

Uh oh!

OlegHahm commented May 13, 2015

Uh oh!

miri64 commented May 13, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OlegHahm commented May 16, 2015

Uh oh!

miri64 commented May 16, 2015

Uh oh!

miri64 commented May 16, 2015

Uh oh!

miri64 commented May 17, 2015

Uh oh!

kaspar030 commented May 17, 2015

Uh oh!

miri64 commented May 17, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants