From 6ccd166d23abf78faf484d6242aefc0f36ce832a Mon Sep 17 00:00:00 2001 From: "Ravi [Marvell]" Date: Tue, 6 Jun 2023 02:33:45 +0530 Subject: [PATCH] [acl-loader] Support for ACL table type L3V4V6 (#2794) Support a new ACL table type called L3V4V6. This table supports both v4 and v6 Match types. Add unit tests for this new ACL table type. HLD: sonic-net/SONiC#1267 --- acl_loader/main.py | 57 +++++++-- doc/Command-Reference.md | 2 +- tests/acl_input/acl1.json | 84 ++++++++++++++ .../illegal_v4v6_rule_no_ethertype.json | 109 ++++++++++++++++++ tests/acl_loader_test.py | 44 ++++++- tests/aclshow_test.py | 2 +- tests/mock_tables/config_db.json | 11 ++ 7 files changed, 299 insertions(+), 10 deletions(-) create mode 100644 tests/acl_input/illegal_v4v6_rule_no_ethertype.json diff --git a/acl_loader/main.py b/acl_loader/main.py index ff5d22f0078..72618674125 100644 --- a/acl_loader/main.py +++ b/acl_loader/main.py @@ -94,7 +94,7 @@ class AclLoader(object): "ETHERTYPE_LLDP": 0x88CC, "ETHERTYPE_VLAN": 0x8100, "ETHERTYPE_ROCE": 0x8915, - "ETHERTYPE_ARP": 0x0806, + "ETHERTYPE_ARP": 0x0806, "ETHERTYPE_IPV4": 0x0800, "ETHERTYPE_IPV6": 0x86DD, "ETHERTYPE_MPLS": 0x8847 @@ -261,7 +261,7 @@ def read_acl_object_status_info(self, cfg_db_table_name, state_db_table_name): else: state_db_info = self.statedb.get_all(self.statedb.STATE_DB, "{}|{}".format(state_db_table_name, state_db_key)) status[key]['status'] = state_db_info.get("status", "N/A") if state_db_info else "N/A" - + return status def get_sessions_db_info(self): @@ -346,6 +346,14 @@ def is_table_l3v6(self, tname): """ return self.tables_db_info[tname]["type"].upper() == "L3V6" + def is_table_l3v4v6(self, tname): + """ + Check if ACL table type is L3V4V6 + :param tname: ACL table name + :return: True if table type is L3V4V6 else False + """ + return self.tables_db_info[tname]["type"].upper() == "L3V4V6" + def is_table_l3(self, tname): """ Check if ACL table type is L3 @@ -509,6 +517,17 @@ def convert_ip(self, table_name, rule_idx, rule): # "IP_ICMP" we need to pick the correct protocol number for the IP version if rule.ip.config.protocol == "IP_ICMP" and self.is_table_ipv6(table_name): rule_props["IP_PROTOCOL"] = self.ip_protocol_map["IP_ICMPV6"] + elif rule.ip.config.protocol == "IP_ICMP" and self.is_table_l3v4v6(table_name): + # For L3V4V6 tables, both ICMP and ICMPv6 are supported, + # so find the IP_PROTOCOL using the ether_type. + try: + ether_type = rule.l2.config.ethertype + except Exception as e: + ether_type = None + if rule.l2.config.ethertype == "ETHERTYPE_IPV6": + rule_props["IP_PROTOCOL"] = self.ip_protocol_map["IP_ICMPV6"] + else: + rule_props["IP_PROTOCOL"] = self.ip_protocol_map[rule.ip.config.protocol] else: rule_props["IP_PROTOCOL"] = self.ip_protocol_map[rule.ip.config.protocol] else: @@ -544,9 +563,20 @@ def convert_ip(self, table_name, rule_idx, rule): def convert_icmp(self, table_name, rule_idx, rule): rule_props = {} - is_table_v6 = self.is_table_ipv6(table_name) - type_key = "ICMPV6_TYPE" if is_table_v6 else "ICMP_TYPE" - code_key = "ICMPV6_CODE" if is_table_v6 else "ICMP_CODE" + is_rule_v6 = False + if self.is_table_ipv6(table_name): + is_rule_v6 = True + elif self.is_table_l3v4v6(table_name): + # get the IP version type using Ether-Type. + try: + ether_type = rule.l2.config.ethertype + if ether_type == "ETHERTYPE_IPV6": + is_rule_v6 = True + except Exception as e: + pass + + type_key = "ICMPV6_TYPE" if is_rule_v6 else "ICMP_TYPE" + code_key = "ICMPV6_CODE" if is_rule_v6 else "ICMP_CODE" if rule.icmp.config.type != "" and rule.icmp.config.type != "null": icmp_type = rule.icmp.config.type @@ -651,7 +681,18 @@ def convert_rule_to_db_schema(self, table_name, rule): rule_props["PRIORITY"] = str(self.max_priority - rule_idx) # setup default ip type match to dataplane acl (could be overriden by rule later) - if self.is_table_l3v6(table_name): + if self.is_table_l3v4v6(table_name): + # ETHERTYPE must be passed and it should be one of IPv4 or IPv6 + try: + ether_type = rule.l2.config.ethertype + except Exception as e: + raise AclLoaderException("l2:ethertype must be provided for rule #{} in table:{} of type L3V4V6".format(rule_idx, table_name)) + if ether_type not in ["ETHERTYPE_IPV4", "ETHERTYPE_IPV6"]: + # Ether type must be v4 or v6 to match IP fields, L4 (TCP/UDP) fields or ICMP fields + if rule.ip or rule.transport: + raise AclLoaderException("ethertype={} is neither ETHERTYPE_IPV4 nor ETHERTYPE_IPV6 for IP rule #{} in table:{} type L3V4V6".format(rule.l2.config.ethertype, rule_idx, table_name)) + rule_props["ETHER_TYPE"] = str(self.ethertype_map[ether_type]) + elif self.is_table_l3v6(table_name): rule_props["IP_TYPE"] = "IPV6ANY" # ETHERTYPE is not supported for DATAACLV6 elif self.is_table_l3(table_name): rule_props["ETHER_TYPE"] = str(self.ethertype_map["ETHERTYPE_IPV4"]) @@ -682,6 +723,8 @@ def deny_rule(self, table_name): rule_props["IP_TYPE"] = "IPV6ANY" # ETHERTYPE is not supported for DATAACLV6 elif self.is_table_l3(table_name): rule_props["ETHER_TYPE"] = str(self.ethertype_map["ETHERTYPE_IPV4"]) + elif self.is_table_l3v4v6(table_name): + rule_props["IP_TYPE"] = "IP" # Drop both v4 and v6 packets else: return {} # Don't add default deny rule if table is not [L3, L3V6] return rule_data @@ -835,7 +878,7 @@ def show_table(self, table_name): for key, val in self.get_tables_db_info().items(): if table_name and key != table_name: continue - + stage = val.get("stage", Stage.INGRESS).lower() # Get ACL table status from STATE_DB if key in self.acl_table_status: diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index 316e8cb86e2..3d6f4bade11 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -1861,7 +1861,7 @@ This command is used to create new ACL tables. - Parameters: - table_name: The name of the ACL table to create. - - table_type: The type of ACL table to create (e.g. "L3", "L3V6", "MIRROR") + - table_type: The type of ACL table to create (e.g. "L3", "L3V6", "L3V4V6", "MIRROR") - description: A description of the table for the user. (default is the table_name) - ports: A comma-separated list of ports/interfaces to add to the table. The behavior is as follows: - Physical ports will be bound as physical ports diff --git a/tests/acl_input/acl1.json b/tests/acl_input/acl1.json index 177d7cb227a..4bcd8049be2 100644 --- a/tests/acl_input/acl1.json +++ b/tests/acl_input/acl1.json @@ -316,6 +316,90 @@ "config": { "name": "bmc_acl_northbound_v6" } + }, + "DATAACLV4V6": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "l2": { + "config": { + "vlan-id": "369", + "ethertype": "ETHERTYPE_IPV4" + } + }, + "ip": { + "config": { + "protocol": "IP_TCP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + } + }, + "2": { + "config": { + "sequence-id": 2 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "l2": { + "config": { + "ethertype": "ETHERTYPE_IPV6" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "::1/128", + "destination-ip-address": "::1/128" + } + }, + "icmp": { + "config": { + "type": "1", + "code": "0" + } + } + }, + "3": { + "config": { + "sequence-id": 3 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "l2": { + "config": { + "ethertype": "ETHERTYPE_IPV6" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "::1/128", + "destination-ip-address": "::1/128" + } + }, + "icmp": { + "config": { + "type": "128" + } + } + } + } + } } } } diff --git a/tests/acl_input/illegal_v4v6_rule_no_ethertype.json b/tests/acl_input/illegal_v4v6_rule_no_ethertype.json new file mode 100644 index 00000000000..acf30773ce6 --- /dev/null +++ b/tests/acl_input/illegal_v4v6_rule_no_ethertype.json @@ -0,0 +1,109 @@ +{ + "acl": { + "acl-sets": { + "acl-set": { + "DATAACLV4V6": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "ip": { + "config": { + "protocol": "IP_TCP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + } + }, + "2": { + "config": { + "sequence-id": 2 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "l2": { + "config": { + "ethertype": "ETHERTYPE_IPV6" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "::1/128", + "destination-ip-address": "::1/128" + } + }, + "icmp": { + "config": { + "type": "1", + "code": "0" + } + } + }, + "3": { + "config": { + "sequence-id": 3 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "::1/128", + "destination-ip-address": "::1/128" + } + }, + "icmp": { + "config": { + "type": "1" + } + } + }, + "4": { + "config": { + "sequence-id": 2 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "l2": { + "config": { + "ethertype": "ETHERTYPE_IPV4" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + }, + "icmp": { + "config": { + "type": "1", + "code": "0" + } + } + } + } + } + } + } + } + } +} diff --git a/tests/acl_loader_test.py b/tests/acl_loader_test.py index 37f64307334..adcf38fe37f 100644 --- a/tests/acl_loader_test.py +++ b/tests/acl_loader_test.py @@ -21,7 +21,7 @@ def test_acl_empty(self): def test_valid(self): yang_acl = AclLoader.parse_acl_json(os.path.join(test_path, 'acl_input/acl1.json')) - assert len(yang_acl.acl.acl_sets.acl_set) == 8 + assert len(yang_acl.acl.acl_sets.acl_set) == 9 def test_invalid(self): with pytest.raises(AclLoaderException): @@ -95,6 +95,42 @@ def test_ethertype_translation(self, acl_loader): "PRIORITY": "9997" } + def test_v4_rule_inv4v6_table(self, acl_loader): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl1.json')) + assert acl_loader.rules_info[("DATAACLV4V6", "RULE_1")] + assert acl_loader.rules_info[("DATAACLV4V6", "RULE_1")] == { + "VLAN_ID": 369, + "ETHER_TYPE": 2048, + "IP_PROTOCOL": 6, + "SRC_IP": "20.0.0.2/32", + "DST_IP": "30.0.0.3/32", + "PACKET_ACTION": "FORWARD", + "PRIORITY": "9999" + } + + def test_v6_rule_inv4v6_table(self, acl_loader): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl1.json')) + assert acl_loader.rules_info[("DATAACLV4V6", "RULE_2")] + assert acl_loader.rules_info[("DATAACLV4V6", "RULE_2")] == { + "ETHER_TYPE": 34525, + "IP_PROTOCOL": 58, + "SRC_IPV6": "::1/128", + "DST_IPV6": "::1/128", + "PACKET_ACTION": "FORWARD", + "PRIORITY": "9998", + 'ICMPV6_CODE': 0, + 'ICMPV6_TYPE': 1 + } + + def test_rule_without_ethertype_inv4v6(self, acl_loader): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/illegal_v4v6_rule_no_ethertype.json')) + assert not acl_loader.rules_info.get(("DATAACLV4V6", "RULE_1")) + assert acl_loader.rules_info[("DATAACLV4V6", "RULE_2")] + assert not acl_loader.rules_info.get(("DATAACLV4V6", "RULE_3")) + def test_icmp_translation(self, acl_loader): acl_loader.rules_info = {} acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl1.json')) @@ -151,6 +187,12 @@ def test_ingress_default_deny_rule(self, acl_loader): 'PACKET_ACTION': 'DROP', 'IP_TYPE': 'IPV6ANY' } + assert acl_loader.rules_info[('DATAACLV4V6', 'DEFAULT_RULE')] == { + 'PRIORITY': '1', + 'PACKET_ACTION': 'DROP', + 'IP_TYPE': 'IP' + } + # Verify acl-loader doesn't add default deny rule to MIRROR assert ('EVERFLOW', 'DEFAULT_RULE') not in acl_loader.rules_info # Verify acl-loader doesn't add default deny rule to MIRRORV6 diff --git a/tests/aclshow_test.py b/tests/aclshow_test.py index 8e2d20cbf18..94615e54434 100644 --- a/tests/aclshow_test.py +++ b/tests/aclshow_test.py @@ -90,7 +90,7 @@ # Expected output for aclshow -r RULE_4,RULE_6 -vv rule4_rule6_verbose_output = '' + \ """Reading ACL info... -Total number of ACL Tables: 15 +Total number of ACL Tables: 16 Total number of ACL Rules: 21 RULE NAME TABLE NAME PRIO PACKETS COUNT BYTES COUNT diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index 5cf11f9f669..538f81d6058 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -556,6 +556,17 @@ "type": "L3", "stage": "ingress" }, + "ACL_TABLE|DATAACLV4V6": { + "expireat": 1602451533.237415, + "ttl": -0.001, + "type": "hash", + "value": { + "policy_desc": "DATAACLV4V6", + "ports@": "PortChannel0002,PortChannel0005,PortChannel0008,PortChannel0011,PortChannel0014,PortChannel0017,PortChannel0020,PortChannel0023,Ethernet64,Ethernet68,Ethernet72,Ethernet76,Ethernet80,Ethernet84,Ethernet88,Ethernet92,Ethernet96,Ethernet100,Ethernet104,Ethernet108,Ethernet112,Ethernet116,Ethernet120,Ethernet124", + "stage": "ingress", + "type": "L3V4V6" + } + }, "ACL_TABLE|EVERFLOW": { "policy_desc": "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",