From 57aedde6ef41a7fb7138e73d7f32d6c13ffe16f9 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 16 May 2023 10:34:22 +0300 Subject: [PATCH 1/4] ripng: Implement `allow-ecmp X` command A port of ripd implementation for ripngd implemented by 75fce4645a7cf0a93ef0109d69365f51b84bc47c. Signed-off-by: Donatas Abraitis --- ripngd/ripng_cli.c | 43 +++++++++++++++++++++------- ripngd/ripng_nb_config.c | 8 ++++-- ripngd/ripng_zebra.c | 11 +++++-- ripngd/ripngd.c | 62 +++++++++++++++++++++++++++++++++++++++- ripngd/ripngd.h | 3 +- yang/frr-ripngd.yang | 4 +-- 6 files changed, 113 insertions(+), 18 deletions(-) diff --git a/ripngd/ripng_cli.c b/ripngd/ripng_cli.c index 5e59dfd2c4..9a96e29313 100644 --- a/ripngd/ripng_cli.c +++ b/ripngd/ripng_cli.c @@ -85,14 +85,32 @@ void cli_show_router_ripng(struct vty *vty, const struct lyd_node *dnode, /* * XPath: /frr-ripngd:ripngd/instance/allow-ecmp */ -DEFPY_YANG (ripng_allow_ecmp, - ripng_allow_ecmp_cmd, - "[no] allow-ecmp", - NO_STR - "Allow Equal Cost MultiPath\n") +DEFUN_YANG (ripng_allow_ecmp, + ripng_allow_ecmp_cmd, + "allow-ecmp [" CMD_RANGE_STR(1, MULTIPATH_NUM) "]", + "Allow Equal Cost MultiPath\n" + "Number of paths\n") { - nb_cli_enqueue_change(vty, "./allow-ecmp", NB_OP_MODIFY, - no ? "false" : "true"); + int idx_number = 0; + char mpaths[3] = {}; + uint32_t paths = MULTIPATH_NUM; + + if (argv_find(argv, argc, CMD_RANGE_STR(1, MULTIPATH_NUM), &idx_number)) + paths = strtol(argv[idx_number]->arg, NULL, 10); + snprintf(mpaths, sizeof(mpaths), "%u", paths); + + nb_cli_enqueue_change(vty, "./allow-ecmp", NB_OP_MODIFY, mpaths); + + return nb_cli_apply_changes(vty, NULL); +} + +DEFUN_YANG (no_ripng_allow_ecmp, + no_ripng_allow_ecmp_cmd, + "no allow-ecmp [" CMD_RANGE_STR(1, MULTIPATH_NUM) "]", NO_STR + "Allow Equal Cost MultiPath\n" + "Number of paths\n") +{ + nb_cli_enqueue_change(vty, "./allow-ecmp", NB_OP_MODIFY, 0); return nb_cli_apply_changes(vty, NULL); } @@ -100,10 +118,14 @@ DEFPY_YANG (ripng_allow_ecmp, void cli_show_ripng_allow_ecmp(struct vty *vty, const struct lyd_node *dnode, bool show_defaults) { - if (!yang_dnode_get_bool(dnode, NULL)) - vty_out(vty, " no"); + uint8_t paths; - vty_out(vty, " allow-ecmp\n"); + paths = yang_dnode_get_uint8(dnode, NULL); + + if (!paths) + vty_out(vty, " no allow-ecmp\n"); + else + vty_out(vty, " allow-ecmp %d\n", paths); } /* @@ -547,6 +569,7 @@ void ripng_cli_init(void) install_element(RIPNG_NODE, &ripng_no_ipv6_distribute_list_cmd); install_element(RIPNG_NODE, &ripng_allow_ecmp_cmd); + install_element(RIPNG_NODE, &no_ripng_allow_ecmp_cmd); install_element(RIPNG_NODE, &ripng_default_information_originate_cmd); install_element(RIPNG_NODE, &ripng_default_metric_cmd); install_element(RIPNG_NODE, &no_ripng_default_metric_cmd); diff --git a/ripngd/ripng_nb_config.c b/ripngd/ripng_nb_config.c index 30f707e061..de72319354 100644 --- a/ripngd/ripng_nb_config.c +++ b/ripngd/ripng_nb_config.c @@ -129,9 +129,13 @@ int ripngd_instance_allow_ecmp_modify(struct nb_cb_modify_args *args) return NB_OK; ripng = nb_running_get_entry(args->dnode, NULL, true); - ripng->ecmp = yang_dnode_get_bool(args->dnode, NULL); - if (!ripng->ecmp) + ripng->ecmp = yang_dnode_get_uint8(args->dnode, NULL); + if (!ripng->ecmp) { ripng_ecmp_disable(ripng); + return NB_OK; + } + + ripng_ecmp_change(ripng); return NB_OK; } diff --git a/ripngd/ripng_zebra.c b/ripngd/ripng_zebra.c index d974d65df5..6122c4255c 100644 --- a/ripngd/ripng_zebra.c +++ b/ripngd/ripng_zebra.c @@ -20,6 +20,7 @@ /* All information about zebra. */ struct zclient *zclient = NULL; +uint32_t zebra_ecmp_count = MULTIPATH_NUM; /* Send ECMP routes to zebra. */ static void ripng_zebra_ipv6_send(struct ripng *ripng, struct agg_node *rp, @@ -30,7 +31,7 @@ static void ripng_zebra_ipv6_send(struct ripng *ripng, struct agg_node *rp, struct zapi_nexthop *api_nh; struct listnode *listnode = NULL; struct ripng_info *rinfo = NULL; - int count = 0; + uint32_t count = 0; const struct prefix *p = agg_node_get_prefix(rp); memset(&api, 0, sizeof(api)); @@ -41,7 +42,7 @@ static void ripng_zebra_ipv6_send(struct ripng *ripng, struct agg_node *rp, SET_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP); for (ALL_LIST_ELEMENTS_RO(list, listnode, rinfo)) { - if (count >= MULTIPATH_NUM) + if (count >= zebra_ecmp_count) break; api_nh = &api.nexthops[count]; api_nh->vrf_id = ripng->vrf->vrf_id; @@ -227,6 +228,11 @@ static zclient_handler *const ripng_handlers[] = { [ZEBRA_REDISTRIBUTE_ROUTE_DEL] = ripng_zebra_read_route, }; +static void ripng_zebra_capabilities(struct zclient_capabilities *cap) +{ + zebra_ecmp_count = MIN(cap->ecmp, zebra_ecmp_count); +} + /* Initialize zebra structure and it's commands. */ void zebra_init(struct event_loop *master) { @@ -236,6 +242,7 @@ void zebra_init(struct event_loop *master) zclient_init(zclient, ZEBRA_ROUTE_RIPNG, 0, &ripngd_privs); zclient->zebra_connected = ripng_zebra_connected; + zclient->zebra_capabilities = ripng_zebra_capabilities; } void ripng_zebra_stop(void) diff --git a/ripngd/ripngd.c b/ripngd/ripngd.c index 2f6409a70d..7269e76656 100644 --- a/ripngd/ripngd.c +++ b/ripngd/ripngd.c @@ -443,7 +443,10 @@ struct ripng_info *ripng_ecmp_add(struct ripng *ripng, { struct agg_node *rp = rinfo_new->rp; struct ripng_info *rinfo = NULL; + struct ripng_info *rinfo_exist = NULL; struct list *list = NULL; + struct listnode *node = NULL; + struct listnode *nnode = NULL; if (rp->info == NULL) rp->info = list_new(); @@ -454,6 +457,33 @@ struct ripng_info *ripng_ecmp_add(struct ripng *ripng, if (listcount(list) && !ripng->ecmp) return NULL; + /* Add or replace an existing ECMP path with lower neighbor IP */ + if (listcount(list) && listcount(list) >= ripng->ecmp) { + struct ripng_info *from_highest = NULL; + + /* Find the rip_info struct that has the highest nexthop IP */ + for (ALL_LIST_ELEMENTS(list, node, nnode, rinfo_exist)) + if (!from_highest || + (from_highest && + IPV6_ADDR_CMP(&rinfo_exist->from, + &from_highest->from) > 0)) { + from_highest = rinfo_exist; + } + + /* If we have a route in ECMP group, delete the old + * one that has a higher next-hop address. Lower IP is + * preferred. + */ + if (ripng->ecmp > 1 && from_highest && + IPV6_ADDR_CMP(&from_highest->from, &rinfo_new->from) > 0) { + ripng_ecmp_delete(ripng, from_highest); + goto add_or_replace; + } + + return NULL; + } + +add_or_replace: rinfo = ripng_info_new(); memcpy(rinfo, rinfo_new, sizeof(struct ripng_info)); listnode_add(list, rinfo); @@ -475,6 +505,36 @@ struct ripng_info *ripng_ecmp_add(struct ripng *ripng, return rinfo; } +/* Update ECMP routes to zebra when `allow-ecmp` changed. */ +void ripng_ecmp_change(struct ripng *ripng) +{ + struct agg_node *rp; + struct ripng_info *rinfo; + struct list *list; + struct listnode *node, *nextnode; + + for (rp = agg_route_top(ripng->table); rp; rp = agg_route_next(rp)) { + list = rp->info; + if (list && listcount(list) > 1) { + while (listcount(list) > ripng->ecmp) { + struct ripng_info *from_highest = NULL; + + for (ALL_LIST_ELEMENTS(list, node, nextnode, + rinfo)) { + if (!from_highest || + (from_highest && + IPV6_ADDR_CMP( + &rinfo->from, + &from_highest->from) > 0)) + from_highest = rinfo; + } + + ripng_ecmp_delete(ripng, from_highest); + } + } + } +} + /* Replace the ECMP list with the new route. * RETURN: the new entry added in the list */ @@ -1814,7 +1874,7 @@ struct ripng *ripng_create(const char *vrf_name, struct vrf *vrf, int socket) "%s/timers/flush-interval", RIPNG_INSTANCE); ripng->default_metric = yang_get_default_uint8("%s/default-metric", RIPNG_INSTANCE); - ripng->ecmp = yang_get_default_bool("%s/allow-ecmp", RIPNG_INSTANCE); + ripng->ecmp = yang_get_default_uint8("%s/allow-ecmp", RIPNG_INSTANCE); /* Make buffer. */ ripng->ibuf = stream_new(RIPNG_MAX_PACKET_SIZE * 5); diff --git a/ripngd/ripngd.h b/ripngd/ripngd.h index eefcb0ee69..3d13097dd6 100644 --- a/ripngd/ripngd.h +++ b/ripngd/ripngd.h @@ -130,7 +130,7 @@ struct ripng { struct event *t_triggered_interval; /* RIPng ECMP flag */ - bool ecmp; + uint8_t ecmp; /* RIPng redistribute configuration. */ struct { @@ -429,6 +429,7 @@ extern struct ripng_info *ripng_ecmp_replace(struct ripng *ripng, struct ripng_info *rinfo); extern struct ripng_info *ripng_ecmp_delete(struct ripng *ripng, struct ripng_info *rinfo); +extern void ripng_ecmp_change(struct ripng *ripng); extern void ripng_vrf_init(void); extern void ripng_vrf_terminate(void); diff --git a/yang/frr-ripngd.yang b/yang/frr-ripngd.yang index 7b2b135fb5..4aeaf36400 100644 --- a/yang/frr-ripngd.yang +++ b/yang/frr-ripngd.yang @@ -86,8 +86,8 @@ module frr-ripngd { "VRF name."; } leaf allow-ecmp { - type boolean; - default "false"; + type uint8; + default 0; description "Allow equal-cost multi-path."; } From 29d3532a7a1e6b90ef8820b21cc328e1f8cb707b Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 16 May 2023 10:28:35 +0300 Subject: [PATCH 2/4] tests: Check if `allow-ecmp` command works for RIPng Signed-off-by: Donatas Abraitis --- tests/topotests/ripng_allow_ecmp/__init__.py | 0 tests/topotests/ripng_allow_ecmp/r1/frr.conf | 9 ++ tests/topotests/ripng_allow_ecmp/r2/frr.conf | 13 +++ tests/topotests/ripng_allow_ecmp/r3/frr.conf | 14 +++ tests/topotests/ripng_allow_ecmp/r4/frr.conf | 14 +++ tests/topotests/ripng_allow_ecmp/r5/frr.conf | 14 +++ .../ripng_allow_ecmp/test_ripng_allow_ecmp.py | 93 +++++++++++++++++++ 7 files changed, 157 insertions(+) create mode 100644 tests/topotests/ripng_allow_ecmp/__init__.py create mode 100644 tests/topotests/ripng_allow_ecmp/r1/frr.conf create mode 100644 tests/topotests/ripng_allow_ecmp/r2/frr.conf create mode 100644 tests/topotests/ripng_allow_ecmp/r3/frr.conf create mode 100644 tests/topotests/ripng_allow_ecmp/r4/frr.conf create mode 100644 tests/topotests/ripng_allow_ecmp/r5/frr.conf create mode 100644 tests/topotests/ripng_allow_ecmp/test_ripng_allow_ecmp.py diff --git a/tests/topotests/ripng_allow_ecmp/__init__.py b/tests/topotests/ripng_allow_ecmp/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/topotests/ripng_allow_ecmp/r1/frr.conf b/tests/topotests/ripng_allow_ecmp/r1/frr.conf new file mode 100644 index 0000000000..effb5df1ea --- /dev/null +++ b/tests/topotests/ripng_allow_ecmp/r1/frr.conf @@ -0,0 +1,9 @@ +! +int r1-eth0 + ipv6 address 2001:db8:1::1/64 +! +router ripng + allow-ecmp + network 2001:db8:1::/64 + timers basic 5 15 10 +exit diff --git a/tests/topotests/ripng_allow_ecmp/r2/frr.conf b/tests/topotests/ripng_allow_ecmp/r2/frr.conf new file mode 100644 index 0000000000..71da101c0d --- /dev/null +++ b/tests/topotests/ripng_allow_ecmp/r2/frr.conf @@ -0,0 +1,13 @@ +! +int lo + ipv6 address 2001:db8:2::1/64 +! +int r2-eth0 + ipv6 address 2001:db8:1::2/64 +! +router ripng + redistribute connected + network 2001:db8:1::/64 + network 2001:db8:2::/64 + timers basic 5 15 10 +exit diff --git a/tests/topotests/ripng_allow_ecmp/r3/frr.conf b/tests/topotests/ripng_allow_ecmp/r3/frr.conf new file mode 100644 index 0000000000..fe9594dcd8 --- /dev/null +++ b/tests/topotests/ripng_allow_ecmp/r3/frr.conf @@ -0,0 +1,14 @@ +! +int lo + ipv6 address 2001:db8:2::1/64 +! +int r3-eth0 + ipv6 address 2001:db8:1::3/64 +! +router ripng + redistribute connected + network 2001:db8:1::/64 + network 2001:db8:2::/64 + timers basic 5 15 10 +exit + diff --git a/tests/topotests/ripng_allow_ecmp/r4/frr.conf b/tests/topotests/ripng_allow_ecmp/r4/frr.conf new file mode 100644 index 0000000000..0d3ea0bdea --- /dev/null +++ b/tests/topotests/ripng_allow_ecmp/r4/frr.conf @@ -0,0 +1,14 @@ +! +int lo + ipv6 address 2001:db8:2::1/64 +! +int r4-eth0 + ipv6 address 2001:db8:1::4/64 +! +router ripng + redistribute connected + network 2001:db8:1::/64 + network 2001:db8:2::/64 + timers basic 5 15 10 +exit + diff --git a/tests/topotests/ripng_allow_ecmp/r5/frr.conf b/tests/topotests/ripng_allow_ecmp/r5/frr.conf new file mode 100644 index 0000000000..6d6ca56571 --- /dev/null +++ b/tests/topotests/ripng_allow_ecmp/r5/frr.conf @@ -0,0 +1,14 @@ +! +int lo + ipv6 address 2001:db8:2::1/64 +! +int r5-eth0 + ipv6 address 2001:db8:1::5/64 +! +router ripng + redistribute connected + network 2001:db8:1::/64 + network 2001:db8:2::/64 + timers basic 5 15 10 +exit + diff --git a/tests/topotests/ripng_allow_ecmp/test_ripng_allow_ecmp.py b/tests/topotests/ripng_allow_ecmp/test_ripng_allow_ecmp.py new file mode 100644 index 0000000000..08bb999928 --- /dev/null +++ b/tests/topotests/ripng_allow_ecmp/test_ripng_allow_ecmp.py @@ -0,0 +1,93 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# Copyright (c) 2023 by +# Donatas Abraitis +# + +""" +Test if RIPng `allow-ecmp` command works correctly. +""" + +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.ripngd] + + +def setup_module(mod): + topodef = {"s1": ("r1", "r2", "r3", "r4", "r5")} + tgen = Topogen(topodef, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + + for _, (rname, router) in enumerate(router_list.items(), 1): + router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname))) + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_ripng_allow_ecmp(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + + def _show_routes(nh_num): + output = json.loads(r1.vtysh_cmd("show ipv6 route json")) + expected = { + "2001:db8:2::/64": [ + { + "internalNextHopNum": nh_num, + "internalNextHopActiveNum": nh_num, + } + ] + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_show_routes, 4) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assert ( + result is None + ), "Can't see 2001:db8:2::/64 as multipath (4) in `show ipv6 route`" + + step( + "Configure allow-ecmp 2, ECMP group routes SHOULD have next-hops with the lowest IPs" + ) + r1.vtysh_cmd( + """ + configure terminal + router ripng + allow-ecmp 2 + """ + ) + + test_func = functools.partial(_show_routes, 2) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assert ( + result is None + ), "Can't see 2001:db8:2::/64 as multipath (2) in `show ipv6 route`" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) From 993b236b2c1a2348ebb6a562956ff82a9e39ae25 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 16 May 2023 10:30:20 +0300 Subject: [PATCH 3/4] doc: Add RIPng allow-ecmp command Signed-off-by: Donatas Abraitis --- doc/user/ripngd.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/user/ripngd.rst b/doc/user/ripngd.rst index df7a0e249e..4c9b734d88 100644 --- a/doc/user/ripngd.rst +++ b/doc/user/ripngd.rst @@ -38,6 +38,10 @@ Currently ripngd supports the following commands: Set RIPng static routing announcement of NETWORK. +.. clicmd:: allow-ecmp [1-MULTIPATH_NUM] + + Control how many ECMP paths RIPng can inject for the same prefix. If specified + without a number, a maximum is taken (compiled with ``--enable-multipath``). .. _ripngd-terminal-mode-commands: From 6c5ffa88963d43204c944f9c798a315d1ee5f104 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 17 May 2023 22:17:02 +0300 Subject: [PATCH 4/4] ripngd: Make sure we do not overuse higher values for ECMP count Use a minimum value of a CLI version and a value of Zebra capabilities. Signed-off-by: Donatas Abraitis --- ripngd/ripng_main.c | 2 ++ ripngd/ripng_nb_config.c | 3 ++- ripngd/ripng_zebra.c | 1 - ripngd/ripngd.h | 2 ++ 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ripngd/ripng_main.c b/ripngd/ripng_main.c index 1d392efdde..9933dae5cd 100644 --- a/ripngd/ripng_main.c +++ b/ripngd/ripng_main.c @@ -32,6 +32,8 @@ struct option longopts[] = {{0}}; /* ripngd privileges */ zebra_capabilities_t _caps_p[] = {ZCAP_NET_RAW, ZCAP_BIND, ZCAP_SYS_ADMIN}; +uint32_t zebra_ecmp_count = MULTIPATH_NUM; + struct zebra_privs_t ripngd_privs = { #if defined(FRR_USER) .user = FRR_USER, diff --git a/ripngd/ripng_nb_config.c b/ripngd/ripng_nb_config.c index de72319354..0b1bd68eca 100644 --- a/ripngd/ripng_nb_config.c +++ b/ripngd/ripng_nb_config.c @@ -129,7 +129,8 @@ int ripngd_instance_allow_ecmp_modify(struct nb_cb_modify_args *args) return NB_OK; ripng = nb_running_get_entry(args->dnode, NULL, true); - ripng->ecmp = yang_dnode_get_uint8(args->dnode, NULL); + ripng->ecmp = + MIN(yang_dnode_get_uint8(args->dnode, NULL), zebra_ecmp_count); if (!ripng->ecmp) { ripng_ecmp_disable(ripng); return NB_OK; diff --git a/ripngd/ripng_zebra.c b/ripngd/ripng_zebra.c index 6122c4255c..49b8a197ad 100644 --- a/ripngd/ripng_zebra.c +++ b/ripngd/ripng_zebra.c @@ -20,7 +20,6 @@ /* All information about zebra. */ struct zclient *zclient = NULL; -uint32_t zebra_ecmp_count = MULTIPATH_NUM; /* Send ECMP routes to zebra. */ static void ripng_zebra_ipv6_send(struct ripng *ripng, struct agg_node *rp, diff --git a/ripngd/ripngd.h b/ripngd/ripngd.h index 3d13097dd6..c7468b6317 100644 --- a/ripngd/ripngd.h +++ b/ripngd/ripngd.h @@ -435,4 +435,6 @@ extern void ripng_vrf_init(void); extern void ripng_vrf_terminate(void); extern void ripng_cli_init(void); +extern uint32_t zebra_ecmp_count; + #endif /* _ZEBRA_RIPNG_RIPNGD_H */