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 "<ifname>.".

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.

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)