From 6b3afa3457a86196383c284047335e1683af4e3a Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 20 Oct 2023 11:59:59 +0300 Subject: [PATCH 1/2] bgpd: Do not suppress conditional advertisement updates if triggered If we have a prefix-list with one entry, and after some time we append a prefix-list with some more additional entries, conditional advertisement is triggered, and the old entries are suppressed (because they look identical as sent before). Hence, the old entries are sent as withdrawals and only new entries sent as updates. Force re-sending all BGP updates for conditional advertisement. The same is done for route-refresh, and/or soft clear operations. Signed-off-by: Donatas Abraitis (cherry picked from commit 2d8e85958526493f59e7cb9bf6dac829ed3d687f) --- bgpd/bgp_conditional_adv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/bgpd/bgp_conditional_adv.c b/bgpd/bgp_conditional_adv.c index 4ad00ed121bb..24d822a745dc 100644 --- a/bgpd/bgp_conditional_adv.c +++ b/bgpd/bgp_conditional_adv.c @@ -103,6 +103,7 @@ static void bgp_conditional_adv_routes(struct peer *peer, afi_t afi, addpath_capable = bgp_addpath_encode_tx(peer, afi, safi); + SET_FLAG(subgrp->sflags, SUBGRP_STATUS_FORCE_UPDATES); for (dest = bgp_table_top(table); dest; dest = bgp_route_next(dest)) { dest_p = bgp_dest_get_prefix(dest); assert(dest_p); From c502cb0e1dad686ab97b6d7240b3821ad6a19ceb Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 19 Oct 2023 20:25:23 +0300 Subject: [PATCH 2/2] tests: Check if BGP conditional advertisement works fine with static routes If we modify the prefix-list that is used to define the routes to be advertised, all of them MUST be advertised. Signed-off-by: Donatas Abraitis (cherry picked from commit 3c9415125818b54416bd89b9f703f987ff91746c) --- .../__init__.py | 0 .../r1/frr.conf | 10 ++ .../r2/frr.conf | 39 ++++++ .../r3/frr.conf | 19 +++ ..._conditional_advertisement_static_route.py | 118 ++++++++++++++++++ 5 files changed, 186 insertions(+) create mode 100644 tests/topotests/bgp_conditional_advertisement_static_route/__init__.py create mode 100644 tests/topotests/bgp_conditional_advertisement_static_route/r1/frr.conf create mode 100644 tests/topotests/bgp_conditional_advertisement_static_route/r2/frr.conf create mode 100644 tests/topotests/bgp_conditional_advertisement_static_route/r3/frr.conf create mode 100644 tests/topotests/bgp_conditional_advertisement_static_route/test_bgp_conditional_advertisement_static_route.py diff --git a/tests/topotests/bgp_conditional_advertisement_static_route/__init__.py b/tests/topotests/bgp_conditional_advertisement_static_route/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/topotests/bgp_conditional_advertisement_static_route/r1/frr.conf b/tests/topotests/bgp_conditional_advertisement_static_route/r1/frr.conf new file mode 100644 index 000000000000..3e51337eee90 --- /dev/null +++ b/tests/topotests/bgp_conditional_advertisement_static_route/r1/frr.conf @@ -0,0 +1,10 @@ +! +int r1-eth0 + ip address 192.168.1.1/24 +! +router bgp 65000 + no bgp ebgp-requires-policy + neighbor 192.168.1.2 remote-as internal + neighbor 192.168.1.2 timers 1 3 + neighbor 192.168.1.2 timers connect 1 +! diff --git a/tests/topotests/bgp_conditional_advertisement_static_route/r2/frr.conf b/tests/topotests/bgp_conditional_advertisement_static_route/r2/frr.conf new file mode 100644 index 000000000000..9dc4099341d9 --- /dev/null +++ b/tests/topotests/bgp_conditional_advertisement_static_route/r2/frr.conf @@ -0,0 +1,39 @@ +! +!debug bgp conditional-advertisement +!debug bgp updates +! +int r2-eth0 + ip address 192.168.1.2/24 +! +int r2-eth1 + ip address 192.168.2.2/24 +! +router bgp 65000 + no bgp ebgp-requires-policy + bgp conditional-advertisement timer 5 + neighbor 192.168.1.1 remote-as internal + neighbor 192.168.1.1 timers 1 3 + neighbor 192.168.1.1 timers connect 1 + neighbor 192.168.2.1 remote-as internal + neighbor 192.168.2.1 timers 1 3 + neighbor 192.168.2.1 timers connect 1 + address-family ipv4 unicast + redistribute static + neighbor 192.168.1.1 advertise-map advertise-map exist-map exist-map + neighbor 192.168.1.1 route-map deny-all out + exit-address-family +! +ip route 10.10.10.1/32 r2-eth0 +ip route 10.10.10.2/32 r2-eth0 +! +ip prefix-list default seq 5 permit 0.0.0.0/0 +ip prefix-list advertise seq 5 permit 10.10.10.1/32 +! +route-map deny-all deny 10 +! +route-map exist-map permit 10 + match ip address prefix-list default +! +route-map advertise-map permit 10 + match ip address prefix-list advertise +! diff --git a/tests/topotests/bgp_conditional_advertisement_static_route/r3/frr.conf b/tests/topotests/bgp_conditional_advertisement_static_route/r3/frr.conf new file mode 100644 index 000000000000..a24a2cb04ed6 --- /dev/null +++ b/tests/topotests/bgp_conditional_advertisement_static_route/r3/frr.conf @@ -0,0 +1,19 @@ +! +int lo + ip address 10.10.10.1/32 + ip address 10.10.10.2/32 +! +int r3-eth0 + ip address 192.168.2.1/24 +! +router bgp 65000 + no bgp ebgp-requires-policy + no bgp network import-check + neighbor 192.168.2.2 remote-as internal + neighbor 192.168.2.2 timers 1 3 + neighbor 192.168.2.2 timers connect 1 + ! + address-family ipv4 unicast + neighbor 192.168.2.2 default-originate + exit-address-family +! diff --git a/tests/topotests/bgp_conditional_advertisement_static_route/test_bgp_conditional_advertisement_static_route.py b/tests/topotests/bgp_conditional_advertisement_static_route/test_bgp_conditional_advertisement_static_route.py new file mode 100644 index 000000000000..9d61bbd643fe --- /dev/null +++ b/tests/topotests/bgp_conditional_advertisement_static_route/test_bgp_conditional_advertisement_static_route.py @@ -0,0 +1,118 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# Copyright (c) 2023 by +# Donatas Abraitis +# + +""" +Test if static route with BGP conditional advertisement works correctly +if we modify the prefix-lists. +""" + +import os +import re +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 +from lib.common_config import step + +pytestmark = [pytest.mark.bgpd] + + +def setup_module(mod): + topodef = {"s1": ("r1", "r2"), "s2": ("r2", "r3")} + tgen = Topogen(topodef, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + + for _, (rname, router) in enumerate(router_list.items(), 1): + router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname))) + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_bgp_conditional_advertisements_static_route(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r2 = tgen.gears["r2"] + + def _bgp_converge(): + output = json.loads( + r2.vtysh_cmd( + "show bgp ipv4 unicast neighbor 192.168.1.1 advertised-routes json" + ) + ) + expected = { + "advertisedRoutes": { + "10.10.10.1/32": { + "valid": True, + } + }, + "totalPrefixCounter": 1, + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_converge, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Can't converge" + + step("Append prefix-list to advertise 10.10.10.2/32") + + r2.vtysh_cmd( + """ + configure terminal + ip prefix-list advertise seq 10 permit 10.10.10.2/32 + """ + ) + + def _bgp_check_advertised_after_update(): + output = json.loads( + r2.vtysh_cmd( + "show bgp ipv4 unicast neighbor 192.168.1.1 advertised-routes json" + ) + ) + expected = { + "advertisedRoutes": { + "10.10.10.1/32": { + "valid": True, + }, + "10.10.10.2/32": { + "valid": True, + }, + }, + "totalPrefixCounter": 2, + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_check_advertised_after_update, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "10.10.10.2/32 is not advertised after prefix-list update" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args))