KRT: Fix route learn scan when route changed

When a kernel route changed, function krt_learn_scan() noticed that and
replaced the route in internal kernel FIB, but after that, function
krt_learn_prune() failed to propagate the new route to the nest, because
it confused the new route with the (removed) old best route and decided
that the best route did not changed.

Wow, the original code (and the bug) is almost 17 years old.
This commit is contained in:
Ondrej Zajicek (work) 2016-03-23 18:25:15 +01:00
parent ea0a8be2ff
commit e86cfd41d9
4 changed files with 34 additions and 22 deletions

View file

@ -223,8 +223,8 @@ typedef struct rte {
struct { /* Routes generated by krt sync (both temporary and inherited ones) */ struct { /* Routes generated by krt sync (both temporary and inherited ones) */
s8 src; /* Alleged route source (see krt.h) */ s8 src; /* Alleged route source (see krt.h) */
u8 proto; /* Kernel source protocol ID */ u8 proto; /* Kernel source protocol ID */
u8 type; /* Kernel route type */
u8 seen; /* Seen during last scan */ u8 seen; /* Seen during last scan */
u8 best; /* Best route in network, propagated to core */
u32 metric; /* Kernel metric */ u32 metric; /* Kernel metric */
} krt; } krt;
} u; } u;

View file

@ -493,9 +493,8 @@ krt_read_route(struct ks_msg *msg, struct krt_proto *p, int scan)
e->net = net; e->net = net;
e->u.krt.src = src; e->u.krt.src = src;
e->u.krt.proto = src2; e->u.krt.proto = src2;
e->u.krt.seen = 0;
/* These are probably too Linux-specific */ e->u.krt.best = 0;
e->u.krt.type = 0;
e->u.krt.metric = 0; e->u.krt.metric = 0;
if (scan) if (scan)

View file

@ -1102,7 +1102,8 @@ nl_parse_route(struct nlmsghdr *h, int scan)
e->net = net; e->net = net;
e->u.krt.src = src; e->u.krt.src = src;
e->u.krt.proto = i->rtm_protocol; e->u.krt.proto = i->rtm_protocol;
e->u.krt.type = i->rtm_type; e->u.krt.seen = 0;
e->u.krt.best = 0;
e->u.krt.metric = 0; e->u.krt.metric = 0;
if (a[RTA_PRIORITY]) if (a[RTA_PRIORITY])

View file

@ -417,46 +417,58 @@ again:
net *n = (net *) f; net *n = (net *) f;
rte *e, **ee, *best, **pbest, *old_best; rte *e, **ee, *best, **pbest, *old_best;
old_best = n->routes; /*
* Note that old_best may be NULL even if there was an old best route in
* the previous step, because it might be replaced in krt_learn_scan().
* But in that case there is a new valid best route.
*/
old_best = NULL;
best = NULL; best = NULL;
pbest = NULL; pbest = NULL;
ee = &n->routes; ee = &n->routes;
while (e = *ee) while (e = *ee)
{ {
if (e->u.krt.best)
old_best = e;
if (!e->u.krt.seen) if (!e->u.krt.seen)
{ {
*ee = e->next; *ee = e->next;
rte_free(e); rte_free(e);
continue; continue;
} }
if (!best || best->u.krt.metric > e->u.krt.metric) if (!best || best->u.krt.metric > e->u.krt.metric)
{ {
best = e; best = e;
pbest = ee; pbest = ee;
} }
e->u.krt.seen = 0; e->u.krt.seen = 0;
e->u.krt.best = 0;
ee = &e->next; ee = &e->next;
} }
if (!n->routes) if (!n->routes)
{ {
DBG("%I/%d: deleting\n", n->n.prefix, n->n.pxlen); DBG("%I/%d: deleting\n", n->n.prefix, n->n.pxlen);
if (old_best) if (old_best)
{
krt_learn_announce_delete(p, n); krt_learn_announce_delete(p, n);
n->n.flags &= ~KRF_INSTALLED;
}
FIB_ITERATE_PUT(&fit, f); FIB_ITERATE_PUT(&fit, f);
fib_delete(fib, f); fib_delete(fib, f);
goto again; goto again;
} }
best->u.krt.best = 1;
*pbest = best->next; *pbest = best->next;
best->next = n->routes; best->next = n->routes;
n->routes = best; n->routes = best;
if (best != old_best || !(n->n.flags & KRF_INSTALLED) || p->reload)
if ((best != old_best) || p->reload)
{ {
DBG("%I/%d: announcing (metric=%d)\n", n->n.prefix, n->n.pxlen, best->u.krt.metric); DBG("%I/%d: announcing (metric=%d)\n", n->n.prefix, n->n.pxlen, best->u.krt.metric);
krt_learn_announce_update(p, best); krt_learn_announce_update(p, best);
n->n.flags |= KRF_INSTALLED;
} }
else else
DBG("%I/%d: uptodate (metric=%d)\n", n->n.prefix, n->n.pxlen, best->u.krt.metric); DBG("%I/%d: uptodate (metric=%d)\n", n->n.prefix, n->n.pxlen, best->u.krt.metric);
@ -515,31 +527,31 @@ krt_learn_async(struct krt_proto *p, rte *e, int new)
best = n->routes; best = n->routes;
bestp = &n->routes; bestp = &n->routes;
for(gg=&n->routes; g=*gg; gg=&g->next) for(gg=&n->routes; g=*gg; gg=&g->next)
{
if (best->u.krt.metric > g->u.krt.metric) if (best->u.krt.metric > g->u.krt.metric)
{ {
best = g; best = g;
bestp = gg; bestp = gg;
} }
g->u.krt.best = 0;
}
if (best) if (best)
{ {
best->u.krt.best = 1;
*bestp = best->next; *bestp = best->next;
best->next = n->routes; best->next = n->routes;
n->routes = best; n->routes = best;
} }
if (best != old_best) if (best != old_best)
{ {
DBG("krt_learn_async: distributing change\n"); DBG("krt_learn_async: distributing change\n");
if (best) if (best)
{
krt_learn_announce_update(p, best); krt_learn_announce_update(p, best);
n->n.flags |= KRF_INSTALLED;
}
else else
{
n->routes = NULL;
krt_learn_announce_delete(p, n); krt_learn_announce_delete(p, n);
n->n.flags &= ~KRF_INSTALLED;
}
} }
} }
@ -564,7 +576,7 @@ krt_dump(struct proto *P)
static void static void
krt_dump_attrs(rte *e) krt_dump_attrs(rte *e)
{ {
debug(" [m=%d,p=%d,t=%d]", e->u.krt.metric, e->u.krt.proto, e->u.krt.type); debug(" [m=%d,p=%d]", e->u.krt.metric, e->u.krt.proto);
} }
#endif #endif