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

aodvv2: port to new network stack #3445

Closed
wants to merge 41 commits into from

Conversation

Lotterleben
Copy link
Member

This PR makes AODVv2 ng ready. :) Also, AODVv2 must now be configured to serve a specific interface.

@Lotterleben Lotterleben added Area: network Area: Networking Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. NSTF labels Jul 19, 2015
@Lotterleben
Copy link
Member Author

One thing, though– the initial efforts of this PR were made by @BytesGalore, and I'd like to preserve his authorship, but I had to create a clean branch since the original implementation became messy beyond repair (at least with my git skills) through several PR merges and rebases and fixes... What would be the best way to do this? Just add him as an author? Add his original commit and then rebase this one on top of it?

@miri64
Copy link
Member

miri64 commented Jul 19, 2015

What would be the best way to do this? Just add him as an author? Add his original commit and then rebase this one on top of it?

Yes add him to as @author in doxygen and maybe to the copyright note too.

@@ -34,7 +35,7 @@ extern "C" {
/**
* @brief Initialize the AODVv2 routing protocol.
*/
void aodv_init(void);
void aodv_init(kernel_pid_t interface);
Copy link
Member

Choose a reason for hiding this comment

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

Documentation of interface. What is it for? And what's with multiple interface support?

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'd say AODVv2 can currently only serve one interface and put that into the documentation? Multiple interfaces would be nifty, but I have a huge backlog of draft changes that I'd like to implement first...

Copy link
Member

Choose a reason for hiding this comment

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

👍

@Lotterleben Lotterleben added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jul 22, 2015
@Lotterleben
Copy link
Member Author

Addressed commits (the “4 days ago” is because my VM got confused), but I haven't tested yet if I broke anything.

struct netaddr _sender;
ipv6_addr_t_to_netaddr(&sa_rcv.sin6_addr, &_sender);
/* convert to correct byteorder */
port = port;
Copy link
Member

Choose a reason for hiding this comment

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

Um, I guess this is not needed anymore. (I know that it was HTONS(port) formerly )

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops :D

@miri64
Copy link
Member

miri64 commented Aug 7, 2015

Needs rebase.

@miri64
Copy link
Member

miri64 commented Aug 17, 2015

And adaptation to the GNRC renames.

@Lotterleben
Copy link
Member Author

Will do, as soon as my vacation is over and the jet lag is manageable (should be around the 26th)

@Lotterleben
Copy link
Member Author

(not ready for review yet, the change from local to global addresses broke some stuff and there is still something amiss with unicast source addresses– will let you know once I'm done with fixing & rebasing)

@Lotterleben Lotterleben added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Aug 27, 2015
@miri64 miri64 mentioned this pull request Aug 31, 2015
36 tasks
@OlegHahm OlegHahm modified the milestone: Release NEXT MAJOR Sep 2, 2015
@OlegHahm OlegHahm modified the milestones: Release 2015.12, Release 2016.03 Dec 3, 2015
@smlng
Copy link
Member

smlng commented Aug 30, 2016

@Lotterleben what's your status on this one, ready for some reviewing?

I could spare some time during today H'n'A - maybe you can join as well? Further, I would give it another try on some real hardware, eg. SAMR21 to see what's missing to get it working.

@Lotterleben
Copy link
Member Author

I'll be at the H'n'A to work on this PR, but I was busy with exams up until now :/

@miri64
Copy link
Member

miri64 commented Oct 31, 2016

Sadly I guess we need to postpone again :(

@miri64 miri64 modified the milestones: Release 2017.01, Release 2016.10 Oct 31, 2016
@Lotterleben
Copy link
Member Author

Indeed :/ For the record, I'm still working on this– there appears to be a bug causing forwarded RREPs to have the wrong source & destination addresses in the IP header (!). I hope I'll be able to fix this asap. But while hunting this bug I fixed one of @smlng s complaints, namely that there are too many threads running (down to 1 from previously 3 threads!). In the process, the weird nested message contents @BytesGalore criticized were chucked as well. And all mallocs are gone too.

}
/* allocate IPv6 header, set pkt_with_udp as payload */
pkt_with_ip = gnrc_ipv6_hdr_build(pkt_with_udp, &aodv_ipv6_addr_local,
&addr_send);
Copy link
Member

Choose a reason for hiding this comment

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

RREPs to have the wrong source & destination addresses in the IP header (!).

How wrong are they? If they are still sensible addresses I guess you are setting the wrong addresses here. If they are total garbage you might overwrite something accidentaly. My choice of tool for this would be gdb:

watch *<address of first byte in destination or source address>

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly, that was what I was thinking. They're not total garbage: the src address is the address that originated the whole route discovery, and the dst address has parts of the suffix of the target of the route discovery, but as a multicast address(so for example ff02::1:ff88:36ed instead of bee2::3068:3cff:fe88:36ed). o.0
so in a topology like this: originator<-A<-B<-target, A receives a RREP packet with the originator as src etc. I'll keep an eye on what I'm feeding gnrc_ipv6_hdr_build ...

@PeterKietzmann PeterKietzmann modified the milestones: Release 2017.04, Release 2017.01 Jan 13, 2017
@OlegHahm
Copy link
Member

OlegHahm commented Jan 15, 2017

The same question as last release? The same question as every release. ;-)

@Lotterleben
Copy link
Member Author

:D :D I'd answer "I'll do my very best" but exam season has started, sooooo.. :(

@miri64
Copy link
Member

miri64 commented Oct 5, 2017

Closing with memo for now. No activity for > 6 month, RFC is not going to happen and original author doesn't have the time to work on it.

@miri64 miri64 closed this Oct 5, 2017
@miri64 miri64 added the State: archived State: The PR has been archived for possible future re-adaptation label Oct 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. State: archived State: The PR has been archived for possible future re-adaptation State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants