From b9d4191a51a8965228f50f5100e5831eb1a5a894 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 5 Sep 2024 15:16:05 +0300 Subject: [PATCH 1/3] bgpd: Allow using `solo` for peer-groups Inherit solo flag for peer-group members also. Signed-off-by: Donatas Abraitis --- bgpd/bgp_updgrp.c | 2 ++ bgpd/bgp_vty.c | 7 ++----- bgpd/bgpd.c | 1 + doc/user/bgp.rst | 3 +-- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/bgpd/bgp_updgrp.c b/bgpd/bgp_updgrp.c index b717793a45..115bc35cdc 100644 --- a/bgpd/bgp_updgrp.c +++ b/bgpd/bgp_updgrp.c @@ -2016,6 +2016,8 @@ int update_group_adjust_soloness(struct peer *peer, int set) struct peer_group *group; struct listnode *node, *nnode; + peer_flag_set(peer, PEER_FLAG_LONESOUL); + if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { peer_lonesoul_or_not(peer, set); if (peer_established(peer->connection)) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index c9c7b80496..338f6e3a50 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -18678,11 +18678,8 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp, peer->password); /* neighbor solo */ - if (CHECK_FLAG(peer->flags, PEER_FLAG_LONESOUL)) { - if (!peer_group_active(peer)) { - vty_out(vty, " neighbor %s solo\n", addr); - } - } + if (peergroup_flag_check(peer, PEER_FLAG_LONESOUL)) + vty_out(vty, " neighbor %s solo\n", addr); /* BGP port */ if (peer->port != BGP_PORT_DEFAULT) { diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index a88de651f5..ad45f5d6e9 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -4701,6 +4701,7 @@ static const struct peer_flag_action peer_flag_action_list[] = { {PEER_FLAG_CAPABILITY_FQDN, 0, peer_change_none}, {PEER_FLAG_AS_LOOP_DETECTION, 0, peer_change_none}, {PEER_FLAG_EXTENDED_LINK_BANDWIDTH, 0, peer_change_none}, + {PEER_FLAG_LONESOUL, 0, peer_change_reset_out}, {0, 0, 0}}; static const struct peer_flag_action peer_af_flag_action_list[] = { diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst index aa62d274f0..b6d289fac9 100644 --- a/doc/user/bgp.rst +++ b/doc/user/bgp.rst @@ -2191,8 +2191,7 @@ and will share updates. .. clicmd:: neighbor PEER solo This command is used to indicate that routes advertised by the peer - should not be reflected back to the peer. This command only is only - meaningful when there is a single peer defined in the peer-group. + should not be reflected back to the peer. .. clicmd:: show [ip] bgp peer-group [json] From 9de74cf0beff75dace53881e11ba40c0d707b8a1 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 5 Sep 2024 15:48:14 +0300 Subject: [PATCH 2/3] bgpd: Show what is the real type of the peer-group ``` ton# sh ip bgp peer-group BGP peer-group pg-a Peer-group type is auto Configured address-families: IPv4 Unicast; BGP peer-group pg-e, remote AS 0 Peer-group type is external Configured address-families: IPv4 Unicast; BGP peer-group pg-i, remote AS 65001 Peer-group type is internal Configured address-families: IPv4 Unicast; ton# ``` `auto` should be handled accordingly. Fixes: 0dfe25697f5299326046fcfb66f2c6beca7c423c ("bgpd: Implement neighbor X remote-as auto") Signed-off-by: Donatas Abraitis --- bgpd/bgp_vty.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 338f6e3a50..2766efc838 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -17072,8 +17072,13 @@ static int bgp_show_one_peer_group(struct vty *vty, struct peer_group *group, vty_out(vty, "\nBGP peer-group %s\n", group->name); } - if ((group->bgp->as == conf->as) || - CHECK_FLAG(conf->as_type, AS_INTERNAL)) { + if (CHECK_FLAG(conf->as_type, AS_AUTO)) { + if (json) + json_object_string_add(json_peer_group, "type", "auto"); + else + vty_out(vty, " Peer-group type is auto\n"); + } else if ((group->bgp->as == conf->as) || + CHECK_FLAG(conf->as_type, AS_INTERNAL)) { if (json) json_object_string_add(json_peer_group, "type", "internal"); From b63315f15ded16e70ec87a0bdcefd8b396150094 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 5 Sep 2024 16:31:37 +0300 Subject: [PATCH 3/3] tests: Check if we can use `solo` with a peer-group Signed-off-by: Donatas Abraitis --- .../topotests/bgp_peer_group_solo/__init__.py | 0 .../topotests/bgp_peer_group_solo/r1/frr.conf | 21 ++++ .../topotests/bgp_peer_group_solo/r2/frr.conf | 10 ++ .../topotests/bgp_peer_group_solo/r3/frr.conf | 10 ++ .../test_bgp_peer_group_solo.py | 102 ++++++++++++++++++ 5 files changed, 143 insertions(+) create mode 100644 tests/topotests/bgp_peer_group_solo/__init__.py create mode 100644 tests/topotests/bgp_peer_group_solo/r1/frr.conf create mode 100644 tests/topotests/bgp_peer_group_solo/r2/frr.conf create mode 100644 tests/topotests/bgp_peer_group_solo/r3/frr.conf create mode 100644 tests/topotests/bgp_peer_group_solo/test_bgp_peer_group_solo.py diff --git a/tests/topotests/bgp_peer_group_solo/__init__.py b/tests/topotests/bgp_peer_group_solo/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/topotests/bgp_peer_group_solo/r1/frr.conf b/tests/topotests/bgp_peer_group_solo/r1/frr.conf new file mode 100644 index 0000000000..53959aa134 --- /dev/null +++ b/tests/topotests/bgp_peer_group_solo/r1/frr.conf @@ -0,0 +1,21 @@ +! +int r1-eth0 + ip address 192.168.1.1/24 +! +int r1-eth1 + ip address 192.168.14.1/24 +! +router bgp 65001 + no bgp ebgp-requires-policy + no bgp network import-check + neighbor pg peer-group + neighbor pg remote-as external + neighbor pg solo + neighbor pg timers 1 3 + neighbor pg timers connect 1 + neighbor 192.168.1.2 peer-group pg + neighbor 192.168.1.3 peer-group pg + address-family ipv4 unicast + network 10.0.0.1/32 + exit-address-family +! diff --git a/tests/topotests/bgp_peer_group_solo/r2/frr.conf b/tests/topotests/bgp_peer_group_solo/r2/frr.conf new file mode 100644 index 0000000000..ba99827a47 --- /dev/null +++ b/tests/topotests/bgp_peer_group_solo/r2/frr.conf @@ -0,0 +1,10 @@ +! +int r2-eth0 + ip address 192.168.1.2/24 +! +router bgp 65002 + no bgp ebgp-requires-policy + neighbor 192.168.1.1 remote-as external + neighbor 192.168.1.1 timers 1 3 + neighbor 192.168.1.1 timers connect 1 +! diff --git a/tests/topotests/bgp_peer_group_solo/r3/frr.conf b/tests/topotests/bgp_peer_group_solo/r3/frr.conf new file mode 100644 index 0000000000..ed06170bf2 --- /dev/null +++ b/tests/topotests/bgp_peer_group_solo/r3/frr.conf @@ -0,0 +1,10 @@ +! +int r3-eth0 + ip address 192.168.1.3/24 +! +router bgp 65003 + no bgp ebgp-requires-policy + neighbor 192.168.1.1 remote-as external + neighbor 192.168.1.1 timers 1 3 + neighbor 192.168.1.1 timers connect 1 +! diff --git a/tests/topotests/bgp_peer_group_solo/test_bgp_peer_group_solo.py b/tests/topotests/bgp_peer_group_solo/test_bgp_peer_group_solo.py new file mode 100644 index 0000000000..cdbc1e02a7 --- /dev/null +++ b/tests/topotests/bgp_peer_group_solo/test_bgp_peer_group_solo.py @@ -0,0 +1,102 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# Copyright (c) 2024 by +# Donatas Abraitis +# + +import os +import re +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, get_topogen + +pytestmark = [pytest.mark.bgpd] + + +def setup_module(mod): + topodef = {"s1": ("r1", "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_remote_as_auto(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + + def _bgp_converge(): + output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast summary json")) + expected = { + "peers": { + "192.168.1.2": { + "remoteAs": 65002, + "state": "Established", + "peerState": "OK", + }, + "192.168.1.3": { + "remoteAs": 65003, + "state": "Established", + "peerState": "OK", + }, + }, + "totalPeers": 2, + } + + 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 initial state" + + def _bgp_update_groups(): + actual = [] + output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast update-groups json")) + expected = [ + {"subGroup": [{"adjListCount": 1, "peers": ["192.168.1.2"]}]}, + {"subGroup": [{"adjListCount": 1, "peers": ["192.168.1.3"]}]}, + ] + + # update-group's number can be random and it's not deterministic, + # so we need to normalize the data a bit before checking. + # We care here about the `peers` array only actually. + for updgrp in output["default"].keys(): + actual.append(output["default"][updgrp]) + + return topotest.json_cmp(actual, expected) + + test_func = functools.partial( + _bgp_update_groups, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Can't see separate update-groups" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args))