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(-) (limited to 'python/vyos/ifconfig') 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(+) (limited to 'python/vyos/ifconfig') 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 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(-) (limited to 'python/vyos/ifconfig') 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 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(-) (limited to 'python/vyos/ifconfig') 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