From 05099eab245c265474f10ffb206ab23c494c8b88 Mon Sep 17 00:00:00 2001 From: fett0 Date: Sat, 4 May 2024 17:07:26 +0000 Subject: bond: T6303: add system mac address on bond (cherry picked from commit 234f35d8bae71b5d33ad97cdabc236ec6b13c3a2) --- interface-definitions/interfaces_bonding.xml.in | 12 +++++++++++ python/vyos/ifconfig/bond.py | 27 ++++++++++++++++++++++++ smoketest/scripts/cli/test_interfaces_bonding.py | 16 ++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/interface-definitions/interfaces_bonding.xml.in b/interface-definitions/interfaces_bonding.xml.in index 92c0911db..e6baed590 100644 --- a/interface-definitions/interfaces_bonding.xml.in +++ b/interface-definitions/interfaces_bonding.xml.in @@ -176,6 +176,18 @@ 0 + + + System MAC address for 802.3ad + + macaddr + MAC address + + + + + + Rate in which we will ask our link partner to transmit LACPDU packets diff --git a/python/vyos/ifconfig/bond.py b/python/vyos/ifconfig/bond.py index c6d0f1cff..b381ba0e1 100644 --- a/python/vyos/ifconfig/bond.py +++ b/python/vyos/ifconfig/bond.py @@ -18,6 +18,7 @@ import os from vyos.ifconfig.interface import Interface from vyos.utils.dict import dict_search from vyos.utils.assertion import assert_list +from vyos.utils.assertion import assert_mac from vyos.utils.assertion import assert_positive @Interface.register @@ -54,6 +55,10 @@ class BondIf(Interface): 'validate': lambda v: assert_list(v, ['slow', 'fast']), 'location': '/sys/class/net/{ifname}/bonding/lacp_rate', }, + 'bond_system_mac': { + 'validate': assert_mac, + 'location': '/sys/class/net/{ifname}/bonding/ad_actor_system', + }, 'bond_miimon': { 'validate': assert_positive, 'location': '/sys/class/net/{ifname}/bonding/miimon' @@ -385,6 +390,24 @@ class BondIf(Interface): """ return self.set_interface('bond_mode', mode) + def set_system_mac(self, mac): + """ + In an AD system, this specifies the mac-address for the actor in + protocol packet exchanges (LACPDUs). The value cannot be NULL or + multicast. It is preferred to have the local-admin bit set for this + mac but driver does not enforce it. If the value is not given then + system defaults to using the masters' mac address as actors' system + address. + + This parameter has effect only in 802.3ad mode and is available through + SysFs interface. + + Example: + >>> from vyos.ifconfig import BondIf + >>> BondIf('bond0').set_system_mac('00:50:ab:cd:ef:01') + """ + return self.set_interface('bond_system_mac', mac) + def update(self, config): """ General helper function which works on a dictionary retrived by get_config_dict(). It's main intention is to consolidate the scattered @@ -433,6 +456,10 @@ class BondIf(Interface): if mode == '802.3ad': self.set_lacp_rate(config.get('lacp_rate')) + # Add system mac address for 802.3ad + if mode == '802.3ad' and 'system_mac' in config: + self.set_system_mac(config.get('system_mac')) + if mode not in ['802.3ad', 'balance-tlb', 'balance-alb']: tmp = dict_search('arp_monitor.interval', config) value = tmp if (tmp != None) else '0' diff --git a/smoketest/scripts/cli/test_interfaces_bonding.py b/smoketest/scripts/cli/test_interfaces_bonding.py index 419de774a..091a7a3c5 100755 --- a/smoketest/scripts/cli/test_interfaces_bonding.py +++ b/smoketest/scripts/cli/test_interfaces_bonding.py @@ -241,6 +241,22 @@ class BondingInterfaceTest(BasicInterfaceTest.TestCase): for member in self._members: self.assertIn(member, slaves) + def test_bonding_system_mac(self): + # configure member interfaces and system-mac + system_mac = '00:50:ab:cd:ef:11' + for interface in self._interfaces: + for option in self._options.get(interface, []): + self.cli_set(self._base_path + [interface] + option.split()) + + self.cli_set(self._base_path + [interface, 'system-mac', system_mac]) + + self.cli_commit() + + # verify config + for interface in self._interfaces: + defined_mac = read_file(f'/sys/class/net/{interface}/bonding/ad_actor_system') + self.assertIn(defined_mac, system_mac) + def test_bonding_evpn_multihoming(self): id = '5' for interface in self._interfaces: -- cgit v1.2.3 From 82552e2abc77640b3a81560e3f8c5be2c21e3ad2 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Fri, 10 May 2024 15:15:53 +0200 Subject: bond: T6303: system-mac is not allowed to be a multicast MAC address (cherry picked from commit d8ddd7191d3004e886fa45a2cf9bd8dd5e7f5e14) --- python/vyos/ifconfig/bond.py | 2 +- python/vyos/utils/assertion.py | 4 ++-- src/conf_mode/interfaces_bonding.py | 11 +++++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/python/vyos/ifconfig/bond.py b/python/vyos/ifconfig/bond.py index b381ba0e1..f26426915 100644 --- a/python/vyos/ifconfig/bond.py +++ b/python/vyos/ifconfig/bond.py @@ -56,7 +56,7 @@ class BondIf(Interface): 'location': '/sys/class/net/{ifname}/bonding/lacp_rate', }, 'bond_system_mac': { - 'validate': assert_mac, + 'validate': lambda v: assert_mac(v, test_all_zero=False), 'location': '/sys/class/net/{ifname}/bonding/ad_actor_system', }, 'bond_miimon': { diff --git a/python/vyos/utils/assertion.py b/python/vyos/utils/assertion.py index 1aaa54dff..c7fa220c3 100644 --- a/python/vyos/utils/assertion.py +++ b/python/vyos/utils/assertion.py @@ -53,7 +53,7 @@ def assert_mtu(mtu, ifname): if (max_mtu and cur_mtu > max_mtu) or cur_mtu > 65536: raise ValueError(f'MTU is too small for interface "{ifname}": {mtu} > {max_mtu}') -def assert_mac(m): +def assert_mac(m, test_all_zero=True): split = m.split(':') size = len(split) @@ -74,7 +74,7 @@ def assert_mac(m): raise ValueError(f'{m} is a multicast MAC address') # overall mac address is not allowed to be 00:00:00:00:00:00 - if sum(octets) == 0: + if test_all_zero and sum(octets) == 0: raise ValueError('00:00:00:00:00:00 is not a valid MAC address') if octets[:5] == (0, 0, 94, 0, 1): diff --git a/src/conf_mode/interfaces_bonding.py b/src/conf_mode/interfaces_bonding.py index 371b219c0..5e5d5fba1 100755 --- a/src/conf_mode/interfaces_bonding.py +++ b/src/conf_mode/interfaces_bonding.py @@ -33,6 +33,7 @@ from vyos.ifconfig import BondIf from vyos.ifconfig.ethernet import EthernetIf from vyos.ifconfig import Section from vyos.template import render_to_string +from vyos.utils.assertion import assert_mac from vyos.utils.dict import dict_search from vyos.utils.dict import dict_to_paths_values from vyos.utils.network import interface_exists @@ -244,6 +245,16 @@ def verify(bond): raise ConfigError('primary interface only works for mode active-backup, ' \ 'transmit-load-balance or adaptive-load-balance') + if 'system_mac' in bond: + if bond['mode'] != '802.3ad': + raise ConfigError('Actor MAC address only available in 802.3ad mode!') + + system_mac = bond['system_mac'] + try: + assert_mac(system_mac, test_all_zero=False) + except: + raise ConfigError(f'Cannot use a multicast MAC address "{system_mac}" as system-mac!') + return None def generate(bond): -- cgit v1.2.3 From bced1b4ef04f9e1a12c083c08839e4c1a54e2549 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Fri, 10 May 2024 15:17:22 +0200 Subject: bond: T6303: must reset system-mac to 00:00:00:00:00:00 on deletion (cherry picked from commit 314901e7b45782fb6266b35b0e788ab7ea1404b8) --- python/vyos/ifconfig/bond.py | 19 +++++++++++-------- smoketest/scripts/cli/test_interfaces_bonding.py | 16 ++++++++++++++-- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/python/vyos/ifconfig/bond.py b/python/vyos/ifconfig/bond.py index f26426915..b8ea90049 100644 --- a/python/vyos/ifconfig/bond.py +++ b/python/vyos/ifconfig/bond.py @@ -449,18 +449,13 @@ class BondIf(Interface): Interface(interface).set_admin_state('up') # Bonding policy/mode - default value, always present - mode = config.get('mode') - self.set_mode(mode) + self.set_mode(config['mode']) # LACPDU transmission rate - default value - if mode == '802.3ad': + if config['mode'] == '802.3ad': self.set_lacp_rate(config.get('lacp_rate')) - # Add system mac address for 802.3ad - if mode == '802.3ad' and 'system_mac' in config: - self.set_system_mac(config.get('system_mac')) - - if mode not in ['802.3ad', 'balance-tlb', 'balance-alb']: + if config['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) @@ -495,6 +490,14 @@ class BondIf(Interface): Interface(interface).flush_addrs() self.add_port(interface) + # Add system mac address for 802.3ad - default address is all zero + # mode is always present (defaultValue) + if config['mode'] == '802.3ad': + mac = '00:00:00:00:00:00' + if 'system_mac' in config: + mac = config['system_mac'] + self.set_system_mac(mac) + # Primary device interface - must be set after 'mode' value = config.get('primary') if value: self.set_primary(value) diff --git a/smoketest/scripts/cli/test_interfaces_bonding.py b/smoketest/scripts/cli/test_interfaces_bonding.py index 091a7a3c5..f436424b8 100755 --- a/smoketest/scripts/cli/test_interfaces_bonding.py +++ b/smoketest/scripts/cli/test_interfaces_bonding.py @@ -243,7 +243,9 @@ class BondingInterfaceTest(BasicInterfaceTest.TestCase): def test_bonding_system_mac(self): # configure member interfaces and system-mac + default_system_mac = '00:00:00:00:00:00' # default MAC is all zeroes system_mac = '00:50:ab:cd:ef:11' + for interface in self._interfaces: for option in self._options.get(interface, []): self.cli_set(self._base_path + [interface] + option.split()) @@ -254,8 +256,18 @@ class BondingInterfaceTest(BasicInterfaceTest.TestCase): # verify config for interface in self._interfaces: - defined_mac = read_file(f'/sys/class/net/{interface}/bonding/ad_actor_system') - self.assertIn(defined_mac, system_mac) + tmp = read_file(f'/sys/class/net/{interface}/bonding/ad_actor_system') + self.assertIn(tmp, system_mac) + + for interface in self._interfaces: + self.cli_delete(self._base_path + [interface, 'system-mac']) + + self.cli_commit() + + # verify default value + for interface in self._interfaces: + tmp = read_file(f'/sys/class/net/{interface}/bonding/ad_actor_system') + self.assertIn(tmp, default_system_mac) def test_bonding_evpn_multihoming(self): id = '5' -- cgit v1.2.3