Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Sep 2, 2015

Conversation

miri64
Copy link
Member

@miri64 miri64 commented May 22, 2015

This imports router behavior for NDP router discovery. This encompasses:

  • building and sending of (periodic) router advertisement (including all available options specified in RFC 4861)
  • handling of router solicitations and their options

Depends on:

@x3ro
Copy link
Contributor

x3ro commented May 26, 2015

Is #2770 really the PR this should be tested with? Asking because it is closed and seems to be superseded by #2776

@miri64
Copy link
Member Author

miri64 commented May 26, 2015

Yes of course, sorry about that.

@x3ro
Copy link
Contributor

x3ro commented May 26, 2015

I haven't managed to get this working yet, mainly because I don't know how the setup works, I suppose :s So I tried the following, as described by @kaspar030:

[snip]

  1. actually there's no need for bridging. I precreate the tap using openvpn:
    # sudo openvpn --mktun --dev tap0
    Then setup the linux side of it (e.g., using ip to give it an address)
  2. I start the resulting binary directly:
    # <path>/bin/native/driver_netdev_eth.elf tap0

To test pinging,

  1. give the RIOT side an ipv6 address using, e.g., ifconfig <n> add 2001:db8::1/64
  2. add the linux side to the neighbour cache: ```ncache add 2001:db8::2
  3. do the opposite on the Linux side, make sure the tap is UP
  4. ping from Linux side should succeed

Adding the interface to the RIOT side worked fine. But the "do the opposite on the Linux side and make sure the tap is up" part is not quite clear to me. What interface do I have add the IPv6 address to? (I've tried this command:)

sudo ip -6 address add 2001:db8::2/64 dev eth0

This will, however, result in ping6 not actually sending over the tap anything over the tap0 interface, which, due to the openvpn thingy, it should do if I've understood things correctly.

Can anyone help me on what I have to do on the linux side to get things working?

@miri64
Copy link
Member Author

miri64 commented May 26, 2015

@x3ro if you want to configure your linux device as router you need to (a) install radvd (router advertisement daemon) and (b) enable forwarding on the interface, so Linux (a) sends router advertisements and (b) acts as a router. I haven't looked into the documentation for that too much however, because I find it far to complicated for a simple PR-test set up. Just configure two virtual RIOT nodes as router and host and let them ping it out. ;-)

@Lotterleben
Copy link
Member

Mhm, but isn't a RIOT-RIOT test kind of... incomplete? Especially since we don't have a border router (yet!), an NDP implementation that doesn't play well with Linux could be a serious hindrance (it cost us many headaches with watr.li, at least ^^). Aaaaanyhow, I'm curious to see @x3ro s results. Fingers crossed! :)

@miri64
Copy link
Member Author

miri64 commented May 26, 2015

I'm not saying you should test it if you want to, for me it was just too much of a hassle. Linux responds to router advertisements, that was everything I tested :D

@miri64
Copy link
Member Author

miri64 commented May 26, 2015

Rebased to current master

@OlegHahm OlegHahm 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 26, 2015
@x3ro
Copy link
Contributor

x3ro commented May 26, 2015

Ah, I think I've misunderstood the scope of this PR. This does not respond to neighbour solicitations, right?

@miri64
Copy link
Member Author

miri64 commented May 26, 2015

Nope, that was #2910

@x3ro
Copy link
Contributor

x3ro commented May 26, 2015

Okay, but that one is merged and RIOT does not seem to be responding to the neighbour solicitations that Linux is sending to ff02::1:ff00:2. Do I have to enable some special for it to work?

@x3ro
Copy link
Contributor

x3ro commented May 26, 2015

Also, when starting radvd, RIOT seems to receive the router advertisements but does not display anything when I query the routers table in the shell

@miri64
Copy link
Member Author

miri64 commented May 26, 2015

When we tested #2910 it worked and I tested it here to. Let me test it again, when I come home. Now that ng_nativenet is merged, it'll be a breeze :D

@miri64
Copy link
Member Author

miri64 commented May 31, 2015

Rebased to current master and squashed fixes.

@OlegHahm
Copy link
Member

OlegHahm commented Sep 2, 2015

Node hangs at _send_rtr_adv().

@miri64
Copy link
Member Author

miri64 commented Sep 2, 2015

Might be a locked mutex?

@OlegHahm
Copy link
Member

OlegHahm commented Sep 2, 2015

Yepp.

@OlegHahm
Copy link
Member

OlegHahm commented Sep 2, 2015

Got an idea or should I continue to debug?

@miri64
Copy link
Member Author

miri64 commented Sep 2, 2015

No I'm just commenting. Go on.

@OlegHahm
Copy link
Member

OlegHahm commented Sep 2, 2015

For some reason https:/RIOT-OS/RIOT/pull/3049/files#diff-f9bf85cb337616822921e4230ede1572R257 is not reached when adding the additional address (though it seems to work for the first (the link local) address).

@OlegHahm
Copy link
Member

OlegHahm commented Sep 2, 2015

I guess it's because the router advertisement is sent from the same thread.

@miri64
Copy link
Member Author

miri64 commented Sep 2, 2015

Ah, of course. So we have to unlock it for a short moment there :/

@OlegHahm
Copy link
Member

OlegHahm commented Sep 2, 2015

Like

diff --git a/sys/net/gnrc/network_layer/ipv6/netif/gnrc_ipv6_netif.c b/sys/net/gnrc/network_layer/ipv6/netif/gnrc_ipv6_netif.c
index 319f626..bd4403c 100644
--- a/sys/net/gnrc/network_layer/ipv6/netif/gnrc_ipv6_netif.c
+++ b/sys/net/gnrc/network_layer/ipv6/netif/gnrc_ipv6_netif.c
@@ -108,7 +108,9 @@ static ipv6_addr_t *_add_addr_to_entry(gnrc_ipv6_netif_t *entry, const ipv6_addr
             if ((entry->flags & GNRC_IPV6_NETIF_FLAGS_ROUTER) &&
                 (entry->flags & GNRC_IPV6_NETIF_FLAGS_RTR_ADV)) {
                 entry->rtr_adv_count = GNRC_NDP_MAX_INIT_RTR_ADV_NUMOF;
+                mutex_unlock(&entry->mutex);
                 gnrc_ndp_router_retrans_rtr_adv(entry);
+                mutex_lock(&entry->mutex);
             }
 #endif
         }

?

@miri64
Copy link
Member Author

miri64 commented Sep 2, 2015

Tell me what you think about my fixup (which I pushed while seeing your comment)

@OlegHahm
Copy link
Member

OlegHahm commented Sep 2, 2015

You can squash immediately. Tested the diff successfully.

@miri64
Copy link
Member Author

miri64 commented Sep 2, 2015

Squashed.

@OlegHahm
Copy link
Member

OlegHahm commented Sep 2, 2015

Now RPL seems to be not working any more. (Cannot ping any more.)

@OlegHahm
Copy link
Member

OlegHahm commented Sep 2, 2015

Was maybe just a hickup. Seems to be working again.

@OlegHahm
Copy link
Member

OlegHahm commented Sep 2, 2015

ACK, merge as soon as Travis green like the sky on Qo'noS.

@miri64
Copy link
Member Author

miri64 commented Sep 2, 2015

Welp, the goal of this PR wasn't certainly to make it stable (ad lib @emmanuelsearch "Get it done on sunday, don't test it; just get it done") ;-P

@OlegHahm
Copy link
Member

OlegHahm commented Sep 2, 2015

Well, I would say that meant the additional features - the already working stuff shouldn't be broken.

@miri64
Copy link
Member Author

miri64 commented Sep 2, 2015

Yes of course, but when a new feature (router advertisements in this case) is destroying something, then already working stuff might fall victim, too.

@OlegHahm
Copy link
Member

OlegHahm commented Sep 2, 2015

Whatever, I can confirm that my usual quick'n'dirty tests on IoT-LAB with 5 nodes and RPL are working. That should be good enough for now.

@miri64
Copy link
Member Author

miri64 commented Sep 2, 2015

ACK, merge as soon as Travis green like the sky on Qo'noS.

You should have ACK'd with "MAJ!" then ;-P

@OlegHahm
Copy link
Member

OlegHahm commented Sep 2, 2015

/home/travis/build/RIOT-OS/RIOT/sys/net/gnrc/network_layer/ipv6/netif/gnrc_ipv6_netif.c:231:1: error: expected identifier or '(' before '{' token

 {

 ^

/home/travis/build/RIOT-OS/RIOT/sys/net/gnrc/network_layer/ipv6/netif/gnrc_ipv6_netif.c:238:1: error: expected identifier or '(' before '{' token

 {

 ^

@miri64
Copy link
Member Author

miri64 commented Sep 2, 2015

Already fixed :-)

miri64 added a commit that referenced this pull request Sep 2, 2015
gnrc_ndp_router: Initial import of router behavior of router discovery
@miri64 miri64 merged commit fc6896b into RIOT-OS:master Sep 2, 2015
@miri64 miri64 deleted the ng_ndp/feat/rtr-dscvry branch September 2, 2015 22:22
@miri64 miri64 added the Area: network Area: Networking label Sep 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants