From aed3dbfa5d29af9de99f32f9b8b6ff66aa5c65df Mon Sep 17 00:00:00 2001 From: Alexander Chernavin Date: Fri, 14 Oct 2022 09:02:06 +0000 Subject: [PATCH] lib: fix on-match when added to existing route-map entry Currently, "on-match (next|goto)" only works if already present in a route-map entry when the route-map is applied to the routes. However, if the command is added to an existing route-map entry, the route-map is not reapplied to the routes in order to accommodate the changes. And service restart is needed. The problem is that setting the command doesn't signal about the change to the listener (i.e. to a routing daemon). With this fix, signal to the listener about addition of "on-match (next|goto)" to a route-map entry. Signed-off-by: Alexander Chernavin (cherry picked from commit 633eb6ee7742449d0644dda7fd83865e3c01f937) --- lib/routemap_northbound.c | 10 ++ .../bgp_route_map_on_match_next/__init__.py | 0 .../bgp_route_map_on_match_next/r1/bgpd.conf | 10 ++ .../bgp_route_map_on_match_next/r1/zebra.conf | 6 + .../bgp_route_map_on_match_next/r2/bgpd.conf | 17 +++ .../bgp_route_map_on_match_next/r2/zebra.conf | 6 + .../test_bgp_route_map_on_match_next.py | 111 ++++++++++++++++++ 7 files changed, 160 insertions(+) create mode 100644 tests/topotests/bgp_route_map_on_match_next/__init__.py create mode 100644 tests/topotests/bgp_route_map_on_match_next/r1/bgpd.conf create mode 100644 tests/topotests/bgp_route_map_on_match_next/r1/zebra.conf create mode 100644 tests/topotests/bgp_route_map_on_match_next/r2/bgpd.conf create mode 100644 tests/topotests/bgp_route_map_on_match_next/r2/zebra.conf create mode 100644 tests/topotests/bgp_route_map_on_match_next/test_bgp_route_map_on_match_next.py diff --git a/lib/routemap_northbound.c b/lib/routemap_northbound.c index 4659850994..5767e0aacf 100644 --- a/lib/routemap_northbound.c +++ b/lib/routemap_northbound.c @@ -353,6 +353,7 @@ static int lib_route_map_entry_exit_policy_modify(struct nb_cb_modify_args *args) { struct route_map_index *rmi; + struct route_map *map; int rm_action; int policy; @@ -382,6 +383,7 @@ lib_route_map_entry_exit_policy_modify(struct nb_cb_modify_args *args) break; case NB_EV_APPLY: rmi = nb_running_get_entry(args->dnode, NULL, true); + map = rmi->map; policy = yang_dnode_get_enum(args->dnode, NULL); switch (policy) { @@ -395,6 +397,14 @@ lib_route_map_entry_exit_policy_modify(struct nb_cb_modify_args *args) rmi->exitpolicy = RMAP_GOTO; break; } + + /* Execute event hook. */ + if (route_map_master.event_hook) { + (*route_map_master.event_hook)(map->name); + route_map_notify_dependencies(map->name, + RMAP_EVENT_CALL_ADDED); + } + break; } diff --git a/tests/topotests/bgp_route_map_on_match_next/__init__.py b/tests/topotests/bgp_route_map_on_match_next/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/topotests/bgp_route_map_on_match_next/r1/bgpd.conf b/tests/topotests/bgp_route_map_on_match_next/r1/bgpd.conf new file mode 100644 index 0000000000..b858fffd81 --- /dev/null +++ b/tests/topotests/bgp_route_map_on_match_next/r1/bgpd.conf @@ -0,0 +1,10 @@ +! +router bgp 65001 + no bgp network import-check + neighbor 192.168.255.2 remote-as 65001 + neighbor 192.168.255.2 timers 3 10 + address-family ipv4 unicast + network 10.100.100.1/32 + exit-address-family + ! +! diff --git a/tests/topotests/bgp_route_map_on_match_next/r1/zebra.conf b/tests/topotests/bgp_route_map_on_match_next/r1/zebra.conf new file mode 100644 index 0000000000..581e2e634f --- /dev/null +++ b/tests/topotests/bgp_route_map_on_match_next/r1/zebra.conf @@ -0,0 +1,6 @@ +! +interface r1-eth0 + ip address 192.168.255.1/30 +! +ip forwarding +! diff --git a/tests/topotests/bgp_route_map_on_match_next/r2/bgpd.conf b/tests/topotests/bgp_route_map_on_match_next/r2/bgpd.conf new file mode 100644 index 0000000000..518d63d692 --- /dev/null +++ b/tests/topotests/bgp_route_map_on_match_next/r2/bgpd.conf @@ -0,0 +1,17 @@ +! +router bgp 65001 + neighbor 192.168.255.1 remote-as 65001 + neighbor 192.168.255.1 timers 3 10 + address-family ipv4 unicast + neighbor 192.168.255.1 route-map RM in + exit-address-family + ! +! +route-map RM permit 10 + set weight 100 +exit +! +route-map RM permit 20 + set metric 20 +exit +! diff --git a/tests/topotests/bgp_route_map_on_match_next/r2/zebra.conf b/tests/topotests/bgp_route_map_on_match_next/r2/zebra.conf new file mode 100644 index 0000000000..fd45c48d6d --- /dev/null +++ b/tests/topotests/bgp_route_map_on_match_next/r2/zebra.conf @@ -0,0 +1,6 @@ +! +interface r2-eth0 + ip address 192.168.255.2/30 +! +ip forwarding +! diff --git a/tests/topotests/bgp_route_map_on_match_next/test_bgp_route_map_on_match_next.py b/tests/topotests/bgp_route_map_on_match_next/test_bgp_route_map_on_match_next.py new file mode 100644 index 0000000000..8fe45a3498 --- /dev/null +++ b/tests/topotests/bgp_route_map_on_match_next/test_bgp_route_map_on_match_next.py @@ -0,0 +1,111 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# +# test_bgp_route_map_on_match_next.py +# +# Copyright (c) 2023 Rubicon Communications, LLC. +# + +""" +Test whether `on-match next` added to an existing route-map entry takes effect. +""" + +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 + +pytestmark = [pytest.mark.bgpd] + + +def build_topo(tgen): + for routern in range(1, 3): + tgen.add_router("r{}".format(routern)) + + switch = tgen.add_switch("s1") + switch.add_link(tgen.gears["r1"]) + switch.add_link(tgen.gears["r2"]) + + +def setup_module(mod): + tgen = Topogen(build_topo, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + + for i, (rname, router) in enumerate(router_list.items(), 1): + 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.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_bgp_route_map_on_match_next(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + router1 = tgen.gears["r1"] + router2 = tgen.gears["r2"] + + def _bgp_converge(router): + output = json.loads(router.vtysh_cmd("show ip bgp neighbor 192.168.255.1 json")) + expected = {"192.168.255.1": {"bgpState": "Established"}} + return topotest.json_cmp(output, expected) + + def _bgp_has_routes(router, metric, weight): + output = json.loads( + router.vtysh_cmd("show ip bgp neighbor 192.168.255.1 routes json") + ) + expected = { + "routes": {"10.100.100.1/32": [{"metric": metric, "weight": weight}]} + } + return topotest.json_cmp(output, expected) + + # Check thst session is established + test_func = functools.partial(_bgp_converge, router2) + success, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + assert result is None, "Failed bgp convergence on r2" + + # Check that metric is 0 and weight is 100 for the received prefix + test_func = functools.partial(_bgp_has_routes, router2, 0, 100) + success, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + assert result is None, "r2 does not receive routes with metric 0 and weight 100" + + # Update the route-map and add "on-match next" to entry 10 + cmd = """ + configure terminal + route-map RM permit 10 + on-match next + exit + """ + router2.vtysh_cmd(cmd) + + # Check that metric is 20 and weight is 100 for the received prefix + test_func = functools.partial(_bgp_has_routes, router2, 20, 100) + success, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + assert result is None, "r2 does not receive routes with metric 20 and weight 100" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) -- 2.39.5