From 580c605194b3893a1d61a997a7b9d62e2d877427 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 30 Nov 2023 17:29:20 +0100 Subject: [PATCH 1/3] staticd: fix changing to source auto in bfd monitor When monitoring a static route with BFD multi-hop, the source IP can be either configured or retrieved from NextHop-Tracking (NHT). After removing a configured source, the source is supposed to be retrieved from NHT but it remains to the previous value. This is problematic if the user desires to fix the configuration of a incorrect source IP. For example, theses two commands results in the incorrect state: > ip route 10.0.0.0/24 10.1.0.1 bfd multi-hop source 10.2.2.2 > ip route 10.0.0.0/24 10.1.0.1 bfd multi-hop When removing the source, BFD is unable to find the source from NHT via bfd_nht_update() were called. Force zebra to resend the information to BFD by unregistering and registering again NHT. The (...)/frr-nexthops/nexthop northbound apply_finish function will trigger a call to static_install_nexthop() that does a call to static_zebra_nht_register(nh, true); Fixes: b7ca809d1c ("lib: BFD automatic source selection") Signed-off-by: Louis Scalbert --- staticd/static_nb_config.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/staticd/static_nb_config.c b/staticd/static_nb_config.c index ede2e387544c..8a497304a81e 100644 --- a/staticd/static_nb_config.c +++ b/staticd/static_nb_config.c @@ -18,6 +18,7 @@ #include "static_vrf.h" #include "static_routes.h" #include "static_nb.h" +#include "static_zebra.h" static int static_path_list_create(struct nb_cb_create_args *args) @@ -960,6 +961,17 @@ int route_next_hop_bfd_source_destroy(struct nb_cb_destroy_args *args) sn = nb_running_get_entry(args->dnode, NULL, true); static_next_hop_bfd_auto_source(sn); + + /* NHT information are needed by BFD to automatically find the source + * + * Force zebra to resend the information to BFD by unregistering and + * registering again NHT. The (...)/frr-nexthops/nexthop northbound + * apply_finish function will trigger a call to static_install_nexthop() + * that does a call to static_zebra_nht_register(nh, true); + * static_zebra_nht_register(sn, false); + */ + static_zebra_nht_register(sn, false); + return NB_OK; } From 8f5bf65f267f5a60a08a4206ea32ced39a51a100 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 30 Nov 2023 16:01:09 +0100 Subject: [PATCH 2/3] topotests: redispatch tests in bfd_topo3 Redispatch tests in bfd_topo3 in order to prepare next commit. Cosmetic change. Signed-off-by: Louis Scalbert --- tests/topotests/bfd_topo3/test_bfd_topo3.py | 78 +++++++++++---------- 1 file changed, 42 insertions(+), 36 deletions(-) diff --git a/tests/topotests/bfd_topo3/test_bfd_topo3.py b/tests/topotests/bfd_topo3/test_bfd_topo3.py index c0dc052a38d9..f6985f25c483 100644 --- a/tests/topotests/bfd_topo3/test_bfd_topo3.py +++ b/tests/topotests/bfd_topo3/test_bfd_topo3.py @@ -65,6 +65,41 @@ def setup_module(mod): tgen.start_router() +def expect_static_bfd_output(router, filename): + "Load JSON file and compare with 'show bfd peer json'" + + tgen = get_topogen() + + logger.info("waiting BFD configuration on router {}".format(router)) + bfd_config = json.loads(open("{}/{}/{}.json".format(CWD, router, filename)).read()) + test_func = partial( + topotest.router_json_cmp, + tgen.gears[router], + "show bfd static route json", + bfd_config, + ) + _, result = topotest.run_and_expect(test_func, None, count=20, wait=1) + assertmsg = '"{}" BFD static route status failure'.format(router) + assert result is None, assertmsg + + +def expect_route_missing(router, iptype, route): + "Wait until route is present on RIB for protocol." + + tgen = get_topogen() + + logger.info("waiting route {} to disapear in {}".format(route, router)) + test_func = partial( + topotest.router_json_cmp, + tgen.gears[router], + "show {} route json".format(iptype), + {route: None}, + ) + rv, result = topotest.run_and_expect(test_func, None, count=20, wait=1) + assertmsg = '"{}" convergence failure'.format(router) + assert result is None, assertmsg + + def test_wait_bgp_convergence(): "Wait for BGP to converge" tgen = get_topogen() @@ -166,7 +201,7 @@ def expect_bfd_configuration(router): expect_bfd_configuration("r6") -def test_static_route_monitoring(): +def test_static_route_monitoring_convergence(): "Test static route monitoring output." tgen = get_topogen() if tgen.routers_have_failure(): @@ -174,32 +209,9 @@ def test_static_route_monitoring(): logger.info("test BFD static route status") - def expect_static_bfd_output(router, filename): - "Load JSON file and compare with 'show bfd peer json'" - logger.info("waiting BFD configuration on router {}".format(router)) - bfd_config = json.loads( - open("{}/{}/{}.json".format(CWD, router, filename)).read() - ) - test_func = partial( - topotest.router_json_cmp, - tgen.gears[router], - "show bfd static route json", - bfd_config, - ) - _, result = topotest.run_and_expect(test_func, None, count=20, wait=1) - assertmsg = '"{}" BFD static route status failure'.format(router) - assert result is None, assertmsg - expect_static_bfd_output("r3", "bfd-static") expect_static_bfd_output("r6", "bfd-static") - logger.info("Setting r4 link down ...") - - tgen.gears["r4"].link_enable("r4-eth0", False) - - expect_static_bfd_output("r3", "bfd-static-down") - expect_static_bfd_output("r6", "bfd-static-down") - def test_expect_static_rib_removal(): "Test that route got removed from RIB (staticd and bgpd)." @@ -208,18 +220,12 @@ def test_expect_static_rib_removal(): if tgen.routers_have_failure(): pytest.skip(tgen.errors) - def expect_route_missing(router, iptype, route): - "Wait until route is present on RIB for protocol." - logger.info("waiting route {} to disapear in {}".format(route, router)) - test_func = partial( - topotest.router_json_cmp, - tgen.gears[router], - "show {} route json".format(iptype), - {route: None}, - ) - rv, result = topotest.run_and_expect(test_func, None, count=20, wait=1) - assertmsg = '"{}" convergence failure'.format(router) - assert result is None, assertmsg + logger.info("Setting r4 link down ...") + + tgen.gears["r4"].link_enable("r4-eth0", False) + + expect_static_bfd_output("r3", "bfd-static-down") + expect_static_bfd_output("r6", "bfd-static-down") expect_route_missing("r1", "ip", "10.254.254.5/32") expect_route_missing("r2", "ip", "10.254.254.5/32") From 94640da23492df86a45c1a919bc64f809cd2a1ec Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 30 Nov 2023 16:07:40 +0100 Subject: [PATCH 3/3] topotests: test wrong bfd source in bfd_topo3 Test setting a wrong bfd source and restore the source auto parameter. Signed-off-by: Louis Scalbert --- tests/topotests/bfd_topo3/test_bfd_topo3.py | 39 +++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/topotests/bfd_topo3/test_bfd_topo3.py b/tests/topotests/bfd_topo3/test_bfd_topo3.py index f6985f25c483..f767b0e7b99b 100644 --- a/tests/topotests/bfd_topo3/test_bfd_topo3.py +++ b/tests/topotests/bfd_topo3/test_bfd_topo3.py @@ -213,6 +213,45 @@ def test_static_route_monitoring_convergence(): expect_static_bfd_output("r6", "bfd-static") +def test_static_route_monitoring_wrong_source(): + "Test that static monitoring fails if setting a wrong source." + + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + logger.info("test route wrong ") + + tgen.gears["r3"].vtysh_cmd( + """ +configure +ipv6 route 2001:db8:5::/64 2001:db8:4::3 bfd multi-hop source 2001:db8:4::2 profile slow-tx +""" + ) + + expect_route_missing("r3", "ipv6", "2001:db8:5::/64") + + +def test_static_route_monitoring_unset_source(): + "Test that static monitoring fails if setting a wrong source." + + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + logger.info("test route wrong ") + + tgen.gears["r3"].vtysh_cmd( + """ +configure +ipv6 route 2001:db8:5::/64 2001:db8:4::3 bfd multi-hop profile slow-tx +""" + ) + + expect_static_bfd_output("r3", "bfd-static") + expect_static_bfd_output("r6", "bfd-static") + + def test_expect_static_rib_removal(): "Test that route got removed from RIB (staticd and bgpd)."