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.

Sign up for GitHub

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

Jump to bottom

ipv6_netif: fix ng_ipv6_netif_find_best_src_addr() for multicast #3048

Merged
merged 2 commits into from
May 25, 2015
Merged

ipv6_netif: fix ng_ipv6_netif_find_best_src_addr() for multicast #3048

merged 2 commits into from
May 25, 2015

Conversation

miri64
Copy link
Member

@miri64 miri64 commented May 22, 2015

Without this fix ng_ipv6_netif_find_best_src_addr() will return :: for most multicast addresses

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer NSTF labels May 22, 2015
@miri64 miri64 added this to the Network Stack Task Force milestone May 22, 2015
@Lotterleben
Copy link
Member

Do you mean ipv6_net_if_get_best_src_addr? Let me figure out how that happens...

@miri64
Copy link
Member Author

miri64 commented May 22, 2015

No I mean ng_ipv6_netif_find_best_src_addr()... sorry wrote this from the top of my head and pretty pissed, that I took longer for #3049 than expected :D

@cgundogan
Copy link
Member

I also came across this bug several days ago and can confirm it.

@miri64 miri64 changed the title ipv6_netif: fix ipv6_get_best_src_addr() for multicast ipv6_netif: fix ng_ipv6_netif_find_best_src_addr() for multicast May 22, 2015
@miri64
ipv6_netif: fix ng_ipv6_netif_find_best_src_addr() for multicast
e93b8a1 
Without this fix ng_ipv6_netif_find_best_src_addr() will return :: for most
multicast addresses
@miri64 miri64 force-pushed the ipv6_netif/fix/get-best-src-mc branch from 5db5722 to e93b8a1 Compare May 22, 2015 15:46
@miri64
Copy link
Member Author

miri64 commented May 22, 2015

Updated the commit message accordingly.

@Lotterleben
Copy link
Member

Aaaah that makes much more sense :D

@Lotterleben
Copy link
Member

@cgundogan is that also an ACK?

@cgundogan
Copy link
Member

@Lotterleben No. Not tested yet (:

@miri64
Copy link
Member Author

miri64 commented May 23, 2015

And now?

@Lotterleben
Copy link
Member

I'd be interested in that too :) I think I understood the problem and the solution looks reasonable to me, but I'm sure @cgundogan has a much more well-informed view on this. I'd merge this fix based on @cgundogan s comment, if that makes sense. (sry, I know youre waiting for this PR but I'm a bit hesistant to merge when I'm unsure I know what I'm doing :D Otherwise we could just reassign to cenk, if thats okay with his workload?)

@cgundogan
Copy link
Member

I tested this briefly and it seems to work. Could you also add a quick unittest to tests-ipv6_netif/tests-ipv6_netif.c please? Afterwards, ACK and merge at will

@miri64
Copy link
Member Author

miri64 commented May 25, 2015

Done.

@miri64
Copy link
Member Author

miri64 commented May 25, 2015

Wait a minute, I just realized an error in my thinking :D

@miri64
Copy link
Member Author

miri64 commented May 25, 2015

Will leave this for a follow-up PR.

@Lotterleben
Copy link
Member

now i'm confused :D

@miri64
Copy link
Member Author

miri64 commented May 25, 2015

See #3061

miri64 added a commit that referenced this pull request May 25, 2015
@miri64
Merge pull request #3048 from authmillenon/ipv6_netif/fix/get-best-sr…
4310ab3 
…c-mc

ipv6_netif: fix ng_ipv6_netif_find_best_src_addr() for multicast
@miri64 miri64 merged commit 4310ab3 into RIOT-OS:master May 25, 2015
@miri64 miri64 deleted the ipv6_netif/fix/get-best-src-mc branch May 25, 2015 21:19
@miri64 miri64 added the Area: network Area: Networking label Sep 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Lotterleben Lotterleben

Labels
Area: network Area: Networking Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Network Stack Task Force
Development

Successfully merging this pull request may close these issues.
3 participants
@miri64 @Lotterleben @cgundogan