gnrc_ipv6_nib: add forwarding table component by miri64 · Pull Request #7253 · RIOT-OS/RIOT

miri64 · 2017-06-27T12:45:35Z

Implements the forwarding table component of the NIB.

~~Depends on #7212.~~

This PR is part of the network layer remodelling effort:

…IOT-OS#7253

…IOT-OS#7479

…IOT-OS#7253

miri64 · 2017-08-21T10:31:22Z

Rebased to current master and current dependencies

…OT-OS#7479

miri64 · 2017-10-06T10:29:03Z

Rebased to current master

cgundogan

more comments to look at.

cgundogan · 2017-10-06T12:47:07Z

sys/include/net/gnrc/ipv6/nib/ft.h

+ *
+ * @param[in] dst   The destination.
+ * @param[in] ctx   Packet that is supposed to go to that destination
+ *                  (is handed over to a reactive routing protocol if one exist


Why isn't this named pkt, but ctx, if the documentation says packet?

Can this be const probably?

Why isn't this named pkt, but ctx, if the documentation says packet?

Because it is the context of the forwarding step, but I don't care. I can call it pkt if you want.

Can this be const probably?

Maybe, but I'm not sure if this is guaranteed for every reactive routing protocol (even RPL adds stuff like routing headers to a packet and that's not even reactive).

Because it is the context of the forwarding step, but I don't care. I can call it pkt if you want.

Yes, please.

cgundogan · 2017-10-06T12:54:59Z

sys/net/gnrc/network_layer/ipv6/nib/_nib-internal.c

+bool _nib_offl_is_entry(const _nib_offl_entry_t *entry)
+{
+    return (entry >= _dsts) &&
+           (entry < (_dsts + GNRC_IPV6_NIB_OFFL_NUMOF));


_in_dsts()

cgundogan · 2017-10-06T12:55:49Z

sys/net/gnrc/network_layer/ipv6/nib/_nib-internal.c

+    DEBUG("nib: get match for destination %s from NIB\n",
+          ipv6_addr_to_str(addr_str, dst, sizeof(addr_str)));
+    for (_nib_offl_entry_t *entry = _dsts;
+         entry < (_dsts + GNRC_IPV6_NIB_OFFL_NUMOF);


_in_dsts()

cgundogan · 2017-10-06T12:58:31Z

sys/net/gnrc/network_layer/ipv6/nib/_nib-internal.c

+    int res = 0;
+
+    assert((dst != NULL) && (fte != NULL));
+    do {    /* goto, but cleaner ;-) */


I don't think abusing a do { } while() construct is cleaner than goto. In this case you can return res anyway instead of using break. There's nothing important after the loop.

True to the last statement! and regarding "abusing" do { } while(0): it still provides a clear (stack-)frame structure, visible in the code. goto does not and can easily get out of hand because of that.

cgundogan · 2017-10-06T13:08:26Z

sys/include/net/gnrc/ipv6/nib/ft.h

+ * }
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * @note    The list may change during iteration, but no duplication of already


Are you sure the latter part of this sentence is true? What happens if an entry gets removed and then added (after 3 other additions). Then the same value would end up later in the list, right?

Yes, that may happen. But then it is also wrong for nib_pl_iter() and nib_nc_iter(). Shall I remove it?

Shall I remove it?

Yup. No need to advertise features we cannot guarantee (:

miri64 · 2017-10-06T14:05:08Z

Rebased to current master

miri64 · 2017-10-06T14:19:22Z

Addressed comments.

cgundogan · 2017-10-06T14:22:49Z

sys/net/gnrc/network_layer/ipv6/nib/_nib-internal.c


+bool _nib_offl_is_entry(const _nib_offl_entry_t *entry)
+{
+    return (entry >= _dsts) && _in_dsts(entry);


isn't _in_dsts() also suppoed to check for >= _dsts?

you could probably remove _nib_offl_is_entry and use _in_dsts() everywhere

Nope. Since it is used as a loop-target this check wouldn't make sense / would be more expensive

But I can do the replacement for the cases where entry >= _dsts must also be true ;-).

(there is no such case in _nib-internal.c so all done ;-))

…IOT-OS#7479

cgundogan · 2017-10-06T20:11:19Z

sys/net/gnrc/network_layer/ipv6/nib/_nib-internal.c

+    else {
+        _nib_ft_get(offl, fte);
+    }
+    return 0;


else { _nib_ft_get(offl, fte); } } else { _nib_ft_get(offl, fte); }

Any chance to remove code duplication and / or level of nestedness?

I'll try something

miri64 · 2017-10-06T20:51:42Z

Rebased and addressed comments

cgundogan · 2017-10-06T20:56:52Z

much better, please squash!

miri64 · 2017-10-06T21:01:29Z

Squashed

cgundogan · 2017-10-06T21:08:15Z

cross another one off the list! (:

miri64 added GNRC Area: network Area: Networking State: waiting for other PR State: The PR requires another PR to be merged first labels Jun 27, 2017

miri64 added this to the Release 2017.07 milestone Jun 27, 2017

miri64 assigned cgundogan Jun 27, 2017

miri64 requested a review from cgundogan June 27, 2017 12:45

miri64 force-pushed the gnrc_ipv6_nib/feat/ft-component branch from 775cad7 to 3fc6298 Compare June 27, 2017 15:43

miri64 added a commit to miri64/RIOT that referenced this pull request Jun 27, 2017

Merge branch 'gnrc_ipv6_nib/feat/internal-offl' (RIOT-OS#7212) into R…

0c25fa8

…IOT-OS#7253

miri64 added a commit to miri64/RIOT that referenced this pull request Jun 27, 2017

Merge branch 'gnrc_ipv6_nib/feat/internal-offl' (RIOT-OS#7212) into R…

a21a5ab

…IOT-OS#7253

miri64 added a commit to miri64/RIOT that referenced this pull request Jun 27, 2017

Merge branch 'gnrc_ipv6_nib/feat/ft-component' (RIOT-OS#7253) into HEAD

93cc1f3

miri64 added a commit to miri64/RIOT that referenced this pull request Jun 29, 2017

Merge branch 'gnrc_ipv6_nib/feat/ft-component' (RIOT-OS#7253) into HEAD

07b92c7

miri64 force-pushed the gnrc_ipv6_nib/feat/ft-component branch from 89a9dff to fa71fc8 Compare June 30, 2017 16:05

miri64 added a commit to miri64/RIOT that referenced this pull request Jun 30, 2017

Merge branch 'gnrc_ipv6_nib/feat/internal-offl' (RIOT-OS#7212) into R…

f485452

…IOT-OS#7253

miri64 added a commit to miri64/RIOT that referenced this pull request Jun 30, 2017

Merge branch 'gnrc_ipv6_nib/feat/ft-component' (RIOT-OS#7253) into HEAD

b7ff78a

aabadie modified the milestones: Release 2017.07, Release 2017.10 Jun 30, 2017

miri64 added a commit to miri64/RIOT that referenced this pull request Jul 4, 2017

Merge branch 'gnrc_ipv6_nib/feat/ft-component' (RIOT-OS#7253) into HEAD

32d6464

miri64 added a commit to miri64/RIOT that referenced this pull request Jul 5, 2017

Merge branch 'gnrc_ipv6_nib/feat/ft-component' (RIOT-OS#7253) into HEAD

2ba1501

miri64 added a commit to miri64/RIOT that referenced this pull request Aug 11, 2017

Merge branch 'gnrc_ipv6_nib/feat/ft-component' (RIOT-OS#7253) into HEAD

937386a

miri64 mentioned this pull request Aug 17, 2017

gnrc_ipv6_nib: implement behavior for router discovery #7479

Merged

3 tasks

miri64 added a commit to miri64/RIOT that referenced this pull request Aug 17, 2017

Merge branch 'gnrc_ipv6_nib/feat/ft-component' (RIOT-OS#7253) into R…

935f564

…IOT-OS#7479

miri64 force-pushed the gnrc_ipv6_nib/feat/ft-component branch from 99a6afe to 5f89b17 Compare August 21, 2017 10:31

miri64 added a commit to miri64/RIOT that referenced this pull request Aug 21, 2017

Merge branch 'gnrc_ipv6_nib/feat/internal-offl' (RIOT-OS#7212) into R…

63dbcd7

…IOT-OS#7253

miri64 force-pushed the gnrc_ipv6_nib/feat/ft-component branch from 5f89b17 to 9b933fb Compare August 21, 2017 10:38

miri64 added a commit to miri64/RIOT that referenced this pull request Aug 21, 2017

Merge branch 'gnrc_ipv6_nib/feat/ft-component' (RIOT-OS#7253) into RI…

d211f0d

…OT-OS#7479

miri64 force-pushed the gnrc_ipv6_nib/feat/ft-component branch from 9b933fb to 9a6b1ff Compare October 6, 2017 10:28

miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Oct 6, 2017

cgundogan requested changes Oct 6, 2017

View reviewed changes

miri64 force-pushed the gnrc_ipv6_nib/feat/ft-component branch from 2a08ed2 to 02aa064 Compare October 6, 2017 14:05

cgundogan reviewed Oct 6, 2017

View reviewed changes

miri64 added a commit to miri64/RIOT that referenced this pull request Oct 6, 2017

Merge branch 'gnrc_ipv6_nib/feat/ft-component' (RIOT-OS#7253) into R…

5af99bd

…IOT-OS#7479

cgundogan reviewed Oct 6, 2017

View reviewed changes

miri64 force-pushed the gnrc_ipv6_nib/feat/ft-component branch from 9cc359a to 2cdae91 Compare October 6, 2017 20:51

miri64 added 3 commits October 6, 2017 23:01

gnrc_ipv6_nib: add forwarding table component

d8acdc9

shell: commands: extend nib shell command for forwarding table

e1a37a0

tests: extend gnrc_ipv6_nib unittests for FT component

c60cacc

miri64 force-pushed the gnrc_ipv6_nib/feat/ft-component branch from 2cdae91 to c60cacc Compare October 6, 2017 21:01

cgundogan approved these changes Oct 6, 2017

View reviewed changes

cgundogan merged commit d5c9644 into RIOT-OS:master Oct 6, 2017

miri64 deleted the gnrc_ipv6_nib/feat/ft-component branch October 6, 2017 21:08

Conversation

miri64 commented Jun 27, 2017 • edited by cgundogan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miri64 commented Aug 21, 2017

Uh oh!

miri64 commented Oct 6, 2017

Uh oh!

cgundogan left a comment

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!

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!

miri64 commented Oct 6, 2017

Uh oh!

miri64 commented Oct 6, 2017

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!

miri64 commented Oct 6, 2017

Uh oh!

cgundogan commented Oct 6, 2017

Uh oh!

miri64 commented Oct 6, 2017

Uh oh!

cgundogan commented Oct 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

miri64 commented Jun 27, 2017 •

edited by cgundogan

Loading