Merge pull request #5036 from c-po/bond-mirror

vyos.ifconfig: T8358: clear qdiscs when deleting mirror CLI node

5 files changed, 28 insertions, 13 deletions

diff --git a/smoketest/scripts/cli/base_interfaces_test.py b/smoketest/scripts/cli/base_interfaces_test.py
index 7e9c5b6a4..b2f700fce 100644
--- a/smoketest/scripts/cli/base_interfaces_test.py
+++ b/smoketest/scripts/cli/base_interfaces_test.py
@@ -13,6 +13,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import re
+import jmespath
 
 from json import loads
 from netifaces import ifaddresses # pylint: disable = no-name-in-module
@@ -121,9 +122,9 @@ def get_certificate_count(interface, cert_type):
     tmp = read_file(f'/run/wpa_supplicant/{interface}_{cert_type}.pem')
     return tmp.count(CERT_BEGIN)
 
-def is_mirrored_to(interface, mirror_if, qdisc):
+def is_mirrored_to(interface, mirror_if, qdisc) -> bool:
     """
-    Ask TC if we are mirroring traffic to a discrete interface.
+    Ask tc(8) if we are mirroring traffic to a specific interface.
 
     interface: source interface
     mirror_if: destination where we mirror our data to
@@ -132,12 +133,11 @@ def is_mirrored_to(interface, mirror_if, qdisc):
     if qdisc not in ['ffff', '1']:
         raise ValueError()
 
-    ret_val = False
-    tmp = cmd(f'tc -s -p filter ls dev {interface} parent {qdisc}: | grep mirred')
-    tmp = tmp.lower()
-    if mirror_if in tmp:
-        ret_val = True
-    return ret_val
+    tmp = loads(cmd(f'tc -json filter ls dev {interface} parent {qdisc}:'))
+    # the following syntax looks odd but we need to filter out the first
+    # result sets from tc which do not have "options.actions...".
+    tmp = jmespath.search("[?options.actions[0].kind=='mirred'].options.actions[0].{mirred_action: mirred_action, to_dev: to_dev} | [0]", tmp)
+    return bool(dict_search('mirred_action', tmp) == 'mirror' and dict_search('to_dev', tmp) == mirror_if)
 
 class BasicInterfaceTest:
     class TestCase(VyOSUnitTestSHIM.TestCase):
@@ -161,7 +161,7 @@ class BasicInterfaceTest:
         _test_addr = ['192.0.2.1/26', '192.0.2.255/31', '192.0.2.64/32',
                       '2001:db8:1::ffff/64', '2001:db8:101::1/112']
 
-        _mirror_interfaces = []
+        _mirror_interfaces = ['dum21354']
         # choose IPv6 minimum MTU value for tests - this must always work
         _mtu = '1280'
 
@@ -181,6 +181,7 @@ class BasicInterfaceTest:
             cls._test_ipv6_pd = cli_defined(cls._base_path + ['dhcpv6-options'], 'pd')
             cls._test_mtu = cli_defined(cls._base_path, 'mtu')
             cls._test_vrf = cli_defined(cls._base_path, 'vrf')
+            cls._test_mirror = cli_defined(cls._base_path, 'mirror')
 
             # Setup mirror interfaces for SPAN (Switch Port Analyzer)
             for span in cls._mirror_interfaces:
@@ -456,9 +457,13 @@ class BasicInterfaceTest:
                 self.cli_set(self._base_path + [interface, 'description', 'test_add_to_invalid_vrf'])
 
         def test_span_mirror(self):
-            if not self._mirror_interfaces:
+            if not self._test_mirror:
                 self.skipTest(MSG_TESTCASE_UNSUPPORTED)
 
+            for interface in self._interfaces:
+                for option in self._options.get(interface, []):
+                    self.cli_set(self._base_path + [interface] + option.split())
+
             # Check the two-way mirror rules of ingress and egress
             for mirror in self._mirror_interfaces:
                 for interface in self._interfaces:
@@ -473,6 +478,18 @@ class BasicInterfaceTest:
                     self.assertTrue(is_mirrored_to(interface, mirror, 'ffff'))
                     self.assertTrue(is_mirrored_to(interface, mirror, '1'))
 
+            # delete interface mirror - check that configuration from tc is removed
+            for mirror in self._mirror_interfaces:
+                for interface in self._interfaces:
+                    self.cli_delete(self._base_path + [interface, 'mirror'])
+            self.cli_commit()
+
+            # Verify config
+            for mirror in self._mirror_interfaces:
+                for interface in self._interfaces:
+                    self.assertFalse(is_mirrored_to(interface, mirror, 'ffff'))
+                    self.assertFalse(is_mirrored_to(interface, mirror, '1'))
+
         def test_interface_disable(self):
             # Check if description can be added to interface and
             # can be read back
diff --git a/smoketest/scripts/cli/test_interfaces_bonding.py b/smoketest/scripts/cli/test_interfaces_bonding.py
index a565168cf..a4afae5da 100755
--- a/smoketest/scripts/cli/test_interfaces_bonding.py
+++ b/smoketest/scripts/cli/test_interfaces_bonding.py
@@ -30,7 +30,6 @@ class BondingInterfaceTest(BasicInterfaceTest.TestCase):
     @classmethod
     def setUpClass(cls):
         cls._base_path = ['interfaces', 'bonding']
-        cls._mirror_interfaces = ['dum21354']
         cls._members = []
 
         # we need to filter out VLAN interfaces identified by a dot (.)
diff --git a/smoketest/scripts/cli/test_interfaces_bridge.py b/smoketest/scripts/cli/test_interfaces_bridge.py
index 1a67a9165..a41ad1482 100755
--- a/smoketest/scripts/cli/test_interfaces_bridge.py
+++ b/smoketest/scripts/cli/test_interfaces_bridge.py
@@ -35,7 +35,6 @@ class BridgeInterfaceTest(BasicInterfaceTest.TestCase):
     @classmethod
     def setUpClass(cls):
         cls._base_path = ['interfaces', 'bridge']
-        cls._mirror_interfaces = ['dum21354']
         cls._members = []
 
         # we need to filter out VLAN interfaces identified by a dot (.)
diff --git a/smoketest/scripts/cli/test_interfaces_dummy.py b/smoketest/scripts/cli/test_interfaces_dummy.py
index b19ab02dd..c011a411b 100755
--- a/smoketest/scripts/cli/test_interfaces_dummy.py
+++ b/smoketest/scripts/cli/test_interfaces_dummy.py
@@ -24,6 +24,7 @@ class DummyInterfaceTest(BasicInterfaceTest.TestCase):
     def setUpClass(cls):
         cls._base_path = ['interfaces', 'dummy']
         cls._interfaces = ['dum435', 'dum8677', 'dum0931', 'dum089']
+        cls._mirror_interfaces = ['eth0']
         # call base-classes classmethod
         super(DummyInterfaceTest, cls).setUpClass()
 
diff --git a/smoketest/scripts/cli/test_interfaces_ethernet.py b/smoketest/scripts/cli/test_interfaces_ethernet.py
index 7c8106521..7e0da2079 100755
--- a/smoketest/scripts/cli/test_interfaces_ethernet.py
+++ b/smoketest/scripts/cli/test_interfaces_ethernet.py
@@ -43,7 +43,6 @@ class EthernetInterfaceTest(BasicInterfaceTest.TestCase):
     @classmethod
     def setUpClass(cls):
         cls._base_path = ['interfaces', 'ethernet']
-        cls._mirror_interfaces = ['dum21354']
 
         # We only test on physical interfaces and not VLAN (sub-)interfaces
         if 'TEST_ETH' in os.environ: