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 <achernavin@netgate.com>
(cherry picked from commit 633eb6ee7742449d0644dda7fd83865e3c01f937)

diff --git a/lib/routemap_northbound.c b/lib/routemap_northbound.c
index 465985099429c3e45eff7b9d5ed9db5abbc3a7d1..5767e0aacf75f0b0890c3e8decae6a4399a8617f 100644 (file)
--- 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 (file)
index 0000000..e69de29

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 (file)
index 0000000..b858fff
--- /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 (file)
index 0000000..581e2e6
--- /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 (file)
index 0000000..518d63d
--- /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 (file)
index 0000000..fd45c48
--- /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 (file)
index 0000000..8fe45a3
--- /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))