From 06801c12b0edc45d6323452382005ae36329ca9c Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 14 Nov 2024 15:45:25 +0100 Subject: [PATCH 1/8] bgpd: merge parent and source_bpi in leak_update They are the same value. Signed-off-by: Louis Scalbert --- bgpd/bgp_mplsvpn.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 46e529f03dfc..81682258b511 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1126,7 +1126,6 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn, struct bgp_path_info *bpi; struct bgp_path_info *new; struct bgp_path_info_extra *extra; - struct bgp_path_info *parent = source_bpi; struct bgp_labels bgp_labels = {}; bool labelssame; uint8_t i; @@ -1159,8 +1158,7 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn, * match parent */ for (bpi = bgp_dest_get_bgp_path_info(bn); bpi; bpi = bpi->next) { - if (bpi->extra && bpi->extra->vrfleak && - bpi->extra->vrfleak->parent == parent) + if (bpi->extra && bpi->extra->vrfleak && bpi->extra->vrfleak->parent == source_bpi) break; } @@ -1297,9 +1295,8 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn, if (bgp_labels.num_labels) new->extra->labels = bgp_labels_intern(&bgp_labels); - new->extra->vrfleak->parent = bgp_path_info_lock(parent); - bgp_dest_lock_node( - (struct bgp_dest *)parent->net); + new->extra->vrfleak->parent = bgp_path_info_lock(source_bpi); + bgp_dest_lock_node((struct bgp_dest *)source_bpi->net); new->extra->vrfleak->bgp_orig = bgp_lock(bgp_orig); From ef0595de49718edb2d54e5cf7efa950b4e7f5187 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 6 Nov 2024 13:52:03 +0100 Subject: [PATCH 2/8] bgpd: check nexthop vrf usability Mark a nexthop as invalid if the origin VRF is unusable, either because it does not exist or its interface is down. Signed-off-by: Louis Scalbert --- bgpd/bgp_mplsvpn.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 81682258b511..5ad5da3286b5 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1052,6 +1052,7 @@ static bool leak_update_nexthop_valid(struct bgp *to_bgp, struct bgp_dest *bn, struct bgp_path_info *bpi_ultimate; struct bgp *bgp_nexthop; struct bgp_table *table; + struct interface *ifp; bool nh_valid; bpi_ultimate = bgp_get_imported_bpi_ultimate(source_bpi); @@ -1062,6 +1063,15 @@ static bool leak_update_nexthop_valid(struct bgp *to_bgp, struct bgp_dest *bn, else bgp_nexthop = bgp_orig; + /* The nexthop is invalid if its VRF does not exist */ + if (bgp_nexthop->vrf_id == VRF_UNKNOWN) + return false; + + /* The nexthop is invalid if its VRF interface is down*/ + ifp = if_get_vrf_loopback(bgp_nexthop->vrf_id); + if (ifp && !if_is_up(ifp)) + return false; + /* * No nexthop tracking for redistributed routes, for * EVPN-imported routes that get leaked, or for routes From 08b862da07fe0de887a59165cdbcaa19ef868d62 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Tue, 5 Nov 2024 15:53:43 +0100 Subject: [PATCH 3/8] bgpd: recheck leak nexthop validity When leak_update() rechecks an existing path, it considers nothing to update if the attributes and labels are not changed. However, it does not take into account the nexthop validity. Perform a leak update if the nexthop validity has changed. Signed-off-by: Louis Scalbert --- bgpd/bgp_mplsvpn.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 5ad5da3286b5..b55e8968b9e0 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1193,9 +1193,11 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn, return NULL; } - if (attrhash_cmp(bpi->attr, new_attr) && labelssame - && !CHECK_FLAG(bpi->flags, BGP_PATH_REMOVED)) { - + if (attrhash_cmp(bpi->attr, new_attr) && labelssame && + !CHECK_FLAG(bpi->flags, BGP_PATH_REMOVED) && + leak_update_nexthop_valid(to_bgp, bn, new_attr, afi, safi, source_bpi, bpi, + bgp_orig, p, + debug) == !!CHECK_FLAG(bpi->flags, BGP_PATH_VALID)) { bgp_attr_unintern(&new_attr); if (debug) zlog_debug( From 5ce2b98b214002bf34ce5bf10664f96faa2904e4 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 6 Nov 2024 15:25:35 +0100 Subject: [PATCH 4/8] bgpd: do not leak if origin vrf is not usable Do not leak if origin vrf is not usable Signed-off-by: Louis Scalbert --- bgpd/bgp_mplsvpn.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index b55e8968b9e0..6a8df3b68e18 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1137,6 +1137,7 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn, struct bgp_path_info *new; struct bgp_path_info_extra *extra; struct bgp_labels bgp_labels = {}; + struct bgp *bgp_nexthop; bool labelssame; uint8_t i; @@ -1182,6 +1183,16 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn, labelssame = bgp_path_info_labels_same(bpi, bgp_labels.label, bgp_labels.num_labels); + bgp_nexthop = bpi->extra->vrfleak->bgp_orig ?: bgp_orig; + if (bgp_nexthop->vrf_id == VRF_UNKNOWN) { + if (debug) { + zlog_debug("%s: ->%s(s_flags: 0x%x b_flags: 0x%x): %pFX: Found route, origin VRF does not exist, not leaking", + __func__, to_bgp->name_pretty, source_bpi->flags, + bpi->flags, p); + } + return NULL; + } + if (CHECK_FLAG(source_bpi->flags, BGP_PATH_REMOVED) && CHECK_FLAG(bpi->flags, BGP_PATH_REMOVED)) { if (debug) { @@ -1284,6 +1295,14 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn, return NULL; } + if (bgp_orig->vrf_id == VRF_UNKNOWN) { + if (debug) { + zlog_debug("%s: ->%s(s_flags: 0x%x): %pFX: New route, origin VRF does not exist, not leaking", + __func__, to_bgp->name_pretty, source_bpi->flags, p); + } + return NULL; + } + new = info_make(ZEBRA_ROUTE_BGP, BGP_ROUTE_IMPORTED, 0, to_bgp->peer_self, new_attr, bn); From 99c122ef56d22da4707c1aa7365ff84c4d711f50 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 6 Nov 2024 14:28:12 +0100 Subject: [PATCH 5/8] tests: update route_leak_basic tests Update route_leak_basic tests. The routes with an unusable nexthop VRF are no more present in the RIB. Signed-off-by: Louis Scalbert --- .../test_bgp_vpnv4_route_leak_basic.py | 75 ++----------------- .../test_bgp-vrf-route-leak-basic.py | 75 ++----------------- 2 files changed, 10 insertions(+), 140 deletions(-) diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/test_bgp_vpnv4_route_leak_basic.py b/tests/topotests/bgp_vpnv4_route_leak_basic/test_bgp_vpnv4_route_leak_basic.py index a44f07b5600e..b7f72ccea166 100644 --- a/tests/topotests/bgp_vpnv4_route_leak_basic/test_bgp_vpnv4_route_leak_basic.py +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/test_bgp_vpnv4_route_leak_basic.py @@ -119,20 +119,7 @@ def test_vrf_route_leak_donna(): ], }, ], - "172.16.101.0/24": [ - { - "protocol": "bgp", - "selected": None, - "nexthops": [ - { - "fib": None, - "interfaceName": "unknown", - "vrf": "Unknown", - "active": None, - }, - ], - }, - ], + "172.16.101.0/24": None, } test_func = partial( @@ -191,20 +178,7 @@ def test_vrf_route_leak_eva(): "protocol": "connected", } ], - "172.16.101.0/24": [ - { - "protocol": "bgp", - "selected": None, - "nexthops": [ - { - "fib": None, - "interfaceName": "unknown", - "vrf": "Unknown", - "active": None, - }, - ], - }, - ], + "172.16.101.0/24": None, } test_func = partial( @@ -298,34 +272,8 @@ def test_vrf_route_leak_donna_after_eva_down(): # Test DONNA VRF. expect = { - "10.0.1.0/24": [ - { - "protocol": "bgp", - "selected": None, - "nexthops": [ - { - "fib": None, - "interfaceName": "EVA", - "vrf": "EVA", - "active": None, - }, - ], - }, - ], - "10.0.3.0/24": [ - { - "protocol": "bgp", - "selected": None, - "nexthops": [ - { - "fib": None, - "interfaceName": "EVA", - "vrf": "EVA", - "active": None, - }, - ], - }, - ], + "10.0.1.0/24": None, + "10.0.3.0/24": None, } test_func = partial( @@ -417,20 +365,7 @@ def test_vrf_route_leak_donna_add_vrf_zita(): # Test DONNA VRF. expect = { - "172.16.101.0/24": [ - { - "protocol": "bgp", - "selected": None, - "nexthops": [ - { - "fib": None, - "interfaceName": "ZITA", - "vrf": "ZITA", - "active": None, - }, - ], - }, - ], + "172.16.101.0/24": None, } test_func = partial( diff --git a/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py b/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py index 6d4b436bccf9..3bc36862bf76 100644 --- a/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py +++ b/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py @@ -123,20 +123,7 @@ def test_vrf_route_leak_donna(): ], }, ], - "172.16.101.0/24": [ - { - "protocol": "bgp", - "selected": None, - "nexthops": [ - { - "fib": None, - "interfaceName": "unknown", - "vrf": "Unknown", - "active": None, - }, - ], - }, - ], + "172.16.101.0/24": None, } test_func = partial( @@ -195,20 +182,7 @@ def test_vrf_route_leak_eva(): "protocol": "connected", } ], - "172.16.101.0/24": [ - { - "protocol": "bgp", - "selected": None, - "nexthops": [ - { - "fib": None, - "interfaceName": "unknown", - "vrf": "Unknown", - "active": None, - }, - ], - }, - ], + "172.16.101.0/24": None, } test_func = partial( @@ -302,34 +276,8 @@ def test_vrf_route_leak_donna_after_eva_down(): # Test DONNA VRF. expect = { - "10.0.1.0/24": [ - { - "protocol": "bgp", - "selected": None, - "nexthops": [ - { - "fib": None, - "interfaceName": "EVA", - "vrf": "EVA", - "active": None, - }, - ], - }, - ], - "10.0.3.0/24": [ - { - "protocol": "bgp", - "selected": None, - "nexthops": [ - { - "fib": None, - "interfaceName": "EVA", - "vrf": "EVA", - "active": None, - }, - ], - }, - ], + "10.0.1.0/24": None, + "10.0.3.0/24": None, } test_func = partial( @@ -421,20 +369,7 @@ def test_vrf_route_leak_donna_add_vrf_zita(): # Test DONNA VRF. expect = { - "172.16.101.0/24": [ - { - "protocol": "bgp", - "selected": None, - "nexthops": [ - { - "fib": None, - "interfaceName": "ZITA", - "vrf": "ZITA", - "active": None, - }, - ], - }, - ], + "172.16.101.0/24": None, } test_func = partial( From 66b233f2fb5577d1271fe88ce775555a577a8662 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 14 Nov 2024 11:28:24 +0100 Subject: [PATCH 6/8] tests: fix no vrf1 in bgp_bmp r2 The bgp_bmp test is failing because r2 lacks the vrf1 VRF, which prevents it from exporting VPN prefixes from the configured vrf1 BGP instance. Previous versions allowed the export of static BGP prefixes from a non-existent VRF, so the test passed under those conditions. Add a vrf1 VRF on r2. Fixes: d748544769 ("topotests: add basic bgp bmp test") Signed-off-by: Louis Scalbert --- tests/topotests/bgp_bmp/test_bgp_bmp_1.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp_1.py b/tests/topotests/bgp_bmp/test_bgp_bmp_1.py index be3e07929a0f..59cfc4ea0896 100644 --- a/tests/topotests/bgp_bmp/test_bgp_bmp_1.py +++ b/tests/topotests/bgp_bmp/test_bgp_bmp_1.py @@ -78,6 +78,13 @@ def setup_module(mod): "tcpdump -nni r1-eth0 -s 0 -w {} &".format(pcap_file), stdout=None ) + tgen.net["r2"].cmd( + """ +ip link add vrf1 type vrf table 10 +ip link set vrf1 up +""" + ) + for rname, router in tgen.routers().items(): logger.info("Loading router %s" % rname) router.load_frr_config( From d6f8a41bce058cb74d735ad810efc77388773d60 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 6 Nov 2024 11:11:19 +0100 Subject: [PATCH 7/8] tests: check bgp vpn table in bgp_vpnv4_route_leak_basic Check bgp vpn table in bgp_vpnv4_route_leak_basic Signed-off-by: Louis Scalbert --- .../bgp_vpnv4_route_leak_basic/r1/frr.conf | 5 + .../r1/show_bgp_ipv4_vpn_add_zita.json | 137 ++++++++++++++++++ .../r1/show_bgp_ipv4_vpn_eva_down.json | 102 +++++++++++++ .../r1/show_bgp_ipv4_vpn_init.json | 102 +++++++++++++ .../r1/show_bgp_ipv4_vpn_zita_up.json | 119 +++++++++++++++ .../test_bgp_vpnv4_route_leak_basic.py | 57 ++++++++ 6 files changed, 522 insertions(+) create mode 100644 tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_add_zita.json create mode 100644 tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_eva_down.json create mode 100644 tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_init.json create mode 100644 tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_zita_up.json diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/frr.conf b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/frr.conf index d3ababde3a81..56657edd9522 100644 --- a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/frr.conf +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/frr.conf @@ -28,6 +28,9 @@ ip router-id 10.0.4.1 ! router bgp 99 no bgp ebgp-requires-policy + ! 10.0.4.254 peer session will not be established + ! it is there just to activate the ipv4 vpn table + neighbor 10.0.4.254 remote-as external address-family ipv4 unicast redistribute connected rd vpn export 10.0.4.1:1 @@ -36,6 +39,8 @@ router bgp 99 export vpn import vpn ! + address-family ipv4 vpn + neighbor 10.0.4.254 activate ! router bgp 99 vrf DONNA no bgp ebgp-requires-policy diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_add_zita.json b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_add_zita.json new file mode 100644 index 000000000000..7056428ca9d5 --- /dev/null +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_add_zita.json @@ -0,0 +1,137 @@ +{ + "routerId": "10.0.4.1", + "localAS": 99, + "routes": { + "routeDistinguishers": { + "10.0.4.1:1": { + "10.0.0.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.1.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "EVA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.2.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.3.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "EVA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.4.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "default", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "172.16.3.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "IGP", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "172.16.101.0/24": [ + { + "valid": null, + "pathFrom": "external", + "path": "", + "origin": "IGP", + "announceNexthopSelf": true, + "nhVrfName": "ZITA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ] + } + } + } +} + diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_eva_down.json b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_eva_down.json new file mode 100644 index 000000000000..17d06f2576cb --- /dev/null +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_eva_down.json @@ -0,0 +1,102 @@ +{ + "routerId": "10.0.4.1", + "localAS": 99, + "routes": { + "routeDistinguishers": { + "10.0.4.1:1": { + "10.0.0.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.1.0/24": [ + { + "valid": null, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "EVA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.2.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.3.0/24": [ + { + "valid": null, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "EVA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.4.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "default", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "172.16.101.0/24": null + } + } + } +} + diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_init.json b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_init.json new file mode 100644 index 000000000000..d9a37f9b2c8f --- /dev/null +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_init.json @@ -0,0 +1,102 @@ +{ + "routerId": "10.0.4.1", + "localAS": 99, + "routes": { + "routeDistinguishers": { + "10.0.4.1:1": { + "10.0.0.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.1.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "EVA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.2.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.3.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "EVA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.4.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "default", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "172.16.101.0/24": null + } + } + } +} + diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_zita_up.json b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_zita_up.json new file mode 100644 index 000000000000..861422c84f48 --- /dev/null +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_zita_up.json @@ -0,0 +1,119 @@ +{ + "routerId": "10.0.4.1", + "localAS": 99, + "routes": { + "routeDistinguishers": { + "10.0.4.1:1": { + "10.0.0.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.1.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "EVA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.2.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.3.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "EVA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.4.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "default", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "172.16.101.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "IGP", + "announceNexthopSelf": true, + "nhVrfName": "ZITA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ] + } + } + } +} + diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/test_bgp_vpnv4_route_leak_basic.py b/tests/topotests/bgp_vpnv4_route_leak_basic/test_bgp_vpnv4_route_leak_basic.py index b7f72ccea166..9026b499066e 100644 --- a/tests/topotests/bgp_vpnv4_route_leak_basic/test_bgp_vpnv4_route_leak_basic.py +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/test_bgp_vpnv4_route_leak_basic.py @@ -13,6 +13,7 @@ Test basic VPNv4 route leaking """ +import json import os import sys from functools import partial @@ -60,6 +61,22 @@ def teardown_module(mod): tgen.stop_topology() +def test_bgp_convergence(): + tgen = get_topogen() + # Don't run this test if we have any failure. + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + + json_file = "{}/{}/show_bgp_ipv4_vpn_init.json".format(CWD, r1.name) + expect = json.loads(open(json_file).read()) + + test_func = partial(topotest.router_json_cmp, r1, "show bgp ipv4 vpn json", expect) + result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result, "BGP IPv4 VPN table check failed:\n{}".format(diff) + + def test_vrf_route_leak_donna(): logger.info("Ensure that routes are leaked back and forth") tgen = get_topogen() @@ -297,6 +314,14 @@ def check_vrf_table(router, vrf, expect): result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) assert result, "BGP VRF DONNA check failed:\n{}".format(diff) + # check BGP IPv4 VPN table + json_file = "{}/{}/show_bgp_ipv4_vpn_eva_down.json".format(CWD, r1.name) + expect = json.loads(open(json_file).read()) + + test_func = partial(topotest.router_json_cmp, r1, "show bgp ipv4 vpn json", expect) + result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result, "BGP IPv4 VPN table check failed:\n{}".format(diff) + def test_vrf_route_leak_donna_after_eva_up(): logger.info("Ensure that route states change after EVA interface goes up") @@ -352,6 +377,14 @@ def test_vrf_route_leak_donna_after_eva_up(): result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) assert result, "BGP VRF DONNA check failed:\n{}".format(diff) + # check BGP IPv4 VPN table + json_file = "{}/{}/show_bgp_ipv4_vpn_init.json".format(CWD, r1.name) + expect = json.loads(open(json_file).read()) + + test_func = partial(topotest.router_json_cmp, r1, "show bgp ipv4 vpn json", expect) + result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result, "BGP IPv4 VPN table check failed:\n{}".format(diff) + def test_vrf_route_leak_donna_add_vrf_zita(): logger.info("Add VRF ZITA and ensure that the route from VRF ZITA is updated") @@ -374,6 +407,14 @@ def test_vrf_route_leak_donna_add_vrf_zita(): result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) assert result, "BGP VRF DONNA check failed:\n{}".format(diff) + # check BGP IPv4 VPN table + json_file = "{}/{}/show_bgp_ipv4_vpn_add_zita.json".format(CWD, r1.name) + expect = json.loads(open(json_file).read()) + + test_func = partial(topotest.router_json_cmp, r1, "show bgp ipv4 vpn json", expect) + result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result, "BGP IPv4 VPN table check failed:\n{}".format(diff) + def test_vrf_route_leak_donna_set_zita_up(): logger.info("Set VRF ZITA up and ensure that the route from VRF ZITA is updated") @@ -415,6 +456,14 @@ def test_vrf_route_leak_donna_set_zita_up(): result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) assert result, "BGP VRF DONNA check failed:\n{}".format(diff) + # check BGP IPv4 VPN table + json_file = "{}/{}/show_bgp_ipv4_vpn_zita_up.json".format(CWD, r1.name) + expect = json.loads(open(json_file).read()) + + test_func = partial(topotest.router_json_cmp, r1, "show bgp ipv4 vpn json", expect) + result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result, "BGP IPv4 VPN table check failed:\n{}".format(diff) + def test_vrf_route_leak_donna_delete_vrf_zita(): logger.info("Delete VRF ZITA and ensure that the route from VRF ZITA is deleted") @@ -437,6 +486,14 @@ def test_vrf_route_leak_donna_delete_vrf_zita(): result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) assert result, "BGP VRF DONNA check failed:\n{}".format(diff) + # check BGP IPv4 VPN table + json_file = "{}/{}/show_bgp_ipv4_vpn_init.json".format(CWD, r1.name) + expect = json.loads(open(json_file).read()) + + test_func = partial(topotest.router_json_cmp, r1, "show bgp ipv4 vpn json", expect) + result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result, "BGP IPv4 VPN table check failed:\n{}".format(diff) + def test_memory_leak(): "Run the memory leak test and report results." From d2ae8945b5dfaf7c06faee91808bf91b3b1a71ac Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Tue, 5 Nov 2024 17:13:38 +0100 Subject: [PATCH 8/8] tests: bgp_vpnv4_route_leak_basic add del static prefix In bgp_vpnv4_route_leak_basic, remove and add back the static prefix 172.16.3.0/24 on VRF DONNA. Before the previous fixes, the 172.16.3.0/24 prefix re-appeared when it was added back in the VPN table but it was marked as invalid. Signed-off-by: Louis Scalbert --- .../bgp_vpnv4_route_leak_basic/r1/frr.conf | 5 + .../r1/show_bgp_ipv4_vpn_add_zita.json | 18 +++ .../show_bgp_ipv4_vpn_del_donna_prefix.json | 103 ++++++++++++++ .../r1/show_bgp_ipv4_vpn_eva_down.json | 18 +++ .../r1/show_bgp_ipv4_vpn_init.json | 18 +++ .../r1/show_bgp_ipv4_vpn_zita_up.json | 18 +++ .../test_bgp_vpnv4_route_leak_basic.py | 128 ++++++++++++++++++ 7 files changed, 308 insertions(+) create mode 100644 tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_del_donna_prefix.json diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/frr.conf b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/frr.conf index 56657edd9522..e3f8b242a142 100644 --- a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/frr.conf +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/frr.conf @@ -1,3 +1,7 @@ +vrf DONNA + ip route 172.16.3.0/24 10.0.0.254 +exit-vrf +! int dummy0 ip address 10.0.4.1/24 no shut @@ -46,6 +50,7 @@ router bgp 99 vrf DONNA no bgp ebgp-requires-policy address-family ipv4 unicast redistribute connected + network 172.16.3.0/24 label vpn export 101 rd vpn export 10.0.4.1:1 rt vpn export 10.0.4.1:101 diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_add_zita.json b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_add_zita.json index 7056428ca9d5..c7ff2f4f8064 100644 --- a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_add_zita.json +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_add_zita.json @@ -112,6 +112,24 @@ ] } ], + "172.16.3.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "IGP", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], "172.16.101.0/24": [ { "valid": null, diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_del_donna_prefix.json b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_del_donna_prefix.json new file mode 100644 index 000000000000..1797d7858232 --- /dev/null +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_del_donna_prefix.json @@ -0,0 +1,103 @@ +{ + "routerId": "10.0.4.1", + "localAS": 99, + "routes": { + "routeDistinguishers": { + "10.0.4.1:1": { + "10.0.0.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.1.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "EVA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.2.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.3.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "EVA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "10.0.4.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "default", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], + "172.16.3.0/24": null, + "172.16.101.0/24": null + } + } + } +} + diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_eva_down.json b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_eva_down.json index 17d06f2576cb..4b0eaaa052a3 100644 --- a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_eva_down.json +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_eva_down.json @@ -94,6 +94,24 @@ ] } ], + "172.16.3.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "IGP", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], "172.16.101.0/24": null } } diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_init.json b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_init.json index d9a37f9b2c8f..de18bc84638f 100644 --- a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_init.json +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_init.json @@ -94,6 +94,24 @@ ] } ], + "172.16.3.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "IGP", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], "172.16.101.0/24": null } } diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_zita_up.json b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_zita_up.json index 861422c84f48..0c249e241ee5 100644 --- a/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_zita_up.json +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/r1/show_bgp_ipv4_vpn_zita_up.json @@ -94,6 +94,24 @@ ] } ], + "172.16.3.0/24": [ + { + "valid": true, + "pathFrom": "external", + "path": "", + "origin": "IGP", + "announceNexthopSelf": true, + "nhVrfName": "DONNA", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "r1", + "afi": "ipv4", + "used": true + } + ] + } + ], "172.16.101.0/24": [ { "valid": true, diff --git a/tests/topotests/bgp_vpnv4_route_leak_basic/test_bgp_vpnv4_route_leak_basic.py b/tests/topotests/bgp_vpnv4_route_leak_basic/test_bgp_vpnv4_route_leak_basic.py index 9026b499066e..67e53cb0ccdd 100644 --- a/tests/topotests/bgp_vpnv4_route_leak_basic/test_bgp_vpnv4_route_leak_basic.py +++ b/tests/topotests/bgp_vpnv4_route_leak_basic/test_bgp_vpnv4_route_leak_basic.py @@ -136,6 +136,19 @@ def test_vrf_route_leak_donna(): ], }, ], + "172.16.3.0/24": [ + { + "protocol": "static", + "selected": True, + "nexthops": [ + { + "fib": True, + "interfaceName": "dummy1", + "active": True, + } + ], + }, + ], "172.16.101.0/24": None, } @@ -195,6 +208,20 @@ def test_vrf_route_leak_eva(): "protocol": "connected", } ], + "172.16.3.0/24": [ + { + "protocol": "bgp", + "selected": True, + "nexthops": [ + { + "fib": True, + "interfaceName": "DONNA", + "vrf": "DONNA", + "active": True, + } + ], + }, + ], "172.16.101.0/24": None, } @@ -249,6 +276,20 @@ def test_vrf_route_leak_default(): "protocol": "connected", } ], + "172.16.3.0/24": [ + { + "protocol": "bgp", + "selected": True, + "nexthops": [ + { + "fib": True, + "interfaceName": "DONNA", + "vrf": "DONNA", + "active": True, + } + ], + }, + ], } test_func = partial(topotest.router_json_cmp, r1, "show ip route json", expect) @@ -495,6 +536,93 @@ def test_vrf_route_leak_donna_delete_vrf_zita(): assert result, "BGP IPv4 VPN table check failed:\n{}".format(diff) +def test_vrf_route_leak_default_delete_prefix(): + logger.info( + "Remove BGP static prefix 172.16.3.0/24 from VRF DONNA and ensure that the route is deleted on default" + ) + tgen = get_topogen() + # Don't run this test if we have any failure. + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + r1.vtysh_cmd( + """ +configure +router bgp 99 vrf DONNA + address-family ipv4 unicast + no network 172.16.3.0/24 +""" + ) + + # Test default VRF. + expect = { + "172.16.3.0/24": None, + } + + test_func = partial(topotest.router_json_cmp, r1, "show ip route json", expect) + result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result, "BGP VRF default check failed:\n{}".format(diff) + + # check BGP IPv4 VPN table + json_file = "{}/{}/show_bgp_ipv4_vpn_del_donna_prefix.json".format(CWD, r1.name) + expect = json.loads(open(json_file).read()) + + test_func = partial(topotest.router_json_cmp, r1, "show bgp ipv4 vpn json", expect) + result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result, "BGP IPv4 VPN table check failed:\n{}".format(diff) + + +def test_vrf_route_leak_default_prefix_back(): + logger.info( + "Set back BGP static prefix 172.16.3.0/24 to VRF DONNA and ensure that the route is set on default" + ) + tgen = get_topogen() + # Don't run this test if we have any failure. + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + r1.vtysh_cmd( + """ +configure +router bgp 99 vrf DONNA + address-family ipv4 unicast + network 172.16.3.0/24 +""" + ) + + # Test default VRF. + expect = { + "172.16.3.0/24": [ + { + "protocol": "bgp", + "selected": True, + "nexthops": [ + { + "fib": True, + "interfaceName": "DONNA", + "vrf": "DONNA", + "active": True, + } + ], + }, + ], + } + + test_func = partial(topotest.router_json_cmp, r1, "show ip route json", expect) + result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result, "BGP VRF default check failed:\n{}".format(diff) + + # check BGP IPv4 VPN table + json_file = "{}/{}/show_bgp_ipv4_vpn_init.json".format(CWD, r1.name) + expect = json.loads(open(json_file).read()) + + test_func = partial(topotest.router_json_cmp, r1, "show bgp ipv4 vpn json", expect) + result, diff = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result, "BGP IPv4 VPN table check failed:\n{}".format(diff) + + def test_memory_leak(): "Run the memory leak test and report results." tgen = get_topogen()