]> git.ipfire.org Git - thirdparty/bird.git/commitdiff
KRT: Fix route learn scan when route changed
authorOndrej Zajicek (work) <santiago@crfreenet.org>
Wed, 23 Mar 2016 17:25:15 +0000 (18:25 +0100)
committerOndrej Zajicek (work) <santiago@crfreenet.org>
Wed, 6 Apr 2016 09:46:25 +0000 (11:46 +0200)
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.

nest/route.h
sysdep/bsd/krt-sock.c
sysdep/linux/netlink.c
sysdep/unix/krt.c

index c435b9e0758d43c7efbac5b6e27fe9229dcfc8c9..9368808fb5d48ff9dc8e039e0ea7d3a68729bbec 100644 (file)
@@ -223,8 +223,8 @@ typedef struct rte {
     struct {                           /* Routes generated by krt sync (both temporary and inherited ones) */
       s8 src;                          /* Alleged route source (see krt.h) */
       u8 proto;                                /* Kernel source protocol ID */
-      u8 type;                         /* Kernel route type */
       u8 seen;                         /* Seen during last scan */
+      u8 best;                         /* Best route in network, propagated to core */
       u32 metric;                      /* Kernel metric */
     } krt;
   } u;
index 29203d1b8168a4ea5d76637d43d7d2a73f765df9..6ff3b2b7363cb4ed0d2126a7e258df891bdbb109 100644 (file)
@@ -493,9 +493,8 @@ krt_read_route(struct ks_msg *msg, struct krt_proto *p, int scan)
   e->net = net;
   e->u.krt.src = src;
   e->u.krt.proto = src2;
-
-  /* These are probably too Linux-specific */
-  e->u.krt.type = 0;
+  e->u.krt.seen = 0;
+  e->u.krt.best = 0;
   e->u.krt.metric = 0;
 
   if (scan)
index 640d18772acf4c4245bbf9e749f441a0bab58a6a..1ffdff07adc5130a7d9679122d4e8900cd952fa7 100644 (file)
@@ -1102,7 +1102,8 @@ nl_parse_route(struct nlmsghdr *h, int scan)
   e->net = net;
   e->u.krt.src = src;
   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;
 
   if (a[RTA_PRIORITY])
index 5e78586bfec9fb27bc5a462c9153b3c052cb9d7d..f5dee877575f6738f007404fe2b4d2f0398d3739 100644 (file)
@@ -417,46 +417,58 @@ again:
       net *n = (net *) f;
       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;
       pbest = NULL;
       ee = &n->routes;
       while (e = *ee)
        {
+         if (e->u.krt.best)
+           old_best = e;
+
          if (!e->u.krt.seen)
            {
              *ee = e->next;
              rte_free(e);
              continue;
            }
+
          if (!best || best->u.krt.metric > e->u.krt.metric)
            {
              best = e;
              pbest = ee;
            }
+
          e->u.krt.seen = 0;
+         e->u.krt.best = 0;
          ee = &e->next;
        }
       if (!n->routes)
        {
          DBG("%I/%d: deleting\n", n->n.prefix, n->n.pxlen);
          if (old_best)
-           {
-             krt_learn_announce_delete(p, n);
-             n->n.flags &= ~KRF_INSTALLED;
-           }
+           krt_learn_announce_delete(p, n);
+
          FIB_ITERATE_PUT(&fit, f);
          fib_delete(fib, f);
          goto again;
        }
+
+      best->u.krt.best = 1;
       *pbest = best->next;
       best->next = n->routes;
       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);
          krt_learn_announce_update(p, best);
-         n->n.flags |= KRF_INSTALLED;
        }
       else
        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;
   bestp = &n->routes;
   for(gg=&n->routes; g=*gg; gg=&g->next)
+  {
     if (best->u.krt.metric > g->u.krt.metric)
       {
        best = g;
        bestp = gg;
       }
+
+    g->u.krt.best = 0;
+  }
+
   if (best)
     {
+      best->u.krt.best = 1;
       *bestp = best->next;
       best->next = n->routes;
       n->routes = best;
     }
+
   if (best != old_best)
     {
       DBG("krt_learn_async: distributing change\n");
       if (best)
-       {
-         krt_learn_announce_update(p, best);
-         n->n.flags |= KRF_INSTALLED;
-       }
+       krt_learn_announce_update(p, best);
       else
-       {
-         n->routes = NULL;
-         krt_learn_announce_delete(p, n);
-         n->n.flags &= ~KRF_INSTALLED;
-       }
+       krt_learn_announce_delete(p, n);
     }
 }
 
@@ -564,7 +576,7 @@ krt_dump(struct proto *P)
 static void
 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