gnrc_ipv6_nib: add forwarding table component #7253
Conversation
775cad7 to
3fc6298
Compare
89a9dff to
fa71fc8
Compare
99a6afe to
5f89b17
Compare
|
Rebased to current master and current dependencies |
5f89b17 to
9b933fb
Compare
9b933fb to
9a6b1ff
Compare
|
Rebased to current master |
sys/include/net/gnrc/ipv6/nib/ft.h
Outdated
| * | ||
| * @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 |
There was a problem hiding this comment.
Why isn't this named pkt, but ctx, if the documentation says packet?
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Because it is the context of the forwarding step, but I don't care. I can call it pkt if you want.
Yes, please.
| bool _nib_offl_is_entry(const _nib_offl_entry_t *entry) | ||
| { | ||
| return (entry >= _dsts) && | ||
| (entry < (_dsts + GNRC_IPV6_NIB_OFFL_NUMOF)); |
| 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); |
| int res = 0; | ||
|
|
||
| assert((dst != NULL) && (fte != NULL)); | ||
| do { /* goto, but cleaner ;-) */ |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
sys/include/net/gnrc/ipv6/nib/ft.h
Outdated
| * } | ||
| * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| * | ||
| * @note The list may change during iteration, but no duplication of already |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Yes, that may happen. But then it is also wrong for nib_pl_iter() and nib_nc_iter(). Shall I remove it?
There was a problem hiding this comment.
Shall I remove it?
Yup. No need to advertise features we cannot guarantee (:
2a08ed2 to
02aa064
Compare
|
Rebased to current master |
|
Addressed comments. |
| @@ -483,6 +495,91 @@ _nib_offl_entry_t *_nib_offl_iter(const _nib_offl_entry_t *last) | |||
| return NULL; | |||
| } | |||
|
|
|||
| bool _nib_offl_is_entry(const _nib_offl_entry_t *entry) | |||
| { | |||
| return (entry >= _dsts) && _in_dsts(entry); | |||
There was a problem hiding this comment.
isn't _in_dsts() also suppoed to check for >= _dsts?
There was a problem hiding this comment.
you could probably remove _nib_offl_is_entry and use _in_dsts() everywhere
There was a problem hiding this comment.
Nope. Since it is used as a loop-target this check wouldn't make sense / would be more expensive
There was a problem hiding this comment.
But I can do the replacement for the cases where entry >= _dsts must also be true ;-).
There was a problem hiding this comment.
(there is no such case in _nib-internal.c so all done ;-))
| else { | ||
| _nib_ft_get(offl, fte); | ||
| } | ||
| return 0; |
There was a problem hiding this comment.
else {
_nib_ft_get(offl, fte);
}
}
else {
_nib_ft_get(offl, fte);
}Any chance to remove code duplication and / or level of nestedness?
9cc359a to
2cdae91
Compare
|
Rebased and addressed comments |
|
much better, please squash! |
2cdae91 to
c60cacc
Compare
|
Squashed |
|
cross another one off the list! (: |
Implements the forwarding table component of the NIB.
Depends on #7212.This PR is part of the network layer remodelling effort:
