Merge pull request #4833 from c-po/fix-veth-removal

veth: T7990: fix stale DHCP clients when removing virtual Ethernet pairs

3 files changed, 27 insertions, 30 deletions

diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index ad634c4d7..f03d0cb68 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -371,7 +371,7 @@ class Interface(Control):
         if 'netns' in self.config: cmd = f'ip netns exec {netns} {cmd}'
         self._cmd(cmd)
 
-    def remove(self):
+    def remove(self, skip_delete=False):
         """
         Remove interface from operating system. Removing the interface
         deconfigures all assigned IP addresses and clear possible DHCP(v6)
@@ -388,12 +388,20 @@ class Interface(Control):
 
         # remove all assigned IP addresses from interface - this is a bit redundant
         # as the kernel will remove all addresses on interface deletion, but we
-        # can not delete ALL interfaces, see below
+        # can not delete ALL interfaces, see below.
+        #
+        # This will internally stop DHCP(v6) if running
         self.flush_addrs()
 
         # remove interface from conntrack VRF interface map
         self._del_interface_from_ct_iface_map()
 
+        # Some interfaces - mainly veth pairs - should be properly de-configured
+        # but not deleted. Deleting one veth pair member will delete the other,
+        # we need need a way to skip the deletion.
+        if skip_delete:
+            return
+
         # ---------------------------------------------------------------------
         # Any class can define an eternal regex in its definition
         # interface matching the regex will not be deleted
diff --git a/python/vyos/ifconfig/veth.py b/python/vyos/ifconfig/veth.py
index f4075fa02..9868ea526 100644
--- a/python/vyos/ifconfig/veth.py
+++ b/python/vyos/ifconfig/veth.py
@@ -14,7 +14,8 @@
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
 from vyos.ifconfig.interface import Interface
-
+from vyos.utils.dict import dict_search
+from vyos.utils.network import get_interface_config
 
 @Interface.register
 class VethIf(Interface):
@@ -51,3 +52,18 @@ class VethIf(Interface):
 
         # interface is always A/D down. It needs to be enabled explicitly
         self.set_admin_state('down')
+
+    def remove(self):
+        # The oddity with virtual-ethernet pairs is, if you delete one - the OS
+        # Kernel will immediately delete the other interface, leaving no
+        # possibility to properly shutdown the peer-interface.
+        #
+        # We deconfigure the second veth pair interface during deletion, but
+        # skip it's actual call to "ip link del dev ..."
+        tmp = get_interface_config(self.ifname)
+        peer = tmp.get('link', dict_search('linkinfo.info_kind', tmp))
+        if peer != None:
+            Interface(peer).remove(skip_delete=True)
+
+        # always forward to the base class
+        super().remove()
diff --git a/smoketest/scripts/cli/test_interfaces_virtual-ethernet.py b/smoketest/scripts/cli/test_interfaces_virtual-ethernet.py
index 0a581951d..9bcc1cb1f 100755
--- a/smoketest/scripts/cli/test_interfaces_virtual-ethernet.py
+++ b/smoketest/scripts/cli/test_interfaces_virtual-ethernet.py
@@ -15,14 +15,10 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import unittest
-from netifaces import interfaces # pylint: disable = no-name-in-module
 
 from base_interfaces_test import BasicInterfaceTest
 from base_vyostest_shim import VyOSUnitTestSHIM
 
-from vyos.frrender import mgmt_daemon
-from vyos.utils.process import process_named_running
-
 class VEthInterfaceTest(BasicInterfaceTest.TestCase):
     @classmethod
     def setUpClass(cls):
@@ -36,28 +32,5 @@ class VEthInterfaceTest(BasicInterfaceTest.TestCase):
         # call base-classes classmethod
         super(VEthInterfaceTest, cls).setUpClass()
 
-    # As we always need a pair of veth interfaces, we can not rely on the base
-    # class check to determine if there is a dhcp6c or dhclient instance
-    # running. This test will always fail as there is an instance still running
-    # on the peer interface.
-    def tearDown(self):
-        self.cli_delete(self._base_path)
-        self.cli_commit()
-
-        # Verify that no previously interface remained on the system
-        for intf in self._interfaces:
-            self.assertNotIn(intf, interfaces())
-
-        # check process health and continuity
-        self.assertEqual(self.mgmt_daemon_pid, process_named_running(mgmt_daemon))
-
-    @classmethod
-    def tearDownClass(cls):
-        # No daemon started during tests should remain running
-        for daemon in ['dhcp6c', 'dhclient']:
-            cls.assertFalse(cls, process_named_running(daemon))
-
-        super(VEthInterfaceTest, cls).tearDownClass()
-
 if __name__ == '__main__':
     unittest.main(verbosity=2, failfast=VyOSUnitTestSHIM.TestCase.debug_on())