From 8cc5f20aa055624bd01890462b65c4e500f75a2e Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Sun, 30 Jun 2019 16:48:42 +0300 Subject: [PATCH 01/14] [acl-loader] output stage in show table Signed-off-by: Stepan Blyschak --- acl_loader/main.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/acl_loader/main.py b/acl_loader/main.py index c633b853ec..7bbc045d46 100644 --- a/acl_loader/main.py +++ b/acl_loader/main.py @@ -507,7 +507,7 @@ 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(): @@ -516,21 +516,21 @@ def show_table(self, table_name): 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"], val.get("stage", "ingress")]) 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"], val.get("stage", "ingress")]) 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"], val.get("stage", "ingress")]) if len(ports) > 1: for port in ports[1:]: - data.append(["", "", port, ""]) + data.append(["", "", port, "", ""]) print(tabulate.tabulate(data, headers=header, tablefmt="simple", missingval="")) @@ -599,7 +599,7 @@ def show_rule(self, table_name, rule_id): if "PACKET_ACTION" in val: action = val["PACKET_ACTION"] elif "MIRROR_ACTION" in val: - action = "MIRROR: %s" % val["MIRROR_ACTION"] + action = "MIRROR: {}".format(val["MIRROR_ACTION"]) else: continue From 6084fcf05a594e68c4438723ba7797c74c87e25b Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Sun, 30 Jun 2019 18:56:06 +0300 Subject: [PATCH 02/14] [acl-loader] add support for mirror action stage and action support check Signed-off-by: Stepan Blyschak --- acl_loader/main.py | 47 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/acl_loader/main.py b/acl_loader/main.py index 7bbc045d46..a3163bacba 100644 --- a/acl_loader/main.py +++ b/acl_loader/main.py @@ -52,6 +52,7 @@ class AclLoader(object): STATE_MIRROR_SESSION_TABLE = "MIRROR_SESSION_TABLE" POLICER = "POLICER" SESSION_PREFIX = "everflow" + SWITCH_CAPABILITY_TABLE = "SWITCH_CAPABILITY" min_priority = 1 max_priority = 10000 @@ -81,6 +82,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 = {} @@ -88,7 +90,7 @@ def __init__(self): self.sessions_db_info = {} self.configdb = ConfigDBConnector() self.configdb.connect() - self.statedb = SonicV2Connector(host="127.0.0.1") + self.statedb = SonicV2Connector() self.statedb.connect(self.statedb.STATE_DB) self.read_tables_info() @@ -176,6 +178,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 + def set_max_priority(self, priority): """ Set rules max priority @@ -235,7 +245,7 @@ def convert_action(self, table_name, rule_idx, rule): if not session_name: raise AclLoaderException("Mirroring session does not exist") - rule_props["MIRROR_ACTION"] = session_name + rule_props["MIRROR_ACTION"] = "{}:{}".format(self.mirror_stage, session_name) else: rule_props["PACKET_ACTION"] = "FORWARD" elif rule.actions.config.forwarding_action == "DROP": @@ -243,11 +253,32 @@ def convert_action(self, table_name, rule_idx, rule): elif rule.actions.config.forwarding_action == "REJECT": rule_props["PACKET_ACTION"] = "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_action(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_action(self, table_name, 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", "ingress") + capability = self.statedb.get_all(self.statedb.STATE_DB, "{}|switch".format(self.SWITCH_CAPABILITY_TABLE)) + for action_key in action_props: + key = "ACL_ACTION|{}|{}".format(stage.upper(), action_key.upper()) + if key not in capability: + return False + + values = capability[key] + if action_props[action_key].split(":")[0].upper() not in values: + return False + + return True + def convert_l2(self, table_name, rule_idx, rule): rule_props = {} @@ -714,9 +745,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. @@ -729,6 +761,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) @@ -739,9 +773,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. """ @@ -750,6 +785,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) From 91d93db95cd9c393655ca7934eba082afdf115e8 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Mon, 1 Jul 2019 12:11:39 +0300 Subject: [PATCH 03/14] [acl_loader_test] add UT for validate_action methodw Signed-off-by: Stepan Blyschak --- sonic-utilities-tests/acl_loader_test.py | 22 +++++++++++++++++-- .../mock_tables/config_db.json | 6 +++++ .../mock_tables/state_db.json | 8 +++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/sonic-utilities-tests/acl_loader_test.py b/sonic-utilities-tests/acl_loader_test.py index 8b13f73e89..42187f1f21 100644 --- a/sonic-utilities-tests/acl_loader_test.py +++ b/sonic-utilities-tests/acl_loader_test.py @@ -1,16 +1,17 @@ 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) sys.path.insert(0, modules_path) +import mock_tables.dbconnector + from acl_loader import * from acl_loader.main import * -class TestAclLoader(TestCase): +class TestAclLoader(object): def setUp(self): pass @@ -25,3 +26,20 @@ 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_ACTION": "ingress:everflow0" + } + + egress_mirror_rule_props = { + "MIRROR_ACTION": "egress:everflow0" + } + + acl_loader = AclLoader() + # switch capability taken from mock_tables/state_db.json SWITCH_CAPABILITY table + assert acl_loader.validate_action("EVERFLOW", ingress_mirror_rule_props) + assert not acl_loader.validate_action("EVERFLOW", egress_mirror_rule_props) + + assert not acl_loader.validate_action("EVERFLOW_EGRESS", ingress_mirror_rule_props) + assert acl_loader.validate_action("EVERFLOW_EGRESS", egress_mirror_rule_props) 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..ab682df7c3 100644 --- a/sonic-utilities-tests/mock_tables/state_db.json +++ b/sonic-utilities-tests/mock_tables/state_db.json @@ -58,6 +58,14 @@ "PSU_INFO|PSU 2": { "presence": "true", "status": "true" + }, + "SWITCH_CAPABILITY|switch": { + "MIRROR": "true", + "MIRRORV6": "true", + "ACL_ACTION|INGRESS|PACKET_ACTION": "DROP,FORWARD,REDIRECT", + "ACL_ACTION|EGRESS|PACKET_ACTION": "DROP,FORWARD", + "ACL_ACTION|INGRESS|MIRROR_ACTION": "INGRESS", + "ACL_ACTION|EGRESS|MIRROR_ACTION": "EGRESS" } } From 08c7496da99887d852609b3eae09f34be879c8ee Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Tue, 2 Jul 2019 18:02:15 +0300 Subject: [PATCH 04/14] [acl-loader] new schema Signed-off-by: Stepan Blyschak --- acl_loader/main.py | 49 ++++++++++++------- sonic-utilities-tests/acl_loader_test.py | 4 +- sonic-utilities-tests/aclshow_test.py | 4 +- .../mock_tables/state_db.json | 6 +-- 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/acl_loader/main.py b/acl_loader/main.py index a3163bacba..b0e61ea96d 100644 --- a/acl_loader/main.py +++ b/acl_loader/main.py @@ -90,7 +90,7 @@ def __init__(self): self.sessions_db_info = {} self.configdb = ConfigDBConnector() self.configdb.connect() - self.statedb = SonicV2Connector() + self.statedb = SonicV2Connector(host='127.0.0.1') self.statedb.connect(self.statedb.STATE_DB) self.read_tables_info() @@ -245,7 +245,8 @@ def convert_action(self, table_name, rule_idx, rule): if not session_name: raise AclLoaderException("Mirroring session does not exist") - rule_props["MIRROR_ACTION"] = "{}:{}".format(self.mirror_stage, session_name) + mirror_action_name = "MIRROR_ACTION:{}".format(self.mirror_stage.upper()) + rule_props[mirror_action_name] = "{}".format(session_name) else: rule_props["PACKET_ACTION"] = "FORWARD" elif rule.actions.config.forwarding_action == "DROP": @@ -269,12 +270,12 @@ def validate_action(self, table_name, action_props): stage = self.tables_db_info[table_name].get("stage", "ingress") capability = self.statedb.get_all(self.statedb.STATE_DB, "{}|switch".format(self.SWITCH_CAPABILITY_TABLE)) for action_key in action_props: - key = "ACL_ACTION|{}|{}".format(stage.upper(), action_key.upper()) + key = "ACL_ACTION|{}".format(stage.upper()) if key not in capability: return False - values = capability[key] - if action_props[action_key].split(":")[0].upper() not in values: + values = capability[key].split(",") + if action_key.upper() not in values: return False return True @@ -613,7 +614,29 @@ def show_rule(self, table_name, rule_id): """ header = ("Table", "Rule", "Priority", "Action", "Match") - ignore_list = ["PRIORITY", "PACKET_ACTION", "MIRROR_ACTION"] + def show_priority(val): + priority = val.pop("PRIORITY") + + def show_action(val): + action = "" + + for key in dict(val): + key = key.upper() + if key == "PACKET_ACTION": + action = val.pop(key) + elif key == "REDIRECT_ACTION": + action = "REDIRECT: {}".format(val.pop(key)) + elif key in ("MIRROR_ACTION", "MIRROR_INGRESS_ACTION"): + action = "MIRROR INGRESS: {}".format(val.pop(key)) + elif key == "MIRROR_ACTION:EGRESS": + action = "MIRROR EGRESS: {}".format(val.pop(key)) + else: + continue + + return action + + def show_matches(val): + return ["%s: %s" % (k, val[k]) for k in val] raw_data = [] for (tname, rid), val in self.get_rules_db_info().iteritems(): @@ -624,17 +647,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: {}".format(val["MIRROR_ACTION"]) - else: - continue - - matches = ["%s: %s" % (k, v) for k, v in val.iteritems() if k not in ignore_list] + priority = show_priority(val) + action = show_action(val) + matches = show_matches(val) matches.sort() diff --git a/sonic-utilities-tests/acl_loader_test.py b/sonic-utilities-tests/acl_loader_test.py index 42187f1f21..33b7cded41 100644 --- a/sonic-utilities-tests/acl_loader_test.py +++ b/sonic-utilities-tests/acl_loader_test.py @@ -29,11 +29,11 @@ def test_invalid(self): def test_validate_mirror_action(self): ingress_mirror_rule_props = { - "MIRROR_ACTION": "ingress:everflow0" + "MIRROR_ACTION:INGRESS": "everflow0" } egress_mirror_rule_props = { - "MIRROR_ACTION": "egress:everflow0" + "mirror_action:egress": "everflow0" } acl_loader = AclLoader() 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/state_db.json b/sonic-utilities-tests/mock_tables/state_db.json index ab682df7c3..681e708435 100644 --- a/sonic-utilities-tests/mock_tables/state_db.json +++ b/sonic-utilities-tests/mock_tables/state_db.json @@ -62,10 +62,8 @@ "SWITCH_CAPABILITY|switch": { "MIRROR": "true", "MIRRORV6": "true", - "ACL_ACTION|INGRESS|PACKET_ACTION": "DROP,FORWARD,REDIRECT", - "ACL_ACTION|EGRESS|PACKET_ACTION": "DROP,FORWARD", - "ACL_ACTION|INGRESS|MIRROR_ACTION": "INGRESS", - "ACL_ACTION|EGRESS|MIRROR_ACTION": "EGRESS" + "ACL_ACTION|INGRESS": "PACKET_ACTION,REDIRECT_ACTION,MIRROR_ACTION:INGRESS", + "ACL_ACTION|EGRESS": "PACKET_ACTION,MIRROR_ACTION:EGRESS" } } From c8beb8d863d7da785052281d8ccc2e1bcc76a797 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Wed, 3 Jul 2019 12:08:53 +0300 Subject: [PATCH 05/14] [.gitignore] add mising build artifacts in gitignore Signed-off-by: Stepan Blyschak --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) 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 From 0ceb9d0afa87d13c98ffc6e5775b409b4a783333 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Wed, 3 Jul 2019 16:41:35 +0300 Subject: [PATCH 06/14] [acl_loader] validate packet action by value as well Signed-off-by: Stepan Blyschak --- acl_loader/main.py | 108 +++++++++++++----- sonic-utilities-tests/acl_loader_test.py | 24 +++- .../mock_tables/state_db.json | 5 +- 3 files changed, 101 insertions(+), 36 deletions(-) diff --git a/acl_loader/main.py b/acl_loader/main.py index b0e61ea96d..5113ce7a96 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 stage """ + + INGRESS = "INGRESS" + EGRESS = "EGRESS" + + class AclLoaderException(Exception): pass @@ -53,6 +78,8 @@ class AclLoader(object): 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 @@ -184,7 +211,7 @@ def set_mirror_stage(self, stage): :param session_name: stage 'ingress'/'egress' :return: """ - self.mirror_stage = stage + self.mirror_stage = stage.upper() def set_max_priority(self, priority): """ @@ -239,46 +266,70 @@ 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") - mirror_action_name = "MIRROR_ACTION:{}".format(self.mirror_stage.upper()) - rule_props[mirror_action_name] = "{}".format(session_name) + if self.mirror_stage == Stage.INGRESS: + mirror_action = AclAction.MIRROR_INGRESS_ACTION + elif self.mirror_stage == Stage.EGRES: + mirror_action = AclAction.MIRROR_EGRESS_ACTION + 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 {} in table {}, rule {}".format( rule.actions.config.forwarding_action, table_name, rule_idx)) - if not self.validate_action(table_name, rule_props): + 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_action(self, table_name, action_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", "ingress") + 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 action_props: - key = "ACL_ACTION|{}".format(stage.upper()) + for action_key in dict(action_props): + key = "{}|{}".format(self.ACL_ACTIONS_CAPABILITY_FIELD, stage.upper()) if key not in capability: - return False + del action_props[action_key] + continue values = capability[key].split(",") if action_key.upper() not in values: - return False + 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] - return True + return action_count == len(action_props) def convert_l2(self, table_name, rule_idx, rule): rule_props = {} @@ -546,19 +597,21 @@ def show_table(self, table_name): 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"], val.get("stage", "ingress")]) + data.append([key, val["type"], services[0], val["policy_desc"], stage]) if len(services) > 1: for service in services[1:]: data.append(["", "", service, "", ""]) else: if not val["ports"]: - data.append([key, val["type"], "", val["policy_desc"], val.get("stage", "ingress")]) + data.append([key, val["type"], "", val["policy_desc"], stage]) else: ports = natsorted(val["ports"]) - data.append([key, val["type"], ports[0], val["policy_desc"], val.get("stage", "ingress")]) + data.append([key, val["type"], ports[0], val["policy_desc"], stage]) if len(ports) > 1: for port in ports[1:]: @@ -616,19 +669,20 @@ def show_rule(self, table_name, rule_id): def show_priority(val): priority = val.pop("PRIORITY") + return priority def show_action(val): action = "" for key in dict(val): key = key.upper() - if key == "PACKET_ACTION": + if key == AclAction.PACKET: action = val.pop(key) - elif key == "REDIRECT_ACTION": + elif key == AclAction.REDIRECT: action = "REDIRECT: {}".format(val.pop(key)) - elif key in ("MIRROR_ACTION", "MIRROR_INGRESS_ACTION"): + elif key in (AclAction.MIRROR, AclAction.MIRROR_INGRESS): action = "MIRROR INGRESS: {}".format(val.pop(key)) - elif key == "MIRROR_ACTION:EGRESS": + elif key == AclAction.MIRROR_EGRESS: action = "MIRROR EGRESS: {}".format(val.pop(key)) else: continue @@ -636,7 +690,10 @@ def show_action(val): return action def show_matches(val): - return ["%s: %s" % (k, val[k]) for k in 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(): @@ -651,11 +708,6 @@ def show_matches(val): action = show_action(val) matches = show_matches(val) - matches.sort() - - if len(matches) == 0: - matches.append("N/A") - rule_data = [[tname, rid, priority, action, matches[0]]] if len(matches) > 1: for m in matches[1:]: diff --git a/sonic-utilities-tests/acl_loader_test.py b/sonic-utilities-tests/acl_loader_test.py index 33b7cded41..45df77e1fd 100644 --- a/sonic-utilities-tests/acl_loader_test.py +++ b/sonic-utilities-tests/acl_loader_test.py @@ -29,17 +29,29 @@ def test_invalid(self): def test_validate_mirror_action(self): ingress_mirror_rule_props = { - "MIRROR_ACTION:INGRESS": "everflow0" + "MIRROR_INGRESS_ACTION": "everflow0" } egress_mirror_rule_props = { - "mirror_action:egress": "everflow0" + "mirror_egress_action": "everflow0" } acl_loader = AclLoader() # switch capability taken from mock_tables/state_db.json SWITCH_CAPABILITY table - assert acl_loader.validate_action("EVERFLOW", ingress_mirror_rule_props) - assert not acl_loader.validate_action("EVERFLOW", egress_mirror_rule_props) + 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_action("EVERFLOW_EGRESS", ingress_mirror_rule_props) - assert acl_loader.validate_action("EVERFLOW_EGRESS", 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/mock_tables/state_db.json b/sonic-utilities-tests/mock_tables/state_db.json index 681e708435..67b0567234 100644 --- a/sonic-utilities-tests/mock_tables/state_db.json +++ b/sonic-utilities-tests/mock_tables/state_db.json @@ -62,8 +62,9 @@ "SWITCH_CAPABILITY|switch": { "MIRROR": "true", "MIRRORV6": "true", - "ACL_ACTION|INGRESS": "PACKET_ACTION,REDIRECT_ACTION,MIRROR_ACTION:INGRESS", - "ACL_ACTION|EGRESS": "PACKET_ACTION,MIRROR_ACTION:EGRESS" + "ACL_ACTIONS|INGRESS": "PACKET_ACTION,REDIRECT_ACTION,MIRROR_INGRESS_ACTION", + "ACL_ACTIONS|EGRESS": "PACKET_ACTION,MIRROR_EGRESS_ACTION", + "ACL_ACTION|PACKET_ACTION": "FORWARD" } } From 8fbf5916011c2dd0a95dfa5ac38835e130a82bbe Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 4 Jul 2019 16:51:02 +0300 Subject: [PATCH 07/14] [acl_loader] fix typos Signed-off-by: Stepan Blyschak --- acl_loader/main.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/acl_loader/main.py b/acl_loader/main.py index 5113ce7a96..efdb97d934 100644 --- a/acl_loader/main.py +++ b/acl_loader/main.py @@ -117,7 +117,7 @@ def __init__(self): self.sessions_db_info = {} self.configdb = ConfigDBConnector() self.configdb.connect() - self.statedb = SonicV2Connector(host='127.0.0.1') + self.statedb = SonicV2Connector(host="127.0.0.1") self.statedb.connect(self.statedb.STATE_DB) self.read_tables_info() @@ -273,9 +273,9 @@ def convert_action(self, table_name, rule_idx, rule): raise AclLoaderException("Mirroring session does not exist") if self.mirror_stage == Stage.INGRESS: - mirror_action = AclAction.MIRROR_INGRESS_ACTION - elif self.mirror_stage == Stage.EGRES: - mirror_action = AclAction.MIRROR_EGRESS_ACTION + 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)) From c3514695e39e66957285b50bc9a66816484f3881 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Fri, 5 Jul 2019 12:02:42 +0300 Subject: [PATCH 08/14] [doc] add documentation for egress mirroring action support Signed-off-by: Stepan Blyschak --- doc/Command-Reference.md | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index 7fbb99dceb..76c4e61b0f 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -940,20 +940,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 ``` @@ -1012,6 +1012,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: @@ -1019,7 +1021,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. @@ -1053,13 +1056,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. From e709ee9a4c01d22ed6245a94712493bc1d0171aa Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Fri, 5 Jul 2019 13:33:07 +0300 Subject: [PATCH 09/14] [doc] update acl show rule output Signed-off-by: Stepan Blyschak --- doc/Command-Reference.md | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index 76c4e61b0f..e64a6c599d 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -964,7 +964,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: @@ -974,15 +986,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 ``` From 841f9d71dbda965ab6ed227bf1be20ccbce122d8 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Fri, 5 Jul 2019 13:40:11 +0300 Subject: [PATCH 10/14] [acl_loader_test] remove unused import Signed-off-by: Stepan Blyschak --- sonic-utilities-tests/acl_loader_test.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/sonic-utilities-tests/acl_loader_test.py b/sonic-utilities-tests/acl_loader_test.py index 45df77e1fd..ae98cd9571 100644 --- a/sonic-utilities-tests/acl_loader_test.py +++ b/sonic-utilities-tests/acl_loader_test.py @@ -6,8 +6,6 @@ modules_path = os.path.dirname(test_path) sys.path.insert(0, modules_path) -import mock_tables.dbconnector - from acl_loader import * from acl_loader.main import * From 6cb194ba8b74fafb1d75a0bf6e43a138ca56c5d6 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Fri, 5 Jul 2019 13:45:53 +0300 Subject: [PATCH 11/14] [doc] fix tabs Signed-off-by: Stepan Blyschak --- doc/Command-Reference.md | 54 ++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index e64a6c599d..bdffabd16a 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -940,20 +940,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 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 + 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 ``` @@ -986,19 +986,19 @@ Users can choose to have a default permit rule or default deny rule. In case of - 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_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 + 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 ``` From ff512e367531fd379cb08522d16fb840b7c1a82b Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Fri, 5 Jul 2019 14:43:00 +0300 Subject: [PATCH 12/14] [acl-loader] change helper functions to pop_* Signed-off-by: Stepan Blyschak --- acl_loader/main.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/acl_loader/main.py b/acl_loader/main.py index efdb97d934..15d9907413 100644 --- a/acl_loader/main.py +++ b/acl_loader/main.py @@ -328,6 +328,7 @@ def validate_actions(self, table_name, action_props): if action_value not in capability[key]: del action_props[action_key] + continue return action_count == len(action_props) @@ -667,11 +668,11 @@ def show_rule(self, table_name, rule_id): """ header = ("Table", "Rule", "Priority", "Action", "Match") - def show_priority(val): + def pop_priority(val): priority = val.pop("PRIORITY") return priority - def show_action(val): + def pop_action(val): action = "" for key in dict(val): @@ -689,7 +690,7 @@ def show_action(val): return action - def show_matches(val): + def pop_matches(val): matches = list(sorted(["%s: %s" % (k, val[k]) for k in val])) if len(matches) == 0: matches.append("N/A") @@ -704,9 +705,9 @@ def show_matches(val): if rule_id and rule_id != rid: continue - priority = show_priority(val) - action = show_action(val) - matches = show_matches(val) + priority = pop_priority(val) + action = pop_action(val) + matches = pop_matches(val) rule_data = [[tname, rid, priority, action, matches[0]]] if len(matches) > 1: From ec51f6f4ec1b477b871657fbbb0f0d6c39ace166 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Tue, 27 Aug 2019 09:26:19 +0300 Subject: [PATCH 13/14] [acl-loader] fix typo Signed-off-by: Stepan Blyschak --- acl_loader/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acl_loader/main.py b/acl_loader/main.py index 15d9907413..a5f2dcf01d 100644 --- a/acl_loader/main.py +++ b/acl_loader/main.py @@ -57,7 +57,7 @@ class PacketAction: class Stage: - """ namespace for ACL stage """ + """ namespace for ACL stages """ INGRESS = "INGRESS" EGRESS = "EGRESS" From f2e4c470ac8a40c8a5cfb808c10149e5a8238f3e Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Tue, 3 Sep 2019 01:02:08 +0300 Subject: [PATCH 14/14] [config] add optional "--stage" parameter to "config acl add table" command Signed-off-by: Stepan Blyschak --- config/main.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/config/main.py b/config/main.py index 14d2e4dfa5..121b9a4c16 100755 --- a/config/main.py +++ b/config/main.py @@ -997,7 +997,7 @@ def remove(ctx, interface_name, ip_addr): ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") except ValueError: ctx.fail("'ip_addr' is not valid.") - + exists = False if if_table: interfaces = config_db.get_table(if_table) @@ -1060,7 +1060,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 """ @@ -1079,6 +1080,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) #