aodvv2: add routes to FIB #2985
Conversation
|
|
||
| fib_register_rp((uint8_t*) &aodvv2_prefix, aodvv2_prefix_len); | ||
|
|
||
| //fib_register_rp((uint8_t*) &aodvv2_prefix, aodvv2_prefix_len); |
|
nice one 👍 will test |
|
Addressed comments and rebased to current master |
| * | ||
| * @} | ||
| */ | ||
|
|
There was a problem hiding this comment.
please add:
#ifdef __cplusplus
extern "C" {
#endif
...
#ifdef __cplusplus
}
#endifand header guards
|
please add an empty |
|
|
||
| /* make sure sender_thread is done */ | ||
| sleep(2); | ||
| aodv_test_writer_write_more_recent_rrep(); |
There was a problem hiding this comment.
This writer is called but doesn't seem to store ./assets/more_recent_rrep, resulting in an access violation in aodv_test_update_fib_regular_rreq() when trying to load the missing file.
There was a problem hiding this comment.
Weird, this works on my machine too. I'd suggest this is because the call to aodv_init() that comes after it overwrites the callback too quickly; I'll add another sleep(2), can you test if it works for you too then?
|
@BytesGalore doesn't git forbid you from adding empty directories? I guess It'd make more sense to write proper code to prevent that error ^^ |
|
Also, judging by your header comments I'm assuming the tests stay in? |
|
@Lotterleben don't know how git handles empty directories, but another possibility is to create the directory by the |
Git just cannot store directory information, only files can be stored. A common workaround is to put an (empty) |
|
@Lotterleben I would vote to store the RREPs/RREQs in a global buffer instead of writing them to files. Currently this test is only applicable on native. |
|
The idea behind storing them in files was that they could be tracked too and any differences in the message format caused by unvoluntary changes could be detected, but I guess that was a little bit too enthusiastic of an approach... |
|
Then I propose to provide an optional feature in the tests to store the RREP/RREQ in files only when |
|
Currently, this PR as well as #2985 are modifying ip.c to get anything working with the routing protocols which are still on the network stack... I really wouldn't proposed my changes to be merged, but I think ít would make sense to introduce an ifdef-based workaround to ip.c so that the fib can be used and tested even before all routing protocols have been moved to the network stack when it's ready... What do you think @OlegHahm @BytesGalore? |
|
I think you meant #2765 :) |
|
Ssoooo tests are updated and everything is squashed :) |
| { | ||
| int i; /* make clang stfu */ | ||
| i = netaddr_from_string(&aodvv2_test_origaddr, "::10"); | ||
| i = netaddr_from_string(&aodvv2_test_sender_oa, "::11"); |
There was a problem hiding this comment.
ok travis seems to be a bit picky here:
tests/aodvv2/main.c:76: performance (redundantAssignment): Variable 'i' is reassigned a value before the old one has been used. ...
I think you can calm travis when using += from this line on :)
|
Squashed and rebased again |
|
(Can I add the ready for CI build flag and see what travis says?) |
BytesGalore
added
the
CI: ready for build
|
lets rock :) |
| static void aodvv2_init_testdata(void) | ||
| { | ||
| volatile int i; /* make clang stfu */ | ||
| i = netaddr_from_string(&aodvv2_test_origaddr, "::10"); |
There was a problem hiding this comment.
ok, volatile doesn't seem to calm travis
tests/aodvv2/main.c:70: performance (redundantAssignment): Variable 'i' is reassigned a value before the old one has been used.
There was a problem hiding this comment.
sigh... += it is, then.
There was a problem hiding this comment.
how about checking if the conversion worked accordingly with a return value isof 0?
There was a problem hiding this comment.
You mean just to make sure? That makes sense...
There was a problem hiding this comment.
I propose this:
static int aodvv2_init_testdata(void)
{
if( netaddr_from_string(&aodvv2_test_origaddr, "::10") == -1 ) {
return -1;
}
if( netaddr_from_string(&aodvv2_test_sender_oa, "::11") == -1 ) {
return -1;
}
if( netaddr_from_string(&aodvv2_test_sender_ta, "::12") == -1 ) {
return -1;
}
if( netaddr_from_string(&aodvv2_test_targaddr, "::13") == -1 ) {
return -1;
}
return 0;
}
Lotterleben
force-pushed
the
aodvv2_fib
branch
2 times, most recently
from
3d5fb7d
to
72baaca
Compare
|
Oha... seems like the test exceeds the memory of a lot of boards... I'll have to investigate that. :S |
|
Seems like my very ambitious values for AODVV2_RREQ_BUF (128) and AODVV2_MAX_ROUTING_ENTRIES (255 !!! What the hell) are the main offenders. I'm wondering if I should just blacklist smaller boards for now, or open a PR trimming down those sizes, or wait for us to figure out a doable solution for setting these variables at compile time (see #3022)... |
|
+1 for blacklisting the test. |
|
(I mean blacklisting the boards not the test ;) ) |
|
I've whitelisted only native for now... |
|
rebased to current master |
| int main(void) | ||
| { | ||
| aodvv2_init_testdata(); | ||
| aodvv2_setup_node(); |
There was a problem hiding this comment.
and I propose to change this to:
if( aodvv2_init_testdata() != 0 ) {
return -1;
}
if( aodvv2_setup_node() != 0 ) {
return -1;
}
|
Addressed comments and rebased onto current master |
|
I guess you can squash and we see if the dynamic duo (travis and strider) get happy :) |
|
nice, the duo is happy and so am I -> ACK |
|
and GO |
aodvv2: add routes to FIB
This PR introduces the FIB to AODVv2. All routes which are found will be added to the FIB, and the FIB now initiates route discoveries.
What's still missing is that routes aren't updated when they are used. This will be introduced after #2815 has been merged.
The tests are pretty rudimentary, but I thought I'd include them anyway for clarity. if you want to, I can remove them.