From 5564d873803612a33b69d357609f58114ffe8cd6 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak <38952541+stepanblyschak@users.noreply.github.com> Date: Mon, 7 Oct 2019 17:35:34 +0300 Subject: [PATCH] [acl-loader] egress mirror action support and action ASIC support check (#575) - support egress mirror action - support action check via state DB --- .gitignore | 3 + acl_loader/main.py | 167 ++++++++++++++---- config/main.py | 5 +- doc/Command-Reference.md | 74 +++++--- sonic-utilities-tests/acl_loader_test.py | 32 +++- sonic-utilities-tests/aclshow_test.py | 4 +- .../mock_tables/config_db.json | 6 + .../mock_tables/state_db.json | 7 + 8 files changed, 236 insertions(+), 62 deletions(-) diff --git a/.gitignore b/.gitignore index 4afb832592..2a9d7b9348 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,6 @@ build/ deb_dist/ dist/ *.egg-info/ +*.pyc +.cache +*.tar.gz diff --git a/acl_loader/main.py b/acl_loader/main.py index b80ac0d840..77fd03a35b 100644 --- a/acl_loader/main.py +++ b/acl_loader/main.py @@ -38,6 +38,31 @@ def deep_update(dst, src): return dst +class AclAction: + """ namespace for ACL action keys """ + + PACKET = "PACKET_ACTION" + REDIRECT = "REDIRECT_ACTION" + MIRROR = "MIRROR_ACTION" + MIRROR_INGRESS = "MIRROR_INGRESS_ACTION" + MIRROR_EGRESS = "MIRROR_EGRESS_ACTION" + + +class PacketAction: + """ namespace for ACL packet actions """ + + DROP = "DROP" + FORWARD = "FORWARD" + ACCEPT = "ACCEPT" + + +class Stage: + """ namespace for ACL stages """ + + INGRESS = "INGRESS" + EGRESS = "EGRESS" + + class AclLoaderException(Exception): pass @@ -52,6 +77,9 @@ class AclLoader(object): STATE_MIRROR_SESSION_TABLE = "MIRROR_SESSION_TABLE" POLICER = "POLICER" SESSION_PREFIX = "everflow" + SWITCH_CAPABILITY_TABLE = "SWITCH_CAPABILITY" + ACL_ACTIONS_CAPABILITY_FIELD = "ACL_ACTIONS" + ACL_ACTION_CAPABILITY_FIELD = "ACL_ACTION" min_priority = 1 max_priority = 10000 @@ -81,6 +109,7 @@ class AclLoader(object): def __init__(self): self.yang_acl = None self.requested_session = None + self.mirror_stage = None self.current_table = None self.tables_db_info = {} self.rules_db_info = {} @@ -179,6 +208,14 @@ def set_session_name(self, session_name): self.requested_session = session_name + def set_mirror_stage(self, stage): + """ + Set mirror stage to be used in ACL mirror rule action + :param session_name: stage 'ingress'/'egress' + :return: + """ + self.mirror_stage = stage.upper() + def set_max_priority(self, priority): """ Set rules max priority @@ -232,25 +269,72 @@ def convert_action(self, table_name, rule_idx, rule): if rule.actions.config.forwarding_action == "ACCEPT": if self.is_table_control_plane(table_name): - rule_props["PACKET_ACTION"] = "ACCEPT" + rule_props[AclAction.PACKET] = PacketAction.ACCEPT elif self.is_table_mirror(table_name): session_name = self.get_session_name() if not session_name: raise AclLoaderException("Mirroring session does not exist") - rule_props["MIRROR_ACTION"] = session_name + if self.mirror_stage == Stage.INGRESS: + mirror_action = AclAction.MIRROR_INGRESS + elif self.mirror_stage == Stage.EGRESS: + mirror_action = AclAction.MIRROR_EGRESS + else: + raise AclLoaderException("Invalid mirror stage passed {}".format(self.mirror_stage)) + + rule_props[mirror_action] = session_name else: - rule_props["PACKET_ACTION"] = "FORWARD" + rule_props[AclAction.PACKET] = PacketAction.FORWARD elif rule.actions.config.forwarding_action == "DROP": - rule_props["PACKET_ACTION"] = "DROP" + rule_props[AclAction.PACKET] = PacketAction.DROP elif rule.actions.config.forwarding_action == "REJECT": - rule_props["PACKET_ACTION"] = "DROP" + rule_props[AclAction.PACKET] = PacketAction.DROP else: - raise AclLoaderException("Unknown rule action %s in table %s, rule %d" % ( + raise AclLoaderException("Unknown rule action {} in table {}, rule {}".format( + rule.actions.config.forwarding_action, table_name, rule_idx)) + + if not self.validate_actions(table_name, rule_props): + raise AclLoaderException("Rule action {} is not supported in table {}, rule {}".format( rule.actions.config.forwarding_action, table_name, rule_idx)) return rule_props + def validate_actions(self, table_name, action_props): + if self.is_table_control_plane(table_name): + return True + + action_count = len(action_props) + + if table_name not in self.tables_db_info: + raise AclLoaderException("Table {} does not exist".format(table_name)) + + stage = self.tables_db_info[table_name].get("stage", Stage.INGRESS) + capability = self.statedb.get_all(self.statedb.STATE_DB, "{}|switch".format(self.SWITCH_CAPABILITY_TABLE)) + for action_key in dict(action_props): + key = "{}|{}".format(self.ACL_ACTIONS_CAPABILITY_FIELD, stage.upper()) + if key not in capability: + del action_props[action_key] + continue + + values = capability[key].split(",") + if action_key.upper() not in values: + del action_props[action_key] + continue + + if action_key == AclAction.PACKET: + # Check if action_value is supported + action_value = action_props[action_key] + key = "{}|{}".format(self.ACL_ACTION_CAPABILITY_FIELD, action_key.upper()) + if key not in capability: + del action_props[action_key] + continue + + if action_value not in capability[key]: + del action_props[action_key] + continue + + return action_count == len(action_props) + def convert_l2(self, table_name, rule_idx, rule): rule_props = {} @@ -510,30 +594,32 @@ def show_table(self, table_name): :param table_name: Optional. ACL table name. Filter tables by specified name. :return: """ - header = ("Name", "Type", "Binding", "Description") + header = ("Name", "Type", "Binding", "Description", "Stage") data = [] for key, val in self.get_tables_db_info().iteritems(): if table_name and key != table_name: continue + stage = val.get("stage", Stage.INGRESS).lower() + if val["type"] == AclLoader.ACL_TABLE_TYPE_CTRLPLANE: services = natsorted(val["services"]) - data.append([key, val["type"], services[0], val["policy_desc"]]) + data.append([key, val["type"], services[0], val["policy_desc"], stage]) if len(services) > 1: for service in services[1:]: - data.append(["", "", service, ""]) + data.append(["", "", service, "", ""]) else: if not val["ports"]: - data.append([key, val["type"], "", val["policy_desc"]]) + data.append([key, val["type"], "", val["policy_desc"], stage]) else: ports = natsorted(val["ports"]) - data.append([key, val["type"], ports[0], val["policy_desc"]]) + data.append([key, val["type"], ports[0], val["policy_desc"], stage]) if len(ports) > 1: for port in ports[1:]: - data.append(["", "", port, ""]) + data.append(["", "", port, "", ""]) print(tabulate.tabulate(data, headers=header, tablefmt="simple", missingval="")) @@ -586,7 +672,33 @@ def show_rule(self, table_name, rule_id): """ header = ("Table", "Rule", "Priority", "Action", "Match") - ignore_list = ["PRIORITY", "PACKET_ACTION", "MIRROR_ACTION"] + def pop_priority(val): + priority = val.pop("PRIORITY") + return priority + + def pop_action(val): + action = "" + + for key in dict(val): + key = key.upper() + if key == AclAction.PACKET: + action = val.pop(key) + elif key == AclAction.REDIRECT: + action = "REDIRECT: {}".format(val.pop(key)) + elif key in (AclAction.MIRROR, AclAction.MIRROR_INGRESS): + action = "MIRROR INGRESS: {}".format(val.pop(key)) + elif key == AclAction.MIRROR_EGRESS: + action = "MIRROR EGRESS: {}".format(val.pop(key)) + else: + continue + + return action + + def pop_matches(val): + matches = list(sorted(["%s: %s" % (k, val[k]) for k in val])) + if len(matches) == 0: + matches.append("N/A") + return matches raw_data = [] for (tname, rid), val in self.get_rules_db_info().iteritems(): @@ -597,22 +709,9 @@ def show_rule(self, table_name, rule_id): if rule_id and rule_id != rid: continue - priority = val["PRIORITY"] - - action = "" - if "PACKET_ACTION" in val: - action = val["PACKET_ACTION"] - elif "MIRROR_ACTION" in val: - action = "MIRROR: %s" % val["MIRROR_ACTION"] - else: - continue - - matches = ["%s: %s" % (k, v) for k, v in val.iteritems() if k not in ignore_list] - - matches.sort() - - if len(matches) == 0: - matches.append("N/A") + priority = pop_priority(val) + action = pop_action(val) + matches = pop_matches(val) rule_data = [[tname, rid, priority, action, matches[0]]] if len(matches) > 1: @@ -718,9 +817,10 @@ def update(ctx): @click.argument('filename', type=click.Path(exists=True)) @click.option('--table_name', type=click.STRING, required=False) @click.option('--session_name', type=click.STRING, required=False) +@click.option('--mirror_stage', type=click.Choice(["ingress", "egress"]), default="ingress") @click.option('--max_priority', type=click.INT, required=False) @click.pass_context -def full(ctx, filename, table_name, session_name, max_priority): +def full(ctx, filename, table_name, session_name, mirror_stage, max_priority): """ Full update of ACL rules configuration. If a table_name is provided, the operation will be restricted in the specified table. @@ -733,6 +833,8 @@ def full(ctx, filename, table_name, session_name, max_priority): if session_name: acl_loader.set_session_name(session_name) + acl_loader.set_mirror_stage(mirror_stage) + if max_priority: acl_loader.set_max_priority(max_priority) @@ -743,9 +845,10 @@ def full(ctx, filename, table_name, session_name, max_priority): @update.command() @click.argument('filename', type=click.Path(exists=True)) @click.option('--session_name', type=click.STRING, required=False) +@click.option('--mirror_stage', type=click.Choice(["ingress", "egress"]), default="ingress") @click.option('--max_priority', type=click.INT, required=False) @click.pass_context -def incremental(ctx, filename, session_name, max_priority): +def incremental(ctx, filename, session_name, mirror_stage, max_priority): """ Incremental update of ACL rule configuration. """ @@ -754,6 +857,8 @@ def incremental(ctx, filename, session_name, max_priority): if session_name: acl_loader.set_session_name(session_name) + acl_loader.set_mirror_stage(mirror_stage) + if max_priority: acl_loader.set_max_priority(max_priority) diff --git a/config/main.py b/config/main.py index 51ef0fc446..dae0c8b604 100755 --- a/config/main.py +++ b/config/main.py @@ -1407,7 +1407,8 @@ def get_acl_bound_ports(): @click.argument("table_type", metavar="") @click.option("-d", "--description") @click.option("-p", "--ports") -def table(table_name, table_type, description, ports): +@click.option("-s", "--stage", type=click.Choice(["ingress", "egress"]), default="ingress") +def table(table_name, table_type, description, ports, stage): """ Add ACL table """ @@ -1426,6 +1427,8 @@ def table(table_name, table_type, description, ports): else: table_info["ports@"] = ",".join(get_acl_bound_ports()) + table_info["stage"] = stage + config_db.set_entry("ACL_TABLE", table_name, table_info) # diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index 417905fcb3..c3000baaf5 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -968,20 +968,20 @@ Output from the command displays the table name, type of the table, the list of - Example: ``` admin@sonic:~$ show acl table - Name Type Binding Description - -------- --------- --------------- ------------- - EVERFLOW MIRROR Ethernet16 EVERFLOW - Ethernet96 - Ethernet108 - Ethernet112 - PortChannel0001 - PortChannel0002 - SNMP_ACL CTRLPLANE SNMP SNMP_ACL - DT_ACL_T1 L3 Ethernet0 DATA_ACL_TABLE_1 - Ethernet4 - Ethernet112 - Ethernet116 - SSH_ONLY CTRLPLANE SSH SSH_ONLY + Name Type Binding Description Stage + -------- --------- --------------- ---------------- ------- + EVERFLOW MIRROR Ethernet16 EVERFLOW ingress + Ethernet96 + Ethernet108 + Ethernet112 + PortChannel0001 + PortChannel0002 + SNMP_ACL CTRLPLANE SNMP SNMP_ACL ingress + DT_ACL_T1 L3 Ethernet0 DATA_ACL_TABLE_1 egress + Ethernet4 + Ethernet112 + Ethernet116 + SSH_ONLY CTRLPLANE SSH SSH_ONLY ingress ``` @@ -992,7 +992,19 @@ Output from the command gives the following information about the rules 1) Table name - ACL table name to which the rule belongs to. 2) Rule name - ACL rule name 3) Priority - Priority for this rule. -4) Action - Action to be performed if the packet matches with this ACL rule. It could be either Drop or Permit. Users can choose to have a default permit rule or default deny rule. In case of default "deny all" rule, add the permitted rules on top of the deny rule. In case of the default "permit all" rule, users can add the deny rules on top of it. If users have not confgured any rule, SONiC allows all traffic (which is "permit all"). +4) Action - Action to be performed if the packet matches with this ACL rule. + +It can be: +- "DROP"/"FORWARD"("ACCEPT" for control plane ACL) +- "REDIRECT: redirect-object" for redirect rule, where "redirect-object" is either: + - physical interface name, e.g. "Ethernet10" + - port channel name, e.g. "PortChannel0002" + - next-hop IP address, e.g. "10.0.0.1" + - next-hop group set of IP addresses with comma seperator, e.g. "10.0.0.1,10.0.0.3" +- "MIRROR INGRESS|EGRESS: session-name" for mirror rules, where "session-name" refers to mirror session + +Users can choose to have a default permit rule or default deny rule. In case of default "deny all" rule, add the permitted rules on top of the deny rule. In case of the default "permit all" rule, users can add the deny rules on top of it. If users have not confgured any rule, SONiC allows all traffic (which is "permit all"). + 5) Match - The fields from the packet header that need to be matched against the same present in the incoming traffic. - Usage: @@ -1002,15 +1014,19 @@ Output from the command gives the following information about the rules - Example: ``` admin@sonic:~$ show acl rule - Table Rule Priority Action Match - -------- ------------ ---------- -------- ------------------ - SNMP_ACL RULE_1 9999 ACCEPT IP_PROTOCOL: 17 - SRC_IP: 1.1.1.1/32 - SSH_ONLY RULE_1 9999 ACCEPT IP_PROTOCOL: 6 - SRC_IP: 1.1.1.1/32 - SNMP_ACL DEFAULT_RULE 1 DROP ETHER_TYPE: 2048 - SSH_ONLY DEFAULT_RULE 1 DROP ETHER_TYPE: 2048 - + Table Rule Priority Action Match + -------- ------------ ---------- ------------------------- ---------------------------- + SNMP_ACL RULE_1 9999 ACCEPT IP_PROTOCOL: 17 + SRC_IP: 1.1.1.1/32 + SSH_ONLY RULE_2 9998 ACCEPT IP_PROTOCOL: 6 + SRC_IP: 1.1.1.1/32 + EVERFLOW RULE_3 9997 MIRROR INGRESS: everflow0 SRC_IP: 20.0.0.2/32 + EVERFLOW RULE_4 9996 MIRROR EGRESS : everflow1 L4_SRC_PORT: 4621 + DATAACL RULE_5 9995 REDIRECT: Ethernet8 IP_PROTOCOL: 126 + DATAACL RULE_6 9994 FORWARD L4_SRC_PORT: 179 + DATAACL RULE_7 9993 FORWARD L4_DST_PORT: 179 + SNMP_ACL DEFAULT_RULE 1 DROP ETHER_TYPE: 2048 + SSH_ONLY DEFAULT_RULE 1 DROP ETHER_TYPE: 2048 ``` @@ -1040,6 +1056,8 @@ This command updates only the ACL rules and it does not disturb the ACL tables; When "--session_name" optional argument is specified, command sets the session_name for the ACL table with this mirror session name. It fails if the specified mirror session name does not exist. +When "--mirror_stage" optional argument is specified, command sets the mirror action to ingress/egress based on this parameter. By default command sets ingress mirror action in case argument is not specified. + When the optional argument "max_priority" is specified, each rule’s priority is calculated by subtracting its “sequence_id” value from the “max_priority”. If this value is not passed, the default “max_priority” 10000 is used. - Usage: @@ -1047,7 +1065,8 @@ When the optional argument "max_priority" is specified, each rule’s priority Some of the possible options are 1) --table_name , Example: config acl update full " --table_name DT_ACL_T1 /etc/sonic/acl_table_1.json " 2) --session_name , Example: config acl update full " --session_name mirror_ses1 /etc/sonic/acl_table_1.json " - 3) --max_priority , Example: config acl update full " --max-priority 100 /etc/sonic/acl_table_1.json " + 3) --mirror_stage ingress|egress, Example: config acl update full " --mirror_stage egress /etc/sonic/acl_table_1.json " + 4) --max_priority , Example: config acl update full " --max-priority 100 /etc/sonic/acl_table_1.json " NOTE: All these optional parameters should be inside the double quotes. If none of the options are provided, double quotes is not required for specifying filename alone. Any number of optional parameters can be configured in the same command. @@ -1081,13 +1100,16 @@ Note that "incremental" is working like "full". When "--session_name" optional argument is specified, command sets the session_name for the ACL table with this mirror session name. It fails if the specified mirror session name does not exist. +When "--mirror_stage" optional argument is specified, command sets the mirror action to ingress/egress based on this parameter. By default command sets ingress mirror action in case argument is not specified. + When the optional argument "max_priority" is specified, each rule’s priority is calculated by subtracting its “sequence_id” value from the “max_priority”. If this value is not passed, the default “max_priority” 10000 is used. - Usage: config acl update incremental FILE_NAME Some of the possible options are 1) --session_name , Example: config acl update full " --session_name mirror_ses1 /etc/sonic/acl_table_1.json " - 2) --max-priority , Example: config acl update full " --max-priority 100 /etc/sonic/acl_table_1.json " + 2) --mirror_stage ingress|egress, Example: config acl update full " --mirror_stage egress /etc/sonic/acl_table_1.json " + 3) --max-priority , Example: config acl update full " --max-priority 100 /etc/sonic/acl_table_1.json " NOTE: All these optional parameters should be inside the double quotes. If none of the options are provided, double quotes is not required for specifying filename alone. Any number of optional parameters can be configured in the same command. diff --git a/sonic-utilities-tests/acl_loader_test.py b/sonic-utilities-tests/acl_loader_test.py index 8b13f73e89..ae98cd9571 100644 --- a/sonic-utilities-tests/acl_loader_test.py +++ b/sonic-utilities-tests/acl_loader_test.py @@ -1,7 +1,6 @@ import sys import os import pytest -from unittest import TestCase test_path = os.path.dirname(os.path.abspath(__file__)) modules_path = os.path.dirname(test_path) @@ -10,7 +9,7 @@ from acl_loader import * from acl_loader.main import * -class TestAclLoader(TestCase): +class TestAclLoader(object): def setUp(self): pass @@ -25,3 +24,32 @@ def test_valid(self): def test_invalid(self): with pytest.raises(AclLoaderException): yang_acl = AclLoader.parse_acl_json(os.path.join(test_path, 'acl_input/acl2.json')) + + def test_validate_mirror_action(self): + ingress_mirror_rule_props = { + "MIRROR_INGRESS_ACTION": "everflow0" + } + + egress_mirror_rule_props = { + "mirror_egress_action": "everflow0" + } + + acl_loader = AclLoader() + # switch capability taken from mock_tables/state_db.json SWITCH_CAPABILITY table + assert acl_loader.validate_actions("EVERFLOW", ingress_mirror_rule_props) + assert not acl_loader.validate_actions("EVERFLOW", egress_mirror_rule_props) + + assert not acl_loader.validate_actions("EVERFLOW_EGRESS", ingress_mirror_rule_props) + assert acl_loader.validate_actions("EVERFLOW_EGRESS", egress_mirror_rule_props) + + forward_packet_action = { + "PACKET_ACTION": "FORWARD" + } + + drop_packet_action = { + "PACKET_ACTION": "DROP" + } + + # switch capability taken from mock_tables/state_db.json SWITCH_CAPABILITY table + assert acl_loader.validate_actions("DATAACL", forward_packet_action) + assert not acl_loader.validate_actions("DATAACL", drop_packet_action) diff --git a/sonic-utilities-tests/aclshow_test.py b/sonic-utilities-tests/aclshow_test.py index b7a62e1202..f93d2ba32c 100644 --- a/sonic-utilities-tests/aclshow_test.py +++ b/sonic-utilities-tests/aclshow_test.py @@ -67,7 +67,7 @@ # Expected output for aclshow -r RULE_4,RULE_6 -vv rule4_rule6_verbose_output = '' + \ """Reading ACL info... -Total number of ACL Tables: 4 +Total number of ACL Tables: 5 Total number of ACL Rules: 10 RULE NAME TABLE NAME PRIO PACKETS COUNT BYTES COUNT @@ -211,4 +211,4 @@ def test_all_after_clear(): assert test.result.getvalue() == clear_output nullify_on_start, nullify_on_exit = False, True test = Aclshow(nullify_on_start, nullify_on_exit, all=True, clear=False, rules=None, tables=None, verbose=None) - assert test.result.getvalue() == all_after_clear_output \ No newline at end of file + assert test.result.getvalue() == all_after_clear_output diff --git a/sonic-utilities-tests/mock_tables/config_db.json b/sonic-utilities-tests/mock_tables/config_db.json index 68c6171f63..6cb8e981c7 100644 --- a/sonic-utilities-tests/mock_tables/config_db.json +++ b/sonic-utilities-tests/mock_tables/config_db.json @@ -101,6 +101,12 @@ "ports@": "PortChannel0002,PortChannel0005,PortChannel0008,PortChannel0011,PortChannel0014,PortChannel0017,PortChannel0020,PortChannel0023,Ethernet100,Ethernet104,Ethernet92,Ethernet96,Ethernet84,Ethernet88,Ethernet76,Ethernet80,Ethernet108,Ethernet112,Ethernet64,Ethernet120,Ethernet116,Ethernet124,Ethernet72,Ethernet68", "type": "MIRROR" }, + "ACL_TABLE|EVERFLOW_EGRESS": { + "policy_desc": "EGRESS EVERFLOW", + "ports@": "PortChannel0002,PortChannel0005,PortChannel0008,PortChannel0011,PortChannel0014,PortChannel0017,PortChannel0020,PortChannel0023,Ethernet100,Ethernet104,Ethernet92,Ethernet96,Ethernet84,Ethernet88,Ethernet76,Ethernet80,Ethernet108,Ethernet112,Ethernet64,Ethernet120,Ethernet116,Ethernet124,Ethernet72,Ethernet68", + "type": "MIRROR", + "stage": "egress" + }, "ACL_TABLE|SNMP_ACL": { "policy_desc": "SNMP_ACL", "services@": "SNMP", diff --git a/sonic-utilities-tests/mock_tables/state_db.json b/sonic-utilities-tests/mock_tables/state_db.json index f44f9de4f3..67b0567234 100644 --- a/sonic-utilities-tests/mock_tables/state_db.json +++ b/sonic-utilities-tests/mock_tables/state_db.json @@ -58,6 +58,13 @@ "PSU_INFO|PSU 2": { "presence": "true", "status": "true" + }, + "SWITCH_CAPABILITY|switch": { + "MIRROR": "true", + "MIRRORV6": "true", + "ACL_ACTIONS|INGRESS": "PACKET_ACTION,REDIRECT_ACTION,MIRROR_INGRESS_ACTION", + "ACL_ACTIONS|EGRESS": "PACKET_ACTION,MIRROR_EGRESS_ACTION", + "ACL_ACTION|PACKET_ACTION": "FORWARD" } }