From 41fac0cfbf1a1558c7c6b9cbde9f7a9786e80162 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Mon, 14 Sep 2026 18:42:10 +0200 Subject: ifconfig: T9313: fix removal of QinQ sub-interfaces EthernetIf.remove() snapshotted Section.sub_interfaces() once and then removed every entry of that list. For Q-in-Q setups the list contains both the vif-s (eth0.10) and its vif-c children (eth0.10.20), as Section.sub_interfaces() matches every configured interface whose name starts with ".". Removing eth0.10 cascades in the Kernel and also deletes eth0.10.20. Once the loop reached eth0.10.20, Interface.__init__() no longer found it and - as "create" defaults to True, re-created it via a bare "ip link add dev eth0.10.20" without any type. This failed and left a stranded interface behind which in turn broke all subsequent smoketests. The same cascade silently killed DHCP clients running on sub-interfaces of any other interface type, as only the interface itself got its addresses flushed: * deletion of a bond or bridge interface carrying vif/vif-s/vif-c * deletion of an entire vif-s, as get_removed_vlans() only populates "vif_c_remove" for a vif-s which still exists in the new configuration Move the sub-interface teardown from EthernetIf.remove() into the base class Interface.remove(), where it benefits every interface type. The list is walked deepest first (vif-c before vif-s/vif), which is what T7813 requires to stop the DHCP client and return the lease, and interfaces which vanished in the meantime are skipped so they are never accidentally re-created. As a side effect of this move, an ethernet interface is now only placed into A/D state after the base class has released the addresses - previously the lease of the interface itself was returned over an already downed link. --- python/vyos/ifconfig/ethernet.py | 13 +++++-------- python/vyos/ifconfig/interface.py | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/python/vyos/ifconfig/ethernet.py b/python/vyos/ifconfig/ethernet.py index 1b10d075e..ab5c8a0b8 100644 --- a/python/vyos/ifconfig/ethernet.py +++ b/python/vyos/ifconfig/ethernet.py @@ -19,7 +19,6 @@ from glob import glob from vyos.base import Warning from vyos.ethtool import Ethtool -from vyos.ifconfig import Section from vyos.ifconfig.interface import Interface from vyos.utils.dict import dict_search from vyos.utils.file import read_file @@ -136,19 +135,17 @@ class EthernetIf(Interface): >>> i.remove() """ - # T7813: we do need to remove the VLAN subinterfaces first so we can - # properly stop the DHCP client and inform the DHCP server that we are - # returning the lease. - for vlan in Section.sub_interfaces(self.ifname): - Interface(vlan).remove() + # T7813: the base class removes all VLAN sub-interfaces and flushes the + # addresses of this interface - both require a link which is still up so + # a DHCP client can return its lease. A physical interface is "eternal" + # 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') - super().remove() - def set_flow_control(self, enable, warn=True): """ Changes the pause parameters of the specified Ethernet device. diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index 3a5efec8c..73f7544dd 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -393,6 +393,20 @@ class Interface(Control): >>> i = Interface('eth0') >>> i.remove() """ + # T7813: VLAN sub-interfaces must be de-configured before this interface + # is torn down, otherwise a running DHCP client can neither talk to the + # server nor return its lease. Deleting an interface implicitly deletes + # all its VLAN children in the Kernel, thus start with the deepest + # interface (vif-c) and work the way up to the vif interfaces. + for vlan in sorted( + Section.sub_interfaces(self.ifname), key=lambda x: x.count('.'), reverse=True + ): + # A previous iteration may have already removed this interface as a + # side effect - do not re-create it when instantiating Interface() + if not Interface.exists(vlan): + continue + Interface(vlan).remove() + # Stop WPA supplicant if EAPoL was in use netns = self.config['netns'] if 'netns' in self.config else None stop_systemd_unit(f'wpa_supplicant-wired@{self.ifname}', netns=netns) -- cgit v1.2.3