summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniil Baturin <daniil@vyos.io>2024-05-02 17:10:54 +0200
committerGitHub <noreply@github.com>2024-05-02 17:10:54 +0200
commita6cc2b4ccbcbf4b427b68000e4b31b9bd95a1d64 (patch)
treef88aaeac98bcef962097e89b4da3453759b58a85
parent36aeda94a2b16b2abe340d165dbc4f8ff01db756 (diff)
parent45897d149473e88dd78bcd58c37d26c9f608c549 (diff)
downloadvyos-1x-a6cc2b4ccbcbf4b427b68000e4b31b9bd95a1d64.tar.gz
vyos-1x-a6cc2b4ccbcbf4b427b68000e4b31b9bd95a1d64.zip
Merge pull request #3393 from vyos/mergify/bp/sagitta/pr-3392
bgp: T6189: L3VPN connectivity is broken after re-enabling VRF (backport #3392)
-rw-r--r--data/configd-include.json3
-rw-r--r--data/templates/frr/zebra.vrf.route-map.frr.j26
-rw-r--r--interface-definitions/vrf.xml.in15
-rwxr-xr-xsmoketest/scripts/cli/test_vrf.py55
-rwxr-xr-xsrc/conf_mode/protocols_bgp.py46
-rwxr-xr-xsrc/conf_mode/vrf.py5
-rw-r--r--src/conf_mode/vrf_vni.py103
7 files changed, 74 insertions, 159 deletions
diff --git a/data/configd-include.json b/data/configd-include.json
index fe27ae2b7..212b260e1 100644
--- a/data/configd-include.json
+++ b/data/configd-include.json
@@ -107,6 +107,5 @@
"vpn_openconnect.py",
"vpn_pptp.py",
"vpn_sstp.py",
-"vrf.py",
-"vrf_vni.py"
+"vrf.py"
]
diff --git a/data/templates/frr/zebra.vrf.route-map.frr.j2 b/data/templates/frr/zebra.vrf.route-map.frr.j2
index f1cc6fe66..8ebb82511 100644
--- a/data/templates/frr/zebra.vrf.route-map.frr.j2
+++ b/data/templates/frr/zebra.vrf.route-map.frr.j2
@@ -1,10 +1,6 @@
!
{% if name is vyos_defined %}
{% for vrf, vrf_config in name.items() %}
-{# code path required for vrf_vni.py as we will only render the required VR configuration and not all of them #}
-{% if only_vrf is vyos_defined and vrf is not vyos_defined(only_vrf) %}
-{% continue %}
-{% endif %}
vrf {{ vrf }}
{% if vrf_config.ip.nht.no_resolve_via_default is vyos_defined %}
no ip nht resolve-via-default
@@ -25,7 +21,7 @@ vrf {{ vrf }}
ipv6 protocol {{ protocol_name }} route-map {{ protocol_config.route_map }}
{% endfor %}
{% endif %}
-{% if vrf_config.vni is vyos_defined and no_vni is not vyos_defined %}
+{% if vrf_config.vni is vyos_defined %}
vni {{ vrf_config.vni }}
{% endif %}
exit-vrf
diff --git a/interface-definitions/vrf.xml.in b/interface-definitions/vrf.xml.in
index 94ed96e4b..a20be995a 100644
--- a/interface-definitions/vrf.xml.in
+++ b/interface-definitions/vrf.xml.in
@@ -120,20 +120,7 @@
<constraintErrorMessage>VRF routing table must be in range from 100 to 65535</constraintErrorMessage>
</properties>
</leafNode>
- <leafNode name="vni" owner="${vyos_conf_scripts_dir}/vrf_vni.py $VAR(../@)">
- <properties>
- <help>Virtual Network Identifier</help>
- <!-- must be after BGP to keep correct order when removing L3VNIs in FRR -->
- <priority>822</priority>
- <valueHelp>
- <format>u32:0-16777214</format>
- <description>VXLAN virtual network identifier</description>
- </valueHelp>
- <constraint>
- <validator name="numeric" argument="--range 0-16777214"/>
- </constraint>
- </properties>
- </leafNode>
+ #include <include/vni.xml.i>
</children>
</tagNode>
</children>
diff --git a/smoketest/scripts/cli/test_vrf.py b/smoketest/scripts/cli/test_vrf.py
index f6e4181c0..243397dc2 100755
--- a/smoketest/scripts/cli/test_vrf.py
+++ b/smoketest/scripts/cli/test_vrf.py
@@ -18,7 +18,6 @@ import re
import os
import unittest
-from netifaces import interfaces
from base_vyostest_shim import VyOSUnitTestSHIM
from vyos.configsession import ConfigSessionError
@@ -27,6 +26,7 @@ from vyos.ifconfig import Section
from vyos.utils.file import read_file
from vyos.utils.network import get_interface_config
from vyos.utils.network import is_intf_addr_assigned
+from vyos.utils.network import interface_exists
from vyos.utils.system import sysctl_read
base_path = ['vrf']
@@ -60,7 +60,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
self.cli_delete(base_path)
self.cli_commit()
for vrf in vrfs:
- self.assertNotIn(vrf, interfaces())
+ self.assertFalse(interface_exists(vrf))
def test_vrf_vni_and_table_id(self):
base_table = '1000'
@@ -89,7 +89,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
iproute2_config = read_file('/etc/iproute2/rt_tables.d/vyos-vrf.conf')
for vrf in vrfs:
description = f'VyOS-VRF-{vrf}'
- self.assertTrue(vrf in interfaces())
+ self.assertTrue(interface_exists(vrf))
vrf_if = Interface(vrf)
# validate proper interface description
self.assertEqual(vrf_if.get_alias(), description)
@@ -131,7 +131,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
loopbacks = ['127.0.0.1', '::1']
for vrf in vrfs:
# Ensure VRF was created
- self.assertIn(vrf, interfaces())
+ self.assertTrue(interface_exists(vrf))
# Verify IP forwarding is 1 (enabled)
self.assertEqual(sysctl_read(f'net.ipv4.conf.{vrf}.forwarding'), '1')
self.assertEqual(sysctl_read(f'net.ipv6.conf.{vrf}.forwarding'), '1')
@@ -171,7 +171,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
self.cli_commit()
# Check if VRF has been created
- self.assertTrue(vrf in interfaces())
+ self.assertTrue(interface_exists(vrf))
table = str(int(table) + 1)
self.cli_set(base + ['table', table])
@@ -228,7 +228,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
next_hop = f'192.0.{table}.1'
prefix = f'10.0.{table}.0/24'
- self.assertTrue(vrf in interfaces())
+ self.assertTrue(interface_exists(vrf))
frrconfig = self.getFRRconfig(f'vrf {vrf}')
self.assertIn(f' vni {table}', frrconfig)
@@ -261,7 +261,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
# Apply VRF config
self.cli_commit()
# Ensure VRF got created
- self.assertIn(vrf, interfaces())
+ self.assertTrue(interface_exists(vrf))
# ... and IP addresses are still assigned
for address in addresses:
self.assertTrue(is_intf_addr_assigned(interface, address))
@@ -293,7 +293,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
loopbacks = ['127.0.0.1', '::1']
for vrf in vrfs:
# Ensure VRF was created
- self.assertIn(vrf, interfaces())
+ self.assertTrue(interface_exists(vrf))
# Verify IP forwarding is 0 (disabled)
self.assertEqual(sysctl_read(f'net.ipv4.conf.{vrf}.forwarding'), '0')
self.assertEqual(sysctl_read(f'net.ipv6.conf.{vrf}.forwarding'), '0')
@@ -425,7 +425,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
# Verify VRF configuration
table = base_table
for vrf in vrfs:
- self.assertTrue(vrf in interfaces())
+ self.assertTrue(interface_exists(vrf))
frrconfig = self.getFRRconfig(f'vrf {vrf}')
self.assertIn(f' vni {table}', frrconfig)
@@ -447,7 +447,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
# Verify VRF configuration
table = base_table
for vrf in vrfs:
- self.assertTrue(vrf in interfaces())
+ self.assertTrue(interface_exists(vrf))
frrconfig = self.getFRRconfig(f'vrf {vrf}')
self.assertIn(f' vni {table}', frrconfig)
@@ -470,13 +470,39 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
# Verify VRF configuration
table = base_table
for vrf in vrfs:
- self.assertTrue(vrf in interfaces())
+ self.assertTrue(interface_exists(vrf))
frrconfig = self.getFRRconfig(f'vrf {vrf}')
self.assertIn(f' vni {table}', frrconfig)
# Increment table ID for the next run
table = str(int(table) + 2)
+
+ # add a new VRF with VNI - this must not delete any existing VRF/VNI
+ purple = 'purple'
+ table = str(int(table) + 10)
+ self.cli_set(base_path + ['name', purple, 'table', table])
+ self.cli_set(base_path + ['name', purple, 'vni', table])
+
+ # commit changes
+ self.cli_commit()
+
+ # Verify VRF configuration
+ table = base_table
+ for vrf in vrfs:
+ self.assertTrue(interface_exists(vrf))
+
+ frrconfig = self.getFRRconfig(f'vrf {vrf}')
+ self.assertIn(f' vni {table}', frrconfig)
+ # Increment table ID for the next run
+ table = str(int(table) + 2)
+
+ # Verify purple VRF/VNI
+ self.assertTrue(interface_exists(purple))
+ table = str(int(table) + 10)
+ frrconfig = self.getFRRconfig(f'vrf {purple}')
+ self.assertIn(f' vni {table}', frrconfig)
+
# Now delete all the VNIs
for vrf in vrfs:
base = base_path + ['name', vrf]
@@ -487,11 +513,16 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
# Verify no VNI is defined
for vrf in vrfs:
- self.assertTrue(vrf in interfaces())
+ self.assertTrue(interface_exists(vrf))
frrconfig = self.getFRRconfig(f'vrf {vrf}')
self.assertNotIn('vni', frrconfig)
+ # Verify purple VNI remains
+ self.assertTrue(interface_exists(purple))
+ frrconfig = self.getFRRconfig(f'vrf {purple}')
+ self.assertIn(f' vni {table}', frrconfig)
+
def test_vrf_ip_ipv6_nht(self):
table = '6910'
diff --git a/src/conf_mode/protocols_bgp.py b/src/conf_mode/protocols_bgp.py
index 4df97d133..44409c0e3 100755
--- a/src/conf_mode/protocols_bgp.py
+++ b/src/conf_mode/protocols_bgp.py
@@ -31,6 +31,7 @@ from vyos.utils.dict import dict_search
from vyos.utils.network import get_interface_vrf
from vyos.utils.network import is_addr_assigned
from vyos.utils.process import process_named_running
+from vyos.utils.process import call
from vyos import ConfigError
from vyos import frr
from vyos import airbag
@@ -50,13 +51,8 @@ def get_config(config=None):
# eqivalent of the C foo ? 'a' : 'b' statement
base = vrf and ['vrf', 'name', vrf, 'protocols', 'bgp'] or base_path
- bgp = conf.get_config_dict(
- base,
- key_mangling=('-', '_'),
- get_first_key=True,
- no_tag_node_value_mangle=True,
- with_recursive_defaults=True,
- )
+ bgp = conf.get_config_dict(base, key_mangling=('-', '_'),
+ get_first_key=True, no_tag_node_value_mangle=True)
bgp['dependent_vrfs'] = conf.get_config_dict(['vrf', 'name'],
key_mangling=('-', '_'),
@@ -75,22 +71,29 @@ def get_config(config=None):
if vrf:
bgp.update({'vrf' : vrf})
# We can not delete the BGP VRF instance if there is a L3VNI configured
+ # FRR L3VNI must be deleted first otherwise we will see error:
+ # "FRR error: Please unconfigure l3vni 3000"
tmp = ['vrf', 'name', vrf, 'vni']
- if conf.exists(tmp):
- bgp.update({'vni' : conf.return_value(tmp)})
+ if conf.exists_effective(tmp):
+ bgp.update({'vni' : conf.return_effective_value(tmp)})
# We can safely delete ourself from the dependent vrf list
if vrf in bgp['dependent_vrfs']:
del bgp['dependent_vrfs'][vrf]
- bgp['dependent_vrfs'].update({'default': {'protocols': {
- 'bgp': conf.get_config_dict(base_path, key_mangling=('-', '_'),
- get_first_key=True,
- no_tag_node_value_mangle=True)}}})
+ bgp['dependent_vrfs'].update({'default': {'protocols': {
+ 'bgp': conf.get_config_dict(base_path, key_mangling=('-', '_'),
+ get_first_key=True,
+ no_tag_node_value_mangle=True)}}})
+
if not conf.exists(base):
# If bgp instance is deleted then mark it
bgp.update({'deleted' : ''})
return bgp
+ # We have gathered the dict representation of the CLI, but there are default
+ # options which we need to update into the dictionary retrived.
+ bgp = conf.merge_defaults(bgp, recursive=True)
+
# We also need some additional information from the config, prefix-lists
# and route-maps for instance. They will be used in verify().
#
@@ -242,10 +245,6 @@ def verify(bgp):
if verify_vrf_as_import(bgp['vrf'], tmp_afi, bgp['dependent_vrfs']):
raise ConfigError(f'Cannot delete VRF instance "{bgp["vrf"]}", ' \
'unconfigure "import vrf" commands!')
- # We can not delete the BGP instance if a L3VNI instance exists
- if 'vni' in bgp:
- raise ConfigError(f'Cannot delete VRF instance "{bgp["vrf"]}", ' \
- f'unconfigure VNI "{bgp["vni"]}" first!')
else:
# We are running in the default VRF context, thus we can not delete
# our main BGP instance if there are dependent BGP VRF instances.
@@ -254,7 +253,11 @@ def verify(bgp):
if vrf != 'default':
if dict_search('protocols.bgp', vrf_options):
raise ConfigError('Cannot delete default BGP instance, ' \
- 'dependent VRF instance(s) exist!')
+ 'dependent VRF instance(s) exist(s)!')
+ if 'vni' in vrf_options:
+ raise ConfigError('Cannot delete default BGP instance, ' \
+ 'dependent L3VNI exists!')
+
return None
if 'system_as' not in bgp:
@@ -607,6 +610,13 @@ def generate(bgp):
return None
def apply(bgp):
+ if 'deleted' in bgp:
+ # We need to ensure that the L3VNI is deleted first.
+ # This is not possible with old config backend
+ # priority bug
+ if {'vrf', 'vni'} <= set(bgp):
+ call('vtysh -c "conf t" -c "vrf {vrf}" -c "no vni {vni}"'.format(**bgp))
+
bgp_daemon = 'bgpd'
# Save original configuration prior to starting any commit actions
diff --git a/src/conf_mode/vrf.py b/src/conf_mode/vrf.py
index 587309005..8d8c234c0 100755
--- a/src/conf_mode/vrf.py
+++ b/src/conf_mode/vrf.py
@@ -130,11 +130,6 @@ def get_config(config=None):
tmp = {'policy' : {'route-map' : conf.get_config_dict(['policy', 'route-map'],
get_first_key=True)}}
- # L3VNI setup is done via vrf_vni.py as it must be de-configured (on node
- # deletetion prior to the BGP process. Tell the Jinja2 template no VNI
- # setup is needed
- vrf.update({'no_vni' : ''})
-
# Merge policy dict into "regular" config dict
vrf = dict_merge(tmp, vrf)
return vrf
diff --git a/src/conf_mode/vrf_vni.py b/src/conf_mode/vrf_vni.py
deleted file mode 100644
index 8dab164d7..000000000
--- a/src/conf_mode/vrf_vni.py
+++ /dev/null
@@ -1,103 +0,0 @@
-#!/usr/bin/env python3
-#
-# Copyright (C) 2023-2024 VyOS maintainers and contributors
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License version 2 or later as
-# published by the Free Software Foundation.
-#
-# This program 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 General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program. If not, see <http://www.gnu.org/licenses/>.
-
-from sys import argv
-from sys import exit
-
-from vyos.config import Config
-from vyos.template import render_to_string
-from vyos import ConfigError
-from vyos import frr
-from vyos import airbag
-airbag.enable()
-
-def get_config(config=None):
- if config:
- conf = config
- else:
- conf = Config()
-
- vrf_name = None
- if len(argv) > 1:
- vrf_name = argv[1]
- else:
- return None
-
- # Using duplicate L3VNIs makes no sense - it's also forbidden in FRR,
- # thus VyOS CLI must deny this, too. Instead of getting only the dict for
- # the requested VRF and den comparing it with depenent VRfs to not have any
- # duplicate we will just grad ALL VRFs by default but only render/apply
- # the configuration for the requested VRF - that makes the code easier and
- # hopefully less error prone
- vrf = conf.get_config_dict(['vrf'], key_mangling=('-', '_'),
- no_tag_node_value_mangle=True,
- get_first_key=True)
-
- # Store name of VRF we are interested in for FRR config rendering
- vrf.update({'only_vrf' : vrf_name})
-
- return vrf
-
-def verify(vrf):
- if not vrf:
- return
-
- if len(argv) < 2:
- raise ConfigError('VRF parameter not specified when valling vrf_vni.py')
-
- if 'name' in vrf:
- vni_ids = []
- for name, vrf_config in vrf['name'].items():
- # VRF VNI (Virtual Network Identifier) must be unique on the system
- if 'vni' in vrf_config:
- if vrf_config['vni'] in vni_ids:
- raise ConfigError(f'VRF "{name}" VNI is not unique!')
- vni_ids.append(vrf_config['vni'])
-
- return None
-
-def generate(vrf):
- if not vrf:
- return
-
- vrf['new_frr_config'] = render_to_string('frr/zebra.vrf.route-map.frr.j2', vrf)
- return None
-
-def apply(vrf):
- frr_daemon = 'zebra'
-
- # add configuration to FRR
- frr_cfg = frr.FRRConfig()
- frr_cfg.load_configuration(frr_daemon)
- # There is only one VRF inside the dict as we read only one in get_config()
- if vrf and 'only_vrf' in vrf:
- vrf_name = vrf['only_vrf']
- frr_cfg.modify_section(f'^vrf {vrf_name}', stop_pattern='^exit-vrf', remove_stop_mark=True)
- if vrf and 'new_frr_config' in vrf:
- frr_cfg.add_before(frr.default_add_before, vrf['new_frr_config'])
- frr_cfg.commit_configuration(frr_daemon)
-
- return None
-
-if __name__ == '__main__':
- try:
- c = get_config()
- verify(c)
- generate(c)
- apply(c)
- except ConfigError as e:
- print(e)
- exit(1)