From: Maria Matejka <mq@ucw.cz>
Date: Thu, 14 Jul 2022 09:09:23 +0000 (+0200)
Subject: Fixed invalid routes handling
X-Git-Tag: v3.0-alpha1~171^2~15
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1c2851ecfa94f3d0b732a267c6c2db8b817c37f4;p=thirdparty%2Fbird.git

Fixed invalid routes handling

The invalid routes were filtered out before they could ever get
exported, yet some of the routines need them available, e.g. for
display or import reload.

Now the invalid routes are properly exported and dropped in channel
export routines instead.
---

diff --git a/nest/rt-table.c b/nest/rt-table.c
index e7281b2e4..5e07c1296 100644
--- a/nest/rt-table.c
+++ b/nest/rt-table.c
@@ -726,8 +726,8 @@ rte_feed_count(net *n)
 {
   uint count = 0;
   for (struct rte_storage *e = n->routes; e; e = e->next)
-    if (rte_is_valid(RTE_OR_NULL(e)))
-      count++;
+    count++;
+
   return count;
 }
 
@@ -736,11 +736,11 @@ rte_feed_obtain(net *n, struct rte **feed, uint count)
 {
   uint i = 0;
   for (struct rte_storage *e = n->routes; e; e = e->next)
-    if (rte_is_valid(RTE_OR_NULL(e)))
     {
       ASSERT_DIE(i < count);
       feed[i++] = &e->rte;
     }
+
   ASSERT_DIE(i == count);
 }
 
@@ -1059,10 +1059,15 @@ void
 rt_notify_optimal(struct rt_export_request *req, const net_addr *net, struct rt_pending_export *rpe)
 {
   struct channel *c = SKIP_BACK(struct channel, out_req, req);
-  rte n0;
 
   if (rpe->new_best != rpe->old_best)
-    rt_notify_basic(c, net, RTE_COPY(rpe->new_best, &n0), RTE_OR_NULL(rpe->old_best));
+  {
+    rte n0 = RTE_COPY_VALID(rpe->new_best);
+    rte *o = RTE_VALID_OR_NULL(rpe->old_best);
+
+    if (n0.src || o)
+      rt_notify_basic(c, net, n0.src ? &n0 : NULL, o);
+  }
 
   /* Drop the old stored rejection if applicable.
    * new->id == old->id happens when updating hostentries. */
@@ -1074,10 +1079,14 @@ void
 rt_notify_any(struct rt_export_request *req, const net_addr *net, struct rt_pending_export *rpe)
 {
   struct channel *c = SKIP_BACK(struct channel, out_req, req);
-  rte n0;
 
   if (rpe->new != rpe->old)
-    rt_notify_basic(c, net, RTE_COPY(rpe->new, &n0), RTE_OR_NULL(rpe->old));
+  {
+    rte n0 = RTE_COPY_VALID(rpe->new);
+    rte *o = RTE_VALID_OR_NULL(rpe->old);
+    if (n0.src || o)
+      rt_notify_basic(c, net, n0.src ? &n0 : NULL, o);
+  }
 
   /* Drop the old stored rejection if applicable.
    * new->id == old->id happens when updating hostentries. */
@@ -1091,10 +1100,11 @@ rt_feed_any(struct rt_export_request *req, const net_addr *net, struct rt_pendin
   struct channel *c = SKIP_BACK(struct channel, out_req, req);
 
   for (uint i=0; i<count; i++)
-  {
-    rte n0 = *feed[i];
-    rt_notify_basic(c, net, &n0, NULL);
-  }
+    if (rte_is_valid(feed[i]))
+    {
+      rte n0 = *feed[i];
+      rt_notify_basic(c, net, &n0, NULL);
+    }
 }
 
 /**
@@ -1132,26 +1142,17 @@ static void
 rte_announce(rtable *tab, net *net, struct rte_storage *new, struct rte_storage *old,
 	     struct rte_storage *new_best, struct rte_storage *old_best)
 {
-  if (!rte_is_valid(RTE_OR_NULL(new)))
-    new = NULL;
-
-  if (!rte_is_valid(RTE_OR_NULL(old)))
-    old = NULL;
-
-  if (!rte_is_valid(RTE_OR_NULL(new_best)))
-    new_best = NULL;
-
-  if (!rte_is_valid(RTE_OR_NULL(old_best)))
-    old_best = NULL;
+  int new_best_valid = rte_is_valid(RTE_OR_NULL(new_best));
+  int old_best_valid = rte_is_valid(RTE_OR_NULL(old_best));
 
   if (!new && !old && !new_best && !old_best)
     return;
 
-  if (new_best != old_best)
+  if (new_best_valid || old_best_valid)
   {
-    if (new_best)
+    if (new_best_valid)
       new_best->rte.sender->stats.pref++;
-    if (old_best)
+    if (old_best_valid)
       old_best->rte.sender->stats.pref--;
 
     if (tab->hostcache)
diff --git a/nest/rt.h b/nest/rt.h
index b13c06bea..bdbea05b4 100644
--- a/nest/rt.h
+++ b/nest/rt.h
@@ -167,8 +167,10 @@ struct rte_storage {
   struct rte rte;			/* Route data */
 };
 
-#define RTE_COPY(r, l) ((r) ? (((*(l)) = (r)->rte), (l)) : NULL)
-#define RTE_OR_NULL(r) ((r) ? &((r)->rte) : NULL)
+#define RTE_COPY(r)		((r) ? (r)->rte : (rte) {})
+#define RTE_COPY_VALID(r)	(((r) && (rte_is_valid(&(r)->rte))) ? (r)->rte : (rte) {})
+#define RTE_OR_NULL(r)		((r) ? &((r)->rte) : NULL)
+#define RTE_VALID_OR_NULL(r)	(((r) && (rte_is_valid(&(r)->rte))) ? &((r)->rte) : NULL)
 
 /* Table-channel connections */
 
diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index 1ca77fd57..883a97462 100644
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -2557,12 +2557,10 @@ bgp_rte_modify_stale(struct rt_export_request *req, const net_addr *n, struct rt
   {
     rte *r = feed[i];
 
-    /* Not our route */
-    if (r->sender != irh)
-      continue;
-
-    /* A new route, do not mark as stale */
-    if (r->stale_cycle == irh->stale_set)
+    if (
+	!rte_is_valid(r) ||			/* Not a valid route */
+	(r->sender != irh) ||			/* Not our route */
+	(r->stale_cycle == irh->stale_set))	/* A new route, do not mark as stale */
       continue;
 
     eattr *ea = ea_find(r->attrs, BGP_EA_ID(BA_COMMUNITY));