bgpd: Do not announce routes immediatelly on filter updates

If we set `bgp route-map delay-timer X`, we should ignore starting to announce
routes immediately, and wait for delay timer to expire (or ignore at all if set
to zero).

f1aa49293a broke this because we always sent
route refresh and on receiving BoRR before sending back EoRR.

Let's get fix this.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

bgpd/bgpd.c

@@ -5675,21 +5675,13 @@ void peer_on_policy_change(struct peer *peer, afi_t afi, safi_t safi,
 		if (bgp_soft_reconfig_in(peer, afi, safi))
 			return;
 
-		if (CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_OLD_RCV) ||
-		    CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_NEW_RCV)) {
-			if (CHECK_FLAG(peer->af_cap[afi][safi],
-				       PEER_CAP_ORF_PREFIX_SM_ADV) &&
-			    (CHECK_FLAG(peer->af_cap[afi][safi],
-					PEER_CAP_ORF_PREFIX_RM_RCV) ||
-			     CHECK_FLAG(peer->af_cap[afi][safi],
-					PEER_CAP_ORF_PREFIX_RM_OLD_RCV)))
-				peer_clear_soft(peer, afi, safi,
-						BGP_CLEAR_SOFT_IN_ORF_PREFIX);
-			else
-				bgp_route_refresh_send(
-					peer, afi, safi, 0, 0, 0,
-					BGP_ROUTE_REFRESH_NORMAL);
-		}
+		if (CHECK_FLAG(peer->af_flags[afi][safi],
+			       PEER_FLAG_SOFT_RECONFIG))
+			bgp_soft_reconfig_in(peer, afi, safi);
+		else if (CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_OLD_RCV) ||
+			 CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_NEW_RCV))
+			bgp_route_refresh_send(peer, afi, safi, 0, 0, 0,
+					       BGP_ROUTE_REFRESH_NORMAL);
 	}
 }
 
@@ -6930,11 +6922,18 @@ static void peer_prefix_list_update(struct prefix_list *plist)
 
 				/* If we touch prefix-list, we need to process
 				 * new updates. This is important for ORF to
-				 * work correctly as well.
+				 * work correctly.
 				 */
-				if (peer->afc_nego[afi][safi])
-					peer_on_policy_change(peer, afi, safi,
-							      0);
+				if (CHECK_FLAG(peer->af_cap[afi][safi],
+					       PEER_CAP_ORF_PREFIX_SM_ADV) &&
+				    (CHECK_FLAG(peer->af_cap[afi][safi],
+						PEER_CAP_ORF_PREFIX_RM_RCV) ||
+				     CHECK_FLAG(
+					     peer->af_cap[afi][safi],
+					     PEER_CAP_ORF_PREFIX_RM_OLD_RCV)))
+					peer_clear_soft(
+						peer, afi, safi,
+						BGP_CLEAR_SOFT_IN_ORF_PREFIX);
 			}
 		}
 		for (ALL_LIST_ELEMENTS(bgp->group, node, nnode, group)) {

tests/topotests/bgp_route_map_delay_timer/r1/bgpd.conf

@@ -0,0 +1,24 @@
+!
+debug bgp updates
+debug bgp neighbor
+!
+bgp route-map delay-timer 5
+!
+router bgp 65001
+ no bgp ebgp-requires-policy
+ no bgp network import-check
+ neighbor 192.168.1.2 remote-as external
+ address-family ipv4 unicast
+  network 10.10.10.1/32
+  network 10.10.10.2/32
+  network 10.10.10.3/32
+  aggregate-address 10.10.10.0/24 summary-only
+  neighbor 192.168.1.2 unsuppress-map r2
+ exit-address-family
+!
+ip prefix-list r1 seq 5 permit 10.10.10.1/32
+ip prefix-list r1 seq 10 permit 10.10.10.2/32
+!
+route-map r2 permit 10
+ match ip address prefix-list r1
+exit

tests/topotests/bgp_route_map_delay_timer/r1/zebra.conf

@@ -0,0 +1,4 @@
+!
+int r1-eth0
+ ip address 192.168.1.1/24
+!

tests/topotests/bgp_route_map_delay_timer/r2/bgpd.conf

@@ -0,0 +1,4 @@
+router bgp 65002
+ no bgp ebgp-requires-policy
+ neighbor 192.168.1.1 remote-as external
+!

tests/topotests/bgp_route_map_delay_timer/r2/zebra.conf

@@ -0,0 +1,4 @@
+!
+int r2-eth0
+ ip address 192.168.1.2/24
+!

tests/topotests/bgp_route_map_delay_timer/test_bgp_route_map_delay_timer.py

@@ -0,0 +1,120 @@
+#!/usr/bin/env python
+# SPDX-License-Identifier: ISC
+
+# Copyright (c) 2023 by
+# Donatas Abraitis <donatas@opensourcerouting.org>
+#
+
+"""
+
+"""
+
+import os
+import sys
+import json
+import pytest
+import functools
+
+pytestmark = pytest.mark.bgpd
+
+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 setup_module(mod):
+    topodef = {"s1": ("r1", "r2")}
+    tgen = Topogen(topodef, 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_delay_timer():
+    tgen = get_topogen()
+
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    r1 = tgen.gears["r1"]
+    r2 = tgen.gears["r2"]
+
+    def _bgp_converge_1():
+        output = json.loads(
+            r1.vtysh_cmd(
+                "show bgp ipv4 unicast neighbor 192.168.1.2 advertised-routes json"
+            )
+        )
+        expected = {
+            "advertisedRoutes": {
+                "10.10.10.0/24": {},
+                "10.10.10.1/32": {},
+                "10.10.10.2/32": {},
+                "10.10.10.3/32": None,
+            }
+        }
+        return topotest.json_cmp(output, expected)
+
+    test_func = functools.partial(_bgp_converge_1)
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
+    assert result is None, "10.10.10.3/32 should not be advertised to r2"
+
+    # Set route-map delay-timer to max value and remove 10.10.10.2/32.
+    # After this, r1 MUST do not announce updates immediately, and wait
+    # 600 seconds before withdrawing 10.10.10.2/32.
+    r2.vtysh_cmd(
+        """
+        configure terminal
+            bgp route-map delay-timer 600
+            no ip prefix-list r1 seq 10 permit 10.10.10.2/32
+    """
+    )
+
+    def _bgp_converge_2():
+        output = json.loads(
+            r1.vtysh_cmd(
+                "show bgp ipv4 unicast neighbor 192.168.1.2 advertised-routes json"
+            )
+        )
+        expected = {
+            "advertisedRoutes": {
+                "10.10.10.0/24": {},
+                "10.10.10.1/32": {},
+                "10.10.10.2/32": None,
+                "10.10.10.3/32": None,
+            }
+        }
+        return topotest.json_cmp(output, expected)
+
+    # We are checking `not None` here to wait count*wait time and if we have different
+    # results than expected, it means good - 10.10.10.2/32 wasn't withdrawn immediately.
+    test_func = functools.partial(_bgp_converge_2)
+    _, result = topotest.run_and_expect(test_func, not None, count=60, wait=0.5)
+    assert (
+        result is not None
+    ), "10.10.10.2/32 advertised, but should not be advertised to r2"
+
+
+if __name__ == "__main__":
+    args = ["-s"] + sys.argv[1:]
+    sys.exit(pytest.main(args))