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

FS#2206 - odhcpd: lifetime of routes announced by RA probably shouldn't exceed ra_lifetime ? #7121

Closed
openwrt-bot opened this issue Mar 27, 2019 · 0 comments
Labels

Comments

@openwrt-bot
Copy link

davidmonro:

Found on openwrt 18.06.2, r7676-cddd7b4c77, x86_64 build but I don't think that matters.

There's code in odhcpd which publishes non-default routes via RA if there's no default route (assuming ra_default is set to 0 in /etc/config/dhcp), which in particular will include the whole ULA prefix.
This is a good thing, since it allows ULA traffic to continue to work between different interfaces on the router even with the upstream link is dead.
However, when it publishes the route, it uses the lifetime of the route in the kernel table, which for the ULA prefix will be infinite.
At first glance this seems fair enough, but when a route is published with an infinite lifetime, it means that even if this router goes away (or changes its IP address) downstream clients will still have the route through this router interface address in their tables.
Which causes problems if the router is replaced with a different one, as now the clients will have a route through both the new one and the dead one in their tables and the result of that is not generally very good.

Surely the maximum time we should publish a route for would be smaller of the validity of the route (infinite in this case), and the ra_lifetime interval?

I think the offending line is line 818 of router.c (at least as of commit 6d23385242c918b0e00f5e21ed41dd655905752b):

routes[routes_cnt].lifetime = htonl(TIME_LEFT(addr->valid, now));

which could probably be fixed by adding something like
if (calc_ra_lifetime(iface,maxival) < TIME_LEFT(addr->valid, now)) {
routes[routes_cnt].lifetime = htons(calc_ra_lifetime(iface,maxival));
} else {
routes[routes_cnt].lifetime = htonl(TIME_LEFT(addr->valid, now));
}

(although it might make sense to store the calc_ra_lifetime() result in a variable rather than calling it for every route).
Cheers

David

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant