Merge pull request #4746 from donaldsharp/zebra_rib_improvements

Zebra rib improvements

tests/topotests/bfd-topo2/r1/ipv4_routes.json

@@ -8,7 +8,7 @@
             "selected": true,
             "installed": true,
             "prefix": "10.0.3.0/24",
-            "internalStatus": 34,
+            "internalStatus": 16,
             "nexthops": [
                 {
                     "interfaceName": "r1-eth0",
@@ -30,7 +30,7 @@
             "selected": true,
             "installed": true,
             "prefix": "10.254.254.2/32",
-            "internalStatus": 34,
+            "internalStatus": 16,
             "nexthops": [
                 {
                     "interfaceName": "r1-eth0",
@@ -52,7 +52,7 @@
             "selected": true,
             "installed": true,
             "prefix": "10.254.254.1/32",
-            "internalStatus": 32,
+            "internalStatus": 16,
             "nexthops": [
                 {
                     "directlyConnected": true,

tests/topotests/bfd-topo2/r1/ipv6_routes.json

@@ -8,7 +8,7 @@
       "selected": true,
       "installed": true,
       "prefix": "2001:db8:4::/64",
-      "internalStatus": 34,
+      "internalStatus": 16,
       "nexthops": [
         {
           "interfaceName": "r1-eth0",
@@ -27,7 +27,7 @@
       "protocol": "bgp",
       "internalFlags": 0,
       "metric": 0,
-      "internalStatus": 2,
+      "internalStatus": 0,
       "prefix": "2001:db8:1::/64",
       "nexthops": [
         {
@@ -47,7 +47,7 @@
       "selected": true,
       "installed": true,
       "prefix": "2001:db8:1::/64",
-      "internalStatus": 32,
+      "internalStatus": 16,
       "nexthops": [
         {
           "directlyConnected": true,

tests/topotests/bfd-topo2/r2/ipv4_routes.json

@@ -5,7 +5,7 @@
       "protocol": "ospf",
       "internalFlags": 0,
       "metric": 10,
-      "internalStatus": 2,
+      "internalStatus": 0,
       "prefix": "10.0.3.0/24",
       "nexthops": [
         {
@@ -25,7 +25,7 @@
       "selected": true,
       "installed": true,
       "prefix": "10.0.3.0/24",
-      "internalStatus": 32,
+      "internalStatus": 16,
       "nexthops": [
         {
           "directlyConnected": true,
@@ -47,7 +47,7 @@
       "selected": true,
       "installed": true,
       "prefix": "10.254.254.3/32",
-      "internalStatus": 34,
+      "internalStatus": 16,
       "nexthops": [
         {
           "interfaceName": "r2-eth1",
@@ -70,7 +70,7 @@
       "selected": true,
       "installed": true,
       "prefix": "10.254.254.2/32",
-      "internalStatus": 32,
+      "internalStatus": 16,
       "nexthops": [
         {
           "directlyConnected": true,
@@ -92,7 +92,7 @@
       "selected": true,
       "installed": true,
       "prefix": "10.254.254.1/32",
-      "internalStatus": 34,
+      "internalStatus": 16,
       "nexthops": [
         {
           "interfaceName": "r2-eth0",

tests/topotests/bfd-topo2/r2/ipv6_routes.json

@@ -5,7 +5,7 @@
       "protocol": "ospf6",
       "internalFlags": 0,
       "metric": 10,
-      "internalStatus": 2,
+      "internalStatus": 0,
       "prefix": "2001:db8:4::/64",
       "nexthops": [
         {
@@ -25,7 +25,7 @@
       "selected": true,
       "installed": true,
       "prefix": "2001:db8:4::/64",
-      "internalStatus": 32,
+      "internalStatus": 16,
       "nexthops": [
         {
           "directlyConnected": true,
@@ -47,7 +47,7 @@
       "selected": true,
       "installed": true,
       "prefix": "2001:db8:1::/64",
-      "internalStatus": 32,
+      "internalStatus": 16,
       "nexthops": [
         {
           "directlyConnected": true,

tests/topotests/bfd-topo2/r3/ipv4_routes.json

@@ -25,7 +25,7 @@
       "selected": true,
       "installed": true,
       "prefix": "10.0.3.0/24",
-      "internalStatus": 32,
+      "internalStatus": 16,
       "nexthops": [
         {
           "directlyConnected": true,
@@ -47,7 +47,7 @@
       "selected": true,
       "installed": true,
       "prefix": "10.254.254.3/32",
-      "internalStatus": 32,
+      "internalStatus": 16,
       "nexthops": [
         {
           "directlyConnected": true,
@@ -69,7 +69,7 @@
       "selected": true,
       "installed": true,
       "prefix": "10.254.254.2/32",
-      "internalStatus": 34,
+      "internalStatus": 16,
       "nexthops": [
         {
           "interfaceName": "r3-eth0",
@@ -92,7 +92,7 @@
       "selected": true,
       "installed": true,
       "prefix": "10.254.254.1/32",
-      "internalStatus": 34,
+      "internalStatus": 16,
       "nexthops": [
         {
           "interfaceName": "r3-eth0",

tests/topotests/bfd-topo2/r4/ipv4_routes.json

@@ -8,7 +8,7 @@
       "selected": true,
       "installed": true,
       "prefix": "10.254.254.4/32",
-      "internalStatus": 32,
+      "internalStatus": 16,
       "nexthops": [
         {
           "directlyConnected": true,

tests/topotests/bfd-topo2/r4/ipv6_routes.json

@@ -5,7 +5,7 @@
       "protocol": "ospf6",
       "internalFlags": 0,
       "metric": 10,
-      "internalStatus": 2,
+      "internalStatus": 0,
       "prefix": "2001:db8:4::/64",
       "nexthops": [
         {
@@ -25,7 +25,7 @@
       "selected": true,
       "installed": true,
       "prefix": "2001:db8:4::/64",
-      "internalStatus": 32,
+      "internalStatus": 16,
       "nexthops": [
         {
           "directlyConnected": true,
@@ -47,7 +47,7 @@
       "selected": true,
       "installed": true,
       "prefix": "2001:db8:1::/64",
-      "internalStatus": 34,
+      "internalStatus": 16,
       "nexthops": [
         {
           "interfaceName": "r4-eth0",

tests/topotests/bgp_vrf_lite_ipv6_rtadv/r1/ipv4_routes.json

@@ -9,7 +9,7 @@
             "distance": 20,
             "metric": 0,
             "installed": true,
-            "internalStatus": 34,
+            "internalStatus": 16,
             "internalFlags": 8,
             "nexthops": [
                 {
@@ -33,7 +33,7 @@
             "distance": 0,
             "metric": 0,
             "installed": true,
-            "internalStatus": 32,
+            "internalStatus": 16,
             "internalFlags": 8,
             "nexthops": [
                 {

tests/topotests/bgp_vrf_lite_ipv6_rtadv/r1/ipv6_routes.json

@@ -6,7 +6,7 @@
       "vrfId":3,
       "distance": 20,
       "metric": 0,
-      "internalStatus": 2,
+      "internalStatus": 0,
       "internalFlags": 0,
       "nexthops": [
         {
@@ -27,7 +27,7 @@
       "distance": 0,
       "metric": 0,
       "installed": true,
-      "internalStatus": 32,
+      "internalStatus": 16,
       "internalFlags": 8,
       "nexthops": [
         {

tests/topotests/bgp_vrf_lite_ipv6_rtadv/r2/ipv4_routes.json

@@ -9,7 +9,7 @@
       "distance": 0,
       "metric": 0,
       "installed": true,
-      "internalStatus": 32,
+      "internalStatus": 16,
       "internalFlags": 8,
        "nexthops": [
         {
@@ -33,7 +33,7 @@
       "distance": 20,
       "metric": 0,
       "installed": true,
-      "internalStatus": 34,
+      "internalStatus": 16,
       "internalFlags": 8,
       "nexthops": [
         {

tests/topotests/bgp_vrf_lite_ipv6_rtadv/r2/ipv6_routes.json

@@ -9,7 +9,7 @@
       "distance": 0,
       "metric": 0,
       "installed": true,
-      "internalStatus": 32,
+      "internalStatus": 16,
       "internalFlags": 8,
       "nexthops": [
         {

zebra/connected.c

@@ -260,16 +260,6 @@ void connected_up(struct interface *ifp, struct connected *ifc)
 	rib_add(afi, SAFI_MULTICAST, zvrf->vrf->vrf_id, ZEBRA_ROUTE_CONNECT,
 		0, 0, &p, NULL, &nh, zvrf->table_id, metric, 0, 0, 0);
 
-	if (IS_ZEBRA_DEBUG_RIB_DETAILED) {
-		char buf[PREFIX_STRLEN];
-
-		zlog_debug(
-			"%u: IF %s address %s add/up, scheduling RIB processing",
-			ifp->vrf_id, ifp->name,
-			prefix2str(&p, buf, sizeof(buf)));
-	}
-	rib_update(zvrf->vrf->vrf_id, RIB_UPDATE_IF_CHANGE);
-
 	/* Schedule LSP forwarding entries for processing, if appropriate. */
 	if (zvrf->vrf->vrf_id == VRF_DEFAULT) {
 		if (IS_ZEBRA_DEBUG_MPLS) {
@@ -433,17 +423,6 @@ void connected_down(struct interface *ifp, struct connected *ifc)
 	rib_delete(afi, SAFI_MULTICAST, zvrf->vrf->vrf_id, ZEBRA_ROUTE_CONNECT,
 		   0, 0, &p, NULL, &nh, zvrf->table_id, 0, 0, false);
 
-	if (IS_ZEBRA_DEBUG_RIB_DETAILED) {
-		char buf[PREFIX_STRLEN];
-
-		zlog_debug(
-			"%u: IF %s IP %s address down, scheduling RIB processing",
-			zvrf->vrf->vrf_id, ifp->name,
-			prefix2str(&p, buf, sizeof(buf)));
-	}
-
-	rib_update(zvrf->vrf->vrf_id, RIB_UPDATE_IF_CHANGE);
-
 	/* Schedule LSP forwarding entries for processing, if appropriate. */
 	if (zvrf->vrf->vrf_id == VRF_DEFAULT) {
 		if (IS_ZEBRA_DEBUG_MPLS) {
@@ -468,16 +447,6 @@ static void connected_delete_helper(struct connected *ifc, struct prefix *p)
 
 	connected_withdraw(ifc);
 
-	if (IS_ZEBRA_DEBUG_RIB_DETAILED) {
-		char buf[PREFIX_STRLEN];
-
-		zlog_debug(
-			"%u: IF %s IP %s address del, scheduling RIB processing",
-			ifp->vrf_id, ifp->name,
-			prefix2str(p, buf, sizeof(buf)));
-	}
-	rib_update(ifp->vrf_id, RIB_UPDATE_IF_CHANGE);
-
 	/* Schedule LSP forwarding entries for processing, if appropriate. */
 	if (ifp->vrf_id == VRF_DEFAULT) {
 		if (IS_ZEBRA_DEBUG_MPLS) {

zebra/interface.c

@@ -799,15 +799,6 @@ void if_handle_vrf_change(struct interface *ifp, vrf_id_t vrf_id)
 	/* Install connected routes (in new VRF). */
 	if (if_is_operative(ifp))
 		if_install_connected(ifp);
-
-	/* Due to connected route change, schedule RIB processing for both old
-	 * and new VRF.
-	 */
-	if (IS_ZEBRA_DEBUG_RIB_DETAILED)
-		zlog_debug("%u: IF %s VRF change, scheduling RIB processing",
-			   ifp->vrf_id, ifp->name);
-	rib_update(old_vrf_id, RIB_UPDATE_IF_CHANGE);
-	rib_update(ifp->vrf_id, RIB_UPDATE_IF_CHANGE);
 }
 
 static void ipv6_ll_address_to_mac(struct in6_addr *address, uint8_t *mac)
@@ -950,11 +941,6 @@ void if_up(struct interface *ifp)
 	/* Install connected routes to the kernel. */
 	if_install_connected(ifp);
 
-	if (IS_ZEBRA_DEBUG_RIB_DETAILED)
-		zlog_debug("%u: IF %s up, scheduling RIB processing",
-			   ifp->vrf_id, ifp->name);
-	rib_update(ifp->vrf_id, RIB_UPDATE_IF_CHANGE);
-
 	/* Handle interface up for specific types for EVPN. Non-VxLAN interfaces
 	 * are checked to see if (remote) neighbor entries need to be installed
 	 * on them for ARP suppression.
@@ -1008,11 +994,6 @@ void if_down(struct interface *ifp)
 	/* Uninstall connected routes from the kernel. */
 	if_uninstall_connected(ifp);
 
-	if (IS_ZEBRA_DEBUG_RIB_DETAILED)
-		zlog_debug("%u: IF %s down, scheduling RIB processing",
-			   ifp->vrf_id, ifp->name);
-	rib_update(ifp->vrf_id, RIB_UPDATE_IF_CHANGE);
-
 	if_nbr_ipv6ll_to_ipv4ll_neigh_del_all(ifp);
 
 	/* Delete all neighbor addresses learnt through IPv6 RA */

zebra/rib.h

@@ -124,18 +124,16 @@ struct route_entry {
 	/* RIB internal status */
 	uint32_t status;
 #define ROUTE_ENTRY_REMOVED          0x1
-/* to simplify NHT logic when NHs change, instead of doing a NH by NH cmp */
-#define ROUTE_ENTRY_NEXTHOPS_CHANGED 0x2
 /* The Route Entry has changed */
-#define ROUTE_ENTRY_CHANGED          0x4
+#define ROUTE_ENTRY_CHANGED          0x2
 /* The Label has changed on the Route entry */
-#define ROUTE_ENTRY_LABELS_CHANGED   0x8
+#define ROUTE_ENTRY_LABELS_CHANGED   0x4
 /* Route is queued for Installation into the Data Plane */
-#define ROUTE_ENTRY_QUEUED   0x10
+#define ROUTE_ENTRY_QUEUED   0x8
 /* Route is installed into the Data Plane */
-#define ROUTE_ENTRY_INSTALLED        0x20
+#define ROUTE_ENTRY_INSTALLED        0x10
 /* Route has Failed installation into the Data Plane in some manner */
-#define ROUTE_ENTRY_FAILED           0x40
+#define ROUTE_ENTRY_FAILED           0x20
 
 	/* Nexthop information. */
 	uint8_t nexthop_num;
@@ -303,7 +301,6 @@ typedef struct rib_tables_iter_t_ {
 
 /* Events/reasons triggering a RIB update. */
 typedef enum {
-	RIB_UPDATE_IF_CHANGE,
 	RIB_UPDATE_RMAP_CHANGE,
 	RIB_UPDATE_OTHER
 } rib_update_event_t;

zebra/rt_netlink.c

@@ -1833,6 +1833,19 @@ enum zebra_dplane_result kernel_route_update(struct zebra_dplane_ctx *ctx)
 		if (p->family == AF_INET || v6_rr_semantics) {
 			/* Single 'replace' operation */
 			cmd = RTM_NEWROUTE;
+
+			/*
+			 * With route replace semantics in place
+			 * for v4 routes and the new route is a system
+			 * route we do not install anything.
+			 * The problem here is that the new system
+			 * route should cause us to withdraw from
+			 * the kernel the old non-system route
+			 */
+			if (RSYSTEM_ROUTE(dplane_ctx_get_type(ctx)) &&
+			    !RSYSTEM_ROUTE(dplane_ctx_get_old_type(ctx)))
+				(void)netlink_route_multipath(RTM_DELROUTE,
+							      ctx);
 		} else {
 			/*
 			 * So v6 route replace semantics are not in

zebra/zebra_nhg.c

@@ -272,8 +272,6 @@ static int nexthop_active(afi_t afi, struct route_entry *re,
 
 				SET_FLAG(nexthop->flags,
 					 NEXTHOP_FLAG_RECURSIVE);
-				SET_FLAG(re->status,
-					 ROUTE_ENTRY_NEXTHOPS_CHANGED);
 				nexthop_set_resolved(afi, newhop, nexthop);
 				resolved = 1;
 			}
@@ -501,10 +499,8 @@ int nexthop_active_update(struct route_node *rn, struct route_entry *re)
 			 && nexthop->type < NEXTHOP_TYPE_BLACKHOLE)
 			&& !(IPV6_ADDR_SAME(&prev_src.ipv6,
 					    &nexthop->rmap_src.ipv6)))
-		    || CHECK_FLAG(re->status, ROUTE_ENTRY_LABELS_CHANGED)) {
+		    || CHECK_FLAG(re->status, ROUTE_ENTRY_LABELS_CHANGED))
 			SET_FLAG(re->status, ROUTE_ENTRY_CHANGED);
-			SET_FLAG(re->status, ROUTE_ENTRY_NEXTHOPS_CHANGED);
-		}
 	}
 
 	return re->nexthop_active_num;

zebra/zebra_rib.c

@@ -1128,8 +1128,6 @@ static void rib_process(struct route_node *rn)
 				re->status, re->flags, re->distance,
 				re->metric);
 
-		UNSET_FLAG(re->status, ROUTE_ENTRY_NEXTHOPS_CHANGED);
-
 		/* Currently selected re. */
 		if (CHECK_FLAG(re->flags, ZEBRA_FLAG_SELECTED)) {
 			assert(old_selected == NULL);
@@ -2968,50 +2966,6 @@ void rib_update_table(struct route_table *table, rib_update_event_t event)
 					   RIB_ROUTE_ANY_QUEUED))
 			continue;
 		switch (event) {
-		case RIB_UPDATE_IF_CHANGE:
-			/* Examine all routes that won't get processed by the
-			 * protocol or
-			 * triggered by nexthop evaluation (NHT). This would be
-			 * system,
-			 * kernel and certain static routes. Note that NHT will
-			 * get
-			 * triggered upon an interface event as connected routes
-			 * always
-			 * get queued for processing.
-			 */
-			RNODE_FOREACH_RE_SAFE (rn, re, next) {
-				struct nexthop *nh;
-
-				if (re->type != ZEBRA_ROUTE_SYSTEM
-				    && re->type != ZEBRA_ROUTE_KERNEL
-				    && re->type != ZEBRA_ROUTE_CONNECT
-				    && re->type != ZEBRA_ROUTE_STATIC)
-					continue;
-
-				if (re->type != ZEBRA_ROUTE_STATIC) {
-					SET_FLAG(re->status,
-						 ROUTE_ENTRY_CHANGED);
-					rib_queue_add(rn);
-					continue;
-				}
-
-				for (nh = re->ng.nexthop; nh; nh = nh->next)
-					if (!(nh->type == NEXTHOP_TYPE_IPV4
-					      || nh->type == NEXTHOP_TYPE_IPV6))
-						break;
-
-				/* If we only have nexthops to a
-				 * gateway, NHT will
-				 * take care.
-				 */
-				if (nh) {
-					SET_FLAG(re->status,
-						 ROUTE_ENTRY_CHANGED);
-					rib_queue_add(rn);
-				}
-			}
-			break;
-
 		case RIB_UPDATE_RMAP_CHANGE:
 		case RIB_UPDATE_OTHER:
 			/* Right now, examine all routes. Can restrict to a

zebra/zebra_rnh.c

@@ -868,10 +868,8 @@ static void zebra_rnh_clear_nhc_flag(struct zebra_vrf *zvrf, afi_t afi,
 		re = zebra_rnh_resolve_nexthop_entry(zvrf, afi, nrn, rnh,
 						     &prn);
 
-	if (re) {
+	if (re)
-		UNSET_FLAG(re->status, ROUTE_ENTRY_NEXTHOPS_CHANGED);
 		UNSET_FLAG(re->status, ROUTE_ENTRY_LABELS_CHANGED);
-	}
 }
 
 /* Evaluate all tracked entries (nexthops or routes for import into BGP)