From 9d24bffcc47b48396c580a48ec9405411e955342 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Wed, 7 Oct 2026 19:56:41 +0200 Subject: ethernet: T3871: an ethernet interface belongs to the Kernel and can not be deleted Every NIC used to carry a hw-id, which was enough to make its node exist. Without it a bare node is empty and never reaches the running configuration, so releasing a bond member looked exactly like a deletion: the port was torn down and left admin-down, with nothing left to bring it back. An ethernet interface is owned by the Kernel and - like 'lo' - outlives its configuration. Removing it now resets the port to the Kernel defaults and leaves the link up. --- smoketest/scripts/cli/test_interfaces_bonding.py | 42 ++++++++++++++++++++++++ 1 file changed, 42 insertions(+) (limited to 'smoketest/scripts/cli/test_interfaces_bonding.py') diff --git a/smoketest/scripts/cli/test_interfaces_bonding.py b/smoketest/scripts/cli/test_interfaces_bonding.py index 3a3b1e5cf..d57c1c276 100755 --- a/smoketest/scripts/cli/test_interfaces_bonding.py +++ b/smoketest/scripts/cli/test_interfaces_bonding.py @@ -24,6 +24,7 @@ from vyos.ifconfig import Section from vyos.ifconfig.interface import Interface from vyos.configsession import ConfigSessionError from vyos.utils.network import get_interface_config +from vyos.utils.network import interface_exists from vyos.utils.file import read_file class BondingInterfaceTest(BasicInterfaceTest.TestCase): @@ -134,6 +135,47 @@ class BondingInterfaceTest(BasicInterfaceTest.TestCase): state = Interface(remove_member).get_admin_state() self.assertEqual('up', state) + def test_bonding_bare_member_node_released(self): + # T3871: a member whose ethernet node exists but carries nothing at + # all - a plain "set interfaces ethernet eth1" and no more - is still + # a configured interface, not an empty one. When it is released from + # the bond it must be brought back up, not torn down as though the + # operator had deleted it. + member = self._members[0] + self.cli_set(['interfaces', 'ethernet', member]) + + for interface in self._interfaces: + for option in self._options.get(interface, []): + self.cli_set(self._base_path + [interface] + option.split()) + self.cli_commit() + + for interface in self._interfaces: + self.cli_delete( + self._base_path + [interface, 'member', 'interface', member]) + self.cli_commit() + + self.assertEqual(Interface(member).get_admin_state(), 'up') + + def test_bonding_delete_and_readd_member(self): + # T3871: deleting a member's whole ethernet node while it is enslaved, + # then re-adding it, must leave the port present and usable - the + # hardware never went anywhere. + member = self._members[0] + + for interface in self._interfaces: + for option in self._options.get(interface, []): + self.cli_set(self._base_path + [interface] + option.split()) + self.cli_commit() + + self.cli_delete(['interfaces', 'ethernet', member]) + self.cli_commit() + self.assertTrue(interface_exists(member), + f'{member} disappeared after its configuration was deleted') + + self.cli_set(['interfaces', 'ethernet', member, 'description', 'readded']) + self.cli_commit() + self.assertEqual(read_file(f'/sys/class/net/{member}/ifalias'), 'readded') + def test_bonding_append_member(self): # T9269: appending a member to a bond that already has one must not # disturb the existing member. leaf_node_changed() used to report the -- cgit v1.2.3 From 32f664077d1012c42e0544ccb5a4e1456f783998 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Wed, 7 Oct 2026 19:56:53 +0200 Subject: ethernet: T3871: leave a bond member's MTU alone while it is still enslaved Deleting the ethernet configuration of a member of a jumbo bond dropped that member to the default MTU while the bond went on using it at 9000. It stayed the active slave, so jumbo frames were discarded in both directions, small ones passed, and nothing was logged. A bond owns its members' MTU just as it owns their address - the ethernet path already refuses to let a member set either, and says so. The reset now waits, like the address, until the bond has released the port. The bonding smoketest covers both halves on a running system: while the member is enslaved it must keep the bond's address and MTU, and once the bond releases it, it must get its own address and the default MTU back. --- python/vyos/ifconfig/ethernet.py | 15 +++---- smoketest/scripts/cli/test_interfaces_bonding.py | 50 +++++++++++++++++++++++- 2 files changed, 55 insertions(+), 10 deletions(-) (limited to 'smoketest/scripts/cli/test_interfaces_bonding.py') diff --git a/python/vyos/ifconfig/ethernet.py b/python/vyos/ifconfig/ethernet.py index 8360c4654..2f53b5e06 100644 --- a/python/vyos/ifconfig/ethernet.py +++ b/python/vyos/ifconfig/ethernet.py @@ -156,21 +156,18 @@ class EthernetIf(Interface): if not self.exists(self.ifname): return - # undo what the configuration changed about the hardware itself. - # - # While the port is still enslaved its address belongs to the bond - - # most modes need every member to carry the bond's one, and set_mac() - # would bounce the link underneath it. BondIf.update() releases the - # member and then runs this cleanup again, which is where the address - # is put back. + # undo what the configuration changed about the hardware itself, but + # not while the port is still enslaved: a bond owns its members' + # address and MTU, so resetting them here would leave the member out + # of step with the bond it is still in. Releasing it runs this again. tmp = get_interface_config(self.ifname) if dict_search('linkinfo.info_slave_kind', tmp) != 'bond': mac = permanent_mac(self.ifname) if mac: self.set_mac(mac) - mtu = min(int(EthernetIf.DEFAULT_MTU), int(self.get_max_mtu())) - self.set_mtu(str(mtu)) + mtu = min(int(EthernetIf.DEFAULT_MTU), int(self.get_max_mtu())) + self.set_mtu(str(mtu)) self.set_admin_state('up') diff --git a/smoketest/scripts/cli/test_interfaces_bonding.py b/smoketest/scripts/cli/test_interfaces_bonding.py index d57c1c276..683fcc947 100755 --- a/smoketest/scripts/cli/test_interfaces_bonding.py +++ b/smoketest/scripts/cli/test_interfaces_bonding.py @@ -160,17 +160,65 @@ class BondingInterfaceTest(BasicInterfaceTest.TestCase): # T3871: deleting a member's whole ethernet node while it is enslaved, # then re-adding it, must leave the port present and usable - the # hardware never went anywhere. - member = self._members[0] + # + # A bond also owns its members' address and MTU + # (BondIf.get_inherit_bond_options()), and it goes on using this one + # after the node is gone. Putting the Kernel defaults back here left + # the member out of step with the bond it was still an active slave + # of: on a jumbo bond every frame the bond was sized for was silently + # discarded, and with the address reset the replies addressed to the + # bond were dropped by the port. Both have to wait until the bond has + # released it - which is asserted at the end. + if len(self._members) < 2: + self.skipTest('need a second member - the bond adopts the first ' + "member's address, so that one cannot show an " + 'address reset at all') + + bond = self._interfaces[0] + # deliberately not _members[0]: the bond takes its address from that + # one, so it could not tell a reset apart from a no-op + member = self._members[-1] + own_mac = Interface(member).get_mac() + # any MTU the bond does not share with the Kernel default will do. + # Staying below it keeps this runnable on ports which cannot carry + # jumbo frames, which is every port the smoketests run on. + bond_mtu = '1400' 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, 'mtu', bond_mtu]) self.cli_commit() + bond_mac = Interface(bond).get_mac() + self.assertNotEqual(own_mac, bond_mac, + f'{member} was picked as the address donor - it ' + 'cannot show an address reset') + self.assertEqual(Interface(member).get_mac(), bond_mac) + self.cli_delete(['interfaces', 'ethernet', member]) self.cli_commit() + self.assertTrue(interface_exists(member), f'{member} disappeared after its configuration was deleted') + self.assertEqual(read_file(f'/sys/class/net/{member}/mtu'), bond_mtu, + f'{member} left out of step with the bond it is still in') + self.assertEqual(Interface(member).get_mac(), bond_mac, + f'{member} no longer carries the address of the bond ' + 'it is still a member of') + + # released: now, and only now, the port goes back to what the Kernel + # gave it - which is what makes the skip above a deferral and not a + # permanent exemption + self.cli_delete(self._base_path + [bond, 'member', 'interface', member]) + self.cli_commit() + + self.assertEqual(Interface(member).get_mac(), own_mac, + f'{member} did not get its own address back once the ' + 'bond released it') + self.assertEqual(read_file(f'/sys/class/net/{member}/mtu'), '1500', + f'{member} did not get the default MTU back once the ' + 'bond released it') self.cli_set(['interfaces', 'ethernet', member, 'description', 'readded']) self.cli_commit() -- cgit v1.2.3