From 90b54e70f9d2afe3c4168569fad1056ac41ff367 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 28 Mar 2023 16:18:47 +0300 Subject: [PATCH] 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). f1aa49293a4a8302b70989aaa9ceb715385c3a7e 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 (cherry picked from commit 4d8e44c7538c6479ac99ec842bebc42a1e6b2ebc) Signed-off-by: Donatas Abraitis --- bgpd/bgpd.c | 36 +++--- .../bgp_route_map_delay_timer/__init__.py | 0 .../bgp_route_map_delay_timer/r1/bgpd.conf | 24 ++++ .../bgp_route_map_delay_timer/r1/zebra.conf | 4 + .../bgp_route_map_delay_timer/r2/bgpd.conf | 4 + .../bgp_route_map_delay_timer/r2/zebra.conf | 4 + .../test_bgp_route_map_delay_timer.py | 120 ++++++++++++++++++ 7 files changed, 172 insertions(+), 20 deletions(-) create mode 100644 tests/topotests/bgp_route_map_delay_timer/__init__.py create mode 100644 tests/topotests/bgp_route_map_delay_timer/r1/bgpd.conf create mode 100644 tests/topotests/bgp_route_map_delay_timer/r1/zebra.conf create mode 100644 tests/topotests/bgp_route_map_delay_timer/r2/bgpd.conf create mode 100644 tests/topotests/bgp_route_map_delay_timer/r2/zebra.conf create mode 100644 tests/topotests/bgp_route_map_delay_timer/test_bgp_route_map_delay_timer.py diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 7e7d57e2c0..eaeed6c600 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -5502,23 +5502,12 @@ static void peer_on_policy_change(struct peer *peer, afi_t afi, safi_t safi, return; if (CHECK_FLAG(peer->af_flags[afi][safi], - PEER_FLAG_SOFT_RECONFIG)) { + 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)) { - 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); - } + 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); } } @@ -6772,11 +6761,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)) { diff --git a/tests/topotests/bgp_route_map_delay_timer/__init__.py b/tests/topotests/bgp_route_map_delay_timer/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/topotests/bgp_route_map_delay_timer/r1/bgpd.conf b/tests/topotests/bgp_route_map_delay_timer/r1/bgpd.conf new file mode 100644 index 0000000000..04dc73a06d --- /dev/null +++ b/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 diff --git a/tests/topotests/bgp_route_map_delay_timer/r1/zebra.conf b/tests/topotests/bgp_route_map_delay_timer/r1/zebra.conf new file mode 100644 index 0000000000..b29940f46a --- /dev/null +++ b/tests/topotests/bgp_route_map_delay_timer/r1/zebra.conf @@ -0,0 +1,4 @@ +! +int r1-eth0 + ip address 192.168.1.1/24 +! diff --git a/tests/topotests/bgp_route_map_delay_timer/r2/bgpd.conf b/tests/topotests/bgp_route_map_delay_timer/r2/bgpd.conf new file mode 100644 index 0000000000..36653e6b1c --- /dev/null +++ b/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 +! diff --git a/tests/topotests/bgp_route_map_delay_timer/r2/zebra.conf b/tests/topotests/bgp_route_map_delay_timer/r2/zebra.conf new file mode 100644 index 0000000000..cffe827363 --- /dev/null +++ b/tests/topotests/bgp_route_map_delay_timer/r2/zebra.conf @@ -0,0 +1,4 @@ +! +int r2-eth0 + ip address 192.168.1.2/24 +! diff --git a/tests/topotests/bgp_route_map_delay_timer/test_bgp_route_map_delay_timer.py b/tests/topotests/bgp_route_map_delay_timer/test_bgp_route_map_delay_timer.py new file mode 100644 index 0000000000..15a077da2e --- /dev/null +++ b/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 +# + +""" + +""" + +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)) -- 2.39.5