From a3c526e7975bb30fada3a6ae834cde794e7e58fe Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Mon, 27 Jan 2025 14:17:24 -0500 Subject: [PATCH 1/2] zebra: include resolving nexthops in nhg hash Ensure that the nhg hash comparison function includes all nexthops, including recursive-resolving nexthops. Signed-off-by: Mark Stapp (cherry picked from commit cb7cf73992847cfd4af796085bf14f2fdc4fa8db) --- zebra/zebra_nhg.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index 804cd48d9ccf..a7e30530af44 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -569,8 +569,7 @@ bool zebra_nhg_hash_equal(const void *arg1, const void *arg2) /* Nexthops should be in-order, so we simply compare them in-place */ for (nexthop1 = nhe1->nhg.nexthop, nexthop2 = nhe2->nhg.nexthop; nexthop1 && nexthop2; - nexthop1 = nexthop1->next, nexthop2 = nexthop2->next) { - + nexthop1 = nexthop_next(nexthop1), nexthop2 = nexthop_next(nexthop2)) { if (!nhg_compare_nexthops(nexthop1, nexthop2)) return false; } @@ -605,8 +604,7 @@ bool zebra_nhg_hash_equal(const void *arg1, const void *arg2) for (nexthop1 = nhe1->backup_info->nhe->nhg.nexthop, nexthop2 = nhe2->backup_info->nhe->nhg.nexthop; nexthop1 && nexthop2; - nexthop1 = nexthop1->next, nexthop2 = nexthop2->next) { - + nexthop1 = nexthop_next(nexthop1), nexthop2 = nexthop_next(nexthop2)) { if (!nhg_compare_nexthops(nexthop1, nexthop2)) return false; } From 30296344d39ac59a81dd4a61b45314febf80af2e Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 27 Jan 2025 10:34:31 -0500 Subject: [PATCH 2/2] tests: Add a test that shows the v6 recursive nexthop problem Currently FRR does not handle v6 recurisive resolution properly when the route being recursed through changes and the most significant bits of the route are not changed. Signed-off-by: Donald Sharp (cherry picked from commit 73ab6a46c51db91df297774221053ab8fc4d12ae) --- .../r1/frr.conf | 4 + ...t_v6_nexthop_group_recursive_resolution.py | 189 ++++++++++++++++++ 2 files changed, 193 insertions(+) create mode 100644 tests/topotests/v6_nexthop_group_recursive_resolution/r1/frr.conf create mode 100644 tests/topotests/v6_nexthop_group_recursive_resolution/test_v6_nexthop_group_recursive_resolution.py diff --git a/tests/topotests/v6_nexthop_group_recursive_resolution/r1/frr.conf b/tests/topotests/v6_nexthop_group_recursive_resolution/r1/frr.conf new file mode 100644 index 000000000000..f4da11af062a --- /dev/null +++ b/tests/topotests/v6_nexthop_group_recursive_resolution/r1/frr.conf @@ -0,0 +1,4 @@ +int r1-eth0 + ipv6 address fc00::1/64 + +ipv6 route 1::1/128 fc00::2 diff --git a/tests/topotests/v6_nexthop_group_recursive_resolution/test_v6_nexthop_group_recursive_resolution.py b/tests/topotests/v6_nexthop_group_recursive_resolution/test_v6_nexthop_group_recursive_resolution.py new file mode 100644 index 000000000000..587a951c855d --- /dev/null +++ b/tests/topotests/v6_nexthop_group_recursive_resolution/test_v6_nexthop_group_recursive_resolution.py @@ -0,0 +1,189 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# +# Copyright (c) 2024 by Nvidia Corporation +# Donald Sharp +# + +""" +Check that the v6 nexthop recursive resolution works when it changes +""" + +import os +import sys +import json +import pytest +import functools + +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + +# pylint: disable=C0413 +from lib import topotest +from lib.topogen import Topogen, TopoRouter, get_topogen +from lib.common_config import step + +pytestmark = [pytest.mark.staticd] + + +def build_topo(tgen): + + tgen.add_router("r1") + + switch = tgen.add_switch("s1") + switch.add_link(tgen.gears["r1"]) + + +def setup_module(mod): + tgen = Topogen(build_topo, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + + for rname, router in router_list.items(): + router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname)), + [(TopoRouter.RD_MGMTD, None), + (TopoRouter.RD_ZEBRA, None), + (TopoRouter.RD_STATIC, None), + (TopoRouter.RD_SHARP, None)]) + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_recursive_v6_nexthop_generation(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + step("Testing v6 nexthop resolution") + + #assert False + router = tgen.gears["r1"] + + def _v6_converge_1_1_initial(): + output = json.loads( + router.vtysh_cmd("show ipv6 route 1::1 json")) + + expected = { + "1::1/128":[ + { + "prefix":"1::1/128", + "prefixLen":128, + "protocol":"static", + "vrfName":"default", + "selected":True, + "destSelected":True, + "distance":1, + "metric":0, + "installed":True, + "table":254, + "nexthops":[ + { + "fib":True, + "ip":"fc00::2", + "afi":"ipv6", + "interfaceName":"r1-eth0", + "active":True, + "weight":1 + } + ] + } + ] + } + + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_v6_converge_1_1_initial) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Failed to install v6 1::1 route" + + router.vtysh_cmd("sharp install routes 2::2 nexthop 1::1 1") + router.vtysh_cmd("conf\nipv6 route 1::1/128 fc00::3\nno ipv6 route 1::1/128 fc00::2") + + def _v6_converge_1_1_post(): + output = json.loads( + router.vtysh_cmd("show ipv6 route 1::1 json")) + + expected = { + "1::1/128":[ + { + "prefix":"1::1/128", + "prefixLen":128, + "protocol":"static", + "vrfName":"default", + "selected":True, + "destSelected":True, + "distance":1, + "metric":0, + "installed":True, + "table":254, + "nexthops":[ + { + "fib":True, + "ip":"fc00::3", + "afi":"ipv6", + "interfaceName":"r1-eth0", + "active":True, + "weight":1 + } + ] + } + ] + } + + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_v6_converge_1_1_post) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Failed to change v6 1::1 route" + + router.vtysh_cmd("sharp install routes 2::2 nexthop 1::1 1") + + def _v6_change_2_2_post(): + output = json.loads( + router.vtysh_cmd("show ipv6 route 2::2 json")) + + expected = { + "2::2/128":[ + { + "prefix":"2::2/128", + "prefixLen":128, + "protocol":"sharp", + "vrfName":"default", + "selected":True, + "destSelected":True, + "distance":150, + "metric":0, + "installed":True, + "table":254, + "nexthops":[ + { + "fib":True, + "ip":"fc00::3", + "afi":"ipv6", + "interfaceName":"r1-eth0", + "active":True, + "weight":1 + } + ] + } + ] + } + + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_v6_change_2_2_post) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Failed to see sharpd route correctly" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args))