From e86cfd41d975122cc944db68383aef4028da9575 Mon Sep 17 00:00:00 2001
From: "Ondrej Zajicek (work)" <santiago@crfreenet.org>
Date: Wed, 23 Mar 2016 18:25:15 +0100
Subject: 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.
---
 nest/route.h           |  2 +-
 sysdep/bsd/krt-sock.c  |  5 ++---
 sysdep/linux/netlink.c |  3 ++-
 sysdep/unix/krt.c      | 46 +++++++++++++++++++++++++++++-----------------
 4 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/nest/route.h b/nest/route.h
index c435b9e0..9368808f 100644
--- a/nest/route.h
+++ b/nest/route.h
@@ -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;
diff --git a/sysdep/bsd/krt-sock.c b/sysdep/bsd/krt-sock.c
index 29203d1b..6ff3b2b7 100644
--- a/sysdep/bsd/krt-sock.c
+++ b/sysdep/bsd/krt-sock.c
@@ -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)
diff --git a/sysdep/linux/netlink.c b/sysdep/linux/netlink.c
index 640d1877..1ffdff07 100644
--- a/sysdep/linux/netlink.c
+++ b/sysdep/linux/netlink.c
@@ -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])
diff --git a/sysdep/unix/krt.c b/sysdep/unix/krt.c
index 5e78586b..f5dee877 100644
--- a/sysdep/unix/krt.c
+++ b/sysdep/unix/krt.c
@@ -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
-- 
cgit v1.2.3