From e12776075f2dde8026f523e0d86f327bac7d398c Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Wed, 7 Oct 2026 19:56:39 +0200 Subject: interfaces: T3871: remove the hw-id node and migrate its bindings The node decides nothing now that the mapping store owns naming, and keeping it would leave two sources of truth free to disagree. Existing bindings are carried into the store by matching each hw-id against the permanent address of the hardware present, so a box named by an older release keeps exactly the names it had. Entries whose hardware is absent are skipped - the configuration may well originate from another machine. The hardware address of an interface is now read off the device rather than from the configuration. --- smoketest/scripts/cli/test_vpp.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'smoketest/scripts') diff --git a/smoketest/scripts/cli/test_vpp.py b/smoketest/scripts/cli/test_vpp.py index 9eb446615..87b901ffd 100755 --- a/smoketest/scripts/cli/test_vpp.py +++ b/smoketest/scripts/cli/test_vpp.py @@ -141,7 +141,7 @@ class TestVPP(VyOSUnitTestSHIM.TestCase): cls.cli_delete(cls, base_path) cls.cli_delete(cls, interfaces_path) # drop any pre-existing custom MAC so the MAC test baseline is the - # interface hardware address (hw-id) + # interface's own hardware address cls.cli_delete(cls, ['interfaces', 'ethernet', interface, 'mac']) # drop any pre-existing MTU so tests start from the default MTU cls.cli_delete(cls, ['interfaces', 'ethernet', interface, 'mtu']) @@ -280,7 +280,7 @@ class TestVPP(VyOSUnitTestSHIM.TestCase): self.cli_discard() # A custom MAC address must reach the VPP dataplane, and removing it must - # restore the hardware address (hw-id). Rejection of drivers that cannot + # restore the hardware address. Rejection of drivers that cannot # change the MAC (e.g. vmxnet3) is not covered here, as the CI dataplane # NIC uses a supported driver. hw_mac = VPPControl().get_mac(interface) @@ -289,7 +289,7 @@ class TestVPP(VyOSUnitTestSHIM.TestCase): self.cli_commit() self.assertEqual(VPPControl().get_mac(interface), mac) - # removing the custom MAC reverts to the hardware address (hw-id) + # removing the custom MAC reverts to the hardware address self.cli_delete(['interfaces', 'ethernet', interface, 'mac']) self.cli_commit() self.assertEqual(VPPControl().get_mac(interface), hw_mac) -- cgit v1.2.3 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. --- python/vyos/ifconfig/ethernet.py | 43 ++++++++++++++++++---- smoketest/scripts/cli/test_interfaces_bonding.py | 42 +++++++++++++++++++++ smoketest/scripts/cli/test_interfaces_ethernet.py | 45 ++++++++++++++++++++--- 3 files changed, 118 insertions(+), 12 deletions(-) (limited to 'smoketest/scripts') diff --git a/python/vyos/ifconfig/ethernet.py b/python/vyos/ifconfig/ethernet.py index 41fd6a0b9..8360c4654 100644 --- a/python/vyos/ifconfig/ethernet.py +++ b/python/vyos/ifconfig/ethernet.py @@ -19,9 +19,11 @@ from glob import glob from vyos.base import Warning from vyos.ethtool import Ethtool +from vyos.ifconfig.ifname_store import permanent_mac from vyos.ifconfig.interface import Interface from vyos.utils.dict import dict_search from vyos.utils.file import read_file +from vyos.utils.network import get_interface_config from vyos.utils.process import run from vyos.utils.assertion import assert_list @@ -83,6 +85,9 @@ class EthernetIf(Interface): }, } + # T5862: clamped against the adapter maximum wherever it is used + DEFAULT_MTU = '1500' + @staticmethod def get_bond_member_allowed_options() -> list: """ @@ -122,14 +127,22 @@ class EthernetIf(Interface): self.ethtool = Ethtool(ifname) def _create(self): + # we cannot create this interface as it is managed by the Kernel pass def remove(self): """ - Remove interface from config. Removing the interface deconfigures all - assigned IP addresses. + Return the interface to the state the Kernel gave it. + + An ethernet interface is owned by the Kernel and - like 'lo' - can not + be removed, so deleting its configuration means resetting the port + rather than tearing it down: addresses flushed, MTU and hardware + address back to the Kernel default and the link left up. Leaving it + admin-down stranded a port with nothing left to bring it back, a + released bond member included. + Example: - >>> from vyos.ifconfig import WWANIf + >>> from vyos.ifconfig import EthernetIf >>> i = EthernetIf('eth0') >>> i.remove() """ @@ -140,10 +153,26 @@ class EthernetIf(Interface): # and thus survives this call. super().remove() - if self.exists(self.ifname): - # interface is placed in A/D state when removed from config! It - # will remain visible for the operating system. - self.set_admin_state('down') + 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. + 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)) + + self.set_admin_state('up') def set_flow_control(self, enable, warn=True): """ 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 diff --git a/smoketest/scripts/cli/test_interfaces_ethernet.py b/smoketest/scripts/cli/test_interfaces_ethernet.py index 0d29d291a..473af051b 100755 --- a/smoketest/scripts/cli/test_interfaces_ethernet.py +++ b/smoketest/scripts/cli/test_interfaces_ethernet.py @@ -32,7 +32,10 @@ from vyos.ethtool import Ethtool from vyos.netlink import coalesce from vyos.frrender import mgmt_daemon from vyos.ifconfig import Section +from vyos.ifconfig.interface import Interface from vyos.utils.file import read_file +from vyos.utils.network import get_interface_config +from vyos.utils.network import interface_exists from vyos.utils.network import is_intf_addr_assigned from vyos.utils.network import is_ipv6_link_local from vyos.utils.process import cmdl @@ -52,10 +55,6 @@ class EthernetInterfaceTest(BasicInterfaceTest.TestCase): for tmp in Section.interfaces('ethernet', vlan=False): cls._interfaces.append(tmp) - cls._macs = {} - for interface in cls._interfaces: - cls._macs[interface] = read_file(f'/sys/class/net/{interface}/address') - # call base-classes classmethod super(EthernetInterfaceTest, cls).setUpClass() @@ -67,7 +66,6 @@ class EthernetInterfaceTest(BasicInterfaceTest.TestCase): self.cli_delete(self._base_path + [interface]) self.cli_set(self._base_path + [interface, 'duplex', 'auto']) self.cli_set(self._base_path + [interface, 'speed', 'auto']) - self.cli_set(self._base_path + [interface, 'hw-id', self._macs[interface]]) self.cli_commit() @@ -168,6 +166,43 @@ class EthernetInterfaceTest(BasicInterfaceTest.TestCase): # manually, else tearDown() will have problem in commit() self.cli_delete(self._base_path + [unknonw_interface]) + def test_delete_and_readd_interface(self): + # T3871: deleting an ethernet interface removes its configuration, but + # the port itself is "eternal" - it must still be present afterwards, + # keep its name, and be configurable again. An unconfigured port is + # not a deleted one. + for interface in self._interfaces: + self.cli_set(self._base_path + [interface, 'description', 'before']) + self.cli_commit() + + for interface in self._interfaces: + self.cli_delete(self._base_path + [interface]) + self.cli_commit() + + for interface in self._interfaces: + # an ethernet port belongs to the kernel - deleting its + # configuration returns it to the state the kernel gave it, it + # does not take the port away or leave it stranded admin-down + self.assertTrue(interface_exists(interface), + f'{interface} disappeared after its configuration ' + 'was deleted') + self.assertEqual(Interface(interface).get_admin_state(), 'up', + f'{interface} was left admin-down after its ' + 'configuration was deleted') + tmp = get_interface_config(interface) + self.assertEqual(tmp['mtu'], 1500, + f'{interface} kept a stale MTU after its ' + 'configuration was deleted') + + # re-adding must work and take effect + for interface in self._interfaces: + self.cli_set(self._base_path + [interface, 'description', 'after']) + self.cli_commit() + + for interface in self._interfaces: + self.assertEqual( + read_file(f'/sys/class/net/{interface}/ifalias'), 'after') + def test_speed_duplex_verify(self): for interface in self._interfaces: self.cli_set(self._base_path + [interface, 'speed', '1000']) -- 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') 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 From 5a70decaaccd0c72871543c8c31fc9a501486b69 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Wed, 7 Oct 2026 19:57:15 +0200 Subject: smoketest: T3871: cover the rescan giving a radio a node A radio is not in the interface mapping file, as every interface on a phy carries that phy's address and there is nothing there to anchor a name to. Nothing checked that the rescan finds one on the system instead, so the regression which left a radio without a configuration node went unnoticed. Run the installed helper against a copy of the running configuration, on an interface the testcase creates for itself so it does not depend on what another one left behind. Reverting the fix makes it fail, with no node for the radio at all. It lives in a file of its own because the test shim keys its configuration session on the process id, and a second testcase class in one file tears that session down underneath the first. --- .../scripts/cli/test_interfaces_wireless_rescan.py | 88 ++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100755 smoketest/scripts/cli/test_interfaces_wireless_rescan.py (limited to 'smoketest/scripts') diff --git a/smoketest/scripts/cli/test_interfaces_wireless_rescan.py b/smoketest/scripts/cli/test_interfaces_wireless_rescan.py new file mode 100755 index 000000000..82dc146c9 --- /dev/null +++ b/smoketest/scripts/cli/test_interfaces_wireless_rescan.py @@ -0,0 +1,88 @@ +#!/usr/bin/env python3 +# +# Copyright 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 . + +import os +import shutil +import tempfile +import unittest +from glob import glob + +from base_vyostest_shim import VyOSUnitTestSHIM + +from vyos.configtree import ConfigTree +from vyos.utils.file import read_file +from vyos.utils.kernel import check_kmod +from vyos.utils.network import interface_exists +from vyos.utils.process import call +from vyos.utils.process import rc_cmd + +rescan = '/usr/libexec/vyos/vyos-interface-rescan.py' +rescan_log = '/var/log/vyatta/vyos-interface-rescan' + +class WirelessRescanTest(VyOSUnitTestSHIM.TestCase): + """T3871: a radio is not in the interface mapping file - every interface on + a phy carries that phy's address, so there is nothing there to anchor a + name to. The rescan has to find radios on the system instead. While it took + its whole list from the map, no radio was ever given a node and 'interfaces + wireless' stayed empty on a box which had one. + """ + + ifname = 'wlan99' + + def setUp(self): + phys = sorted(os.path.basename(p) + for p in glob('/sys/class/ieee80211/*')) + if not phys: + self.skipTest('no wireless phy on this system') + self.phy = phys[0] + + # an interface of our own, so the test does not depend on which radios + # another testcase left behind + if interface_exists(self.ifname): + call(f'sudo iw dev {self.ifname} del') + if call(f'sudo iw phy {self.phy} interface add {self.ifname} ' + 'type managed') != 0: + self.skipTest(f'could not create an interface on {self.phy}') + self.addCleanup(call, f'sudo iw dev {self.ifname} del') + + def test_a_radio_is_given_its_physical_device(self): + # a directory of our own, not /tmp itself - the rescan runs as root + # and fs.protected_regular refuses it an O_CREAT on a file it does not + # own in a world writable sticky directory + tmp_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, tmp_dir) + tmp = os.path.join(tmp_dir, 'config.boot') + shutil.copy('/config/config.boot', tmp) + + config = ConfigTree(read_file(tmp)) + self.assertFalse(config.exists(['interfaces', 'wireless', self.ifname])) + + rc, out = rc_cmd(f'sudo {rescan} {tmp}') + if rc != 0: + # it reports a failure to its own log rather than to stderr, so + # without this a failure here is just an exit code + _, log = rc_cmd(f'sudo tail -n 20 {rescan_log}') + self.fail(f'rescan exited {rc}: {out}\n{log}') + + config = ConfigTree(read_file(tmp)) + self.assertTrue(config.exists(['interfaces', 'wireless', self.ifname])) + self.assertEqual( + config.return_value(['interfaces', 'wireless', self.ifname, + 'physical-device']), self.phy) + +if __name__ == '__main__': + check_kmod('mac80211_hwsim') + unittest.main(verbosity=2, failfast=VyOSUnitTestSHIM.TestCase.debug_on()) -- cgit v1.2.3