sys/net/fib: added function to request a set of destination addresses#2818
Conversation
5118b0e to
c010736
Compare
sys/net/network_layer/fib/fib.c
Outdated
There was a problem hiding this comment.
Could you move the check dst_set != NULL outside of the loop?
There was a problem hiding this comment.
Basically yes, but the it would require a construct like:
if( dst_set != NULL ){
for(...) ... // like above without the check
} else {
for(...) ... // just counting the entries
}There was a problem hiding this comment.
I see. Actually I thought that this function should return an error when no dst_set was specified. Anyhow, can you extend the doc that this function still counts and returns the number of all matching entries even when they are not copied to dst_set?
And add parens to the condition, please (:
There was a problem hiding this comment.
Added parens and wrote a line that the matching count is being determined.
The idea to allow a dst_set == NULL and continue, is that if you're interested at first in the current number of matching entries, to check if your out buffer can hold them, you probably don't want to have any copy operations.
sys/include/net/ng_fib.h
Outdated
There was a problem hiding this comment.
could you also add the fact, that the buffer is allowed to be null? Something like:
If the out buffer is insufficient low or null, ...
There was a problem hiding this comment.
sure, sorry for the delay.
|
besides my comment this PR looks good and needs squashing |
|
does anyone else wants to participate in this discussion (because it's marked as RFC)? Otherwise, my ACK holds and my intention is to merge this ASAP |
0ad152f to
6106c8f
Compare
|
squashed and rebased. |
|
I guess I am satisfied with this PR and I am in favor of dropping the RFC label here and opt for a quick merge, since we need this functionality for #2765 . What do you say @BytesGalore @OlegHahm ? |
|
@cgundogan sure, removed the RFC label |
|
in a followup PR we could make this feature optional/exclusive when RPL is loaded |
|
Do we need to think about a rationale such that their are no flag collisions between different protocols that write into the FIB? Like a prefix based on protocol ID, and then the rest of the bits are managed per protocol? |
|
@emmanuelsearch definitely, I think an [1] https://github.com/RIOT-OS/RIOT/blob/master/sys/include/net/ng_nettype.h#L89 |
|
I don't expect that we will have like 100 routing-protocols available and using more than 3 at the same time on a node |
|
@BytesGalore are regardless of the fact that we will not have more than max 3 routing protocols running at the same time, an |
|
@cgundogan the But I think you're both are right. A collision free mechanism based on protocol IDs and additional management bits is the right way to do it and this should be a done/merged prior to this PR. |
|
@BytesGalore do you have a concept in mind for such a mechanism? |
|
@cgundogan after rethinking a bit I think using flags/protocol IDs for that purpose is not necessary. |
|
The prototype would change to: int fib_get_destination_set(uint32_t* prefix, size_t prefix_length, fib_destination_set_entry_t *dst_set, size_t* dst_set_size);@cgundogan what do you think, any cons on change it this way? |
|
@BytesGalore I guess your suggestion is reasonable. Assigning the routing protocols to handle certain prefixes may be the right way. To stress a point: What happens if prefix ranges overlap? longest prefix matches? |
|
@cgundogan overlapping should MUST NOT happen since then both (or multiple) routing protocols would take responsibility/royalty over the intersected address range. |
e4ff998 to
cdbca42
Compare
|
ok, changed the approach to use a prefix match instead of flags, and rebased. |
|
I think there's only one choice: add it to |
|
@OlegHahm isn't it the rom section, which is overflowing? |
|
Ah, yes, you're right. Which is actually weird. |
|
The macro |
|
Then we should opt to change this macro's name to be more verbose and semantically correct |
|
Maybe we should rename it to |
+1 |
|
+1 |
|
I added the samr board to |
|
@BytesGalore please squash. Merge when travis is happy |
|
@BytesGalore I think this PR also needs a rebase? |
72d0dc2 to
18c69e0
Compare
|
rebased and squashed |
|
@BytesGalore can you trigger travis? |
|
hmm seems that travis is currently not available :( |
|
Why is travis not building this PR? It works for other PRs.. |
|
@BytesGalore can you again rebase to master so that #2783 is also included in this branch? This would simplify the rebase of my branch in #3050 (: |
18c69e0 to
9079e30
Compare
|
travis is green => GO |
sys/net/fib: added function to request a set of destination addresses
This PR introduces an access function to the FIB, that collects all destination addresses matching specific prefix.
(This is meant as a base for discussion)
Rationale:
Using the FIB with RPL requires in storing MOP that all destination addresses reachable from a node, which are constructed by receiving DIOs and all target options from all received DAOs recursively, are put in the DAO messages disseminated by the node.
The former FIB did not provided access for a set of destination addresses matching a specific prefix.