OpenWrt/LEDE Project

  • Status Closed
  • Percent Complete
  • Task Type Bug Report
  • Category Base system
  • Assigned To
    Hans Dedecker
  • Operating System All
  • Severity Medium
  • Priority Very Low
  • Reported Version Trunk
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: OpenWrt/LEDE Project
Opened by davidmonro - 27.03.2019
Last edited by Hans Dedecker - 17.04.2019

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

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).


Closed by  Hans Dedecker
17.04.2019 12:37
Reason for closing:  Fixed
Additional comments about closing:  

Fixed in commit https://git.op;a=commit ;h=38bc630be6816eb5bd0a368437c97ee1aba3e e56


Available keyboard shortcuts


Task Details

Task Editing