On Tue, Feb 25, 2014 at 06:59:10PM +0000, Pierluigi Rolando wrote:
Hi,
This patch fixes a situation where a route that becomes unviable because of a configuration change (i.e. next hop of its new version is unreachable) is marked as not installed internally, but actually remains in the kernel routing table until a subsequent configuration change makes the next hop reachable again.
The code that updates static routes seems to process changes as deletions + insertions, but if the insertion can't happen for any reason the deletion won't happen as well, even though the route is already marked as uninstalled by then. When that happens BIRD and the kernel get out of sync until BIRD tries to install the route again, which might be never.
The patch is supposed to fix the problem by detecting routes that (a) have changed but (b) are not getting deleted because their updated version can't get installed, and deletes them anyway. There's also a quick sanity check to make sure we don't end up with 3 states for the installed flag (which should obviously be a base 2 boolean), even though 3 states are needed temporarily when processing updates.
Thanks, the analysis and the patch is more-or-less correct, with one minor note:
for (r2 = r->mp_next; r2; r2 = r2->mp_next) - if (r2->installed) + if (r2->installed > 0) { struct mpnh *nh = alloca(sizeof(struct mpnh)); nh->gw = r2->via;
No need for this change, secondary multipath nodes from mp_next cannot be -1, they are not modified in static_match() but in static_add() and the meaning is slightly different. -- Elen sila lumenn' omentielvo Ondrej 'SanTiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."