From 625fe20b6d31ede873dbffef50c1bcb2cdaa1841 Mon Sep 17 00:00:00 2001 From: Ryoga Saito Date: Sun, 4 Dec 2022 16:51:24 +0900 Subject: [PATCH 1/2] tests: Fix topotests for bgp_srv6l3vpn In bgp_srv6l3vpn tests, check_ping checks reachability. However, this function have a bug and if we set expect_connected to True, check will pass even if all ping packets are lost. This commit fixes this issue. Signed-off-by: Ryoga Saito --- .../test_bgp_srv6l3vpn_to_bgp_vrf.py | 2 +- .../test_bgp_srv6l3vpn_to_bgp_vrf2.py | 34 +++++++------- .../test_bgp_srv6l3vpn_to_bgp_vrf3.py | 46 +++++++++++++------ 3 files changed, 51 insertions(+), 31 deletions(-) diff --git a/tests/topotests/bgp_srv6l3vpn_to_bgp_vrf/test_bgp_srv6l3vpn_to_bgp_vrf.py b/tests/topotests/bgp_srv6l3vpn_to_bgp_vrf/test_bgp_srv6l3vpn_to_bgp_vrf.py index 10d890effb..7c23d4c576 100755 --- a/tests/topotests/bgp_srv6l3vpn_to_bgp_vrf/test_bgp_srv6l3vpn_to_bgp_vrf.py +++ b/tests/topotests/bgp_srv6l3vpn_to_bgp_vrf/test_bgp_srv6l3vpn_to_bgp_vrf.py @@ -155,7 +155,7 @@ def _check(name, dest_addr, match): if match not in output: return "ping fail" - match = "{} packet loss".format("0%" if expect_connected else "100%") + match = ", {} packet loss".format("0%" if expect_connected else "100%") logger.info("[+] check {} {} {}".format(name, dest_addr, match)) tgen = get_topogen() func = functools.partial(_check, name, dest_addr, match) diff --git a/tests/topotests/bgp_srv6l3vpn_to_bgp_vrf2/test_bgp_srv6l3vpn_to_bgp_vrf2.py b/tests/topotests/bgp_srv6l3vpn_to_bgp_vrf2/test_bgp_srv6l3vpn_to_bgp_vrf2.py index af66a5a791..1209e9d6e3 100755 --- a/tests/topotests/bgp_srv6l3vpn_to_bgp_vrf2/test_bgp_srv6l3vpn_to_bgp_vrf2.py +++ b/tests/topotests/bgp_srv6l3vpn_to_bgp_vrf2/test_bgp_srv6l3vpn_to_bgp_vrf2.py @@ -69,10 +69,12 @@ def setup_module(mod): tgen.start_topology() for rname, router in tgen.routers().items(): router.run("/bin/bash {}/{}/setup.sh".format(CWD, rname)) - router.load_config(TopoRouter.RD_ZEBRA, - os.path.join(CWD, '{}/zebra.conf'.format(rname))) - router.load_config(TopoRouter.RD_BGP, - os.path.join(CWD, '{}/bgpd.conf'.format(rname))) + router.load_config( + TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname)) + ) + router.load_config( + TopoRouter.RD_BGP, os.path.join(CWD, "{}/bgpd.conf".format(rname)) + ) tgen.gears["r1"].run("sysctl net.vrf.strict_mode=1") tgen.gears["r1"].run("ip link add vrf10 type vrf table 10") @@ -114,7 +116,7 @@ def _check(name, dest_addr, match): logger.info(output) assert match in output, "ping fail" - match = "{} packet loss".format("0%" if expect_connected else "100%") + match = ", {} packet loss".format("0%" if expect_connected else "100%") logger.info("[+] check {} {} {}".format(name, dest_addr, match)) tgen = get_topogen() func = functools.partial(_check, name, dest_addr, match) @@ -131,7 +133,7 @@ def _check(name, dest_addr, match): expected = open_json_file("{}/{}".format(CWD, expected_file)) return topotest.json_cmp(output, expected) - logger.info("[+] check {} \"{}\" {}".format(name, cmd, expected_file)) + logger.info('[+] check {} "{}" {}'.format(name, cmd, expected_file)) tgen = get_topogen() func = functools.partial(_check, name, cmd, expected_file) success, result = topotest.run_and_expect(func, None, count=10, wait=0.5) @@ -154,16 +156,16 @@ def test_rib(): def test_ping(): - check_ping("ce1", "192.168.2.2", " 0% packet loss") - check_ping("ce1", "192.168.3.2", " 0% packet loss") - check_ping("ce1", "192.168.4.2", " 100% packet loss") - check_ping("ce1", "192.168.5.2", " 100% packet loss") - check_ping("ce1", "192.168.6.2", " 100% packet loss") - check_ping("ce4", "192.168.1.2", " 100% packet loss") - check_ping("ce4", "192.168.2.2", " 100% packet loss") - check_ping("ce4", "192.168.3.2", " 100% packet loss") - check_ping("ce4", "192.168.5.2", " 0% packet loss") - check_ping("ce4", "192.168.6.2", " 0% packet loss") + check_ping("ce1", "192.168.2.2", True) + check_ping("ce1", "192.168.3.2", True) + check_ping("ce1", "192.168.4.2", False) + check_ping("ce1", "192.168.5.2", False) + check_ping("ce1", "192.168.6.2", False) + check_ping("ce4", "192.168.1.2", False) + check_ping("ce4", "192.168.2.2", False) + check_ping("ce4", "192.168.3.2", False) + check_ping("ce4", "192.168.5.2", True) + check_ping("ce4", "192.168.6.2", True) if __name__ == "__main__": diff --git a/tests/topotests/bgp_srv6l3vpn_to_bgp_vrf3/test_bgp_srv6l3vpn_to_bgp_vrf3.py b/tests/topotests/bgp_srv6l3vpn_to_bgp_vrf3/test_bgp_srv6l3vpn_to_bgp_vrf3.py index e410bbbdeb..61ac7418a4 100644 --- a/tests/topotests/bgp_srv6l3vpn_to_bgp_vrf3/test_bgp_srv6l3vpn_to_bgp_vrf3.py +++ b/tests/topotests/bgp_srv6l3vpn_to_bgp_vrf3/test_bgp_srv6l3vpn_to_bgp_vrf3.py @@ -66,10 +66,12 @@ def setup_module(mod): tgen.start_topology() for rname, router in tgen.routers().items(): router.run("/bin/bash {}/{}/setup.sh".format(CWD, rname)) - router.load_config(TopoRouter.RD_ZEBRA, - os.path.join(CWD, '{}/zebra.conf'.format(rname))) - router.load_config(TopoRouter.RD_BGP, - os.path.join(CWD, '{}/bgpd.conf'.format(rname))) + router.load_config( + TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname)) + ) + router.load_config( + TopoRouter.RD_BGP, os.path.join(CWD, "{}/bgpd.conf".format(rname)) + ) tgen.gears["r1"].run("sysctl net.vrf.strict_mode=1") tgen.gears["r1"].run("ip link add vrf10 type vrf table 10") @@ -111,7 +113,7 @@ def _check(name, dest_addr, match): logger.info(output) assert match in output, "ping fail" - match = "{} packet loss".format("0%" if expect_connected else "100%") + match = ", {} packet loss".format("0%" if expect_connected else "100%") logger.info("[+] check {} {} {}".format(name, dest_addr, match)) tgen = get_topogen() func = functools.partial(_check, name, dest_addr, match) @@ -144,7 +146,7 @@ def _check(name, dest_addr, match): expected = open_json_file("{}/{}".format(CWD, expected_file)) return topotest.json_cmp(output, expected) - logger.info("[+] check {} \"{}\" {}".format(name, cmd, expected_file)) + logger.info('[+] check {} "{}" {}'.format(name, cmd, expected_file)) tgen = get_topogen() func = functools.partial(_check, name, cmd, expected_file) success, result = topotest.run_and_expect(func, None, count=10, wait=0.5) @@ -214,10 +216,18 @@ def test_bgp_sid_vpn_export_disable(): no sid vpn per-vrf export """ ) - check_rib("r1", "show bgp ipv4 vpn json", "r1/vpnv4_rib_sid_vpn_export_disabled.json") - check_rib("r2", "show bgp ipv4 vpn json", "r2/vpnv4_rib_sid_vpn_export_disabled.json") - check_rib("r1", "show bgp ipv6 vpn json", "r1/vpnv6_rib_sid_vpn_export_disabled.json") - check_rib("r2", "show bgp ipv6 vpn json", "r2/vpnv6_rib_sid_vpn_export_disabled.json") + check_rib( + "r1", "show bgp ipv4 vpn json", "r1/vpnv4_rib_sid_vpn_export_disabled.json" + ) + check_rib( + "r2", "show bgp ipv4 vpn json", "r2/vpnv4_rib_sid_vpn_export_disabled.json" + ) + check_rib( + "r1", "show bgp ipv6 vpn json", "r1/vpnv6_rib_sid_vpn_export_disabled.json" + ) + check_rib( + "r2", "show bgp ipv6 vpn json", "r2/vpnv6_rib_sid_vpn_export_disabled.json" + ) check_ping4("ce1", "192.168.2.2", False) check_ping6("ce1", "2001:2::2", False) @@ -233,10 +243,18 @@ def test_bgp_sid_vpn_export_reenable(): sid vpn per-vrf export auto """ ) - check_rib("r1", "show bgp ipv4 vpn json", "r1/vpnv4_rib_sid_vpn_export_reenabled.json") - check_rib("r2", "show bgp ipv4 vpn json", "r2/vpnv4_rib_sid_vpn_export_reenabled.json") - check_rib("r1", "show bgp ipv6 vpn json", "r1/vpnv6_rib_sid_vpn_export_reenabled.json") - check_rib("r2", "show bgp ipv6 vpn json", "r2/vpnv6_rib_sid_vpn_export_reenabled.json") + check_rib( + "r1", "show bgp ipv4 vpn json", "r1/vpnv4_rib_sid_vpn_export_reenabled.json" + ) + check_rib( + "r2", "show bgp ipv4 vpn json", "r2/vpnv4_rib_sid_vpn_export_reenabled.json" + ) + check_rib( + "r1", "show bgp ipv6 vpn json", "r1/vpnv6_rib_sid_vpn_export_reenabled.json" + ) + check_rib( + "r2", "show bgp ipv6 vpn json", "r2/vpnv6_rib_sid_vpn_export_reenabled.json" + ) check_ping4("ce1", "192.168.2.2", True) check_ping6("ce1", "2001:2::2", True) From 4915b5fd8878b76c9122f302ed7466a98c6aa357 Mon Sep 17 00:00:00 2001 From: Ryoga Saito Date: Sun, 4 Dec 2022 16:53:48 +0900 Subject: [PATCH 2/2] bgpd: Fix delete_vrf_tovpn_sid The first argument of sid_unregister should be default bgp instance. However, these functions passed VRF bgp instance to this funciton. Signed-off-by: Ryoga Saito --- bgpd/bgp_mplsvpn.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 18cb90763c..12b68f2607 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -888,7 +888,7 @@ void delete_vrf_tovpn_sid_per_af(struct bgp *bgp_vpn, struct bgp *bgp_vrf, srv6_locator_chunk_free(&bgp_vrf->vpn_policy[afi].tovpn_sid_locator); if (bgp_vrf->vpn_policy[afi].tovpn_sid) { - sid_unregister(bgp_vrf, bgp_vrf->vpn_policy[afi].tovpn_sid); + sid_unregister(bgp_vpn, bgp_vrf->vpn_policy[afi].tovpn_sid); XFREE(MTYPE_BGP_SRV6_SID, bgp_vrf->vpn_policy[afi].tovpn_sid); } bgp_vrf->vpn_policy[afi].tovpn_sid_transpose_label = 0; @@ -915,7 +915,7 @@ void delete_vrf_tovpn_sid_per_vrf(struct bgp *bgp_vpn, struct bgp *bgp_vrf) srv6_locator_chunk_free(&bgp_vrf->tovpn_sid_locator); if (bgp_vrf->tovpn_sid) { - sid_unregister(bgp_vrf, bgp_vrf->tovpn_sid); + sid_unregister(bgp_vpn, bgp_vrf->tovpn_sid); XFREE(MTYPE_BGP_SRV6_SID, bgp_vrf->tovpn_sid); } bgp_vrf->tovpn_sid_transpose_label = 0;