diff options
| author | Nataliia Solomko <natalirs1985@gmail.com> | 2026-05-26 18:27:56 +0300 |
|---|---|---|
| committer | Nataliia Solomko <natalirs1985@gmail.com> | 2026-06-09 18:41:56 +0300 |
| commit | b49e8f00560bdbd69f85539396afb6391b281519 (patch) | |
| tree | 57990da78f634de549947fd28f408a137061aaa7 | |
| parent | 6a5f1961cde6116457cb9e6b958bee7b66603c3b (diff) | |
| download | vyos-1x-b49e8f00560bdbd69f85539396afb6391b281519.tar.gz vyos-1x-b49e8f00560bdbd69f85539396afb6391b281519.zip | |
bgp: T8223: Prevent `advertise-all-vni` in multiple BGP VRF instances simultaneously
FRR only allows one BGP instance to hold `advertise-all-vni` at a time
(FRR issue #9405). When a default BGP instance is present it is always
started before named VRF instances, so if a named VRF holds
the flag FRR silently rejects it on every boot (regardless of default
EVPN config), causing the running config to diverge from what is
stored in VyOS.
Enforce the following policy in verify():
- Default BGP instance may always hold `advertise-all-vni`.
- A named VRF may hold it only when no default BGP instance exists.
- Only one BGP instance (default or named VRF) may hold it at a time.
The default BGP verify path additionally scans dependent VRFs so that
adding or modifying the default BGP instance while a named VRF already
holds the flag is caught even when the VRF node is not part of the
current commit.
| -rw-r--r-- | interface-definitions/include/version/bgp-version.xml.i | 2 | ||||
| -rw-r--r-- | smoketest/configs/bgp-evpn-l3vpn-pe-router | 2 | ||||
| -rwxr-xr-x | smoketest/scripts/cli/test_protocols_bgp.py | 72 | ||||
| -rwxr-xr-x | src/conf_mode/protocols_bgp.py | 51 | ||||
| -rw-r--r-- | src/migration-scripts/bgp/6-to-7 | 58 |
5 files changed, 184 insertions, 1 deletions
diff --git a/interface-definitions/include/version/bgp-version.xml.i b/interface-definitions/include/version/bgp-version.xml.i index c90276151..21fddf9ae 100644 --- a/interface-definitions/include/version/bgp-version.xml.i +++ b/interface-definitions/include/version/bgp-version.xml.i @@ -1,3 +1,3 @@ <!-- include start from include/version/bgp-version.xml.i --> -<syntaxVersion component='bgp' version='6'></syntaxVersion> +<syntaxVersion component='bgp' version='7'></syntaxVersion> <!-- include end --> diff --git a/smoketest/configs/bgp-evpn-l3vpn-pe-router b/smoketest/configs/bgp-evpn-l3vpn-pe-router index 0deda4c8f..17d03c27d 100644 --- a/smoketest/configs/bgp-evpn-l3vpn-pe-router +++ b/smoketest/configs/bgp-evpn-l3vpn-pe-router @@ -226,6 +226,7 @@ vrf { } } } + advertise-all-vni } } local-as 100 @@ -251,6 +252,7 @@ vrf { } } } + advertise-all-vni } } local-as 100 diff --git a/smoketest/scripts/cli/test_protocols_bgp.py b/smoketest/scripts/cli/test_protocols_bgp.py index c7c0dc6f0..e9f615921 100755 --- a/smoketest/scripts/cli/test_protocols_bgp.py +++ b/smoketest/scripts/cli/test_protocols_bgp.py @@ -1018,6 +1018,78 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase): for route_target in route_targets: self.assertIn(f' ead-es-route-target export {route_target}', frrconfig) + def test_bgp_08_l2vpn_evpn_advertise_all_vni_restriction(self): + # T8223: FRR only allows advertise-all-vni in one BGP instance at a time + # (FRR issue #9405). + vrf = 'VRF-A' + vrf2 = 'VRF-B' + vrf_path = ['vrf', 'name', vrf] + vrf2_path = ['vrf', 'name', vrf2] + + # advertise-all-vni in default BGP is valid + self.cli_set(base_path + ['address-family', 'l2vpn-evpn', 'advertise-all-vni']) + self.cli_commit() + + # Verify FRR bgpd configuration + frrconfig = self.getFRRconfig(f'router bgp {ASN}', stop_section='^exit') + self.assertIn(f'router bgp {ASN}', frrconfig) + self.assertIn(' advertise-all-vni', frrconfig) + + # delete advertise-all-vni + self.cli_delete( + base_path + ['address-family', 'l2vpn-evpn', 'advertise-all-vni'] + ) + self.cli_commit() + + # advertise-all-vni in a named VRF is rejected when default BGP exists + self.cli_set(vrf_path + ['table', '1001']) + self.cli_set(vrf_path + ['protocols', 'bgp', 'system-as', ASN]) + self.cli_set( + vrf_path + + ['protocols', 'bgp', 'address-family', 'l2vpn-evpn', 'advertise-all-vni'] + ) + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + self.cli_delete(base_path) + self.cli_delete(vrf_path) + self.cli_commit() + + # reapply VRF and advertise-all-vni in VRF + self.cli_set(vrf_path + ['table', '1001']) + self.cli_set(vrf_path + ['protocols', 'bgp', 'system-as', ASN]) + self.cli_set( + vrf_path + + ['protocols', 'bgp', 'address-family', 'l2vpn-evpn', 'advertise-all-vni'] + ) + self.cli_commit() + + # Verify BGP VRF configuration + frr_vrf_config = self.getFRRconfig( + f'router bgp {ASN} vrf {vrf}', stop_section='^exit' + ) + self.assertIn(f'router bgp {ASN} vrf {vrf}', frr_vrf_config) + self.assertIn(' advertise-all-vni', frr_vrf_config) + + # two named VRFs cannot both have advertise-all-vni simultaneously + self.cli_set(vrf2_path + ['table', '1002']) + self.cli_set(vrf2_path + ['protocols', 'bgp', 'system-as', ASN]) + self.cli_set( + vrf2_path + + ['protocols', 'bgp', 'address-family', 'l2vpn-evpn', 'advertise-all-vni'] + ) + with self.assertRaises(ConfigSessionError): + self.cli_commit() + self.cli_delete(vrf2_path) + + # default BGP is rejected when a named VRF has advertise-all-vni + self.cli_set(base_path + ['system-as', ASN]) + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + # delete advertise-all-vni from VRF and verify default BGP can be created + self.cli_delete(vrf_path + ['protocols', 'bgp', 'address-family']) + self.cli_commit() def test_bgp_09_distance_and_flowspec(self): distance_external = '25' diff --git a/src/conf_mode/protocols_bgp.py b/src/conf_mode/protocols_bgp.py index 53561a9a3..50d8e5cab 100755 --- a/src/conf_mode/protocols_bgp.py +++ b/src/conf_mode/protocols_bgp.py @@ -214,6 +214,26 @@ def verify(config_dict): if 'system_as' not in bgp: raise ConfigError('BGP system-as number must be defined!') + # FRR #9405 blocks "advertise-all-vni" in named VRFs if a default + # BGP instance is initialized first (regardless of default EVPN config). + # On boot the default instance starts first, so if a named VRF also has + # the flag FRR silently rejects it, causing config divergence. Block the + # combination here even when the VRF node is not part of the current commit. + # Could be removed when FRR fixes it upstream. + if not vrf: + for dep_vrf, dep_config in bgp.get('dependent_vrfs', {}).items(): + if dep_vrf == 'default': + continue + dep_evpn = dict_search( + 'protocols.bgp.address_family.l2vpn_evpn', dep_config + ) + if dep_evpn and 'advertise_all_vni' in dep_evpn: + raise ConfigError( + f'BGP EVPN "advertise-all-vni" is configured in named VRF "{dep_vrf}". ' + f'Remove it from the VRF before adding a default BGP instance, ' + f'or move it to the default BGP instance instead.' + ) + # Verify BMP if 'bmp' in bgp: # check bmp flag "bgpd -d -F traditional --daemon -A 127.0.0.1 -M rpki -M bmp" @@ -580,6 +600,37 @@ def verify(config_dict): # Checks only required for L2VPN EVPN if afi in ['l2vpn_evpn']: + # T8223: FRR #9405 — only one BGP instance may hold "advertise-all-vni" + # at a time. When a default BGP instance coexists with a named VRF that + # has the flag, FRR silently rejects the VRF's copy on every boot because + # the default instance is always started first. + if 'advertise_all_vni' in afi_config: + if vrf: + # Named VRF: block whenever a default BGP instance exists. + default_bgp = dict_search( + 'dependent_vrfs.default.protocols.bgp', bgp + ) + if default_bgp is not None and 'deleted' not in default_bgp: + raise ConfigError( + f'BGP EVPN "advertise-all-vni" is not supported in named VRF "{vrf}" ' + f'when a default BGP instance exists. ' + f'Configure it in the default BGP instance instead.' + ) + + # Block if multiple BGP instances have advertise-all-vni simultaneously. + advertise_all_vni_vrfs = [vrf if vrf else 'default'] + for dep_vrf, dep_config in bgp.get('dependent_vrfs', {}).items(): + dep_evpn = dict_search( + 'protocols.bgp.address_family.l2vpn_evpn', dep_config + ) + if dep_evpn and 'advertise_all_vni' in dep_evpn: + advertise_all_vni_vrfs.append(dep_vrf) + if len(advertise_all_vni_vrfs) > 1: + raise ConfigError( + f'BGP EVPN "advertise-all-vni" cannot be configured in multiple ' + f'VRFs simultaneously: {", ".join(advertise_all_vni_vrfs)}.' + ) + if 'vni' in afi_config: for vni, vni_config in afi_config['vni'].items(): if 'rd' in vni_config and 'advertise_all_vni' not in afi_config: diff --git a/src/migration-scripts/bgp/6-to-7 b/src/migration-scripts/bgp/6-to-7 new file mode 100644 index 000000000..efba9678e --- /dev/null +++ b/src/migration-scripts/bgp/6-to-7 @@ -0,0 +1,58 @@ +# Copyright (C) VyOS Inc. +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this library. If not, see <http://www.gnu.org/licenses/>. + +# T8223: remove "advertise-all-vni" from named VRFs where it conflicts with +# a default BGP instance or another named VRF + +from vyos.configtree import ConfigTree + +base = ['vrf', 'name'] +bgp_base = ['protocols', 'bgp'] +bgp_l2vpn_evpn = bgp_base + ['address-family', 'l2vpn-evpn'] + + +def migrate(config: ConfigTree) -> None: + if not config.exists(base): + # Nothing to do + return + + def _cleanup_address_family(config: ConfigTree, vrf) -> None: + """Remove advertise-all-vni from the named VRF and clean up any + empty parent nodes left behind.""" + config.delete(base + [vrf] + bgp_l2vpn_evpn + ['advertise-all-vni']) + if len(config.list_nodes(base + [vrf] + bgp_l2vpn_evpn)) == 0: + config.delete(base + [vrf] + bgp_l2vpn_evpn) + if len(config.list_nodes(base + [vrf] + ['protocols', 'bgp', 'address-family'])) == 0: + config.delete(base + [vrf] + ['protocols', 'bgp', 'address-family']) + + # Collect named VRFs that have advertise-all-vni configured. + advertise_all_vni_vrfs = [] + + for vrf in config.list_nodes(base): + if config.exists(base + [vrf] + bgp_l2vpn_evpn + ['advertise-all-vni']): + if config.exists(bgp_base): + # A default BGP instance exists. FRR always starts it before + # named VRFs, so the named VRF's advertise-all-vni would be + # silently rejected on every boot. Remove it. + _cleanup_address_family(config, vrf) + else: + advertise_all_vni_vrfs.append(vrf) + + # No default BGP instance, but multiple named VRFs hold advertise-all-vni. + # FRR only allows one instance to hold it at a time. Keep the first VRF + # and remove it from the rest. + if len(advertise_all_vni_vrfs) > 1: + for vrf in advertise_all_vni_vrfs[1:]: + _cleanup_address_family(config, vrf) |
