From b24bad5632eae14192fe123963be85b2d2c15a5c Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 1 Jun 2023 11:39:30 +0200 Subject: [PATCH 1/3] tests: fix mpls table check in isis_sr_flex_algo_topo1 Some test steps result in removing some entries in the MPLS forwarding table. However, these steps pass before the entries are actually removed. Use the exact JSON comparison so that the removal of the entries is checked. Fixes: 1a61ef95b2 ("tests: add isis_sr_flex_algo_topo1 for flex-algo") Signed-off-by: Louis Scalbert --- .../test_isis_sr_flex_algo_topo1.py | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/topotests/isis_sr_flex_algo_topo1/test_isis_sr_flex_algo_topo1.py b/tests/topotests/isis_sr_flex_algo_topo1/test_isis_sr_flex_algo_topo1.py index 85600beb0e..fbe179bd3f 100755 --- a/tests/topotests/isis_sr_flex_algo_topo1/test_isis_sr_flex_algo_topo1.py +++ b/tests/topotests/isis_sr_flex_algo_topo1/test_isis_sr_flex_algo_topo1.py @@ -38,6 +38,7 @@ import sys import pytest import json import tempfile +from copy import deepcopy from functools import partial # Save the Current Working Directory to find configuration files. @@ -130,6 +131,30 @@ def setup_testcase(msg): return tgen +def router_json_cmp_exact_filter(router, cmd, expected): + output = router.vtysh_cmd(cmd) + logger.info("{}: {}\n{}".format(router.name, cmd, output)) + + json_output = json.loads(output) + router_output = deepcopy(json_output) + + # filter out dynamic data from "show mpls table" + for label, data in json_output.items(): + if "1500" in label: + # filter out SR local labels + router_output.pop(label) + continue + nexthops = data.get("nexthops", []) + for i in range(len(nexthops)): + if "fe80::" in nexthops[i].get("nexthop"): + router_output.get(label).get("nexthops")[i].pop("nexthop") + elif "." in nexthops[i].get("nexthop"): + # IPv4, just checking the nexthop + router_output.get(label).get("nexthops")[i].pop("interface") + + return topotest.json_cmp(router_output, expected, exact=True) + + def router_compare_json_output(rname, command, reference): "Compare router JSON output" @@ -139,7 +164,9 @@ def router_compare_json_output(rname, command, reference): expected = json.loads(reference) # Run test function until we get an result. Wait at most 60 seconds. - test_func = partial(topotest.router_json_cmp, tgen.gears[rname], command, expected) + test_func = partial( + router_json_cmp_exact_filter, tgen.gears[rname], command, expected + ) _, diff = topotest.run_and_expect(test_func, None, count=120, wait=0.5) assertmsg = '"{}" JSON output mismatches the expected result'.format(rname) assert diff is None, assertmsg From 9ed86fe07e83f4073b1a97fe4d4367545064ff81 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 31 May 2023 16:53:58 +0200 Subject: [PATCH 2/3] isisd: fix wrongly disabled flex-algo A configured flex-algo algorithm may remain in disabled state after its definition is advertised on the area. It happens sometimes that, in isis_sr_flex_algo_topo1 topotest step 4 or 8, flex-algo 203 is disabled. It depends on the following sequence: 1. Flex-algo 203 is configured on a remote router to be re-advertised. 2. A LSP is received on the local router and contains the algo 203 definition. 3. The local router re-builds its own LSP with lsp_build(). 4. local router isis_run_spf() recomputes the algo 203 SPF tree. A 1. 2. 3. 4. sequence results in a working test. The reception of the remote LSP (2.) does not trigger the built of the local LSP. If for some reasons, the sequence is 1. 3. 4. 2. 4., isis_run_spf() will not knows that flex-algo 203 has been re-enabled because flex_algo_get_state() only returns the state from the local LSP. Compare in sequence step 4. the flex-algo state from the local LSP with the actual state. If the state is not the same, request a new local LSP generation and quits the re-computation of algo SPF tree. The SPF tree will be recomputed just after the built of the local LSP. Fixes: 3f55b8c621 ("isisd: fix disabled flex-algo on race condition") Signed-off-by: Louis Scalbert --- isisd/isis_spf.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/isisd/isis_spf.c b/isisd/isis_spf.c index de467c8262..e114467e07 100644 --- a/isisd/isis_spf.c +++ b/isisd/isis_spf.c @@ -1843,6 +1843,9 @@ void isis_run_spf(struct isis_spftree *spftree) struct timeval time_end; struct isis_mt_router_info *mt_router_info; uint16_t mtid = 0; +#ifndef FABRICD + bool flex_algo_enabled; +#endif /* ifndef FABRICD */ /* Get time that can't roll backwards. */ monotime(&time_start); @@ -1885,16 +1888,27 @@ void isis_run_spf(struct isis_spftree *spftree) * not supported by the node, it MUST stop participating in such * Flexible-Algorithm. */ - if (flex_algo_id_valid(spftree->algorithm) && - !flex_algo_get_state(spftree->area->flex_algos, - spftree->algorithm)) { - if (!CHECK_FLAG(spftree->flags, F_SPFTREE_DISABLED)) { - isis_spftree_clear(spftree); - SET_FLAG(spftree->flags, F_SPFTREE_DISABLED); + if (flex_algo_id_valid(spftree->algorithm)) { + flex_algo_enabled = isis_flex_algo_elected_supported( + spftree->algorithm, spftree->area); + if (flex_algo_enabled != + flex_algo_get_state(spftree->area->flex_algos, + spftree->algorithm)) { + /* actual state is inconsistent with local LSP */ lsp_regenerate_schedule(spftree->area, spftree->area->is_type, 0); + goto out; + } + if (!flex_algo_enabled) { + if (!CHECK_FLAG(spftree->flags, F_SPFTREE_DISABLED)) { + isis_spftree_clear(spftree); + SET_FLAG(spftree->flags, F_SPFTREE_DISABLED); + lsp_regenerate_schedule(spftree->area, + spftree->area->is_type, + 0); + } + goto out; } - goto out; } #endif /* ifndef FABRICD */ From 5f230545e18f85bdc99f33d38e72651d4385ec62 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 1 Jun 2023 14:18:47 +0200 Subject: [PATCH 3/3] tests: re-style isis_sr_flex_algo_topo1 Re-style isis_sr_flex_algo_topo1. Cosmetic change. Signed-off-by: Louis Scalbert --- .../test_isis_sr_flex_algo_topo1.py | 141 ++++++++++-------- 1 file changed, 81 insertions(+), 60 deletions(-) diff --git a/tests/topotests/isis_sr_flex_algo_topo1/test_isis_sr_flex_algo_topo1.py b/tests/topotests/isis_sr_flex_algo_topo1/test_isis_sr_flex_algo_topo1.py index fbe179bd3f..c81f63942b 100755 --- a/tests/topotests/isis_sr_flex_algo_topo1/test_isis_sr_flex_algo_topo1.py +++ b/tests/topotests/isis_sr_flex_algo_topo1/test_isis_sr_flex_algo_topo1.py @@ -112,8 +112,12 @@ def setup_module(mod): # For all registered routers, load the zebra configuration file for rname, router in router_list.items(): - router.load_config( TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname))) - router.load_config( TopoRouter.RD_ISIS, os.path.join(CWD, "{}/isisd.conf".format(rname))) + router.load_config( + TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname)) + ) + router.load_config( + TopoRouter.RD_ISIS, os.path.join(CWD, "{}/isisd.conf".format(rname)) + ) tgen.start_router() @@ -180,9 +184,13 @@ def router_compare_output(rname, command, reference): tgen = get_topogen() # Run test function until we get an result. Wait at most 60 seconds. - test_func = partial(topotest.router_output_cmp, tgen.gears[rname], command, reference) + test_func = partial( + topotest.router_output_cmp, tgen.gears[rname], command, reference + ) result, diff = topotest.run_and_expect(test_func, "", count=120, wait=0.5) - assertmsg = '{} command "{}" output mismatches the expected result:\n{}'.format(rname, command, diff) + assertmsg = '{} command "{}" output mismatches the expected result:\n{}'.format( + rname, command, diff + ) assert result, assertmsg @@ -203,11 +211,11 @@ def test_step1_mpls_lfib(): # tgen.mininet_cli() for rname in ["rt1", "rt2", "rt3"]: router_compare_output( - rname, "show isis flex-algo", - outputs[rname][1]["show_isis_flex_algo.ref"]) + rname, "show isis flex-algo", outputs[rname][1]["show_isis_flex_algo.ref"] + ) router_compare_json_output( - rname, "show mpls table json", - outputs[rname][1]["show_mpls_table.ref"]) + rname, "show mpls table json", outputs[rname][1]["show_mpls_table.ref"] + ) # @@ -234,17 +242,18 @@ def test_step2_mpls_lfib(): router isis 1 flex-algo 203 no advertise-definition - """) + """ + ) # For Developers # tgen.mininet_cli() for rname in ["rt1", "rt2", "rt3"]: router_compare_output( - rname, "show isis flex-algo", - outputs[rname][2]["show_isis_flex_algo.ref"]) + rname, "show isis flex-algo", outputs[rname][2]["show_isis_flex_algo.ref"] + ) router_compare_json_output( - rname, "show mpls table json", - outputs[rname][2]["show_mpls_table.ref"]) + rname, "show mpls table json", outputs[rname][2]["show_mpls_table.ref"] + ) # @@ -271,17 +280,18 @@ def test_step3_mpls_lfib(): router isis 1 flex-algo 203 no advertise-definition - """) + """ + ) # For Developers # tgen.mininet_cli() for rname in ["rt1", "rt2", "rt3"]: router_compare_output( - rname, "show isis flex-algo", - outputs[rname][3]["show_isis_flex_algo.ref"]) + rname, "show isis flex-algo", outputs[rname][3]["show_isis_flex_algo.ref"] + ) router_compare_json_output( - rname, "show mpls table json", - outputs[rname][3]["show_mpls_table.ref"]) + rname, "show mpls table json", outputs[rname][3]["show_mpls_table.ref"] + ) # @@ -308,17 +318,18 @@ def test_step4_mpls_lfib(): router isis 1 flex-algo 203 advertise-definition - """) + """ + ) # For Developers # tgen.mininet_cli() for rname in ["rt1", "rt2", "rt3"]: router_compare_output( - rname, "show isis flex-algo", - outputs[rname][4]["show_isis_flex_algo.ref"]) + rname, "show isis flex-algo", outputs[rname][4]["show_isis_flex_algo.ref"] + ) router_compare_json_output( - rname, "show mpls table json", - outputs[rname][4]["show_mpls_table.ref"]) + rname, "show mpls table json", outputs[rname][4]["show_mpls_table.ref"] + ) # @@ -346,17 +357,18 @@ def test_step5_mpls_lfib(): router isis 1 flex-algo 203 advertise-definition - """) + """ + ) # For Developers # tgen.mininet_cli() for rname in ["rt1", "rt2", "rt3"]: router_compare_output( - rname, "show isis flex-algo", - outputs[rname][5]["show_isis_flex_algo.ref"]) + rname, "show isis flex-algo", outputs[rname][5]["show_isis_flex_algo.ref"] + ) router_compare_json_output( - rname, "show mpls table json", - outputs[rname][5]["show_mpls_table.ref"]) + rname, "show mpls table json", outputs[rname][5]["show_mpls_table.ref"] + ) # @@ -387,17 +399,18 @@ def test_step6_mpls_lfib(): router isis 1 flex-algo 203 no dataplane sr-mpls - """) + """ + ) # For Developers # tgen.mininet_cli() for rname in ["rt1", "rt2", "rt3"]: router_compare_output( - rname, "show isis flex-algo", - outputs[rname][6]["show_isis_flex_algo.ref"]) + rname, "show isis flex-algo", outputs[rname][6]["show_isis_flex_algo.ref"] + ) router_compare_json_output( - rname, "show mpls table json", - outputs[rname][6]["show_mpls_table.ref"]) + rname, "show mpls table json", outputs[rname][6]["show_mpls_table.ref"] + ) # @@ -427,17 +440,19 @@ def test_step7_mpls_lfib(): configure terminal router isis 1 no flex-algo 203 - """) + """ + ) # For Developers # tgen.mininet_cli() for rname in ["rt1", "rt2", "rt3"]: router_compare_output( - rname, "show isis flex-algo", - outputs[rname][7]["show_isis_flex_algo.ref"]) + rname, "show isis flex-algo", outputs[rname][7]["show_isis_flex_algo.ref"] + ) router_compare_json_output( - rname, "show mpls table json", - outputs[rname][7]["show_mpls_table.ref"]) + rname, "show mpls table json", outputs[rname][7]["show_mpls_table.ref"] + ) + # # Step 8 @@ -467,7 +482,8 @@ def test_step8_mpls_lfib(): advertise-definition affinity exclude-any green dataplane sr-mpls - """) + """ + ) tgen.gears["rt2"].vtysh_cmd( """ @@ -477,7 +493,8 @@ def test_step8_mpls_lfib(): advertise-definition affinity exclude-any green dataplane sr-mpls - """) + """ + ) tgen.gears["rt3"].vtysh_cmd( """ @@ -485,17 +502,18 @@ def test_step8_mpls_lfib(): router isis 1 flex-algo 203 dataplane sr-mpls - """) + """ + ) # For Developers # tgen.mininet_cli() for rname in ["rt1", "rt2", "rt3"]: router_compare_output( - rname, "show isis flex-algo", - outputs[rname][8]["show_isis_flex_algo.ref"]) + rname, "show isis flex-algo", outputs[rname][8]["show_isis_flex_algo.ref"] + ) router_compare_json_output( - rname, "show mpls table json", - outputs[rname][8]["show_mpls_table.ref"]) + rname, "show mpls table json", outputs[rname][8]["show_mpls_table.ref"] + ) # @@ -521,17 +539,18 @@ def test_step9_mpls_lfib(): router isis 1 no segment-routing prefix 1.1.1.1/32 algorithm 203 index 301 no segment-routing prefix 2001:db8:1000::1/128 algorithm 203 index 1301 - """) + """ + ) # For Developers # tgen.mininet_cli() for rname in ["rt1", "rt2", "rt3"]: router_compare_output( - rname, "show isis flex-algo", - outputs[rname][9]["show_isis_flex_algo.ref"]) + rname, "show isis flex-algo", outputs[rname][9]["show_isis_flex_algo.ref"] + ) router_compare_json_output( - rname, "show mpls table json", - outputs[rname][9]["show_mpls_table.ref"]) + rname, "show mpls table json", outputs[rname][9]["show_mpls_table.ref"] + ) # @@ -557,17 +576,18 @@ def test_step10_mpls_lfib(): router isis 1 segment-routing prefix 1.1.1.1/32 algorithm 203 index 301 segment-routing prefix 2001:db8:1000::1/128 algorithm 203 index 1301 - """) + """ + ) # For Developers # tgen.mininet_cli() for rname in ["rt1", "rt2", "rt3"]: router_compare_output( - rname, "show isis flex-algo", - outputs[rname][10]["show_isis_flex_algo.ref"]) + rname, "show isis flex-algo", outputs[rname][10]["show_isis_flex_algo.ref"] + ) router_compare_json_output( - rname, "show mpls table json", - outputs[rname][10]["show_mpls_table.ref"]) + rname, "show mpls table json", outputs[rname][10]["show_mpls_table.ref"] + ) # @@ -592,17 +612,18 @@ def test_step11_mpls_lfib(): router isis 1 segment-routing prefix 1.1.1.1/32 algorithm 203 index 311 segment-routing prefix 2001:db8:1000::1/128 algorithm 203 index 1311 - """) + """ + ) # For Developers # tgen.mininet_cli() for rname in ["rt1", "rt2", "rt3"]: router_compare_output( - rname, "show isis flex-algo", - outputs[rname][11]["show_isis_flex_algo.ref"]) + rname, "show isis flex-algo", outputs[rname][11]["show_isis_flex_algo.ref"] + ) router_compare_json_output( - rname, "show mpls table json", - outputs[rname][11]["show_mpls_table.ref"]) + rname, "show mpls table json", outputs[rname][11]["show_mpls_table.ref"] + ) if __name__ == "__main__":