Merge pull request FRRouting#12636 from opensourcerouting/fix/bgp_acc…

…ept-own_connected_routes

bgpd: Allow importing local routes with accept-own mechanism

bgpd/bgp_mplsvpn.c

@@ -1070,12 +1070,14 @@ static bool leak_update_nexthop_valid(struct bgp *to_bgp, struct bgp_dest *bn,
 		bgp_nexthop = bgp_orig;
 
 	/*
-	 * No nexthop tracking for redistributed routes or for
-	 * EVPN-imported routes that get leaked.
+	 * No nexthop tracking for redistributed routes, for
+	 * EVPN-imported routes that get leaked, or for routes
+	 * leaked between VRFs with accept-own community.
 	 */
 	if (bpi_ultimate->sub_type == BGP_ROUTE_REDISTRIBUTE ||
-	    is_pi_family_evpn(bpi_ultimate))
-		nh_valid = 1;
+	    is_pi_family_evpn(bpi_ultimate) ||
+	    CHECK_FLAG(bpi_ultimate->flags, BGP_PATH_ACCEPT_OWN))
+		nh_valid = true;
 	else
 		/*
 		 * TBD do we need to do anything about the
@@ -1866,6 +1868,7 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 	struct bgp_path_info *bpi_ultimate = NULL;
 	int origin_local = 0;
 	struct bgp *src_vrf;
+	struct interface *ifp;
 
 	int debug = BGP_DEBUG(vpn, VPN_LEAK_TO_VRF);
 
@@ -1875,6 +1878,22 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 		return false;
 	}
 
+	/*
+	 * For VRF-2-VRF route-leaking,
+	 * the source will be the originating VRF.
+	 *
+	 * If ACCEPT_OWN mechanism is enabled, then we SHOULD(?)
+	 * get the source VRF (BGP) by looking at the RD.
+	 */
+	struct bgp *src_bgp = bgp_lookup_by_rd(path_vpn, prd, afi);
+
+	if (path_vpn->extra && path_vpn->extra->bgp_orig)
+		src_vrf = path_vpn->extra->bgp_orig;
+	else if (src_bgp)
+		src_vrf = src_bgp;
+	else
+		src_vrf = from_bgp;
+
 	/* Check for intersection of route targets */
 	if (!ecommunity_include(
 		    to_bgp->vpn_policy[afi].rtlist[BGP_VPN_POLICY_DIR_FROMVPN],
@@ -1940,6 +1959,20 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 	memset(&nexthop_orig, 0, sizeof(nexthop_orig));
 	nexthop_orig.family = nhfamily;
 
+	/* If the path has accept-own community and the source VRF
+	 * is valid, reset next-hop to self, to allow importing own
+	 * routes between different VRFs on the same node.
+	 * Set the nh ifindex to VRF's interface, not the real interface.
+	 * Let the kernel to decide with double lookup the real next-hop
+	 * interface when installing the route.
+	 */
+	if (src_bgp) {
+		subgroup_announce_reset_nhop(nhfamily, &static_attr);
+		ifp = if_get_vrf_loopback(src_vrf->vrf_id);
+		if (ifp)
+			static_attr.nh_ifindex = ifp->ifindex;
+	}
+
 	switch (nhfamily) {
 	case AF_INET:
 		/* save */
@@ -2051,22 +2084,6 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 		zlog_debug("%s: pfx %pBD: num_labels %d", __func__,
 			   path_vpn->net, num_labels);
 
-	/*
-	 * For VRF-2-VRF route-leaking,
-	 * the source will be the originating VRF.
-	 *
-	 * If ACCEPT_OWN mechanism is enabled, then we SHOULD(?)
-	 * get the source VRF (BGP) by looking at the RD.
-	 */
-	struct bgp *src_bgp = bgp_lookup_by_rd(path_vpn, prd, afi);
-
-	if (path_vpn->extra && path_vpn->extra->bgp_orig)
-		src_vrf = path_vpn->extra->bgp_orig;
-	else if (src_bgp)
-		src_vrf = src_bgp;
-	else
-		src_vrf = from_bgp;
-
 	leak_update(to_bgp, bn, new_attr, afi, safi, path_vpn, pLabels,
 		    num_labels, src_vrf, &nexthop_orig, nexthop_self_flag,
 		    debug);

lib/if.c

@@ -564,6 +564,20 @@ size_t if_lookup_by_hwaddr(const uint8_t *hw_addr, size_t addrsz,
 	return count;
 }
 
+/* Get the VRF loopback interface, i.e. the loopback on the default VRF
+ * or the VRF interface.
+ */
+struct interface *if_get_vrf_loopback(vrf_id_t vrf_id)
+{
+	struct interface *ifp = NULL;
+	struct vrf *vrf = vrf_lookup_by_id(vrf_id);
+
+	FOR_ALL_INTERFACES (vrf, ifp)
+		if (if_is_loopback(ifp))
+			return ifp;
+
+	return NULL;
+}
 
 /* Get interface by name if given name interface doesn't exist create
    one. */

lib/if.h

@@ -532,6 +532,7 @@ static inline bool if_address_is_local(const void *matchaddr, int family,
 struct vrf;
 extern struct interface *if_lookup_by_name_vrf(const char *name, struct vrf *vrf);
 extern struct interface *if_lookup_by_name(const char *ifname, vrf_id_t vrf_id);
+extern struct interface *if_get_vrf_loopback(vrf_id_t vrf_id);
 extern struct interface *if_get_by_name(const char *ifname, vrf_id_t vrf_id,
 					const char *vrf_name);
 

tests/topotests/bgp_accept_own/pe1/bgpd.conf

@@ -24,6 +24,7 @@ router bgp 65001 vrf Customer
  neighbor 192.168.1.1 timers 1 3
  neighbor 192.168.1.1 timers connect 1
  address-family ipv4 unicast
+  redistribute connected
   label vpn export 10
   rd vpn export 192.168.1.2:2
   rt vpn import 192.168.1.2:2

tests/topotests/bgp_accept_own/pe1/zebra.conf

@@ -11,5 +11,8 @@ interface pe1-eth1 vrf Service
 interface pe1-eth2
  ip address 10.0.1.1/24
 !
+interface pe1-eth3 vrf Customer
+ ip address 192.0.2.1/24
+!
 ip forwarding
 !

tests/topotests/bgp_accept_own/test_bgp_accept_own.py

@@ -58,6 +58,9 @@ def build_topo(tgen):
     switch.add_link(tgen.gears["pe1"])
     switch.add_link(tgen.gears["rr1"])
 
+    switch = tgen.add_switch("s4")
+    switch.add_link(tgen.gears["pe1"])
+
 
 def setup_module(mod):
     tgen = Topogen(build_topo, mod.__name__)
@@ -72,6 +75,7 @@ def setup_module(mod):
     pe1.run("ip link add Service type vrf table 1002")
     pe1.run("ip link set up dev Service")
     pe1.run("ip link set pe1-eth1 master Service")
+    pe1.run("ip link set pe1-eth3 master Customer")
     pe1.run("sysctl -w net.mpls.conf.pe1-eth2.input=1")
     rr1.run("sysctl -w net.mpls.conf.rr1-eth0.input=1")
 
@@ -112,7 +116,7 @@ def test_bgp_accept_own():
 
     def _bgp_check_received_routes_due_originator_id():
         output = json.loads(pe1.vtysh_cmd("show bgp ipv4 vpn summary json"))
-        expected = {"peers": {"10.10.10.101": {"pfxRcd": 0, "pfxSnt": 4}}}
+        expected = {"peers": {"10.10.10.101": {"pfxRcd": 0, "pfxSnt": 5}}}
         return topotest.json_cmp(output, expected)
 
     test_func = functools.partial(_bgp_check_received_routes_due_originator_id)
@@ -134,7 +138,7 @@ def _bgp_check_received_routes_due_originator_id():
 
     def _bgp_check_received_routes_with_modified_rts():
         output = json.loads(pe1.vtysh_cmd("show bgp ipv4 vpn summary json"))
-        expected = {"peers": {"10.10.10.101": {"pfxRcd": 4, "pfxSnt": 4}}}
+        expected = {"peers": {"10.10.10.101": {"pfxRcd": 5, "pfxSnt": 5}}}
         return topotest.json_cmp(output, expected)
 
     test_func = functools.partial(_bgp_check_received_routes_with_modified_rts)
@@ -154,9 +158,7 @@ def _bgp_check_received_routes_with_changed_rts():
         expected = {
             "paths": [
                 {
-                    "community": {
-                        "string": "65001:111"
-                    },
+                    "community": {"string": "65001:111"},
                     "extendedCommunity": {
                         "string": "RT:192.168.1.2:2 RT:192.168.2.2:2"
                     },
@@ -171,6 +173,37 @@ def _bgp_check_received_routes_with_changed_rts():
         result is None
     ), "Failed, routes are not imported from RR1 with modified RT list"
 
+    step("Check if 192.0.2.0/24 is imported to vrf Service from vrf Customer")
+
+    def _bgp_check_imported_local_routes_from_vrf_service():
+        output = json.loads(
+            pe1.vtysh_cmd("show ip route vrf Service 192.0.2.0/24 json")
+        )
+        expected = {
+            "192.0.2.0/24": [
+                {
+                    "vrfName": "Service",
+                    "table": 1002,
+                    "installed": True,
+                    "selected": True,
+                    "nexthops": [
+                        {
+                            "fib": True,
+                            "vrf": "Customer",
+                            "active": True,
+                        }
+                    ],
+                }
+            ]
+        }
+        return topotest.json_cmp(output, expected)
+
+    test_func = functools.partial(_bgp_check_imported_local_routes_from_vrf_service)
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
+    assert (
+        result is None
+    ), "Failed, didn't import local route 192.0.2.0/24 from vrf Customer to vrf Service"
+
     step("Check if 172.16.255.1/32 is announced to CE2")
 
     def _bgp_check_received_routes_from_pe():
@@ -188,9 +221,7 @@ def _bgp_check_received_routes_from_pe():
 
     test_func = functools.partial(_bgp_check_received_routes_from_pe)
     _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
-    assert (
-        result is None
-    ), "Failed, didn't receive 172.16.255.1/32 from PE1"
+    assert result is None, "Failed, didn't receive 172.16.255.1/32 from PE1"
 
 
 if __name__ == "__main__":