bgpd: Fix route install upon non-best nexthop change

After BGP path selection, even if the best route entry selected has not
changed, ensure that the route is installed again in zebra if any non-best
but multipath route entry has a nexthop resolution change.

In the absence of this fix, if a non-best multipath route entry had a
nexthop resolution change (such as being resolved over two first hops instead
of one), the route would get reinstalled into zebra only in some situations
(i.e., when the best route entry had its IGP change flag set). If the route
does not get reinstalled by BGP, the corresponding route in the zebra RIB
would not have all the first hops.

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by:   Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by:   Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by:   Sid Khot <sidkhot@cumulusnetworks.com>
Ticket: CM-12390
Reviewed By: CCR-5134
Testing Done: Manual, bgp-smoke

(cherry picked from commit 3064bf43a7d8162dadada2934132f915a45d2bcb)

diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c
index 9d8d8f3b527ce0dafeb450e268f461804a0e7782..23c64731aa058661e21ca0027096933c4b254fd6 100644 (file)
--- a/bgpd/bgp_nht.c
+++ b/bgpd/bgp_nht.c
@@ -681,8 +681,6 @@ evaluate_paths (struct bgp_nexthop_cache *bnc)
       if (CHECK_FLAG(bnc->change_flags, BGP_NEXTHOP_METRIC_CHANGED) ||
          CHECK_FLAG(bnc->change_flags, BGP_NEXTHOP_CHANGED))
        SET_FLAG(path->flags, BGP_INFO_IGP_CHANGED);
-      else
-        UNSET_FLAG (path->flags, BGP_INFO_IGP_CHANGED);
 
       bgp_process(bgp, rn, afi, SAFI_UNICAST);
     }

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index f47e2f607437d9a11731e4b3488b27ebec4f78b2..26b5ef86d6165ffd20833da61eb6538994188b23 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -1657,7 +1657,7 @@ bgp_best_selection (struct bgp *bgp, struct bgp_node *rn,
               if (debug)
                 zlog_debug("%s: %s is the bestpath, add to the multipath list",
                            pfx_buf, path_buf);
-             bgp_mp_list_add (&mp_list, ri);
+              bgp_mp_list_add (&mp_list, ri);
               continue;
             }
 
@@ -1749,6 +1749,50 @@ subgroup_process_announce_selected (struct update_subgroup *subgrp,
   return 0;
 }
 
+/*
+ * Clear IGP changed flag for a route (all paths). This is called at
+ * the end of route processing.
+ */
+static void
+bgp_zebra_clear_route_change_flags (struct bgp_node *rn)
+{
+  struct bgp_info *ri;
+
+  for (ri = rn->info; ri; ri = ri->next)
+    {
+       if (BGP_INFO_HOLDDOWN (ri))
+          continue;
+        UNSET_FLAG (ri->flags, BGP_INFO_IGP_CHANGED);
+    }
+}
+
+/*
+ * Has the route changed from the RIB's perspective? This is invoked only
+ * if the route selection returns the same best route as earlier - to
+ * determine if we need to update zebra or not.
+ */
+static int
+bgp_zebra_has_route_changed (struct bgp_node *rn, struct bgp_info *selected)
+{
+  struct bgp_info *mpinfo;
+
+  /* There is a multipath change or best path has some nexthop change. */
+  if (CHECK_FLAG (selected->flags, BGP_INFO_IGP_CHANGED) ||
+      CHECK_FLAG (selected->flags, BGP_INFO_MULTIPATH_CHG))
+    return 1;
+
+  /* If this is multipath, check all selected paths for any nexthop change */
+  for (mpinfo = bgp_info_mpath_first (selected); mpinfo;
+       mpinfo = bgp_info_mpath_next (mpinfo))
+    {
+      if (CHECK_FLAG (mpinfo->flags, BGP_INFO_IGP_CHANGED))
+        return 1;
+    }
+
+  /* Nothing has changed from the RIB's perspective. */
+  return 0;
+}
+
 struct bgp_process_queue
 {
   struct bgp *bgp;
@@ -1799,11 +1843,11 @@ bgp_process_main (struct work_queue *wq, void *data)
       !CHECK_FLAG(old_select->flags, BGP_INFO_ATTR_CHANGED) &&
       !bgp->addpath_tx_used[afi][safi])
     {
-      if (CHECK_FLAG (old_select->flags, BGP_INFO_IGP_CHANGED) ||
-          CHECK_FLAG (old_select->flags, BGP_INFO_MULTIPATH_CHG))
+      if (bgp_zebra_has_route_changed (rn, old_select))
         bgp_zebra_announce (p, old_select, bgp, afi, safi);
           
       UNSET_FLAG (old_select->flags, BGP_INFO_MULTIPATH_CHG);
+      bgp_zebra_clear_route_change_flags (rn);
       UNSET_FLAG (rn->flags, BGP_NODE_PROCESS_SCHEDULED);
       return WQ_SUCCESS;
     }
@@ -1856,7 +1900,10 @@ bgp_process_main (struct work_queue *wq, void *data)
            bgp_zebra_withdraw (p, old_select, safi);
        }
     }
-    
+
+  /* Clear any route change flags. */
+  bgp_zebra_clear_route_change_flags (rn);
+
   /* Reap old select bgp_info, if it has been removed */
   if (old_select && CHECK_FLAG (old_select->flags, BGP_INFO_REMOVED))
     bgp_info_reap (rn, old_select);