bgpd: fix "bgp max-med on-startup"

Currently, if `bgp max-med on-startup` is configured, after BGP session
is established for the first time, a timer for the specified time is
started. When the timer is expired, an UPDATE message should be sent to
reflect changes in the routes' MED value. The problem is that the routes
are being suppressed because based on the attributes they look like they
have not changed. However, in the case of max-med, the value is copied
to the packet directly from `bgp->maxmed_value`, not from the
attributes. Thus, changes in this case cannot be detected by comparing
attributes.

With this fix, avoid route suppressing when the `max-med on-startup`
timer expires and initiates an UPDATE.

Signed-off-by: Alexander Chernavin <achernavin@netgate.com>

bgpd/bgp_updgrp_adv.c

@@ -355,6 +355,11 @@ static int update_group_announce_walkcb(struct update_group *updgrp, void *arg)
 	struct update_subgroup *subgrp;
 
 	UPDGRP_FOREACH_SUBGRP (updgrp, subgrp) {
+		/* Avoid supressing duplicate routes later
+		 * when processing in subgroup_announce_table().
+		 */
+		SET_FLAG(subgrp->sflags, SUBGRP_STATUS_FORCE_UPDATES);
+
 		subgroup_announce_all(subgrp);
 	}
 

tests/topotests/bgp_max_med_on_startup/r1/bgpd.conf

@@ -0,0 +1,11 @@
+!
+router bgp 65001
+  bgp max-med on-startup 5 777
+  no bgp ebgp-requires-policy
+  neighbor 192.168.255.2 remote-as 65001
+  neighbor 192.168.255.2 timers 3 10
+  address-family ipv4 unicast
+    redistribute connected
+  exit-address-family
+  !
+!

tests/topotests/bgp_max_med_on_startup/r1/zebra.conf

@@ -0,0 +1,9 @@
+!
+interface lo
+ ip address 172.16.255.254/32
+!
+interface r1-eth0
+ ip address 192.168.255.1/30
+!
+ip forwarding
+!

tests/topotests/bgp_max_med_on_startup/r2/bgpd.conf

@@ -0,0 +1,7 @@
+!
+router bgp 65001
+  no bgp ebgp-requires-policy
+  neighbor 192.168.255.1 remote-as 65001
+  neighbor 192.168.255.1 timers 3 10
+  !
+!

tests/topotests/bgp_max_med_on_startup/r2/zebra.conf

@@ -0,0 +1,6 @@
+!
+interface r2-eth0
+ ip address 192.168.255.2/30
+!
+ip forwarding
+!

tests/topotests/bgp_max_med_on_startup/test_bgp_max_med_on_startup.py

@@ -0,0 +1,114 @@
+#!/usr/bin/env python
+
+#
+# test_bgp_max_med_on_startup.py
+#
+# Copyright (c) 2022 Rubicon Communications, LLC.
+#
+# Permission to use, copy, modify, and/or distribute this software
+# for any purpose with or without fee is hereby granted, provided
+# that the above copyright notice and this permission notice appear
+# in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND NETDEF DISCLAIMS ALL WARRANTIES
+# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL NETDEF BE LIABLE FOR
+# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY
+# DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS,
+# WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS
+# ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+# OF THIS SOFTWARE.
+#
+
+"""
+Test whether `bgp max-med on-startup (5-86400) [(0-4294967295)]` is working
+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
+
+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_max_med_on_startup():
+    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):
+        output = json.loads(
+            router.vtysh_cmd("show ip bgp neighbor 192.168.255.1 routes json")
+        )
+        expected = {"routes": {"172.16.255.254/32": [{"metric": metric}]}}
+        return topotest.json_cmp(output, expected)
+
+    # Check 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 metric has value of max-med
+    test_func = functools.partial(_bgp_has_routes, router2, 777)
+    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 777"
+
+    # Check that when the max-med timer expires, metric is updated
+    test_func = functools.partial(_bgp_has_routes, router2, 0)
+    success, result = topotest.run_and_expect(test_func, None, count=16, wait=0.5)
+    assert result is None, "r2 does not receive routes with metric 0"
+
+
+if __name__ == "__main__":
+    args = ["-s"] + sys.argv[1:]
+    sys.exit(pytest.main(args))