diff options
| -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) |