mirror: T3089: remove redundant code paths

- remove redundant code paths apply_mirror() / apply_mirror_of_monitor()
- have single source available

2 files changed, 72 insertions, 112 deletions

diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index b299f4a18..e5e758a8b 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -21,7 +21,6 @@ import json
 
 from vyos.util import dict_search
 from vyos.xml import defaults
-from vyos import ConfigError
 from vyos.util import cmd
 
 def retrieve_config(path_hash, base_path, config):
@@ -215,49 +214,49 @@ def is_member(conf, interface, intftype=None):
         for intf in conf.list_nodes(base):
             member = base + [intf, 'member', 'interface', interface]
             if conf.exists(member):
-                tmp = conf.get_config_dict(member, key_mangling=('-', '_'), get_first_key=True)
+                tmp = conf.get_config_dict(member, key_mangling=('-', '_'),
+                                           get_first_key=True)
                 ret_val = {intf : tmp}
 
     old_level = conf.set_level(old_level)
     return ret_val
 
-def is_monitor_intf(conf,interface,direction=None):
+def is_mirror_intf(conf, interface, direction=None):
     """
-    Check whether the passed interface is the mirror monitoring interface of 
-    other interfaces of the specified type.
-    direction is optional, if not passed it will search all known direction
+    Check whether the passed interface is used for port mirroring. Direction
+    is optional, if not passed it will search all known direction
     (currently ingress and egress)
 
     Returns:
     None -> Interface is not a monitor interface
     Array() -> This interface is a monitor interface of interfaces
     """
-    
+    from vyos.ifconfig import Section
+
     directions = ['ingress', 'egress']
     if direction not in directions + [None]:
-        raise ValueError(f'unknown interface mirror direction "{direction}"')
-    
+        raise ValueError(f'Unknown interface mirror direction "{direction}"')
+
     direction = directions if direction == None else [direction]
-    
-    ret_val = []
+
+    ret_val = None
     old_level = conf.get_level()
     conf.set_level([])
     base = ['interfaces']
-    
-    for dire in direction:
+
+    for dir in direction:
         for iftype in conf.list_nodes(base):
             iftype_base = base + [iftype]
             for intf in conf.list_nodes(iftype_base):
-                mirror = iftype_base + [intf, 'mirror', dire, interface]
+                mirror = iftype_base + [intf, 'mirror', dir, interface]
                 if conf.exists(mirror):
-                    ret_val.append({intf : dire})
-    
+                    path = ['interfaces', Section.section(intf), intf]
+                    tmp = conf.get_config_dict(path, key_mangling=('-', '_'),
+                                               get_first_key=True)
+                    ret_val = {intf : tmp}
+
     old_level = conf.set_level(old_level)
-    if len(ret_val) == 0:
-        return None
-    else:
-        return ret_val
-    
+    return ret_val
 
 def has_vlan_subinterface_configured(conf, intf):
     """
@@ -265,7 +264,7 @@ def has_vlan_subinterface_configured(conf, intf):
     Checks the following config nodes:
     'vif', 'vif-s'
 
-    Returns True if interface has VLAN subinterface configured, False if it doesn't.
+    Return True if interface has VLAN subinterface configured.
     """
     from vyos.ifconfig import Section
     ret = False
@@ -325,7 +324,9 @@ def get_interface_dict(config, base, ifname=''):
 
     Return a dictionary with the necessary interface config keys.
     """
+
     if not ifname:
+        from vyos import ConfigError
         # determine tagNode instance
         if 'VYOS_TAGNODE_VALUE' not in os.environ:
             raise ConfigError('Interface (VYOS_TAGNODE_VALUE) not specified')
@@ -342,13 +343,14 @@ def get_interface_dict(config, base, ifname=''):
 
     # setup config level which is extracted in get_removed_vlans()
     config.set_level(base + [ifname])
-    dict = config.get_config_dict([], key_mangling=('-', '_'), get_first_key=True)
+    dict = config.get_config_dict([], key_mangling=('-', '_'),
+                                  get_first_key=True)
 
-    # Check if interface has been removed. We must use exists() as get_config_dict()
-    # will always return {} - even when an empty interface node like
+    # Check if interface has been removed. We must use exists() as
+    # get_config_dict() will always return {} - even when an empty interface
+    # node like the following exists.
     # +macsec macsec1 {
     # +}
-    # exists.
     if not config.exists([]):
         dict.update({'deleted' : ''})
 
@@ -372,10 +374,10 @@ def get_interface_dict(config, base, ifname=''):
     # Check if we are a member of a bridge device
     bridge = is_member(config, ifname, 'bridge')
     if bridge: dict.update({'is_bridge_member' : bridge})
-    
+
     # Check if it is a monitor interface
-    mirror = is_monitor_intf(config, ifname)
-    if mirror: dict.update({'is_monitor_intf' : mirror})
+    mirror = is_mirror_intf(config, ifname)
+    if mirror: dict.update({'is_mirror_intf' : mirror})
 
     # Check if we are a member of a bond device
     bond = is_member(config, ifname, 'bonding')
@@ -469,7 +471,7 @@ def get_vlan_ids(interface):
     Get the VLAN ID of the interface bound to the bridge
     """
     vlan_ids = set()
-        
+
     bridge_status = cmd('bridge -j vlan show', shell=True)
     vlan_filter_status = json.loads(bridge_status)
 
@@ -481,7 +483,7 @@ def get_vlan_ids(interface):
                 for vlan_status in vlans_status:
                     vlan_id = vlan_status['vlan']
                     vlan_ids.add(vlan_id)
-        
+
     return vlan_ids
 
 
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 9a1483a1e..163ab2f6a 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -1008,86 +1008,45 @@ class Interface(Control):
             if os.path.isfile(config_file):
                 os.remove(config_file)
 
-    def apply_mirror(self):
+    def set_mirror(self):
         # Please refer to the document for details
-        # https://man7.org/linux/man-pages/man8/tc.8.html
-        # https://man7.org/linux/man-pages/man8/tc-mirred.8.html
-        ifname = self._config['ifname']
+        #   - https://man7.org/linux/man-pages/man8/tc.8.html
+        #   - https://man7.org/linux/man-pages/man8/tc-mirred.8.html
+        # Depening if we are the source or the target interface of the port
+        # mirror we need to setup some variables.
+        source_if = self._config['ifname']
+        config = self._config.get('mirror', None)
+
+        if 'is_mirror_intf' in self._config:
+            source_if = next(iter(self._config['is_mirror_intf']))
+            config = self._config['is_mirror_intf'][source_if]
+
         # Remove existing mirroring rules
-        try:
-            delete_tc_cmd = f'tc qdisc del dev {ifname} handle ffff: ingress'
-            self._cmd(delete_tc_cmd)
-        except:
-            pass
-        try:
-            delete_tc_cmd = f'tc qdisc del dev {ifname} handle 1: root prio'
-            self._cmd(delete_tc_cmd)
-        except:
-            pass
-
-        # Setting up packet mirroring
-        ingress_mirror = dict_search('mirror.ingress', self._config)
-        # if interface does yet not exist bail out early and
-        # add it later
-        if ingress_mirror and ingress_mirror in interfaces():
-            # Mirror ingress traffic
-            mirror_cmd = f'tc qdisc add dev {ifname} handle ffff: ingress'
-            self._cmd(mirror_cmd)
-            # Export the mirrored traffic to the interface
-            mirror_cmd = f'tc filter add dev {ifname} parent ffff: protocol all prio 10 u32 match u32 0 0 flowid 1:1 action mirred egress mirror dev {ingress_mirror}'
-            self._cmd(mirror_cmd)
+        delete_tc_cmd  = f'tc qdisc del dev {source_if} handle ffff: ingress; '
+        delete_tc_cmd += f'tc qdisc del dev {source_if} handle 1: root prio'
+        self._popen(delete_tc_cmd)
+
+        # Bail out early if nothing needs to be configured
+        if not config:
+            return
+
+        for direction, mirror_if in config.items():
+            if mirror_if not in interfaces():
+                continue
+
+            if direction == 'ingress':
+                handle = 'ffff: ingress'
+                parent = 'ffff:'
+            elif direction == 'egress':
+                handle = '1: root prio'
+                parent = '1:'
 
-        egress_mirror = dict_search('mirror.egress', self._config)
-        # if interface does yet not exist bail out early and
-        # add it later
-        if egress_mirror and egress_mirror in interfaces():
             # Mirror egress traffic
-            mirror_cmd = f'tc qdisc add dev {ifname} handle 1: root prio'
-            self._cmd(mirror_cmd)
+            mirror_cmd  = f'tc qdisc add dev {source_if} handle {handle}; '
             # Export the mirrored traffic to the interface
-            mirror_cmd = f'tc filter add dev {ifname} parent 1: protocol all prio 10 u32 match u32 0 0 flowid 1:1 action mirred egress mirror dev {egress_mirror}'
-            self._cmd(mirror_cmd)
+            mirror_cmd += f'tc filter add dev {source_if} parent {parent} protocol all prio 10 u32 match u32 0 0 flowid 1:1 action mirred egress mirror dev {mirror_if}'
+            self._popen(mirror_cmd)
 
-    def apply_mirror_of_monitor(self):
-        # Please refer to the document for details
-        # https://man7.org/linux/man-pages/man8/tc.8.html
-        # https://man7.org/linux/man-pages/man8/tc-mirred.8.html
-        ifname = self._config['ifname']
-        mirror_rules = self._config.get('is_monitor_intf')
-
-        # Remove existing mirroring rules
-        # The rule must be completely deleted first
-        for rule in mirror_rules:
-            for intf, dire in rule.items():
-                try:
-                    delete_tc_cmd = f'tc qdisc del dev {intf} handle ffff: ingress'
-                    self._cmd(delete_tc_cmd)
-                except:
-                    pass
-                try:
-                    delete_tc_cmd = f'tc qdisc del dev {intf} handle 1: root prio'
-                    self._cmd(delete_tc_cmd)
-                except:
-                    pass
-
-        # Setting mirror rules
-        for rule in mirror_rules:
-            for intf, dire in rule.items():
-                # Setting up packet mirroring
-                if dire == "ingress":
-                    # Mirror ingress traffic
-                    mirror_cmd = f'tc qdisc add dev {intf} handle ffff: ingress'
-                    self._cmd(mirror_cmd)
-                    # Export the mirrored traffic to the interface
-                    mirror_cmd = f'tc filter add dev {intf} parent ffff: protocol all prio 10 u32 match u32 0 0 flowid 1:1 action mirred egress mirror dev {ifname}'
-                    self._cmd(mirror_cmd)
-                elif dire == "egress":
-                    # Mirror egress traffic
-                    mirror_cmd = f'tc qdisc add dev {intf} handle 1: root prio'
-                    self._cmd(mirror_cmd)
-                    # Export the mirrored traffic to the interface
-                    mirror_cmd = f'tc filter add dev {intf} parent 1: protocol all prio 10 u32 match u32 0 0 flowid 1:1 action mirred egress mirror dev {ifname}'
-                    self._cmd(mirror_cmd)
 
     def set_xdp(self, state):
         """
@@ -1281,13 +1240,12 @@ class Interface(Control):
             bridge_dict = config.get('is_bridge_member')
             self.add_to_bridge(bridge_dict)
 
-        # Re-set rules for the mirror monitoring interface
-        if 'is_monitor_intf' in config:
-            self.apply_mirror_of_monitor()
-
         # eXpress Data Path - highly experimental
         self.set_xdp('xdp' in config)
 
+        # configure port mirror
+        self.set_mirror()
+
         # remove no longer required 802.1ad (Q-in-Q VLANs)
         ifname = config['ifname']
         for vif_s_id in config.get('vif_s_remove', {}):
@@ -1338,9 +1296,6 @@ class Interface(Control):
             vlan = VLANIf(vif_ifname, **tmp)
             vlan.update(vif_config)
 
-        self.apply_mirror()
-
-
 
 class VLANIf(Interface):
     """ Specific class which abstracts 802.1q and 802.1ad (Q-in-Q) VLAN interfaces """
@@ -1417,6 +1372,9 @@ class VLANIf(Interface):
 
         return super().set_admin_state(state)
 
+    def set_mirror(self):
+        return
+
     def update(self, config):
         """ General helper function which works on a dictionary retrived by
         get_config_dict(). It's main intention is to consolidate the scattered