From f4625c9ee2f54992ed96f113ff52c5a9993ea769 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Sun, 3 Jan 2021 20:58:40 +0100 Subject: mirror: T3089: remove redundant code paths - remove redundant code paths apply_mirror() / apply_mirror_of_monitor() - have single source available --- python/vyos/configdict.py | 64 ++++++++++---------- python/vyos/ifconfig/interface.py | 120 +++++++++++++------------------------- 2 files changed, 72 insertions(+), 112 deletions(-) (limited to 'python') 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 -- cgit v1.2.3