From 3907233b51e721ea894f383b51e42118a2012d78 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Sun, 10 Jul 2022 22:27:29 +0200 Subject: smoketest: bond: add testcase for source-interface re-use A bond member is not allowed to also be used as a source interface for e.g. PPPoE or MACsec. (cherry picked from commit 6fca4854aa2e950795ff0411abe4601f86bdeac0) --- smoketest/scripts/cli/test_interfaces_bonding.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/smoketest/scripts/cli/test_interfaces_bonding.py b/smoketest/scripts/cli/test_interfaces_bonding.py index 9bb561275..a6e5ea59c 100755 --- a/smoketest/scripts/cli/test_interfaces_bonding.py +++ b/smoketest/scripts/cli/test_interfaces_bonding.py @@ -49,7 +49,7 @@ class BondingInterfaceTest(BasicInterfaceTest.TestCase): if not '.' in tmp: cls._members.append(tmp) - cls._options['bond0'] = [] + cls._options = {'bond0' : []} for member in cls._members: cls._options['bond0'].append(f'member interface {member}') cls._interfaces = list(cls._options) @@ -165,6 +165,26 @@ class BondingInterfaceTest(BasicInterfaceTest.TestCase): self.cli_commit() + def test_bonding_source_interface(self): + # Re-use member interface that is already a source-interface + bond = 'bond99' + pppoe = 'pppoe98756' + member = next(iter(self._members)) + + self.cli_set(self._base_path + [bond, 'member', 'interface', member]) + self.cli_set(['interfaces', 'pppoe', pppoe, 'source-interface', member]) + + # check validate() - can not add interface to bond, it is the source-interface of ... + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + self.cli_delete(['interfaces', 'pppoe', pppoe]) + self.cli_commit() + + # verify config + slaves = read_file(f'/sys/class/net/{bond}/bonding/slaves').split() + self.assertIn(member, slaves) + def test_bonding_uniq_member_description(self): ethernet_path = ['interfaces', 'ethernet'] for interface in self._interfaces: -- cgit v1.2.3 From d88912616a7d1fe221069773a1102e17880739ed Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Sun, 10 Jul 2022 22:28:12 +0200 Subject: smoketest: bond: add testcase for conflicting bridge member A bond member can not also be used as a member of a bridge interface. (cherry picked from commit 19bfed0abd75adacb61f170606fff8b4d2e7713f) --- smoketest/scripts/cli/test_interfaces_bonding.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/smoketest/scripts/cli/test_interfaces_bonding.py b/smoketest/scripts/cli/test_interfaces_bonding.py index a6e5ea59c..af70c004b 100755 --- a/smoketest/scripts/cli/test_interfaces_bonding.py +++ b/smoketest/scripts/cli/test_interfaces_bonding.py @@ -185,6 +185,26 @@ class BondingInterfaceTest(BasicInterfaceTest.TestCase): slaves = read_file(f'/sys/class/net/{bond}/bonding/slaves').split() self.assertIn(member, slaves) + def test_bonding_source_bridge_interface(self): + # Re-use member interface that is already a source-interface + bond = 'bond1097' + bridge = 'br6327' + member = next(iter(self._members)) + + self.cli_set(self._base_path + [bond, 'member', 'interface', member]) + self.cli_set(['interfaces', 'bridge', bridge, 'member', 'interface', member]) + + # check validate() - can not add interface to bond, it is a member of bridge ... + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + self.cli_delete(['interfaces', 'bridge', bridge]) + self.cli_commit() + + # verify config + slaves = read_file(f'/sys/class/net/{bond}/bonding/slaves').split() + self.assertIn(member, slaves) + def test_bonding_uniq_member_description(self): ethernet_path = ['interfaces', 'ethernet'] for interface in self._interfaces: -- cgit v1.2.3 From 84f2c0bf47315f1da7bff185621aa07f3540e440 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Sun, 10 Jul 2022 22:31:49 +0200 Subject: bond: T4521: ARP monitor interval is not configured despite set via CLI The code path for changing the interval is never executed. (cherry picked from commit daddb0ad7812843f64a7ae0bf4b5e15db7b1758e) --- python/vyos/ifconfig/bond.py | 57 +++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/python/vyos/ifconfig/bond.py b/python/vyos/ifconfig/bond.py index 27d0182e9..dbe089801 100644 --- a/python/vyos/ifconfig/bond.py +++ b/python/vyos/ifconfig/bond.py @@ -199,16 +199,7 @@ class BondIf(Interface): >>> from vyos.ifconfig import BondIf >>> BondIf('bond0').set_arp_interval('100') """ - if int(interval) == 0: - """ - Specifies the MII link monitoring frequency in milliseconds. - This determines how often the link state of each slave is - inspected for link failures. A value of zero disables MII - link monitoring. A value of 100 is a good starting point. - """ - return self.set_interface('bond_miimon', interval) - else: - return self.set_interface('bond_arp_interval', interval) + return self.set_interface('bond_arp_interval', interval) def get_arp_ip_target(self): """ @@ -365,26 +356,6 @@ class BondIf(Interface): if 'shutdown_required' in config: self.set_admin_state('down') - # ARP monitor targets need to be synchronized between sysfs and CLI. - # Unfortunately an address can't be send twice to sysfs as this will - # result in the following exception: OSError: [Errno 22] Invalid argument. - # - # We remove ALL addresses prior to adding new ones, this will remove - # addresses manually added by the user too - but as we are limited to 16 adresses - # from the kernel side this looks valid to me. We won't run into an error - # when a user added manual adresses which would result in having more - # then 16 adresses in total. - arp_tgt_addr = list(map(str, self.get_arp_ip_target().split())) - for addr in arp_tgt_addr: - self.set_arp_ip_target('-' + addr) - - # Add configured ARP target addresses - value = dict_search('arp_monitor.target', config) - if isinstance(value, str): - value = [value] - if value: - for addr in value: - self.set_arp_ip_target('+' + addr) # Bonding transmit hash policy value = config.get('hash_policy') @@ -414,6 +385,32 @@ class BondIf(Interface): if mode == '802.3ad': self.set_lacp_rate(config.get('lacp_rate')) + if mode not in ['802.3ad', 'balance-tlb', 'balance-alb']: + tmp = dict_search('arp_monitor.interval', config) + value = tmp if (tmp != None) else '0' + self.set_arp_interval(value) + + # ARP monitor targets need to be synchronized between sysfs and CLI. + # Unfortunately an address can't be send twice to sysfs as this will + # result in the following exception: OSError: [Errno 22] Invalid argument. + # + # We remove ALL addresses prior to adding new ones, this will remove + # addresses manually added by the user too - but as we are limited to 16 adresses + # from the kernel side this looks valid to me. We won't run into an error + # when a user added manual adresses which would result in having more + # then 16 adresses in total. + arp_tgt_addr = list(map(str, self.get_arp_ip_target().split())) + for addr in arp_tgt_addr: + self.set_arp_ip_target('-' + addr) + + # Add configured ARP target addresses + value = dict_search('arp_monitor.target', config) + if isinstance(value, str): + value = [value] + if value: + for addr in value: + self.set_arp_ip_target('+' + addr) + # Add (enslave) interfaces to bond value = dict_search('member.interface', config) for interface in (value or []): -- cgit v1.2.3 From 872423e103a756ec470846108143e28f70c4d254 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Sun, 10 Jul 2022 22:33:25 +0200 Subject: bond: T1557: re-add miimon configuration - lost in translation (cherry picked from commit cabfd006bed9cd2d1512cb313616a8e97fe29b9e) --- python/vyos/ifconfig/bond.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/python/vyos/ifconfig/bond.py b/python/vyos/ifconfig/bond.py index dbe089801..54e47f18a 100644 --- a/python/vyos/ifconfig/bond.py +++ b/python/vyos/ifconfig/bond.py @@ -176,6 +176,21 @@ class BondIf(Interface): """ self.set_interface('bond_lacp_rate', slow_fast) + def set_miimon_interval(self, interval): + """ + Specifies the MII link monitoring frequency in milliseconds. This + determines how often the link state of each slave is inspected for link + failures. A value of zero disables MII link monitoring. A value of 100 + is a good starting point. + + The default value is 0. + + Example: + >>> from vyos.ifconfig import BondIf + >>> BondIf('bond0').set_miimon_interval('100') + """ + return self.set_interface('bond_miimon', interval) + def set_arp_interval(self, interval): """ Specifies the ARP link monitoring frequency in milliseconds. @@ -356,6 +371,8 @@ class BondIf(Interface): if 'shutdown_required' in config: self.set_admin_state('down') + # Specifies the MII link monitoring frequency in milliseconds + self.set_miimon_interval('250') # Bonding transmit hash policy value = config.get('hash_policy') -- cgit v1.2.3 From 905fe01e1d9bf0930e0dd113f825777786daaa57 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Sun, 10 Jul 2022 22:44:03 +0200 Subject: vyos.configdict(): T4228: is_member() must use the "real" hardware interface When is_member() is inspecting the bridge/Bond member interfaces it must work with the real interface (e.g. eth1) under the "ethernet" node and not work on the "member interface eth1" CLI tree, that makes no sense at all. (cherry picked from commit 3915791216998a18bf6831450df68ee199e2e4f8) --- python/vyos/configdict.py | 14 +++++++------- src/conf_mode/interfaces-bonding.py | 12 +++++++----- src/conf_mode/interfaces-bridge.py | 2 +- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py index be10cbdfc..cc50c5167 100644 --- a/python/vyos/configdict.py +++ b/python/vyos/configdict.py @@ -203,11 +203,12 @@ def is_member(conf, interface, intftype=None): intftype is optional, if not passed it will search all known types (currently bridge and bonding) - Returns: - None -> Interface is not a member - interface name -> Interface is a member of this interface - False -> interface type cannot have members + Returns: dict + empty -> Interface is not a member + key -> Interface is a member of this interface """ + from vyos.ifconfig import Section + ret_val = {} intftypes = ['bonding', 'bridge'] @@ -227,9 +228,8 @@ def is_member(conf, interface, intftype=None): for intf in conf.list_nodes(base): member = base + [intf, 'member', 'interface', interface] if conf.exists(member): - tmp = conf.get_config_dict(member, key_mangling=('-', '_'), - get_first_key=True, no_tag_node_value_mangle=True) - ret_val.update({intf : tmp}) + if conf.exists(['interfaces', Section.section(interface), interface]): + ret_val.update({intf : {}}) old_level = conf.set_level(old_level) return ret_val diff --git a/src/conf_mode/interfaces-bonding.py b/src/conf_mode/interfaces-bonding.py index d5be21949..55ffb184d 100755 --- a/src/conf_mode/interfaces-bonding.py +++ b/src/conf_mode/interfaces-bonding.py @@ -108,20 +108,22 @@ def get_config(config=None): for interface, interface_config in bond['member']['interface'].items(): # Check if member interface is already member of another bridge tmp = is_member(conf, interface, 'bridge') - if tmp: interface_config.update({'is_bridge_member' : tmp}) + if tmp: bond['member']['interface'][interface].update({'is_bridge_member' : tmp}) # Check if member interface is already member of a bond tmp = is_member(conf, interface, 'bonding') - if tmp and bond['ifname'] not in tmp: - interface_config.update({'is_bond_member' : tmp}) + for tmp in is_member(conf, interface, 'bonding'): + if bond['ifname'] == tmp: + continue + bond['member']['interface'][interface].update({'is_bond_member' : tmp}) # Check if member interface is used as source-interface on another interface tmp = is_source_interface(conf, interface) - if tmp: interface_config.update({'is_source_interface' : tmp}) + if tmp: bond['member']['interface'][interface].update({'is_source_interface' : tmp}) # bond members must not have an assigned address tmp = has_address_configured(conf, interface) - if tmp: interface_config.update({'has_address' : ''}) + if tmp: bond['member']['interface'][interface].update({'has_address' : {}}) return bond diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py index f4dba9d4a..a0cfca0af 100755 --- a/src/conf_mode/interfaces-bridge.py +++ b/src/conf_mode/interfaces-bridge.py @@ -132,7 +132,7 @@ def verify(bridge): if 'enable_vlan' in bridge: if 'has_vlan' in interface_config: - raise ConfigError(error_msg + 'it has an VLAN subinterface assigned!') + raise ConfigError(error_msg + 'it has VLAN subinterface(s) assigned!') if 'wlan' in interface: raise ConfigError(error_msg + 'VLAN aware cannot be set!') -- cgit v1.2.3 From d5ed752207bbb016df40fb0fe18c27bb428c446c Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Sun, 10 Jul 2022 23:09:52 +0200 Subject: smoketest: bond: remove second instance of layer2+3 hash-policy test (cherry picked from commit 8d1bb953b784d03e02ba26e78da5488a79aaf20d) --- smoketest/scripts/cli/test_interfaces_bonding.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smoketest/scripts/cli/test_interfaces_bonding.py b/smoketest/scripts/cli/test_interfaces_bonding.py index af70c004b..c0ed916f3 100755 --- a/smoketest/scripts/cli/test_interfaces_bonding.py +++ b/smoketest/scripts/cli/test_interfaces_bonding.py @@ -136,7 +136,7 @@ class BondingInterfaceTest(BasicInterfaceTest.TestCase): def test_bonding_hash_policy(self): # Define available bonding hash policies - hash_policies = ['layer2', 'layer2+3', 'layer2+3', 'encap2+3', 'encap3+4'] + hash_policies = ['layer2', 'layer2+3', 'encap2+3', 'encap3+4'] for hash_policy in hash_policies: for interface in self._interfaces: for option in self._options.get(interface, []): -- cgit v1.2.3 From 57d54b249ff04f70177b31fae0c10ac6677148ee Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Sun, 10 Jul 2022 23:01:09 +0200 Subject: bond: T4522: add ability to specify mii monitor interval via CLI Linux Kernel supports to specify the MII link monitoring frequency in milliseconds. This determines how often the link state of each slave is inspected for link failures. A value of zero disables MII link monitoring. A value of 100 is a good starting point. The default value is 100. set interfaces bonding bond0 mii-mon-interval (cherry picked from commit 4315c8fa5bb090e2b7edd6bda205041623e2511d) --- interface-definitions/interfaces-bonding.xml.in | 17 +++++++++++++++++ python/vyos/ifconfig/bond.py | 3 ++- smoketest/scripts/cli/test_interfaces_bonding.py | 23 +++++++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/interface-definitions/interfaces-bonding.xml.in b/interface-definitions/interfaces-bonding.xml.in index 5a4f08bef..307dd7558 100644 --- a/interface-definitions/interfaces-bonding.xml.in +++ b/interface-definitions/interfaces-bonding.xml.in @@ -92,6 +92,23 @@ #include #include #include + + + Specifies the MII link monitoring frequency in milliseconds + + u32:0 + Disable MII link monitoring + + + u32:50-1000 + MII link monitoring frequency in milliseconds + + + + + + 100 + Minimum number of member interfaces required up before enabling bond diff --git a/python/vyos/ifconfig/bond.py b/python/vyos/ifconfig/bond.py index 54e47f18a..47e0598ab 100644 --- a/python/vyos/ifconfig/bond.py +++ b/python/vyos/ifconfig/bond.py @@ -372,7 +372,8 @@ class BondIf(Interface): self.set_admin_state('down') # Specifies the MII link monitoring frequency in milliseconds - self.set_miimon_interval('250') + value = config.get('mii_mon_interval') + self.set_miimon_interval(value) # Bonding transmit hash policy value = config.get('hash_policy') diff --git a/smoketest/scripts/cli/test_interfaces_bonding.py b/smoketest/scripts/cli/test_interfaces_bonding.py index c0ed916f3..ab75cdcc9 100755 --- a/smoketest/scripts/cli/test_interfaces_bonding.py +++ b/smoketest/scripts/cli/test_interfaces_bonding.py @@ -151,6 +151,29 @@ class BondingInterfaceTest(BasicInterfaceTest.TestCase): defined_policy = read_file(f'/sys/class/net/{interface}/bonding/xmit_hash_policy').split() self.assertEqual(defined_policy[0], hash_policy) + def test_bonding_mii_monitoring_interval(self): + for interface in self._interfaces: + for option in self._options.get(interface, []): + self.cli_set(self._base_path + [interface] + option.split()) + + self.cli_commit() + + # verify default + for interface in self._interfaces: + tmp = read_file(f'/sys/class/net/{interface}/bonding/miimon').split() + self.assertIn('100', tmp) + + mii_mon = '250' + for interface in self._interfaces: + self.cli_set(self._base_path + [interface, 'mii-mon-interval', mii_mon]) + + self.cli_commit() + + # verify new CLI value + for interface in self._interfaces: + tmp = read_file(f'/sys/class/net/{interface}/bonding/miimon').split() + self.assertIn(mii_mon, tmp) + def test_bonding_multi_use_member(self): # Define available bonding hash policies for interface in ['bond10', 'bond20']: -- cgit v1.2.3 From 3bcc6df1a917e027998052223ba7db5424bfd370 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Mon, 11 Jul 2022 07:58:13 +0200 Subject: vyos.configdict(): T4228: is_member() must split VLAN interfaces Commit 39157912 ("vyos.configdict(): T4228: is_member() must use the "real" hardware interface") added a bugfix on calling is_member() to retrieve the real physical information about an interface. It did not include a code path to also split up VLAN interfaces. This has been fixed. (cherry picked from commit fdeae251431cb747e8f60d96269b4365b7401807) --- python/vyos/configdict.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py index cc50c5167..37cf2e114 100644 --- a/python/vyos/configdict.py +++ b/python/vyos/configdict.py @@ -228,8 +228,18 @@ def is_member(conf, interface, intftype=None): for intf in conf.list_nodes(base): member = base + [intf, 'member', 'interface', interface] if conf.exists(member): - if conf.exists(['interfaces', Section.section(interface), interface]): - ret_val.update({intf : {}}) + member_type = Section.section(interface) + # Check if it's a VLAN (QinQ) interface + interface = interface.split('.') + if len(interface) == 3: + if conf.exists(['interfaces', member_type, interface[0], 'vif-s', interface[1], 'vif-c', interface[2]]): + ret_val.update({intf : {}}) + elif len(interface) == 2: + if conf.exists(['interfaces', member_type, interface[0], 'vif', interface[1]]): + ret_val.update({intf : {}}) + else: + if conf.exists(['interfaces', member_type, interface[0]]): + ret_val.update({intf : {}}) old_level = conf.set_level(old_level) return ret_val -- cgit v1.2.3 From 51455fc033cd41e1a105800b2a901e4a32df6058 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Mon, 11 Jul 2022 08:13:02 +0200 Subject: smoketest: bridge: also test QinQ bridge member interfaces (cherry picked from commit 74d6a7e4fc9e2e929c5f899070e6fc3e3e3b5ceb) --- smoketest/scripts/cli/test_interfaces_bridge.py | 31 ++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/smoketest/scripts/cli/test_interfaces_bridge.py b/smoketest/scripts/cli/test_interfaces_bridge.py index 09441326a..b4b91f226 100755 --- a/smoketest/scripts/cli/test_interfaces_bridge.py +++ b/smoketest/scripts/cli/test_interfaces_bridge.py @@ -224,7 +224,7 @@ class BridgeInterfaceTest(BasicInterfaceTest.TestCase): self.cli_delete(self._base_path + [interface, 'member']) - def test_bridge_vlan_members(self): + def test_bridge_vif_members(self): # T2945: ensure that VIFs are not dropped from bridge vifs = ['300', '400'] for interface in self._interfaces: @@ -249,5 +249,34 @@ class BridgeInterfaceTest(BasicInterfaceTest.TestCase): self.cli_delete(['interfaces', 'ethernet', member, 'vif', vif]) self.cli_delete(['interfaces', 'bridge', interface, 'member', 'interface', f'{member}.{vif}']) + def test_bridge_vif_s_vif_c_members(self): + # T2945: ensure that VIFs are not dropped from bridge + vifs = ['300', '400'] + vifc = ['301', '401'] + for interface in self._interfaces: + for member in self._members: + for vif_s in vifs: + for vif_c in vifc: + self.cli_set(['interfaces', 'ethernet', member, 'vif-s', vif_s, 'vif-c', vif_c]) + self.cli_set(['interfaces', 'bridge', interface, 'member', 'interface', f'{member}.{vif_s}.{vif_c}']) + + self.cli_commit() + + # Verify config + for interface in self._interfaces: + for member in self._members: + for vif_s in vifs: + for vif_c in vifc: + # member interface must be assigned to the bridge + self.assertTrue(os.path.exists(f'/sys/class/net/{interface}/lower_{member}.{vif_s}.{vif_c}')) + + # delete all members + for interface in self._interfaces: + for member in self._members: + for vif_s in vifs: + self.cli_delete(['interfaces', 'ethernet', member, 'vif-s', vif_s]) + for vif_c in vifc: + self.cli_delete(['interfaces', 'bridge', interface, 'member', 'interface', f'{member}.{vif_s}.{vif_c}']) + if __name__ == '__main__': unittest.main(verbosity=2) -- cgit v1.2.3 From 3a1c690e22e397bbdecf47364c3f885df7a8e8ef Mon Sep 17 00:00:00 2001 From: Viacheslav Hletenko Date: Tue, 12 Jul 2022 08:39:36 +0000 Subject: vrf: T4527: Prevent to create VRF with reserved names VRF names: "add, all, broadcast, default, delete, dev, get, inet, mtu, link, type, vrf" are reserved and cannot be used for vrf name (cherry picked from commit 52342f389af2da2995b858d026e6fbcad5c8bfaa) --- src/conf_mode/vrf.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/conf_mode/vrf.py b/src/conf_mode/vrf.py index fb2182fff..def4cc70d 100755 --- a/src/conf_mode/vrf.py +++ b/src/conf_mode/vrf.py @@ -108,8 +108,14 @@ def verify(vrf): f'static routes installed!') if 'name' in vrf: + reserved_names = ["add", "all", "broadcast", "default", "delete", "dev", "get", "inet", "mtu", "link", "type", + "vrf"] table_ids = [] for name, config in vrf['name'].items(): + # Reserved VRF names + if name in reserved_names: + raise ConfigError(f'VRF name "{name}" is reserved and connot be used!') + # table id is mandatory if 'table' not in config: raise ConfigError(f'VRF "{name}" table id is mandatory!') -- cgit v1.2.3 From 752ddaff0a806527ab6cc787b4ab2a60fec01886 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 14 Jul 2022 20:13:53 +0200 Subject: bond: bridge: T4534: error out if member interface is assigned to a VRF instance It makes no sense to enslave an interface to a bond or a bridge device if it is bound to a given VRF. If VRFs should be used - the encapuslating/master interface should be part of the VRF. Error out if the member interface is part of a VRF. (cherry picked from commit 87d2dff241d9ab4de9f3a2c7fbf9852934557aef) --- python/vyos/validate.py | 19 +++++++++++++++++++ src/conf_mode/interfaces-bonding.py | 7 +++++++ src/conf_mode/interfaces-bridge.py | 8 ++++++++ 3 files changed, 34 insertions(+) diff --git a/python/vyos/validate.py b/python/vyos/validate.py index 23e88b5ac..9aa23d3dc 100644 --- a/python/vyos/validate.py +++ b/python/vyos/validate.py @@ -260,3 +260,22 @@ def has_address_configured(conf, intf): conf.set_level(old_level) return ret + +def has_vrf_configured(conf, intf): + """ + Checks if interface has a VRF configured. + + Returns True if interface has VRF configured, False if it doesn't. + """ + from vyos.ifconfig import Section + ret = False + + old_level = conf.get_level() + conf.set_level([]) + + tmp = ['interfaces', Section.get_config_path(intf), 'vrf'] + if conf.exists(tmp): + ret = True + + conf.set_level(old_level) + return ret diff --git a/src/conf_mode/interfaces-bonding.py b/src/conf_mode/interfaces-bonding.py index 55ffb184d..06e6f059a 100755 --- a/src/conf_mode/interfaces-bonding.py +++ b/src/conf_mode/interfaces-bonding.py @@ -35,6 +35,7 @@ from vyos.ifconfig import BondIf from vyos.ifconfig import Section from vyos.util import dict_search from vyos.validate import has_address_configured +from vyos.validate import has_vrf_configured from vyos import ConfigError from vyos import airbag airbag.enable() @@ -125,6 +126,10 @@ def get_config(config=None): tmp = has_address_configured(conf, interface) if tmp: bond['member']['interface'][interface].update({'has_address' : {}}) + # bond members must not have a VRF attached + tmp = has_vrf_configured(conf, interface) + if tmp: bond['member']['interface'][interface].update({'has_vrf' : {}}) + return bond @@ -181,6 +186,8 @@ def verify(bond): if 'has_address' in interface_config: raise ConfigError(error_msg + 'it has an address assigned!') + if 'has_vrf' in interface_config: + raise ConfigError(error_msg + 'it has a VRF assigned!') if 'primary' in bond: if bond['primary'] not in bond['member']['interface']: diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py index a0cfca0af..9ad39e080 100755 --- a/src/conf_mode/interfaces-bridge.py +++ b/src/conf_mode/interfaces-bridge.py @@ -30,6 +30,7 @@ from vyos.configverify import verify_dhcpv6 from vyos.configverify import verify_vrf from vyos.ifconfig import BridgeIf from vyos.validate import has_address_configured +from vyos.validate import has_vrf_configured from vyos.xml import defaults from vyos.util import cmd @@ -92,6 +93,10 @@ def get_config(config=None): tmp = has_address_configured(conf, interface) if tmp: bridge['member']['interface'][interface].update({'has_address' : ''}) + # Bridge members must not have a VRF attached + tmp = has_vrf_configured(conf, interface) + if tmp: bridge['member']['interface'][interface].update({'has_vrf' : ''}) + # VLAN-aware bridge members must not have VLAN interface configuration tmp = has_vlan_subinterface_configured(conf,interface) if 'enable_vlan' in bridge and tmp: @@ -130,6 +135,9 @@ def verify(bridge): if 'has_address' in interface_config: raise ConfigError(error_msg + 'it has an address assigned!') + if 'has_vrf' in interface_config: + raise ConfigError(error_msg + 'it has a VRF assigned!') + if 'enable_vlan' in bridge: if 'has_vlan' in interface_config: raise ConfigError(error_msg + 'it has VLAN subinterface(s) assigned!') -- cgit v1.2.3 From 37416e6503998c8d9ef3d20e6b88e92a49bc1115 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 14 Jul 2022 20:14:53 +0200 Subject: vyos.configdict(): T4228: is_member() must return member interface config dict This extends commit 39157912 ("vyos.configdict(): T4228: is_member() must use the "real" hardware interface") and returns the config dict of the used member interfaces. (cherry picked from commit 5b4f76429989a6ab8ca64aeed5a1fae09fe7c6ca) --- python/vyos/configdict.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py index 37cf2e114..70e5cd889 100644 --- a/python/vyos/configdict.py +++ b/python/vyos/configdict.py @@ -233,13 +233,19 @@ def is_member(conf, interface, intftype=None): interface = interface.split('.') if len(interface) == 3: if conf.exists(['interfaces', member_type, interface[0], 'vif-s', interface[1], 'vif-c', interface[2]]): - ret_val.update({intf : {}}) + tmp = conf.get_config_dict(['interfaces', member_type, interface[0]], + key_mangling=('-', '_'), get_first_key=True) + ret_val.update({intf : tmp}) elif len(interface) == 2: if conf.exists(['interfaces', member_type, interface[0], 'vif', interface[1]]): - ret_val.update({intf : {}}) + tmp = conf.get_config_dict(['interfaces', member_type, interface[0]], + key_mangling=('-', '_'), get_first_key=True) + ret_val.update({intf : tmp}) else: if conf.exists(['interfaces', member_type, interface[0]]): - ret_val.update({intf : {}}) + tmp = conf.get_config_dict(['interfaces', member_type, interface[0]], + key_mangling=('-', '_'), get_first_key=True) + ret_val.update({intf : tmp}) old_level = conf.set_level(old_level) return ret_val -- cgit v1.2.3 From 8bbde65979519d38712b1bd55cf50042513546bf Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 14 Jul 2022 21:46:55 +0200 Subject: bond: T4525: fix adding member interface to bond after removing VRF When removing a VRF from an ethernet interface and adding the interface to a bond in the same commit led to an OSError: [Errno 16] Device or resource busy! (cherry picked from commit 3592f56a8deb6c44dcdd7a44ef54fc2c39eb1a3b) --- python/vyos/ifconfig/interface.py | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index 9eed3acb9..9b08d83de 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -1304,14 +1304,22 @@ class Interface(Control): if dhcpv6pd: self.set_dhcpv6(True) - # There are some items in the configuration which can only be applied - # if this instance is not bound to a bridge. This should be checked - # by the caller but better save then sorry! - if not any(k in ['is_bond_member', 'is_bridge_member'] for k in config): - # Bind interface to given VRF or unbind it if vrf node is not set. - # unbinding will call 'ip link set dev eth0 nomaster' which will - # also drop the interface out of a bridge or bond - thus this is - # checked before + # XXX: Bind interface to given VRF or unbind it if vrf is not set. Unbinding + # will call 'ip link set dev eth0 nomaster' which will also drop the + # interface out of any bridge or bond - thus this is checked before. + if 'is_bond_member' in config: + bond_if = next(iter(config['is_bond_member'])) + tmp = get_interface_config(config['ifname']) + if 'master' in tmp and tmp['master'] != bond_if: + self.set_vrf('') + + elif 'is_bridge_member' in config: + bridge_if = next(iter(config['is_bridge_member'])) + tmp = get_interface_config(config['ifname']) + if 'master' in tmp and tmp['master'] != bridge_if: + self.set_vrf('') + + else: self.set_vrf(config.get('vrf', '')) # Add this section after vrf T4331 @@ -1412,8 +1420,8 @@ class Interface(Control): # re-add ourselves to any bridge we might have fallen out of if 'is_bridge_member' in config: - bridge_dict = config.get('is_bridge_member') - self.add_to_bridge(bridge_dict) + tmp = config.get('is_bridge_member') + self.add_to_bridge(tmp) # configure port mirror self.set_mirror() -- cgit v1.2.3 From 6f53eb48d5c9d79354d4349c042eec02b8051560 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 14 Jul 2022 21:48:23 +0200 Subject: interfaces: T4525: interfaces can not be member of a bridge/bond and a VRF (cherry picked from commit 81e0f4a8dece85da7169ba05448e870206aaf57b) --- python/vyos/configverify.py | 12 ++++++++++++ src/conf_mode/interfaces-ethernet.py | 2 ++ src/conf_mode/interfaces-geneve.py | 2 ++ src/conf_mode/interfaces-l2tpv3.py | 2 ++ src/conf_mode/interfaces-macsec.py | 2 ++ src/conf_mode/interfaces-openvpn.py | 2 ++ src/conf_mode/interfaces-tunnel.py | 2 ++ src/conf_mode/interfaces-vxlan.py | 3 +++ src/conf_mode/interfaces-wireguard.py | 2 ++ src/conf_mode/interfaces-wireless.py | 2 ++ 10 files changed, 31 insertions(+) diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py index 6566e8863..44342f289 100644 --- a/python/vyos/configverify.py +++ b/python/vyos/configverify.py @@ -84,6 +84,18 @@ def verify_mtu_ipv6(config): tmp = dict_search('ipv6.address.eui64', config) if tmp != None: raise ConfigError(error_msg) +def verify_bond_bridge_member(config): + """ + Checks if interface has a VRF configured and is also part of a bond or + bridge, which is not allowed! + """ + if 'vrf' in config: + ifname = config['ifname'] + if 'is_bond_member' in config: + raise ConfigError(f'Can not add interface "{ifname}" to bond, it has a VRF assigned!') + if 'is_bridge_member' in config: + raise ConfigError(f'Can not add interface "{ifname}" to bridge, it has a VRF assigned!') + def verify_tunnel(config): """ This helper is used to verify the common part of the tunnel diff --git a/src/conf_mode/interfaces-ethernet.py b/src/conf_mode/interfaces-ethernet.py index de851262b..e8236bf89 100755 --- a/src/conf_mode/interfaces-ethernet.py +++ b/src/conf_mode/interfaces-ethernet.py @@ -30,6 +30,7 @@ from vyos.configverify import verify_mtu from vyos.configverify import verify_mtu_ipv6 from vyos.configverify import verify_vlan_config from vyos.configverify import verify_vrf +from vyos.configverify import verify_bond_bridge_member from vyos.ethtool import Ethtool from vyos.ifconfig import EthernetIf from vyos.template import render @@ -67,6 +68,7 @@ def verify(ethernet): verify_dhcpv6(ethernet) verify_address(ethernet) verify_vrf(ethernet) + verify_bond_bridge_member(ethernet) verify_eapol(ethernet) verify_mirror(ethernet) diff --git a/src/conf_mode/interfaces-geneve.py b/src/conf_mode/interfaces-geneve.py index f49d5b304..a5213fbce 100755 --- a/src/conf_mode/interfaces-geneve.py +++ b/src/conf_mode/interfaces-geneve.py @@ -25,6 +25,7 @@ from vyos.configdict import leaf_node_changed from vyos.configverify import verify_address from vyos.configverify import verify_mtu_ipv6 from vyos.configverify import verify_bridge_delete +from vyos.configverify import verify_bond_bridge_member from vyos.ifconfig import GeneveIf from vyos import ConfigError @@ -59,6 +60,7 @@ def verify(geneve): verify_mtu_ipv6(geneve) verify_address(geneve) + verify_bond_bridge_member(geneve) if 'remote' not in geneve: raise ConfigError('Remote side must be configured') diff --git a/src/conf_mode/interfaces-l2tpv3.py b/src/conf_mode/interfaces-l2tpv3.py index 9b6ddd5aa..c1ed8ec59 100755 --- a/src/conf_mode/interfaces-l2tpv3.py +++ b/src/conf_mode/interfaces-l2tpv3.py @@ -25,6 +25,7 @@ from vyos.configdict import leaf_node_changed from vyos.configverify import verify_address from vyos.configverify import verify_bridge_delete from vyos.configverify import verify_mtu_ipv6 +from vyos.configverify import verify_bond_bridge_member from vyos.ifconfig import L2TPv3If from vyos.util import check_kmod from vyos.validate import is_addr_assigned @@ -76,6 +77,7 @@ def verify(l2tpv3): verify_mtu_ipv6(l2tpv3) verify_address(l2tpv3) + verify_bond_bridge_member(l2tpv3) return None def generate(l2tpv3): diff --git a/src/conf_mode/interfaces-macsec.py b/src/conf_mode/interfaces-macsec.py index bfebed7e4..6ec34a961 100755 --- a/src/conf_mode/interfaces-macsec.py +++ b/src/conf_mode/interfaces-macsec.py @@ -30,6 +30,7 @@ from vyos.configverify import verify_address from vyos.configverify import verify_bridge_delete from vyos.configverify import verify_mtu_ipv6 from vyos.configverify import verify_source_interface +from vyos.configverify import verify_bond_bridge_member from vyos import ConfigError from vyos import airbag airbag.enable() @@ -66,6 +67,7 @@ def verify(macsec): verify_vrf(macsec) verify_mtu_ipv6(macsec) verify_address(macsec) + verify_bond_bridge_member(macsec) if not (('security' in macsec) and ('cipher' in macsec['security'])): diff --git a/src/conf_mode/interfaces-openvpn.py b/src/conf_mode/interfaces-openvpn.py index f7edddcbf..7d20b3fd0 100755 --- a/src/conf_mode/interfaces-openvpn.py +++ b/src/conf_mode/interfaces-openvpn.py @@ -33,6 +33,7 @@ from vyos.configdict import is_node_changed from vyos.configverify import verify_vrf from vyos.configverify import verify_bridge_delete from vyos.configverify import verify_diffie_hellman_length +from vyos.configverify import verify_bond_bridge_member from vyos.ifconfig import VTunIf from vyos.template import render from vyos.template import is_ipv4 @@ -425,6 +426,7 @@ def verify(openvpn): raise ConfigError('Username for authentication is missing') verify_vrf(openvpn) + verify_bond_bridge_member(openvpn) return None diff --git a/src/conf_mode/interfaces-tunnel.py b/src/conf_mode/interfaces-tunnel.py index c4023586a..3bfc7d665 100755 --- a/src/conf_mode/interfaces-tunnel.py +++ b/src/conf_mode/interfaces-tunnel.py @@ -28,6 +28,7 @@ from vyos.configverify import verify_interface_exists from vyos.configverify import verify_mtu_ipv6 from vyos.configverify import verify_vrf from vyos.configverify import verify_tunnel +from vyos.configverify import verify_bond_bridge_member from vyos.ifconfig import Interface from vyos.ifconfig import Section from vyos.ifconfig import TunnelIf @@ -137,6 +138,7 @@ def verify(tunnel): verify_mtu_ipv6(tunnel) verify_address(tunnel) verify_vrf(tunnel) + verify_bond_bridge_member(tunnel) if 'source_interface' in tunnel: verify_interface_exists(tunnel['source_interface']) diff --git a/src/conf_mode/interfaces-vxlan.py b/src/conf_mode/interfaces-vxlan.py index 5ee4603af..bdf47eba0 100755 --- a/src/conf_mode/interfaces-vxlan.py +++ b/src/conf_mode/interfaces-vxlan.py @@ -26,6 +26,7 @@ from vyos.configverify import verify_address from vyos.configverify import verify_bridge_delete from vyos.configverify import verify_mtu_ipv6 from vyos.configverify import verify_source_interface +from vyos.configverify import verify_bond_bridge_member from vyos.ifconfig import Interface from vyos.ifconfig import VXLANIf from vyos.template import is_ipv6 @@ -113,6 +114,8 @@ def verify(vxlan): verify_mtu_ipv6(vxlan) verify_address(vxlan) + verify_bond_bridge_member(vxlan) + return None def generate(vxlan): diff --git a/src/conf_mode/interfaces-wireguard.py b/src/conf_mode/interfaces-wireguard.py index 024ab8f59..34e80cca3 100755 --- a/src/conf_mode/interfaces-wireguard.py +++ b/src/conf_mode/interfaces-wireguard.py @@ -28,6 +28,7 @@ from vyos.configverify import verify_vrf from vyos.configverify import verify_address from vyos.configverify import verify_bridge_delete from vyos.configverify import verify_mtu_ipv6 +from vyos.configverify import verify_bond_bridge_member from vyos.ifconfig import WireGuardIf from vyos.util import check_kmod from vyos import ConfigError @@ -69,6 +70,7 @@ def verify(wireguard): verify_mtu_ipv6(wireguard) verify_address(wireguard) verify_vrf(wireguard) + verify_bond_bridge_member(wireguard) if not os.path.exists(wireguard['private_key']): raise ConfigError('Wireguard private-key not found! Execute: ' \ diff --git a/src/conf_mode/interfaces-wireless.py b/src/conf_mode/interfaces-wireless.py index b25fcd4e0..7e967ed79 100755 --- a/src/conf_mode/interfaces-wireless.py +++ b/src/conf_mode/interfaces-wireless.py @@ -29,6 +29,7 @@ from vyos.configverify import verify_dhcpv6 from vyos.configverify import verify_source_interface from vyos.configverify import verify_vlan_config from vyos.configverify import verify_vrf +from vyos.configverify import verify_bond_bridge_member from vyos.ifconfig import WiFiIf from vyos.template import render from vyos.util import call @@ -188,6 +189,7 @@ def verify(wifi): verify_address(wifi) verify_vrf(wifi) + verify_bond_bridge_member(wifi) # use common function to verify VLAN configuration verify_vlan_config(wifi) -- cgit v1.2.3