-
Notifications
You must be signed in to change notification settings - Fork 112
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
dhcpcd not ignoring source-based routes on linux #372
dhcpcd not ignoring source-based routes on linux #372
Conversation
2dc202d
to
b5b0c89
Compare
RTA_SRC is just a hint for the address selection algo used by the kernel. It doesn't make the route any more special. Can you post a real world example, maybe with your routes and dhcpcd's default route and highlight which one is from dhcpcd please? |
I believe the "hint" is RTA_PREFSRC. RTA_SRC is part of the route itself, and is generally ::0/0 (or "default" in ip route lingo)
Source-based routing is basically the same as adding an Let me know if that doesn't answer your question... PS As an aside, I just tested with |
src/if-linux.c
Outdated
@@ -686,6 +686,22 @@ if_copyrt(struct dhcpcd_ctx *ctx, struct rt *rt, struct nlmsghdr *nlm) | |||
case RTA_DST: | |||
sa = &rt->rt_dest; | |||
break; | |||
case RTA_SRC: | |||
{ | |||
struct sockaddr ssa; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
struct sockaddr won't hold an INET6 address.
We have the sa_ss union to help with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lol, my bad, been a few decades since I did socket programming in C, forgot how weird it can be :)
src/if-linux.c
Outdated
salen = sa_addrlen(&ssa); | ||
memcpy((char *)&ssa + sa_addroffset(&ssa), | ||
RTA_DATA(rta), MIN(salen, RTA_PAYLOAD(rta))); | ||
/* if source-route, ignore it */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we expand on this comment to say that "source routing can conflict with dhcpcd's notion of routing, so ignore for now." I feel that a simple comment may cause someone like me to read this in a few years and say "heh it works" but I've lost the context for why we want to ignore it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a more detailed comment, let me know if it's clear enough...
Thanks for the summary. Can we put part of the in a comment in the code please just to clarify it? The one liner is not verbose enough as to why we are ignoring it. |
Since source-based routes aren't used by dhcpcd, it's best if they are ignored so that they aren't confused with default routes.
b5b0c89
to
363b9bc
Compare
@rsmarples Just curious, I fixed the two issues from your review a couple weeks ago, was there something else you wanted me to change? |
Sorry, I've been distracted of late. |
I'm using ipv6 source-based routing on a machine with multiple WANs. I'm adding routes for each delegated subnet to their respective WAN interface, eg:
However, dhcpcd on the WAN doesn't parse the RTA_SRC route attribute, so it adds the route to it's internal kernel route list as the "default route" - the net effect being that if the upstream router removes and adds back the real default route (eg. ISP goes down momentarily), dhcpcd will think these source-routes are existing kernel default routes, and won't add the new default route back.
This simple pull request patches if-linux.c to parse the RTA_SRC attribute for a "specified" route and ignores the route, after which dhcpcd behaves correctly.
I haven't modified the code for other platforms as I'm not sure how/if they support source-based routing :)
Please feel free to use a totally different method to exclude source routes if you feel this is "kludgy"...