diff --git a/acl_loader/main.py b/acl_loader/main.py index cdd463da2..5cfbca4a5 100644 --- a/acl_loader/main.py +++ b/acl_loader/main.py @@ -72,6 +72,7 @@ class AclLoader(object): ACL_TABLE = "ACL_TABLE" ACL_RULE = "ACL_RULE" + ACL_TABLE_TYPE = "ACL_TABLE_TYPE" CFG_ACL_TABLE = "ACL_TABLE" APPL_ACL_TABLE = "ACL_TABLE_TABLE" STATE_ACL_TABLE = "ACL_TABLE_TABLE" @@ -121,6 +122,7 @@ def __init__(self): self.mirror_stage = None self.current_table = None self.tables_db_info = {} + self.tables_type_info = {} self.rules_db_info = {} self.rules_info = {} self.tables_state_info = None @@ -203,6 +205,19 @@ def read_tables_info(self): self.tables_db_info[table]['ports'] += entry.get( 'ports', []) + # Update user defined acl table types + if self.per_npu_configdb: + # On multi-asic the acl table type should be defined in both the global namespace + # and the asic namespaces but using the asic namespace definition to be consistent + # with the way tables_db_info is populated. + for ns, config_db in self.per_npu_configdb.items(): + acl_table_type = config_db.get_table(self.ACL_TABLE_TYPE) + for table_type, entry in acl_table_type.items(): + if table_type not in self.tables_type_info: + self.tables_type_info[table_type] = entry + else: + self.tables_type_info.update(self.configdb.get_table(self.ACL_TABLE_TYPE)) + if self.per_npu_configdb: # Note: Ability to read table information from APPL_DB is not yet supported for masic devices return @@ -456,6 +471,22 @@ def is_table_control_plane(self, tname): """ return self.tables_db_info[tname]['type'].upper() == self.ACL_TABLE_TYPE_CTRLPLANE + def acl_table_has_match(self, tname, match): + """ + Check if the ACL table supports matching on a given qualifier. + Non-user defined ACL table types will always return true here as we assume they + support all qualifiers + :param tname: ACL table name + :param match: ACL qualifier to query support for + :return: True if qualifier is supported or table type is not user defined + """ + table_type = self.tables_db_info[tname]["type"] + # Predefined table types aren't defined in the tables_type_info + if table_type not in self.tables_type_info: + return True + else: + return match in self.tables_type_info[table_type]["MATCHES"] + @staticmethod def parse_acl_json(filename): yang_acl = pybindJSON.load(filename, openconfig_acl, "openconfig_acl") @@ -795,7 +826,8 @@ def convert_rule_to_db_schema(self, table_name, rule, skip_action_validation=Fal deep_update(rule_props, self.convert_transport(table_name, rule_idx, rule)) deep_update(rule_props, self.convert_input_interface(table_name, rule_idx, rule)) - if "IP_PROTOCOL" in rule_props and "IP_TYPE" not in rule_props: + if ("IP_PROTOCOL" in rule_props and "IP_TYPE" not in rule_props + and self.acl_table_has_match(table_name, "IP_TYPE")): # If we don't include IP_TYPE as a qualifier in the IP_PROTOCOL rule # we could match on non-IP packets if the bits at the same offset match # https://github.com/sonic-net/sonic-mgmt/issues/23960 diff --git a/generic_config_updater/field_operation_validators.py b/generic_config_updater/field_operation_validators.py index fe6c3f8df..fe4a2c2e6 100644 --- a/generic_config_updater/field_operation_validators.py +++ b/generic_config_updater/field_operation_validators.py @@ -46,6 +46,7 @@ def get_asic_name(): spc3_hwskus = asic_mapping["mellanox_asics"]["spc3"] spc4_hwskus = asic_mapping["mellanox_asics"]["spc4"] spc5_hwskus = asic_mapping["mellanox_asics"]["spc5"] + spc6_hwskus = asic_mapping["mellanox_asics"]["spc6"] if hwsku.lower() in [spc1_hwsku.lower() for spc1_hwsku in spc1_hwskus]: asic = "spc1" return asic @@ -61,6 +62,9 @@ def get_asic_name(): if hwsku.lower() in [spc5_hwsku.lower() for spc5_hwsku in spc5_hwskus]: asic = "spc5" return asic + if hwsku.lower() in [spc6_hwsku.lower() for spc6_hwsku in spc6_hwskus]: + asic = "spc6" + return asic if asic_type == 'broadcom' or asic_type == 'vs': broadcom_asics = asic_mapping["broadcom_asics"] for asic_shorthand, hwskus in broadcom_asics.items(): diff --git a/generic_config_updater/gcu_field_operation_validators.conf.json b/generic_config_updater/gcu_field_operation_validators.conf.json index 955187c43..f61f4a194 100644 --- a/generic_config_updater/gcu_field_operation_validators.conf.json +++ b/generic_config_updater/gcu_field_operation_validators.conf.json @@ -24,7 +24,7 @@ "Mellanox-SN4700-A96C8V8", "Mellanox-SN4700-C128", "Mellanox-SN4700-O28", "Mellanox-SN4700-O8V48", "Mellanox-SN4700-V48C32", "Mellanox-SN4280-O28", "Mellanox-SN4280-O8C80", "Mellanox-SN4280-C48", "Mellanox-SN4280-O8C40", "Mellanox-SN4280-O8V40", "Mellanox-SN4280-O4X96"], "spc4": [ "ACS-SN5600", "Mellanox-SN5600-O128", "Mellanox-SN5600-V256", "Mellanox-SN5600-C256S1", "ACS-SN5400", "Mellanox-SN5600-C224O8", "Mellanox-SN5610N-C256S2", "Mellanox-SN5610N-C224O8" ], "spc5": ["ACS-SN5640", "Mellanox-SN5640-C512S2", "Mellanox-SN5640-C448O16", "Mellanox-SN5640-C512X2", "Mellanox-SN5640-C508O1X2", "Mellanox-SN5640-O128X2" ], - "spc6": ["ACS-SN6600_LD"] + "spc6": ["ACS-SN6600_LD", "Mellanox-SN6600_LD-P128C2", "Mellanox-SN6600_LD-P64O128C2"] }, "broadcom_asics": { "th": [ "Force10-S6100", "Arista-7060CX-32S-C32", "Arista-7060CX-32S-C32-T1", "Arista-7060CX-32S-D48C8", "Celestica-DX010-C32", "Seastone-DX010" ], @@ -67,6 +67,7 @@ "spc3": "20220500", "spc4": "20221100", "spc5": "20241200", + "spc6": "20260500", "td2": "20181100", "th": "20181100", "th2": "20181100", @@ -119,6 +120,7 @@ "spc3": "20220500", "spc4": "20221100", "spc5": "20241200", + "spc6": "20260500", "td2": "20181100", "th": "20181100", "th2": "20181100", @@ -148,6 +150,7 @@ "spc3": "20220500", "spc4": "20221100", "spc5": "20241200", + "spc6": "20260500", "td2": "", "th": "20221100", "th2": "20221100", @@ -174,6 +177,7 @@ "platforms": { "spc4": "20241200", "spc5": "20241200", + "spc6": "20260500", "th5": "20241200", "th6": "20251100" } diff --git a/tests/acl_input/acl1.json b/tests/acl_input/acl1.json index 586661bbc..712f6e69b 100644 --- a/tests/acl_input/acl1.json +++ b/tests/acl_input/acl1.json @@ -359,6 +359,30 @@ "name": "bmc_acl_northbound_v6" } }, + "acl_noiptype": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "ip": { + "config": { + "protocol": "1" + } + } + } + } + }, + "config": { + "name": "acl_noiptype" + } + }, "DATAACLV4V6": { "acl-entries": { "acl-entry": { diff --git a/tests/acl_loader_test.py b/tests/acl_loader_test.py index 48c0fd02b..a68cb93cc 100644 --- a/tests/acl_loader_test.py +++ b/tests/acl_loader_test.py @@ -22,7 +22,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) == 9 + assert len(yang_acl.acl.acl_sets.acl_set) == 10 def test_invalid(self): with pytest.raises(AclLoaderException): @@ -231,6 +231,16 @@ def test_icmpv6_translation_in_custom_acl_table_type(self, acl_loader): "PRIORITY": "9998" } + def test_noiptype_custom_acl_table_type(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[("ACL_NOIPTYPE", "RULE_1")] + assert acl_loader.rules_info[("ACL_NOIPTYPE", "RULE_1")] == { + "IP_PROTOCOL": 1, + "PACKET_ACTION": "FORWARD", + "PRIORITY": "9999" + } + def test_ingress_default_deny_rule(self, acl_loader): acl_loader.set_mirror_stage("ingress") acl_loader.get_session_name = mock.MagicMock(return_value="everflow_session_mock") diff --git a/tests/aclshow_test.py b/tests/aclshow_test.py index 94615e544..5fb838fa9 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: 16 +Total number of ACL Tables: 17 Total number of ACL Rules: 21 RULE NAME TABLE NAME PRIO PACKETS COUNT BYTES COUNT diff --git a/tests/config_dpb_test.py b/tests/config_dpb_test.py index b69d2e4e7..df75e96e5 100644 --- a/tests/config_dpb_test.py +++ b/tests/config_dpb_test.py @@ -1,6 +1,7 @@ import json import os import re +from pathlib import Path from unittest import mock import pytest @@ -10,6 +11,8 @@ import config.main as config +from .utils import worker_tmp_path + # Load sonic-cfggen from source since /usr/local/bin/sonic-cfggen does not have .py extension. sonic_cfggen = load_module_from_source('sonic_cfggen', '/usr/local/bin/sonic-cfggen') @@ -162,8 +165,9 @@ def config_mgmt_dpb(cfgdb): ''' curConfig = read_config_db(cfgdb) # create object - config_mgmt.CONFIG_DB_JSON_FILE = "/tmp/startConfigDb.json" - config_mgmt.DEFAULT_CONFIG_DB_JSON_FILE = "/tmp/portBreakOutConfigDb.json" + config_mgmt.CONFIG_DB_JSON_FILE = worker_tmp_path('startConfigDb.json') + config_mgmt.DEFAULT_CONFIG_DB_JSON_FILE = worker_tmp_path( + 'portBreakOutConfigDb.json') # write in temp file writeJson(curConfig, config_mgmt.CONFIG_DB_JSON_FILE) writeJson(portBreakOutConfigDbJson, config_mgmt.DEFAULT_CONFIG_DB_JSON_FILE) @@ -704,8 +708,8 @@ def config_dpb_port4_4x25G_2x50G_f_l(): @classmethod def teardown_class(cls): print("TEARDOWN") - os.system("rm -f /tmp/startConfigDb.json") - os.system("rm -f /tmp/portBreakOutConfigDb.json") + Path(worker_tmp_path('startConfigDb.json')).unlink(missing_ok=True) + Path(worker_tmp_path('portBreakOutConfigDb.json')).unlink(missing_ok=True) ###########GLOBAL Configs##################################### ''' diff --git a/tests/config_mgmt_test.py b/tests/config_mgmt_test.py index b09fdd6b2..84d072fe7 100644 --- a/tests/config_mgmt_test.py +++ b/tests/config_mgmt_test.py @@ -7,6 +7,8 @@ import pytest from utilities_common.general import load_module_from_source +from .utils import worker_tmp_path + # Import file under test i.e., config_mgmt.py config_mgmt_py_path = os.path.join(os.path.dirname(__file__), '..', 'config', 'config_mgmt.py') config_mgmt = load_module_from_source('config_mgmt', config_mgmt_py_path) @@ -19,8 +21,11 @@ class TestConfigMgmt(TestCase): ''' def setUp(self): - config_mgmt.CONFIG_DB_JSON_FILE = "startConfigDb.json" - config_mgmt.DEFAULT_CONFIG_DB_JSON_FILE = "portBreakOutConfigDb.json" + # Per-worker paths: shared basenames in the repo root race under pytest-xdist + # (see sonic-utilities #4516 / config_override_test startConfigDb.json). + config_mgmt.CONFIG_DB_JSON_FILE = worker_tmp_path('startConfigDb.json') + config_mgmt.DEFAULT_CONFIG_DB_JSON_FILE = worker_tmp_path( + 'portBreakOutConfigDb.json') return def test_config_get_module_check(self): @@ -80,8 +85,9 @@ def test_upper_case_mac_fix(self): Libyang converts from 'XX:XX:XX:E4:B3:DD' -> 'xx:xx:xx:e4:b3:dd' ''' curConfig = deepcopy(configDbJson) - # Keep only PORT part to skip dependencies. - curConfig = {'PORT': curConfig['PORT']} + # Use the full sample CONFIG_DB so YANG loadData() satisfies cross-table + # must/when constraints (a PORT-only snapshot can fail model validation on + # some branches). DEVICE_METADATA is added below for the MAC case test. # add DEVICE_METADATA Config curConfig['DEVICE_METADATA'] = { "localhost": { diff --git a/tests/config_override_test.py b/tests/config_override_test.py index 77b579387..8105593ba 100644 --- a/tests/config_override_test.py +++ b/tests/config_override_test.py @@ -11,6 +11,8 @@ from utilities_common.general import load_module_from_source from minigraph import minigraph_encoder +from .utils import worker_tmp_path + SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) DATA_DIR = os.path.join(SCRIPT_DIR, "config_override_input") EMPTY_INPUT = os.path.join(DATA_DIR, "empty_input.json") @@ -202,7 +204,7 @@ def config_mgmt_side_effect(configdb): # ConfigMgmt will call ConfigDBConnector to load default config_db.json. # Here I modify the ConfigMgmt initialization and make it initiated with # a source file which share the same as what we write to cfgdb. - CONFIG_DB_JSON_FILE = "startConfigDb.json" + CONFIG_DB_JSON_FILE = worker_tmp_path('startConfigDb.json') write_config_to_file(read_data['running_config'], CONFIG_DB_JSON_FILE) with mock.patch('config.main.device_info.is_yang_config_validation_enabled', mock.MagicMock(side_effect=is_yang_config_validation_enabled_side_effect)), \ @@ -246,7 +248,7 @@ def config_mgmt_side_effect(configdb): # ConfigMgmt will call ConfigDBConnector to load default config_db.json. # Here I modify the ConfigMgmt initialization and make it initiated with # a source file which share the same as what we write to cfgdb. - CONFIG_DB_JSON_FILE = "startConfigDb.json" + CONFIG_DB_JSON_FILE = worker_tmp_path('startConfigDb.json') write_config_to_file(running_config, CONFIG_DB_JSON_FILE) with mock.patch('config.main.read_json_file', mock.MagicMock(side_effect=read_json_file_side_effect)), \ diff --git a/tests/generic_config_updater/field_operation_validator_test.py b/tests/generic_config_updater/field_operation_validator_test.py index 3bf68cf4e..f8f7feb93 100644 --- a/tests/generic_config_updater/field_operation_validator_test.py +++ b/tests/generic_config_updater/field_operation_validator_test.py @@ -783,6 +783,15 @@ def test_get_asic_spc5(self, mock_popen, mock_get_sonic_version_info): for scope in ["localhost", "asic0"]: self.assertEqual(fov.get_asic_name(), "spc5") + @patch('sonic_py_common.device_info.get_sonic_version_info') + @patch('subprocess.Popen') + def test_get_asic_spc6(self, mock_popen, mock_get_sonic_version_info): + mock_get_sonic_version_info.return_value = {'asic_type': 'mellanox'} + mock_popen.return_value = mock.Mock() + mock_popen.return_value.communicate.return_value = ["Mellanox-SN6600_LD-P64O128C2", 0] + for scope in ["localhost", "asic0"]: + self.assertEqual(fov.get_asic_name(), "spc6") + @patch('sonic_py_common.device_info.get_sonic_version_info') @patch('subprocess.Popen') def test_get_asic_th(self, mock_popen, mock_get_sonic_version_info): diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index c7f0dd96a..71af17466 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -588,6 +588,12 @@ "stage": "ingress", "type": "BMCDATAV6" }, + "ACL_TABLE|ACL_NOIPTYPE": { + "policy_desc": "ACL_NOIPTYPE", + "ports@": "Ethernet8,Ethernet0,Ethernet17,Ethernet16,Ethernet37,Ethernet4,Ethernet13,Ethernet45,Ethernet19,Ethernet24,Ethernet31,Ethernet30,Ethernet39,Ethernet40,Ethernet27,Ethernet43,Ethernet7,Ethernet3,Ethernet20,Ethernet6,Ethernet12,Ethernet34,Ethernet26,Ethernet11,Ethernet42,Ethernet5,Ethernet32,Ethernet36,Ethernet15,Ethernet33,Ethernet35,Ethernet9,Ethernet29,Ethernet21,Ethernet18,Ethernet38,Ethernet23,Ethernet41,Ethernet14,Ethernet2,Ethernet28,Ethernet22,Ethernet10,Ethernet25,Ethernet44,Ethernet1", + "stage": "ingress", + "type": "NOIPTYPE" + }, "ACL_TABLE|DATAACL": { "policy_desc": "DATAACL", "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", @@ -669,6 +675,11 @@ "BIND_POINTS": "PORT", "MATCHES": "SRC_IPV6,DST_IPV6,ETHER_TYPE,IP_TYPE,IP_PROTOCOL,IN_PORTS,TCP_FLAGS" }, + "ACL_TABLE_TYPE|NOIPTYPE": { + "ACTIONS": "PACKET_ACTION,COUNTER", + "BIND_POINTS": "PORT", + "MATCHES": "SRC_IP,DST_IP,ETHER_TYPE,IP_PROTOCOL,IN_PORTS,TCP_FLAGS" + }, "PBH_TABLE|pbh_table1": { "description": "NVGRE", "interface_list@": "Ethernet8,Ethernet60" diff --git a/tests/utils.py b/tests/utils.py index 1baa2e2fb..c369b76ef 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -3,8 +3,27 @@ import importlib.util import importlib.machinery +import os import subprocess import sys +import tempfile + + +def worker_tmp_path(filename): + """ + Per-xdist-worker scratch file path (see tests/conftest.py setup_db_config). + + Avoids races when multiple pytest workers read/write the same basename in + the source tree (e.g. startConfigDb.json under --dist loadfile). + """ + base_dir = os.environ.get('WORKER_TMP') + if not base_dir: + worker_id = os.environ.get('PYTEST_XDIST_WORKER', 'gw0') + base_dir = os.path.join( + tempfile.gettempdir(), f'sonic-utilities-{worker_id}') + + os.makedirs(base_dir, exist_ok=True) + return os.path.join(base_dir, filename) def load_source(modname, filename, cache_module=False):