From c479b28e51bedd8d5be25d32bf9fbd1e42ee02ba Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 13 Jan 2023 12:04:01 +0200 Subject: [PATCH 1/4] bgpd: Allow importing local routes with accept-own mechanism Before this patch we allowed importing routes between VRFs in the same node, only for external routes, but not for local (e.g.: redistribute). Relax here a bit, and allow importing local routes between VRFs when the RT list is modified using route reflectors. Signed-off-by: Donatas Abraitis --- bgpd/bgp_mplsvpn.c | 49 +++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index c92d678eff84..ddc9a525f935 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/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 @@ -1875,6 +1877,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 +1958,13 @@ 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. + */ + if (src_bgp) + subgroup_announce_reset_nhop(nhfamily, &static_attr); + switch (nhfamily) { case AF_INET: /* save */ @@ -2051,22 +2076,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); From b4710d034dc84ad1dfd3023e736d43418231de8f Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 13 Jan 2023 09:19:27 +0200 Subject: [PATCH 2/4] tests: Check if connected routes are leaked between VRFs using accept-own Signed-off-by: Donatas Abraitis --- tests/topotests/bgp_accept_own/pe1/bgpd.conf | 1 + tests/topotests/bgp_accept_own/pe1/zebra.conf | 3 ++ .../bgp_accept_own/test_bgp_accept_own.py | 47 +++++++++++++++---- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/tests/topotests/bgp_accept_own/pe1/bgpd.conf b/tests/topotests/bgp_accept_own/pe1/bgpd.conf index 8631293730ee..109e0eadbb1d 100644 --- a/tests/topotests/bgp_accept_own/pe1/bgpd.conf +++ b/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 diff --git a/tests/topotests/bgp_accept_own/pe1/zebra.conf b/tests/topotests/bgp_accept_own/pe1/zebra.conf index 71476d2aefea..2b7aefadf250 100644 --- a/tests/topotests/bgp_accept_own/pe1/zebra.conf +++ b/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 ! diff --git a/tests/topotests/bgp_accept_own/test_bgp_accept_own.py b/tests/topotests/bgp_accept_own/test_bgp_accept_own.py index 161530b48390..daef6dcd52c7 100644 --- a/tests/topotests/bgp_accept_own/test_bgp_accept_own.py +++ b/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__": From b2cfd204a8ec751c9c9ac00a27272f4b5c0721e3 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 22 Apr 2022 18:08:08 +0200 Subject: [PATCH 3/4] lib: add a function to get the VRF or loopback interface Add a function to find the VRF or the loopback interface: the loopback interface for the default VRF and the VRF master interface otherwise. Signed-off-by: Louis Scalbert --- lib/if.c | 14 ++++++++++++++ lib/if.h | 1 + 2 files changed, 15 insertions(+) diff --git a/lib/if.c b/lib/if.c index 70c0c1814169..db7321003644 100644 --- a/lib/if.c +++ b/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. */ diff --git a/lib/if.h b/lib/if.h index 91dcd462474f..a653246ccb2e 100644 --- a/lib/if.h +++ b/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); From 8a02d9fe1e167bc57bcf8f8968107a739b41b654 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Sun, 15 Jan 2023 12:43:37 +0200 Subject: [PATCH 4/4] bgpd: Set nh ifindex to VRF's interface, not the real The kernel will lookup the real interface later. Signed-off-by: Donatas Abraitis --- bgpd/bgp_mplsvpn.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index ddc9a525f935..ab568d1d6fc3 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1868,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); @@ -1961,9 +1962,16 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp, /* to */ /* 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) + 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: