Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the AODVv2 Routing Protocol #1767

Merged
merged 1 commit into from
Nov 27, 2014
Merged

Conversation

Lotterleben
Copy link
Member

This PR depends on #1766.

It contains a minimal implementation of the AODVv2 routing protocol.
Not implemented are:

- AckReqs
- alternate metrics
- multiple interfaces
- clients and Client Networks
- buffering
- all addresses, TLVs, and features that are marked as optional

An example application can be found at https://github.com/Lotterleben/RIOT-AODVv2/tree/master/aodvv2_demo.

The implementation relies heavily on a functioning Neighbor Discovery Protocol.
It might be necessary to fill the neighbor cache manually with the current state
of RIOTs NDP implementation.

The value of AODVV2_MAX_UNREACHABLE_NODES has been chosen arbitrarily and will be subject to
future improvement.

Please note that based on my experience, with the default transceiver
buffer size (3) of the native port, about 2/3 of the route discoveries
will fail. This has been addressed in issue #1747. It is advised to increase
the transceiver buffer size when using AODVv2 as a routing protocol.

@LudwigKnuepfer LudwigKnuepfer added State: waiting for other PR State: The PR requires another PR to be merged first Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: network Area: Networking labels Oct 7, 2014
@Lotterleben
Copy link
Member Author

I just noticed that Travis throws some style errors-- I remember
sys/net/routing/aodvv2/writer.c:142: error (arrayIndexOutOfBounds): Array '_rreq_addrtlvs[2]' accessed at index 3, which is out of bounds.
and the like being an error of cppcheck which I had encountered earlier as well. But I'll get to the other ones right away.

@Lotterleben
Copy link
Member Author

@LudwigOrtmann Also, PR #1766 has been merged already. ;)

@OlegHahm OlegHahm added the PR-award-nominee Deprecated. Will be removed soon. label Oct 7, 2014
@N8Fear
Copy link

N8Fear commented Oct 7, 2014

@Lotterleben Travis is unhappy because of the license headers of the following files:

file has an unknown license header: 'sys/net/include/aodvv2/aodvv2.h'
file has an unknown license header: 'sys/net/routing/aodvv2/aodv.c'
file has an unknown license header: 'sys/net/routing/aodvv2/aodv.h'
file has an unknown license header: 'sys/net/routing/aodvv2/constants.h'
file has an unknown license header: 'sys/net/routing/aodvv2/reader.c'
file has an unknown license header: 'sys/net/routing/aodvv2/reader.h'
file has an unknown license header: 'sys/net/routing/aodvv2/routing.c'
file has an unknown license header: 'sys/net/routing/aodvv2/routing.h'
file has an unknown license header: 'sys/net/routing/aodvv2/seqnum.c'
file has an unknown license header: 'sys/net/routing/aodvv2/seqnum.h'
file has an unknown license header: 'sys/net/routing/aodvv2/utils.c'
file has an unknown license header: 'sys/net/routing/aodvv2/utils.h'
file has an unknown license header: 'sys/net/routing/aodvv2/writer.c'
file has an unknown license header: 'sys/net/routing/aodvv2/writer.h'

#ifndef AODVV2_H_
#define AODVV2_H_

void aodv_init(void);
Copy link
Member

Choose a reason for hiding this comment

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

This header would be happy about gettting documented. ;-)

@LudwigKnuepfer
Copy link
Member

@Lotterleben Feel free to remove the obsolete tag yourself next time ;)

@LudwigKnuepfer LudwigKnuepfer removed the State: waiting for other PR State: The PR requires another PR to be merged first label Oct 7, 2014
if (ndp_nc_entry != NULL)
{

// Case 2: Broken Link (detected by lower layer)
Copy link
Member

Choose a reason for hiding this comment

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

C style comments please.

@Lotterleben
Copy link
Member Author

@LudwigOrtmann Will do, wasn't sure if I was allowed to do that ;)

@N8Fear
Copy link

N8Fear commented Oct 7, 2014

The cppcheck errors of the form sys/net/routing/aodvv2/writer.c:142: error (arrayIndexOutOfBounds): Array '_rreq_addrtlvs[2]' accessed at index 3, which is out of bounds. can be disabled by adding 'dummy array members' along the lines of [0] = { .type = NULL}, (the index should be adjusted accordingly).
I'm not sure if this is a problem in cppcheck or if this is an actual error. If it's an issue with cppcheck that should likely be reported upstream. The error should be blacklisted in your PR in the meantime.
Can you provide a link where this is documented as valid c99? (The question is essentially if it's ok to leave out members/indices of the struct).

{
AODV_DEBUG("%s()\n", __func__);
rfc5444_writer_cleanup(&writer);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I dislike the prefix writer_. It's bound to clash with user code.

Copy link
Member Author

Choose a reason for hiding this comment

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

I used it for consistency with the oonf_api and Ben's OLSR code. I can rename it to aodv_packet_writer, but only after we've decided which is worse ugly newlines or long lines.

@OlegHahm
Copy link
Member

In general I would like you to address the non-newline-realated comments that @Kijewski made. For the newlines, it's up to you if you want to obey strictly the 80-chars limit.

@Lotterleben
Copy link
Member Author

@OlegHahm @Kijewski most of the unnecessary line breaks were made for consistency reasons. It seems weird to me to cram one ||-divided expression on one line and then split up the next one over two lines, and so on. If it's okay, I'd rather keep it this way (also because I have another deadline approaching and it's 23° C outside here ;) ) for now and if it ends up annoying somebody terribly, I'll fix it in another PR.
I'll take care of the other comments as soon as I can.

@OlegHahm
Copy link
Member

Fair enough. Enjoy Honolulu and the cookies. ;)

@OlegHahm
Copy link
Member

Just let me know (write here, use jabber, scream, wave your hands, ask @LudwigOrtmann to beat me...) when this is ready for the next review.

@Lotterleben
Copy link
Member Author

@OlegHahm ACK, I'll notify you as soon as I'm done here... I hope I'll figure out why travis can't compile my stuff until then. :D

@OlegHahm
Copy link
Member

sys/net/routing/aodvv2/utils.c:236: warning (pointerSize): Size of pointer 'dst' used instead of size of its data.
sys/net/routing/aodvv2/aodv.c:264: style (variableScope): The scope of the variable 'rcv_size' can be reduced.

@Lotterleben
Copy link
Member Author

Oh, where did you get that from?

@OlegHahm
Copy link
Member

Ran the Travis script manually locally. :-/ I think the || exit causes Travis to exit sometimes before it prints the error message. Need to look into this.

@Lotterleben
Copy link
Member Author

I actually got rid of the || exit because it logged me out of my VM when the script was done a while ago, but I couldn't reproduce the messages...

@miri64
Copy link
Member

miri64 commented Nov 26, 2014

It's not related to this PR, but have you tried this?

{ foobar || exit } &
wait

Edit: added crucial &

@OlegHahm
Copy link
Member

Opened #2089

@N8Fear
Copy link

N8Fear commented Nov 27, 2014

Triaged in #2089 #2101 but I guess that's an upstream cppcheck bug.

Edited: seems like I'm running too low on caffeine..

@Lotterleben
Copy link
Member Author

Thank you! :)

@OlegHahm
Copy link
Member

#2101 is merged, I guess you need to rebase on this.

@Lotterleben
Copy link
Member Author

Travis is happy!
giphy
Let me just squash that again, 1sec...

This PR depends on RIOT-OS#1766.

It contains a minimal implementation of the AODVv2 routing protocol.
*Not* implemented are:

	- AckReqs
	- alternate metrics
	- multiple interfaces
	- clients and Client Networks
	- buffering
	- all addresses, TLVs, and features that are marked as optional

An example application can be found at https://github.com/Lotterleben/RIOT-AODVv2/tree/master/aodvv2_demo.

The implementation relies heavily on a functioning Neighbor Discovery Protocol.
It might be necessary to fill the neighbor cache manually with the current state
of RIOTs NDP implementation.

The value of AODVV2_MAX_UNREACHABLE_NODES has been chosen arbitrarily and will be subject to
future improvement.

Please note that based on my experience, with the default transceiver
buffer size (3) of the native port, about 2/3 of the route discoveries
will fail. This has been addressed in issue RIOT-OS#1747. It is advised to increase
the transceiver buffer size when using AODVv2 as a routing protocol.
OlegHahm added a commit that referenced this pull request Nov 27, 2014
Add the AODVv2 Routing Protocol
@OlegHahm OlegHahm merged commit 5ae6ca0 into RIOT-OS:master Nov 27, 2014
@OlegHahm
Copy link
Member

Congrats!

@emmanuelsearch
Copy link
Member

Congrats indeed ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking PR-award-nominee Deprecated. Will be removed soon. 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.

9 participants