From 80a4f87c9a38d5e893f7e24da11cc0c885db682e Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 12 Jul 2024 17:09:16 +0300 Subject: [PATCH 1/5] bgpd: Mark VRF instance as auto created if import vrf is configured for this instance If we create a new BGP instance (in this case VRF instance), it MUST be marked as auto created, to avoid bgpd changing VRF instance's ASN to the default VRF's. That's because of the ordering when FRR reload is happening. Signed-off-by: Donatas Abraitis --- bgpd/bgp_vty.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 0ef1351835..624edff0af 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -10567,12 +10567,20 @@ DEFPY(bgp_imexport_vrf, bgp_imexport_vrf_cmd, vrf_bgp = bgp_lookup_by_name(import_name); if (!vrf_bgp) { - if (strcmp(import_name, VRF_DEFAULT_NAME) == 0) + if (strcmp(import_name, VRF_DEFAULT_NAME) == 0) { vrf_bgp = bgp_default; - else + } else { /* Auto-create assuming the same AS */ ret = bgp_get_vty(&vrf_bgp, &as, import_name, bgp_type, NULL, ASNOTATION_UNDEFINED); + + /* Auto created VRF instances should be marked + * properly, otherwise we have a state after bgpd + * restart where VRF instance has default VRF's ASN. + */ + SET_FLAG(vrf_bgp->vrf_flags, BGP_VRF_AUTO); + } + if (ret) { vty_out(vty, "VRF %s is not configured as a bgp instance\n", From 7540364e58b08da7442927c1a9ffbd535d94fc46 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Sat, 13 Jul 2024 12:43:31 +0300 Subject: [PATCH 2/5] tests: Check if multiple VRF instances can have different ASNs Signed-off-by: Donatas Abraitis --- .../bgp_vrf_different_asn/__init__.py | 0 .../bgp_vrf_different_asn/r1/frr.conf | 18 ++++ .../test_bgp_vrf_different_asn.py | 89 +++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 tests/topotests/bgp_vrf_different_asn/__init__.py create mode 100644 tests/topotests/bgp_vrf_different_asn/r1/frr.conf create mode 100644 tests/topotests/bgp_vrf_different_asn/test_bgp_vrf_different_asn.py diff --git a/tests/topotests/bgp_vrf_different_asn/__init__.py b/tests/topotests/bgp_vrf_different_asn/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/topotests/bgp_vrf_different_asn/r1/frr.conf b/tests/topotests/bgp_vrf_different_asn/r1/frr.conf new file mode 100644 index 0000000000..b325dfb7f0 --- /dev/null +++ b/tests/topotests/bgp_vrf_different_asn/r1/frr.conf @@ -0,0 +1,18 @@ +! +vrf vrf100 + vni 10100 +exit-vrf +! +interface r1-eth0 vrf vrf100 + ip address 192.168.1.1/24 +! +router bgp 65000 + address-family ipv4 unicast + import vrf vrf100 + exit-address-family +! +router bgp 65100 vrf vrf100 + address-family ipv4 unicast + redistribute connected + exit-address-family +! diff --git a/tests/topotests/bgp_vrf_different_asn/test_bgp_vrf_different_asn.py b/tests/topotests/bgp_vrf_different_asn/test_bgp_vrf_different_asn.py new file mode 100644 index 0000000000..7334305ae4 --- /dev/null +++ b/tests/topotests/bgp_vrf_different_asn/test_bgp_vrf_different_asn.py @@ -0,0 +1,89 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# +# Copyright (c) 2024 by +# Donatas Abraitis +# + +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, 2): + tgen.add_router("r{}".format(routern)) + + switch = tgen.add_switch("s1") + switch.add_link(tgen.gears["r1"]) + + +def setup_module(mod): + tgen = Topogen(build_topo, mod.__name__) + tgen.start_topology() + + r1 = tgen.gears["r1"] + r1.run("ip link add vrf100 type vrf table 1001") + r1.run("ip link set up dev vrf100") + r1.run("ip link set r1-eth0 master vrf100") + + 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_vrf_different_asn(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + def _bgp_check_imported_route(): + output = json.loads( + tgen.gears["r1"].vtysh_cmd("show ip route 192.168.1.0/24 json") + ) + expected = { + "192.168.1.0/24": [ + { + "installed": True, + "selected": True, + "nexthops": [ + { + "interfaceName": "vrf100", + "vrf": "vrf100", + "active": True, + } + ], + } + ] + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_bgp_check_imported_route) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Can't see 192.168.1.0/24 being imported into a default VRF" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) From 03c086866bdee9daf55420b88593345b9eb6be15 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Sat, 13 Jul 2024 23:19:57 +0300 Subject: [PATCH 3/5] bgpd: Skip automatically created BGP instances for show CMDs When using e.g. `adverise-all-vni`, and/or `import vrf ...`, the VRF instance is created with a default's VRF ASN and tagged as AUTO_VRF. We MUST skip them here also. Signed-off-by: Donatas Abraitis --- bgpd/bgp_vty.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 624edff0af..0d50a23902 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -12753,6 +12753,9 @@ static void bgp_show_all_instances_summary_vty(struct vty *vty, afi_t afi, vty_out(vty, "{\n"); for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) { + if (CHECK_FLAG(bgp->vrf_flags, BGP_VRF_AUTO)) + continue; + nbr_output = true; if (use_json) { if (!is_first) @@ -16138,6 +16141,9 @@ static void bgp_show_all_instances_neighbors_vty(struct vty *vty, vty_out(vty, "{\n"); for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) { + if (CHECK_FLAG(bgp->vrf_flags, BGP_VRF_AUTO)) + continue; + nbr_output = true; if (use_json) { if (!(json = json_object_new_object())) { @@ -16697,6 +16703,9 @@ static int bgp_show_all_instance_route_leak_vty(struct vty *vty, afi_t afi, if (bgp->inst_type != BGP_INSTANCE_TYPE_DEFAULT) vrf_name = bgp->name; + if (CHECK_FLAG(bgp->vrf_flags, BGP_VRF_AUTO)) + continue; + if (use_json) { json_vrf = json_object_new_object(); } else { @@ -16787,6 +16796,9 @@ static void bgp_show_all_instances_updgrps_vty(struct vty *vty, afi_t afi, struct bgp *bgp; for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) { + if (CHECK_FLAG(bgp->vrf_flags, BGP_VRF_AUTO)) + continue; + if (!uj) vty_out(vty, "\nInstance %s:\n", (bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT) From c6c0403c61c157a9507781c332e152a2b220da52 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Sat, 13 Jul 2024 13:14:33 +0300 Subject: [PATCH 4/5] tests: Check if VRF instance has a different ASN than a default VRF Signed-off-by: Donatas Abraitis --- .../test_bgp_vrf_different_asn.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/topotests/bgp_vrf_different_asn/test_bgp_vrf_different_asn.py b/tests/topotests/bgp_vrf_different_asn/test_bgp_vrf_different_asn.py index 7334305ae4..9a1a9ec766 100644 --- a/tests/topotests/bgp_vrf_different_asn/test_bgp_vrf_different_asn.py +++ b/tests/topotests/bgp_vrf_different_asn/test_bgp_vrf_different_asn.py @@ -58,6 +58,24 @@ def test_bgp_vrf_different_asn(): if tgen.routers_have_failure(): pytest.skip(tgen.errors) + def _bgp_check_instances(): + output = json.loads(tgen.gears["r1"].vtysh_cmd("show bgp vrf all json")) + expected = { + "default": { + "vrfName": "default", + "localAS": 65000, + }, + "vrf100": { + "vrfName": "vrf100", + "localAS": 65100, + }, + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_bgp_check_instances) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Can't see vrf100 to be under 65100 ASN" + def _bgp_check_imported_route(): output = json.loads( tgen.gears["r1"].vtysh_cmd("show ip route 192.168.1.0/24 json") From bfedb38110e8d3e5471718a0f9abe8836ffc7143 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Mon, 15 Jul 2024 16:20:31 +0300 Subject: [PATCH 5/5] bgpd: Skip empty (auto created) VRF instances when deleting a default BGP instance Auto created VRF instances does not have any config, so it's not relevant depending on them. Signed-off-by: Donatas Abraitis --- bgpd/bgp_vty.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 0d50a23902..bce8202377 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -1701,6 +1701,10 @@ DEFUN (no_router_bgp, for (ALL_LIST_ELEMENTS_RO(bm->bgp, node, tmp_bgp)) { if (tmp_bgp->inst_type != BGP_INSTANCE_TYPE_VRF) continue; + + if (CHECK_FLAG(tmp_bgp->vrf_flags, BGP_VRF_AUTO)) + continue; + if (CHECK_FLAG( tmp_bgp->af_flags[AFI_IP] [SAFI_UNICAST],