Merge pull request #15238 from louis-6wind/bgp-leak-network

bgpd: fix VRF leaking with 'network import-check'

bgpd/bgp_attr.c

@@ -864,6 +864,7 @@ bool attrhash_cmp(const void *p1, const void *p2)
 		    attr1->df_alg == attr2->df_alg &&
 		    attr1->nh_ifindex == attr2->nh_ifindex &&
 		    attr1->nh_lla_ifindex == attr2->nh_lla_ifindex &&
+		    attr1->nh_flags == attr2->nh_flags &&
 		    attr1->distance == attr2->distance &&
 		    srv6_l3vpn_same(attr1->srv6_l3vpn, attr2->srv6_l3vpn) &&
 		    srv6_vpn_same(attr1->srv6_vpn, attr2->srv6_vpn) &&

bgpd/bgp_attr.h

@@ -155,6 +155,9 @@ struct attr {
 	uint32_t med;
 	uint32_t local_pref;
 	ifindex_t nh_ifindex;
+	uint8_t nh_flags;
+
+#define BGP_ATTR_NH_VALID 0x01
 
 	/* Path origin attribute */
 	uint8_t origin;

bgpd/bgp_mplsvpn.c

@@ -1034,13 +1034,19 @@ static bool leak_update_nexthop_valid(struct bgp *to_bgp, struct bgp_dest *bn,
 	else if (bpi_ultimate->type == ZEBRA_ROUTE_BGP &&
 		 bpi_ultimate->sub_type == BGP_ROUTE_STATIC && table &&
 		 (table->safi == SAFI_UNICAST ||
-		  table->safi == SAFI_LABELED_UNICAST) &&
-		 !CHECK_FLAG(bgp_nexthop->flags, BGP_FLAG_IMPORT_CHECK)) {
-		/* if the route is defined with the "network <prefix>" command
-		 * and "no bgp network import-check" is set,
-		 * then mark the nexthop as valid.
-		 */
-		nh_valid = true;
+		  table->safi == SAFI_LABELED_UNICAST)) {
+		/* the route is defined with the "network <prefix>" command */
+
+		if (CHECK_FLAG(bgp_nexthop->flags, BGP_FLAG_IMPORT_CHECK))
+			nh_valid = bgp_find_or_add_nexthop(to_bgp, bgp_nexthop,
+							   afi, SAFI_UNICAST,
+							   bpi_ultimate, NULL,
+							   0, p);
+		else
+			/* if "no bgp network import-check" is set,
+			 * then mark the nexthop as valid.
+			 */
+			nh_valid = true;
 	} else
 		/*
 		 * TBD do we need to do anything about the
@@ -2084,6 +2090,7 @@ static void vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 	uint32_t num_labels = 0;
 	int nexthop_self_flag = 1;
 	struct bgp_path_info *bpi_ultimate = NULL;
+	struct bgp_path_info *bpi;
 	int origin_local = 0;
 	struct bgp *src_vrf;
 	struct interface *ifp;
@@ -2173,6 +2180,20 @@ static void vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 
 	community_strip_accept_own(&static_attr);
 
+	bn = bgp_afi_node_get(to_bgp->rib[afi][safi], afi, safi, p, NULL);
+
+	for (bpi = bgp_dest_get_bgp_path_info(bn); bpi; bpi = bpi->next) {
+		if (bpi->extra && bpi->extra->vrfleak &&
+		    bpi->extra->vrfleak->parent == path_vpn)
+			break;
+	}
+
+	if (bpi && leak_update_nexthop_valid(to_bgp, bn, &static_attr, afi, safi,
+					     path_vpn, bpi, src_vrf, p, debug))
+		SET_FLAG(static_attr.nh_flags, BGP_ATTR_NH_VALID);
+	else
+		UNSET_FLAG(static_attr.nh_flags, BGP_ATTR_NH_VALID);
+
 	/*
 	 * Nexthop: stash and clear
 	 *
@@ -2265,8 +2286,6 @@ static void vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 	new_attr = bgp_attr_intern(&static_attr);
 	bgp_attr_flush(&static_attr);
 
-	bn = bgp_afi_node_get(to_bgp->rib[afi][safi], afi, safi, p, NULL);
-
 	/*
 	 * ensure labels are copied
 	 *

bgpd/bgp_nht.c

@@ -406,7 +406,7 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop,
 	if (pi && is_route_parent_evpn(pi))
 		bnc->is_evpn_gwip_nexthop = true;
 
-	if (is_bgp_static_route) {
+	if (is_bgp_static_route && !CHECK_FLAG(bnc->flags, BGP_STATIC_ROUTE)) {
 		SET_FLAG(bnc->flags, BGP_STATIC_ROUTE);
 
 		/* If we're toggling the type, re-register */
@@ -903,7 +903,10 @@ void bgp_nexthop_update(struct vrf *vrf, struct prefix *match,
 {
 	struct bgp_nexthop_cache_head *tree = NULL;
 	struct bgp_nexthop_cache *bnc_nhc, *bnc_import;
-	struct bgp *bgp;
+	struct bgp *bgp, *bgp_default;
+	struct bgp_path_info *pi;
+	struct bgp_dest *dest;
+	safi_t safi;
 	afi_t afi;
 
 	if (!vrf->info) {
@@ -918,25 +921,38 @@ void bgp_nexthop_update(struct vrf *vrf, struct prefix *match,
 	tree = &bgp->nexthop_cache_table[afi];
 
 	bnc_nhc = bnc_find(tree, match, nhr->srte_color, 0);
-	if (!bnc_nhc) {
-		if (BGP_DEBUG(nht, NHT))
-			zlog_debug("parse nexthop update %pFX(%u)(%s): bnc info not found for nexthop cache",
-				   &nhr->prefix, nhr->srte_color,
-				   bgp->name_pretty);
-	} else
+	if (bnc_nhc)
 		bgp_process_nexthop_update(bnc_nhc, nhr, false);
+	else if (BGP_DEBUG(nht, NHT))
+		zlog_debug("parse nexthop update %pFX(%u)(%s): bnc info not found for nexthop cache",
+			   &nhr->prefix, nhr->srte_color,
+			   bgp->name_pretty);
 
 	tree = &bgp->import_check_table[afi];
 
 	bnc_import = bnc_find(tree, match, nhr->srte_color, 0);
-	if (!bnc_import) {
-		if (BGP_DEBUG(nht, NHT))
-			zlog_debug("parse nexthop update %pFX(%u)(%s): bnc info not found for import check",
-				   &nhr->prefix, nhr->srte_color,
-				   bgp->name_pretty);
-	} else
+	if (bnc_import) {
 		bgp_process_nexthop_update(bnc_import, nhr, true);
 
+		bgp_default = bgp_get_default();
+		safi = nhr->safi;
+		if (bgp != bgp_default && bgp->rib[afi][safi]) {
+			dest = bgp_afi_node_get(bgp->rib[afi][safi], afi, safi,
+						match, NULL);
+
+			for (pi = bgp_dest_get_bgp_path_info(dest); pi;
+			     pi = pi->next)
+				if (pi->peer == bgp->peer_self &&
+				    pi->type == ZEBRA_ROUTE_BGP &&
+				    pi->sub_type == BGP_ROUTE_STATIC)
+					vpn_leak_from_vrf_update(bgp_default,
+								 bgp, pi);
+		}
+	} else if (BGP_DEBUG(nht, NHT))
+		zlog_debug("parse nexthop update %pFX(%u)(%s): bnc info not found for import check",
+			   &nhr->prefix, nhr->srte_color,
+			   bgp->name_pretty);
+
 	/*
 	 * HACK: if any BGP route is dependant on an SR-policy that doesn't
 	 * exist, zebra will never send NH updates relative to that policy. In

tests/topotests/bgp_l3vpn_to_bgp_vrf/customize.py

@@ -165,6 +165,8 @@ def ltemplatePreRouterStartHook():
     cmds = [
         "ip link add {0}-cust4 type vrf table 30",
         "ip link set dev {0}-cust4 up",
+        "ip link add {0}-cust5 type vrf table 40",
+        "ip link set dev {0}-cust5 up",
     ]
     rtr = "r1"
     for cmd in cmds:

tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/bgpd.conf

@@ -64,5 +64,17 @@ router bgp 5227 vrf r1-cust4
      import vpn
      export vpn
    exit-address-family
-!
-end
+
+router bgp 5227 vrf r1-cust5
+   bgp router-id 192.168.1.1
+
+   address-family ipv4 unicast
+     network 172.16.1.1/32
+
+     label vpn export 105
+     rd vpn export 10:15
+     rt vpn both 52:100
+
+     import vpn
+     export vpn
+   exit-address-family

tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/zebra.conf

@@ -18,6 +18,10 @@ interface r1-eth4
  ip address 192.168.1.1/24
  no link-detect
 
+interface r1-cust5
+ ip address 172.16.1.1/32
+ no link-detect
+
 ip forwarding
 
 

tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py

@@ -60,6 +60,7 @@ want_r1_cust1_routes = [
     {"p": "6.0.1.0/24", "n": "99.0.0.1"},
     {"p": "6.0.2.0/24", "n": "99.0.0.1"},
     {"p": "172.16.0.0/24", "n": "0.0.0.0", "bp": True},
+    {"p": "172.16.1.1/32", "n": "0.0.0.0", "bp": True},
     {"p": "99.0.0.1/32", "n": "192.168.1.2"},
 ]
 bgpribRequireUnicastRoutes(
@@ -73,6 +74,13 @@ bgpribRequireUnicastRoutes(
     "r1", "ipv4", "r1-cust4", "Customer 4 routes in r1 vrf", want_r1_cust4_routes
 )
 
+want_r1_cust5_routes = [
+    {"p": "172.16.1.1/32", "n": "0.0.0.0", "bp": True},
+]
+bgpribRequireUnicastRoutes(
+    "r1", "ipv4", "r1-cust5", "Customer 5 routes in r1 vrf", want_r1_cust5_routes
+)
+
 want_r3_cust1_routes = [
     {"p": "5.1.0.0/24", "n": "99.0.0.2"},
     {"p": "5.1.1.0/24", "n": "99.0.0.2"},
@@ -675,7 +683,7 @@ bgpribRequireUnicastRoutes(
 luCommand(
     "ce1",
     'vtysh -c "show bgp ipv4 uni"',
-    "13 routes and 13",
+    "14 routes and 14",
     "wait",
     "Local and remote routes",
     10,
@@ -697,7 +705,7 @@ bgpribRequireUnicastRoutes(
 luCommand(
     "ce2",
     'vtysh -c "show bgp ipv4 uni"',
-    "13 routes and 16",
+    "14 routes and 17",
     "wait",
     "Local and remote routes",
     10,
@@ -729,7 +737,7 @@ luCommand("r4", 'vtysh -c "show ip route vrf r4-cust2"')
 luCommand(
     "ce3",
     'vtysh -c "show bgp ipv4 uni"',
-    "13 routes and 14",
+    "14 routes and 15",
     "wait",
     "Local and remote routes",
     10,
@@ -751,7 +759,7 @@ bgpribRequireUnicastRoutes(
 luCommand(
     "ce4",
     'vtysh -c "show bgp vrf ce4-cust2 ipv4 uni"',
-    "13 routes and 15",
+    "14 routes and 16",
     "wait",
     "Local and remote routes",
     10,

tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/scale_down.py

@@ -49,7 +49,7 @@ if ret != False and found != None:
             luCommand(
                 rtr,
                 'vtysh -c "show bgp ipv4 uni" | grep Display',
-                " 13 route",
+                " 14 route",
                 "wait",
                 "BGP routes removed",
                 wait,