Skip to content

gnrc_sixlowpan_nd: initial import of host behavior of 6LoWPAN-ND#3748

Merged
miri64 merged 2 commits intoRIOT-OS:masterfrom
miri64:gnrc_sixlowpan_nd/feat/initial
Sep 9, 2015
Merged

gnrc_sixlowpan_nd: initial import of host behavior of 6LoWPAN-ND#3748
miri64 merged 2 commits intoRIOT-OS:masterfrom
miri64:gnrc_sixlowpan_nd/feat/initial

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Aug 31, 2015

This imports host behavior for 6LoWPAN-ND and adapts GNRC accordingly

Depends on:

@miri64 miri64 added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT State: waiting for other PR State: The PR requires another PR to be merged first NSTF labels Aug 31, 2015
@miri64 miri64 added this to the Release 2015.08 milestone Aug 31, 2015
@miri64 miri64 force-pushed the gnrc_sixlowpan_nd/feat/initial branch from b53e88b to e4751a8 Compare August 31, 2015 23:22
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Aug 31, 2015

Rebased to current master and current dependencies

@miri64 miri64 force-pushed the gnrc_sixlowpan_nd/feat/initial branch from e4751a8 to 43a8189 Compare September 2, 2015 12:22
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 2, 2015

Rebased to current master and dependencies

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 2, 2015

Rebased to current master and dependencies

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Propose a bit of code and I fix it. Every solution I come up with seems to complicated to me to just keep inexperienced users from wasting memory (because this is what you essentially are proposing, right?).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right. Will propose something - later.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Also, technically speaking, this already describes a non-border-router case. We only got 1 interface, that is not a 6LoWPAN interface, if we have one interface more we just include it (see below). The else case for >1 interface could just be a little more sophisticated (maybe check if dev_eth or gnrc_slip are included… I don't know…)

@miri64 miri64 force-pushed the gnrc_sixlowpan_nd/feat/initial branch from a31ab94 to 096fafb Compare September 2, 2015 20:12
@miri64 miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Sep 2, 2015
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 2, 2015

Rebased to current master. No longer dependent on any PRs.

@miri64 miri64 force-pushed the gnrc_sixlowpan_nd/feat/initial branch from 161de70 to e7487df Compare September 2, 2015 22:30
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 2, 2015

Rebased to current master and squashed already.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just checking: the snip is released elsewhere, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Japp, in gnrc_ipv6_demux(), when there are no more receivers for the packet.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 3, 2015

Addressed comments

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Sep 3, 2015

Please squash and kick Travis, will test now.

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 3, 2015
@miri64 miri64 force-pushed the gnrc_sixlowpan_nd/feat/initial branch from 5f567cf to f3b45b2 Compare September 3, 2015 08:06
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 3, 2015

Done

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

a parameter (0?) is missing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Sep 7, 2015

For some reason neighbor cache doesn't get filled correctly with RPL. In master after some time:

2015-09-07 23:01:47,599 - INFO # ncache
2015-09-07 23:01:47,602 - INFO # IPv6 address                    if  L2 address                state       type
2015-09-07 23:01:47,604 - INFO # ------------------------------------------------------------------------------
2015-09-07 23:01:47,607 - INFO # fe80::3432:4833:46da:9e72        7  36:32:48:33:46:da:9e:72   STALE       -
2015-09-07 23:01:47,609 - INFO # fe80::1                          7  36:32:48:33:46:da:9e:72   REACHABLE   -

with this PR:

2015-09-07 23:03:17,223 - INFO # ncache
2015-09-07 23:03:17,225 - INFO # IPv6 address                    if  L2 address                state       type
2015-09-07 23:03:17,228 - INFO # ------------------------------------------------------------------------------
2015-09-07 23:03:17,230 - INFO # fe80::3432:4833:46da:9e72        7  36:32:48:33:46:da:9e:72   STALE       -

Trying to figure out what's going wrong.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 7, 2015

For link-local addresses the neighbor cache won't be filled. See https://tools.ietf.org/html/rfc6775#section-5.6

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Sep 7, 2015

Then master and this PR are broken.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 7, 2015

This PR is introducing 6LoWPAN-ND, so master does not know about RFC6775. Will investigate the brokenness of this PR as I get ping timeouts with

diff --git a/examples/gnrc_networking/Makefile b/examples/gnrc_networking/Makefile
index 3347e0c..0bd2560 100644
--- a/examples/gnrc_networking/Makefile
+++ b/examples/gnrc_networking/Makefile
@@ -20,10 +20,8 @@ BOARD_BLACKLIST        := arduino-mega2560 spark-core
 USEMODULE += gnrc_netif_default
 USEMODULE += auto_init_gnrc_netif
 # Specify the mandatory networking modules for IPv6 and UDP
-USEMODULE += gnrc_ipv6_router_default
+USEMODULE += gnrc_ipv6_default
 USEMODULE += gnrc_udp
-# Add a routing protocol
-USEMODULE += gnrc_rpl
 # This application dumps received packets to STDIO using the pktdump module
 USEMODULE += gnrc_pktdump
 # Additional networking modules that can be dropped if not needed

too

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 7, 2015

Double whammy: I checked for dst instead of next_hop in the address resolution and the top-level function overwrote the result of the next-hop-determination+address-resolution because of a CPP-fuck-up. Now it is working for 6LoWPAN-host behavior with link-local addresses: ping6 has no packet-loss and neighbor cache stays empty.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 7, 2015

On your tests: fe80::1 is not a legal link-local address in 6LoWPANs (at least for hosts [edit]without routers[/edit], again: see https://tools.ietf.org/html/rfc6775#section-5.6) so it still won't work, at least without #3749. I'm not really in favor of fixing this for this PR as this is out of scope of RFC 6775 and would be reverted for #3749. I think seperating #3748 and #3749 was a bad idea and they should be merged in unison.

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Sep 7, 2015

It is working.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 7, 2015

Maybe RPL is doing something on its own ;-)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How do we check that these are all 802.15.4 interfaces? But I'm not in favor to check this at runtime anyway.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well… you kind of only land at this point if you are a 6LoWPAN host (meaning that this one interface is a 6Lo interface [no guarantee for being a 802.15.4 device though, but I'm not even sure how other 6Lo-L2s handle this case]).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The node still may have several other interfaces.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Assumption: a host has only one interface (otherwise it would be a router IMHO). We only come to this point, if the node is a 6LoWPAN node and if it is a host, ergo at this point we can assume to only have one 6Lo interface.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wouldn't agree on the assumption that hosts are only allowed to have 1 interface. IMO it should be possible for a node to participate in the network as a host and be able to communicate e.g. by using one 802.15 transceiver and a BLE transceiver

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, but that has nothing to do with ND, because ND is for IP. I now take the first 6LoWPAN interface if there is more than one. Is there is only one, I assume it to be a 6LoWPAN interface because otherwise we wouldn't be in this function. Please check the updated code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would elide the ifdef. If there's only one interface and it's not a 6LoWPAN interface, something is very broken anyway.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, and that's why I assume that I wouldn't even get there.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Then it's just a superfluous ifdef.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@miri64 miri64 force-pushed the gnrc_sixlowpan_nd/feat/initial branch from 4208350 to 813eaff Compare September 8, 2015 12:06
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

From #3748 (comment) (in outdated diff)

Then it's just a superfluous ifdef.

Just return the only interface that there is vs. for-loop + getting of the IPv6 interface + checking if that interface is a 6LoWPAN interface. How is that superfluous, when it prevents a lot of "useless" checking?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you turn it into a real if() {} else{} block?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO there's a lot of code in gnrc that can be tremendously simplified (and even improved) if we add this #if (GNRC_NETIF_NUMOF == 1) at several places on the cost of readability. For now, I would prefer to go without these preprocessor ifs and leave optimization for the single-interface case for later.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed "superfluous" CPP if, but used input iface in case it was set (e.g. by FIB)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Can you turn it into a real if() {} else{} block?

What good would that be for?

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Sep 8, 2015

Please squash

@miri64 miri64 force-pushed the gnrc_sixlowpan_nd/feat/initial branch from 5831caa to b8f27ee Compare September 8, 2015 18:37
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 8, 2015

Squashed

@OlegHahm OlegHahm added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Sep 9, 2015
@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Sep 9, 2015

ACK

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Sep 9, 2015

Btw this PR adds about 1.7k ROM for iotlab-m3 and about 2k ROM for samr21 with gnrc_networking:

text    data    bss dec BOARD/BINDIRBASE
16  0   0   16  arduino-due
59444   204 21112   80760   master-bin
59460   204 21112   80776   6lowpan-nd-host-bin

32  0   0   32  avsextrem
141080  2308    95989   239377  master-bin
141112  2308    95989   239409  6lowpan-nd-host-bin

16  0   0   16  cc2538dk
58992   200 21032   80224   master-bin
59008   200 21032   80240   6lowpan-nd-host-bin

16  0   0   16  ek-lm4f120xl
58900   200 21008   80108   master-bin
58916   200 21008   80124   6lowpan-nd-host-bin

16  0   0   16  f4vi1
60432   204 21048   81684   master-bin
60448   204 21048   81700   6lowpan-nd-host-bin

16  0   0   16  fox
61096   200 21176   82472   master-bin
61112   200 21176   82488   6lowpan-nd-host-bin

16  0   0   16  frdm-k64f
58868   1228    23168   83264   master-bin
58884   1228    23168   83280   6lowpan-nd-host-bin

1728    0   288 2016    iotlab-m3
73592   200 24152   97944   master-bin
75320   200 24440   99960   6lowpan-nd-host-bin

16  0   0   16  limifrog-v1
60368   200 21056   81624   master-bin
60384   200 21056   81640   6lowpan-nd-host-bin

16  0   0   16  mbed_lpc1768
58736   200 21056   79992   master-bin
58752   200 21056   80008   6lowpan-nd-host-bin

32  0   0   32  msba2
140760  2308    95989   239057  master-bin
140792  2308    95989   239089  6lowpan-nd-host-bin

16  0   0   16  msbiot
60748   204 21072   82024   master-bin
60764   204 21072   82040   6lowpan-nd-host-bin

1732    0   288 2020    mulle
74440   1252    25904   101596  master-bin
76172   1252    26192   103616  6lowpan-nd-host-bin

64  0   0   64  native
185927  592 115000  301519  master-bin
185991  592 115000  301583  6lowpan-nd-host-bin

8   0   0   8   nucleo-f091
64804   200 21056   86060   master-bin
64812   200 21056   86068   6lowpan-nd-host-bin

16  0   0   16  nucleo-f303
60408   200 21064   81672   master-bin
60424   200 21064   81688   6lowpan-nd-host-bin

16  0   0   16  nucleo-l1
60372   200 21048   81620   master-bin
60388   200 21048   81636   6lowpan-nd-host-bin

16  0   0   16  openmote
58900   200 21032   80132   master-bin
58916   200 21032   80148   6lowpan-nd-host-bin

1732    0   288 2020    pba-d-01-kw2x
74124   1228    25752   101104  master-bin
75856   1228    26040   103124  6lowpan-nd-host-bin

32  0   0   32  pttu
141288  2308    95989   239585  master-bin
141320  2308    95989   239617  6lowpan-nd-host-bin

48  0   0   48  qemu-i386
256831  5596    131608  394035  master-bin
256879  5596    131608  394083  6lowpan-nd-host-bin

16  0   0   16  remote
59576   200 21032   80808   master-bin
59592   200 21032   80824   6lowpan-nd-host-bin

8   0   0   8   saml21-xpro
62452   200 21136   83788   master-bin
62460   200 21136   83796   6lowpan-nd-host-bin

2040    0   288 2328    samr21-xpro
76424   200 24136   100760  master-bin
78464   200 24424   103088  6lowpan-nd-host-bin

16  0   0   16  stm32f3discovery
60416   200 21064   81680   master-bin
60432   200 21064   81696   6lowpan-nd-host-bin

16  0   0   16  stm32f4discovery
60612   204 21056   81872   master-bin
60628   204 21056   81888   6lowpan-nd-host-bin

16  0   0   16  udoo
59444   204 21112   80760   master-bin
59460   204 21112   80776   6lowpan-nd-host-bin

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

add (void) pkt;

@miri64 miri64 force-pushed the gnrc_sixlowpan_nd/feat/initial branch from b8f27ee to ea3426e Compare September 9, 2015 09:16
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 9, 2015

Btw this PR adds about 1.7k ROM for iotlab-m3 and about 2k ROM for samr21 with gnrc_networking

Then we have a lot of room for optimization. :-)

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 9, 2015

Fixed unused variable issue and squashed immediately

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 9, 2015

Travis is finally happy \o/

miri64 added a commit that referenced this pull request Sep 9, 2015
…tial

gnrc_sixlowpan_nd: initial import of host behavior of 6LoWPAN-ND
@miri64 miri64 merged commit 84768b1 into RIOT-OS:master Sep 9, 2015
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 9, 2015

Thanks for the review!

@miri64 miri64 deleted the gnrc_sixlowpan_nd/feat/initial branch September 9, 2015 09:58
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 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.

4 participants