From bc8084b5639963d27ac5668dc321eb4a5c1ef39c Mon Sep 17 00:00:00 2001 From: Xu Chen Date: Mon, 23 Feb 2026 22:03:39 +0800 Subject: [PATCH 01/12] test: Add production probe test and infrastructure updates Enable probe framework for production testbed usage with infrastructure updates and physical hardware test cases. Infrastructure Updates: 1. tests/conftest.py: - Add --enable_qos_ptf_pdb option for PTF debugging with pdb breakpoint - Add --ingress_drop_probing option to switch between PFC/Drop probing modes 2. tests/ptf_runner.py: - Add 'probe' subdirectory support alongside 'py3' - Add test_subdir parameter for flexible PTF test location - Enable probe tests to run via PTF runner infrastructure 3. tests/qos/qos_sai_base.py (QosSaiBase refactoring): - Move replaceNonExistentPortId() from TestQosSai to base class - Move updateTestPortIdIp() from TestQosSai to base class - Add bufferConfig to dut_qos_maps fixture for all devices - Enable probe tests to access buffer configuration - Shared utility methods for port ID/IP management 4. tests/qos/test_qos_sai.py: - Remove replaceNonExistentPortId() (moved to base) - Remove updateTestPortIdIp() (moved to base) - Reduce code duplication Production Test Cases: 5. tests/qos/test_qos_probe.py (NEW - 544 lines): - TestQosProbe class for physical testbed probing - test_pfc_xoff_probing: PFC Xoff threshold detection on hardware - test_ingress_drop_probing: Ingress drop threshold detection - test_headroom_pool_probing: Headroom pool size probing - Integrates with existing QoS test infrastructure - Uses physical executors for real hardware validation - Validates probe framework on production testbeds This PR completes the probe framework integration, enabling threshold probing tests to run on physical SONiC testbeds alongside existing QoS tests. Signed-off-by: Xu Chen --- tests/conftest.py | 4 + tests/ptf_runner.py | 10 +- tests/qos/qos_sai_base.py | 102 ++++++- tests/qos/test_qos_probe.py | 544 ++++++++++++++++++++++++++++++++++++ tests/qos/test_qos_sai.py | 91 ------ 5 files changed, 654 insertions(+), 97 deletions(-) create mode 100644 tests/qos/test_qos_probe.py diff --git a/tests/conftest.py b/tests/conftest.py index e9bdbe709e1..d70924f2ffd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -236,6 +236,10 @@ def pytest_addoption(parser): parser.addoption("--py_saithrift_url", action="store", default=None, type=str, help="Specify the url of the saithrift package to be installed on the ptf " "(should be http:///path/python-saithrift_0.9.4_amd64.deb") + parser.addoption("--enable_qos_ptf_pdb", action="store_true", default=False, + help="Enable QoS PTF test debugging mode with pdb breakpoint") + parser.addoption("--ingress_drop_probing", action="store_true", default=False, + help="Enable ingress drop threshold probing instead of PFC xoff probing") ######################### # post-test options # diff --git a/tests/ptf_runner.py b/tests/ptf_runner.py index eeb74642753..99a90e43a01 100644 --- a/tests/ptf_runner.py +++ b/tests/ptf_runner.py @@ -83,7 +83,7 @@ def get_test_path(testdir, testname): """ Returns two values - first: the complete path of the test based on testdir and testname. - - second: True if file is in 'py3' False otherwise + - second: True if file is in 'py3' or 'probe' False otherwise Raises FileNotFoundError if file is not found """ curr_path = os.path.dirname(os.path.abspath(__file__)) @@ -91,6 +91,9 @@ def get_test_path(testdir, testname): idx = testname.find('.') test_fname = testname + '.py' if idx == -1 else testname[:idx] + '.py' chk_path = base_path.joinpath('py3').joinpath(test_fname) + if chk_path.exists(): + return chk_path, True + chk_path = base_path.joinpath('probe').joinpath(test_fname) if chk_path.exists(): return chk_path, True chk_path = base_path.joinpath(test_fname) @@ -122,7 +125,8 @@ def ptf_runner(host, testdir, testname, platform_dir=None, params={}, socket_recv_size=None, log_file=None, ptf_collect_dir="./logs/ptf_collect/", device_sockets=[], timeout=0, custom_options="", - module_ignore_errors=False, is_python3=None, async_mode=False, pdb=False): + module_ignore_errors=False, is_python3=None, async_mode=False, pdb=False, + test_subdir='py3'): dut_type = get_dut_type(host) asic_type = get_asic_type(host) kvm_support = params.get("kvm_support", False) @@ -162,7 +166,7 @@ def ptf_runner(host, testdir, testname, platform_dir=None, params={}, raise Exception(err_msg) if in_py3: - tdir = pathlib.Path(testdir).joinpath('py3') + tdir = pathlib.Path(testdir).joinpath(test_subdir) cmd = "{} --test-dir {} {}".format(ptf_cmd, tdir, testname) else: cmd = "{} --test-dir {} {}".format(ptf_cmd, testdir, testname) diff --git a/tests/qos/qos_sai_base.py b/tests/qos/qos_sai_base.py index 4e7a0dbe070..653200d577a 100644 --- a/tests/qos/qos_sai_base.py +++ b/tests/qos/qos_sai_base.py @@ -145,7 +145,7 @@ def dutTestParams(self, duthosts, dut_test_params_qos, tbinfo, get_src_dst_asic_ yield dut_test_params_qos - def runPtfTest(self, ptfhost, testCase='', testParams={}, relax=False, pdb=False, skip_pcap=False): + def runPtfTest(self, ptfhost, testCase='', testParams={}, relax=False, pdb=False, skip_pcap=False, test_subdir='py3'): """ Runs QoS SAI test case on PTF host @@ -186,7 +186,8 @@ def runPtfTest(self, ptfhost, testCase='', testParams={}, relax=False, pdb=False timeout=1850, socket_recv_size=16384, custom_options=custom_options, - pdb=pdb + pdb=pdb, + test_subdir=test_subdir ) @@ -635,6 +636,97 @@ def __assignTestPortIps(self, mgFacts, topo, lower_tor_host): # noqa: F811 return dutPortIps + def replaceNonExistentPortId(self, availablePortIds, portIds): + ''' + if port id of availablePortIds/dst_port_ids is not existing in availablePortIds + replace it with correct one, make sure all port id is valid + e.g. + Given below parameter: + availablePortIds: [0, 2, 4, 6, 8, 10, 16, 18, 20, 22, 24, 26, + 28, 30, 32, 34, 36, 38, 44, 46, 48, 50, 52, 54] + portIds: [1, 2, 3, 4, 5, 6, 7, 8, 9] + get result: + portIds: [0, 2, 16, 4, 18, 6, 20, 8, 22] + ''' + if len(portIds) > len(availablePortIds): + logger.info('no enough ports for test') + return False + + # cache available as free port pool + freePorts = [pid for pid in availablePortIds] + + # record invaild port + # and remove valid port from free port pool + invalid = [] + for idx, pid in enumerate(portIds): + if pid not in freePorts: + invalid.append(idx) + else: + freePorts.remove(pid) + + # replace invalid port from free port pool + for idx in invalid: + portIds[idx] = freePorts.pop(0) + + return True + + def updateTestPortIdIp(self, dutConfig, get_src_dst_asic_and_duts, qosParams=None): + src_dut_index = get_src_dst_asic_and_duts['src_dut_index'] + dst_dut_index = get_src_dst_asic_and_duts['dst_dut_index'] + src_asic_index = get_src_dst_asic_and_duts['src_asic_index'] + dst_asic_index = get_src_dst_asic_and_duts['dst_asic_index'] + src_testPortIds = dutConfig["testPortIds"][src_dut_index][src_asic_index] + dst_testPortIds = dutConfig["testPortIds"][dst_dut_index][dst_asic_index] + testPortIds = src_testPortIds + list(set(dst_testPortIds) - set(src_testPortIds)) + + portIdNames = [] + portIds = [] + + for idName in dutConfig["testPorts"]: + if re.match(r'(?:src|dst)_port\S+id', idName): + portIdNames.append(idName) + ipName = idName.replace('id', 'ip') + pytest_assert( + ipName in dutConfig["testPorts"], 'Not find {} for {} in dutConfig'.format(ipName, idName)) + portIds.append(dutConfig["testPorts"][idName]) + pytest_assert(self.replaceNonExistentPortId(testPortIds, set(portIds)), "No enough test ports") + for idx, idName in enumerate(portIdNames): + dutConfig["testPorts"][idName] = portIds[idx] + ipName = idName.replace('id', 'ip') + if 'src' in ipName: + testPortIps = dutConfig["testPortIps"][src_dut_index][src_asic_index] + else: + testPortIps = dutConfig["testPortIps"][dst_dut_index][dst_asic_index] + dutConfig["testPorts"][ipName] = testPortIps[portIds[idx]]['peer_addr'] + + if qosParams is not None: + portIdNames = [] + portNumbers = [] + portIds = [] + for idName in qosParams.keys(): + if re.match(r'(?:src|dst)_port\S+ids?', idName): + portIdNames.append(idName) + ids = qosParams[idName] + if isinstance(ids, list): + portIds += ids + # if it's port list, record number of pots + portNumbers.append(len(ids)) + else: + portIds.append(ids) + # record None to indicate it's just one port + portNumbers.append(None) + pytest_assert(self.replaceNonExistentPortId(testPortIds, portIds), "No enough test ports") + startPos = 0 + for idx, idName in enumerate(portIdNames): + if portNumbers[idx] is not None: # port list + qosParams[idName] = [ + portId for portId in portIds[startPos:startPos + portNumbers[idx]]] + startPos += portNumbers[idx] + else: # not list, just one port + qosParams[idName] = portIds[startPos] + startPos += 1 + logger.debug('updateTestPortIdIp dutConfig["testPorts"]: {}'.format(dutConfig["testPorts"])) + @pytest.fixture(scope='module') def swapSyncd_on_selected_duts(self, request, duthosts, creds, tbinfo, lower_tor_host, # noqa: F811 core_dump_and_config_check): # noqa: F811 @@ -1019,7 +1111,6 @@ def dutConfig( downlinkPortIps = [] downlinkPortNames = [] sysPortMap = {} - src_dut_index = get_src_dst_asic_and_duts['src_dut_index'] src_asic_index = get_src_dst_asic_and_duts['src_asic_index'] src_dut = get_src_dst_asic_and_duts['src_dut'] @@ -1886,9 +1977,14 @@ def dutQosConfig( portSpeedCableLength = dutQosConfig["portSpeedCableLength"] else: qosParams = qosConfigs['qos_params'][dutAsic][dutTopo] + + # Get buffer config for all devices (not just cisco/broadcom) + bufferConfig = dutBufferConfig(duthost, dut_asic) + yield { "param": qosParams, "portSpeedCableLength": portSpeedCableLength, + "bufferConfig": bufferConfig, } @pytest.fixture(scope='class') diff --git a/tests/qos/test_qos_probe.py b/tests/qos/test_qos_probe.py new file mode 100644 index 00000000000..bcd4cfb45ad --- /dev/null +++ b/tests/qos/test_qos_probe.py @@ -0,0 +1,544 @@ +"""SAI thrift-based buffer threshold probing tests for SONiC. + +This module contains probe-based tests for buffer threshold detection, including: +- testQosPfcXoffProbe: PFC XOFF threshold probing +- testQosIngressDropProbe: Ingress drop threshold probing +- testQosHeadroomPoolProbe: Headroom pool threshold probing + +These tests use advanced probing algorithms to automatically detect buffer thresholds. + +Parameters: + --enable_qos_ptf_pdb (bool): Enable pdb debugger in PTF tests. Default is False. +""" + +import logging +import pytest +from collections import defaultdict + +from tests.common.fixtures.conn_graph_facts import fanout_graph_facts, conn_graph_facts, get_graph_facts # noqa: F401 +from tests.common.fixtures.duthost_utils import dut_qos_maps, \ + separated_dscp_to_tc_map_on_uplink, load_dscp_to_pg_map # noqa: F401 +from tests.common.fixtures.ptfhost_utils import copy_ptftests_directory # noqa: F401 +from tests.common.fixtures.ptfhost_utils import copy_saitests_directory # noqa: F401 +from tests.common.fixtures.ptfhost_utils import change_mac_addresses # noqa: F401 +from tests.common.fixtures.ptfhost_utils import ptf_portmap_file # noqa: F401 +from tests.common.fixtures.ptfhost_utils import iptables_drop_ipv6_tx # noqa: F401 +from tests.common.dualtor.dual_tor_utils import dualtor_ports, is_tunnel_qos_remap_enabled # noqa: F401 +from tests.common.helpers.assertions import pytest_assert +from .qos_sai_base import QosSaiBase +from tests.common.helpers.ptf_tests_helper import downstream_links, upstream_links, select_random_link,\ + get_stream_ptf_ports, apply_dscp_cfg_setup, apply_dscp_cfg_teardown, fetch_test_logs_ptf # noqa: F401 + +logger = logging.getLogger(__name__) + +pytestmark = [ + pytest.mark.topology('any') +] + + +class TestQosProbe(QosSaiBase): + """TestQosProbe contains probe-based buffer threshold detection tests. + + These tests use advanced algorithms to automatically detect buffer thresholds: + - Binary search for PFC XOFF threshold + - Ingress drop threshold detection + - Headroom pool size probing + """ + + @pytest.fixture(scope="class", autouse=True) + def setup(self, disable_voq_watchdog_class_scope): + return + + @pytest.mark.parametrize("xoffProfile", ["xoff_1", "xoff_2", "xoff_3", "xoff_4"]) + def testQosPfcXoffProbe( + self, xoffProfile, duthost, get_src_dst_asic_and_duts, + ptfhost, dutTestParams, dutConfig, dutQosConfig, + ingressLosslessProfile, egressLosslessProfile, change_lag_lacp_timer, tbinfo, request + ): + # NOTE: this test will be skipped for t2 cisco 8800 if it's not xoff_1 or xoff_2 + """ + Test QoS XOFF limits using PFC XOFF probing algorithm + + Args: + xoffProfile (pytest parameter): XOFF profile + ptfhost (AnsibleHost): Packet Test Framework (PTF) + dutTestParams (Fixture, dict): DUT host test params + dutConfig (Fixture, dict): Map of DUT config containing dut interfaces, test port IDs, test port IPs, + and test ports + dutQosConfig (Fixture, dict): Map containing DUT host QoS configuration + ingressLosslessProfile (Fxiture): Map of ingress lossless buffer profile attributes + egressLosslessProfile (Fxiture): Map of egress lossless buffer profile attributes + set_static_route (Fixture): Setup the static route if the src + and dst ASICs are different. + + Returns: + None + + Raises: + RunAnsibleModuleFail if ptf test fails + """ + normal_profile = ["xoff_1", "xoff_2"] + if not dutConfig["dualTor"] and xoffProfile not in normal_profile: + pytest.skip( + "Additional DSCPs are not supported on non-dual ToR ports") + + portSpeedCableLength = dutQosConfig["portSpeedCableLength"] + if dutTestParams['hwsku'] in self.BREAKOUT_SKUS and 'backend' not in dutTestParams['topo']: + qosConfig = dutQosConfig["param"][portSpeedCableLength]["breakout"] + else: + qosConfig = dutQosConfig["param"][portSpeedCableLength] + + self.updateTestPortIdIp(dutConfig, get_src_dst_asic_and_duts) + + probing_port_ids = [dutConfig["testPorts"]["src_port_id"], dutConfig["testPorts"]["dst_port_id"]] + logger.info(f"Simplified probing strategy: using src_port_id={dutConfig['testPorts']['src_port_id']}, dst_port_id={dutConfig['testPorts']['dst_port_id']}") + + testParams = dict() + testParams.update(dutTestParams["basicParams"]) + testParams.update({"test_port_ids": dutConfig["testPortIds"]}) + testParams.update({"test_port_ips": dutConfig["testPortIps"]}) + testParams.update({"probing_port_ids": probing_port_ids}) + testParams.update({ + "dscp": qosConfig[xoffProfile]["dscp"], + "ecn": qosConfig[xoffProfile]["ecn"], + "pg": qosConfig[xoffProfile]["pg"], + "buffer_max_size": ingressLosslessProfile["size"], + "queue_max_size": egressLosslessProfile["static_th"], + "dst_port_id": dutConfig["testPorts"]["dst_port_id"], + "dst_port_ip": dutConfig["testPorts"]["dst_port_ip"], + "src_port_id": dutConfig["testPorts"]["src_port_id"], + "src_port_ip": dutConfig["testPorts"]["src_port_ip"], + "src_port_vlan": dutConfig["testPorts"]["src_port_vlan"], + "pkts_num_leak_out": qosConfig["pkts_num_leak_out"], + "pkts_num_trig_pfc": qosConfig[xoffProfile]["pkts_num_trig_pfc"], + "pkts_num_trig_ingr_drp": qosConfig[xoffProfile]["pkts_num_trig_ingr_drp"], + "hwsku": dutTestParams['hwsku'], + "src_dst_asic_diff": (dutConfig['dutAsic'] != dutConfig['dstDutAsic']) + }) + + if "platform_asic" in dutTestParams["basicParams"]: + testParams["platform_asic"] = dutTestParams["basicParams"]["platform_asic"] + else: + testParams["platform_asic"] = None + + if "pkts_num_egr_mem" in list(qosConfig.keys()): + testParams["pkts_num_egr_mem"] = qosConfig["pkts_num_egr_mem"] + + if dutTestParams["basicParams"].get("platform_asic", None) == "cisco-8000" \ + and not get_src_dst_asic_and_duts["src_long_link"] and get_src_dst_asic_and_duts["dst_long_link"]: + if "pkts_num_egr_mem_short_long" in list(qosConfig.keys()): + testParams["pkts_num_egr_mem"] = qosConfig["pkts_num_egr_mem_short_long"] + else: + pytest.skip( + "pkts_num_egr_mem_short_long is missing in yaml file ") + + if "pkts_num_margin" in list(qosConfig[xoffProfile].keys()): + testParams["pkts_num_margin"] = qosConfig[xoffProfile]["pkts_num_margin"] + + if "packet_size" in list(qosConfig[xoffProfile].keys()): + testParams["packet_size"] = qosConfig[xoffProfile]["packet_size"] + + if 'cell_size' in list(qosConfig[xoffProfile].keys()): + testParams["cell_size"] = qosConfig[xoffProfile]["cell_size"] + + bufferConfig = dutQosConfig["bufferConfig"] + testParams["ingress_lossless_pool_size"] = bufferConfig["BUFFER_POOL"]["ingress_lossless_pool"]["size"] + testParams["egress_lossy_pool_size"] = bufferConfig["BUFFER_POOL"]["egress_lossy_pool"]["size"] + + # Get cell_size with fallback to sub-layers if not found at top level + def find_cell_size(config_dict): + """Recursively find cell_size in config dictionary""" + if isinstance(config_dict, dict): + if 'cell_size' in config_dict: + return config_dict['cell_size'] + for value in config_dict.values(): + result = find_cell_size(value) + if result is not None: + return result + return None + + cell_size = dutQosConfig["param"].get("cell_size", None) + if cell_size is None: + cell_size = find_cell_size(dutQosConfig["param"]) + testParams["cell_size"] = cell_size + + # Get pdb parameter from command line + enable_qos_ptf_pdb = request.config.getoption("--enable_qos_ptf_pdb", default=False) + + self.runPtfTest( + ptfhost, testCase="pfc_xoff_probing.PfcXoffProbing", testParams=testParams, + pdb=enable_qos_ptf_pdb, test_subdir='probe' + ) + + @pytest.mark.parametrize("xoffProfile", ["xoff_1", "xoff_2", "xoff_3", "xoff_4"]) + def testQosIngressDropProbe( + self, xoffProfile, duthost, get_src_dst_asic_and_duts, + ptfhost, dutTestParams, dutConfig, dutQosConfig, + ingressLosslessProfile, egressLosslessProfile, change_lag_lacp_timer, tbinfo, request + ): + """ + Test QoS Ingress Drop limits using IngressDropProbe + + Args: + xoffProfile (pytest parameter): XOFF profile (used for config lookup) + ptfhost (AnsibleHost): Packet Test Framework (PTF) + dutTestParams (Fixture, dict): DUT host test params + dutConfig (Fixture, dict): Map of DUT config containing dut interfaces, test port IDs, test port IPs, + and test ports + dutQosConfig (Fixture, dict): Map containing DUT host QoS configuration + ingressLosslessProfile (Fxiture): Map of ingress lossless buffer profile attributes + egressLosslessProfile (Fxiture): Map of egress lossless buffer profile attributes + + Returns: + None + + Raises: + RunAnsibleModuleFail if ptf test fails + """ + normal_profile = ["xoff_1", "xoff_2"] + if not dutConfig["dualTor"] and xoffProfile not in normal_profile: + pytest.skip( + "Additional DSCPs are not supported on non-dual ToR ports") + + portSpeedCableLength = dutQosConfig["portSpeedCableLength"] + if dutTestParams['hwsku'] in self.BREAKOUT_SKUS and 'backend' not in dutTestParams['topo']: + qosConfig = dutQosConfig["param"][portSpeedCableLength][" breakout"] + else: + qosConfig = dutQosConfig["param"][portSpeedCableLength] + + self.updateTestPortIdIp(dutConfig, get_src_dst_asic_and_duts) + + probing_port_ids = [dutConfig["testPorts"]["src_port_id"], dutConfig["testPorts"]["dst_port_id"]] + logger.info(f"Simplified probing strategy: using src_port_id={dutConfig['testPorts']['src_port_id']}, dst_port_id={dutConfig['testPorts']['dst_port_id']}") + + testParams = dict() + testParams.update(dutTestParams["basicParams"]) + testParams.update({"test_port_ids": dutConfig["testPortIds"]}) + testParams.update({"test_port_ips": dutConfig["testPortIps"]}) + testParams.update({"probing_port_ids": probing_port_ids}) + testParams.update({ + "dscp": qosConfig[xoffProfile]["dscp"], + "ecn": qosConfig[xoffProfile]["ecn"], + "pg": qosConfig[xoffProfile]["pg"], + "buffer_max_size": ingressLosslessProfile["size"], + "queue_max_size": egressLosslessProfile["static_th"], + "dst_port_id": dutConfig["testPorts"]["dst_port_id"], + "dst_port_ip": dutConfig["testPorts"]["dst_port_ip"], + "src_port_id": dutConfig["testPorts"]["src_port_id"], + "src_port_ip": dutConfig["testPorts"]["src_port_ip"], + "src_port_vlan": dutConfig["testPorts"]["src_port_vlan"], + "pkts_num_leak_out": qosConfig["pkts_num_leak_out"], + "pkts_num_trig_pfc": qosConfig[xoffProfile]["pkts_num_trig_pfc"], + "pkts_num_trig_ingr_drp": qosConfig[xoffProfile]["pkts_num_trig_ingr_drp"], + "hwsku": dutTestParams['hwsku'], + "src_dst_asic_diff": (dutConfig['dutAsic'] != dutConfig['dstDutAsic']) + }) + + if "platform_asic" in dutTestParams["basicParams"]: + testParams["platform_asic"] = dutTestParams["basicParams"]["platform_asic"] + else: + testParams["platform_asic"] = None + + if "pkts_num_egr_mem" in list(qosConfig.keys()): + testParams["pkts_num_egr_mem"] = qosConfig["pkts_num_egr_mem"] + + if dutTestParams["basicParams"].get("platform_asic", None) == "cisco-8000" \ + and not get_src_dst_asic_and_duts["src_long_link"] and get_src_dst_asic_and_duts["dst_long_link"]: + if "pkts_num_egr_mem_short_long" in list(qosConfig.keys()): + testParams["pkts_num_egr_mem"] = qosConfig["pkts_num_egr_mem_short_long"] + else: + pytest.skip( + "pkts_num_egr_mem_short_long is missing in yaml file ") + + if "pkts_num_margin" in list(qosConfig[xoffProfile].keys()): + testParams["pkts_num_margin"] = qosConfig[xoffProfile]["pkts_num_margin"] + + if "packet_size" in list(qosConfig[xoffProfile].keys()): + testParams["packet_size"] = qosConfig[xoffProfile]["packet_size"] + + if 'cell_size' in list(qosConfig[xoffProfile].keys()): + testParams["cell_size"] = qosConfig[xoffProfile]["cell_size"] + + bufferConfig = dutQosConfig["bufferConfig"] + testParams["ingress_lossless_pool_size"] = bufferConfig["BUFFER_POOL"]["ingress_lossless_pool"]["size"] + testParams["egress_lossy_pool_size"] = bufferConfig["BUFFER_POOL"]["egress_lossy_pool"]["size"] + + # Get cell_size with fallback to sub-layers if not found at top level + def find_cell_size(config_dict): + """Recursively find cell_size in config dictionary""" + if isinstance(config_dict, dict): + if 'cell_size' in config_dict: + return config_dict['cell_size'] + for value in config_dict.values(): + result = find_cell_size(value) + if result is not None: + return result + return None + + cell_size = dutQosConfig["param"].get("cell_size", None) + if cell_size is None: + cell_size = find_cell_size(dutQosConfig["param"]) + testParams["cell_size"] = cell_size + + # Get pdb parameter from command line + enable_qos_ptf_pdb = request.config.getoption("--enable_qos_ptf_pdb", default=False) + + self.runPtfTest( + ptfhost, testCase="ingress_drop_probing.IngressDropProbing", testParams=testParams, + pdb=enable_qos_ptf_pdb, test_subdir='probe' + ) + + def testQosHeadroomPoolProbe( + self, duthosts, get_src_dst_asic_and_duts, ptfhost, dutTestParams, dutConfig, dutQosConfig, + ingressLosslessProfile, iptables_drop_ipv6_tx, change_lag_lacp_timer, tbinfo, request): # noqa: F811 + # NOTE: cisco-8800 will skip this test since there are no headroom pool + """ + Test QoS Headroom pool size using advanced probing + + This test uses a multi-source probing strategy to detect headroom pool limits. + + Args: + ptfhost (AnsibleHost): Packet Test Framework (PTF) + dutTestParams (Fixture, dict): DUT host test params + dutConfig (Fixture, dict): Map of DUT config containing dut interfaces, test port IDs, test port IPs, + and test ports + dutQosConfig (Fixture, dict): Map containing DUT host QoS configuration + ingressLosslessProfile (Fxiture): Map of ingress lossless buffer profile attributes + + Returns: + None + + Raises: + RunAnsibleModuleFail if ptf test fails + """ + + portSpeedCableLength = dutQosConfig["portSpeedCableLength"] + qosConfig = dutQosConfig["param"][portSpeedCableLength] + testPortIps = dutConfig["testPortIps"] + + if 'hdrm_pool_size' not in list(qosConfig.keys()): + pytest.skip("Headroom pool size is not enabled on this DUT") + + # if no enough ports, src_port_ids is empty list, skip the test + if not qosConfig['hdrm_pool_size'].get('src_port_ids', None): + pytest.skip("No enough test ports on this DUT") + + # run 4 pgs and 4 dscps test for dualtor and T1 dualtor scenario + if not dutConfig['dualTor'] and not dutConfig['dualTorScenario']: + qosConfig['hdrm_pool_size']['pgs'] = qosConfig['hdrm_pool_size']['pgs'][:2] + qosConfig['hdrm_pool_size']['dscps'] = qosConfig['hdrm_pool_size']['dscps'][:2] + + src_dut_index = get_src_dst_asic_and_duts['src_dut_index'] + dst_dut_index = get_src_dst_asic_and_duts['dst_dut_index'] + src_asic_index = get_src_dst_asic_and_duts['src_asic_index'] + dst_asic_index = get_src_dst_asic_and_duts['dst_asic_index'] + + if ('platform_asic' in dutTestParams["basicParams"] and + dutTestParams["basicParams"]["platform_asic"] == "broadcom-dnx"): + # for 100G port speed the number of ports required to fill headroom is huge, + # hence skipping the test with speed 100G or cable length of 2k + if portSpeedCableLength not in ['400000_120000m']: + pytest.skip("Insufficient number of ports to fill the headroom") + # Need to adjust hdrm_pool_size src_port_ids, dst_port_id and pgs_num based on how many source and dst ports + # present + src_ports = dutConfig['testPortIds'][src_dut_index][src_asic_index] + if len(duthosts) == 1: + if len(src_ports) < 3: + pytest.skip("Insufficient number of src ports for testQosHeadroomPoolProbe") + qosConfig["hdrm_pool_size"]["src_port_ids"] = src_ports[1:3] + qosConfig["hdrm_pool_size"]["pgs_num"] = 2 * len(qosConfig["hdrm_pool_size"]["src_port_ids"]) + else: + if len(src_ports) < 5: + pytest.skip("Insufficient number of src ports for testQosHeadroomPoolProbe") + qosConfig["hdrm_pool_size"]["src_port_ids"] = src_ports[1:5] + qosConfig["hdrm_pool_size"]["pgs_num"] = 2 * len(qosConfig["hdrm_pool_size"]["src_port_ids"]) + + if get_src_dst_asic_and_duts['src_asic'] == get_src_dst_asic_and_duts['dst_asic']: + # Src and dst are the same asics, leave one for dst port and the rest for src ports + qosConfig["hdrm_pool_size"]["dst_port_id"] = src_ports[0] + + else: + qosConfig["hdrm_pool_size"]["dst_port_id"] = dutConfig['testPortIds'][dst_dut_index][dst_asic_index][0] + + src_port_vlans = [testPortIps[src_dut_index][src_asic_index][port]['vlan_id'] + if 'vlan_id' in testPortIps[src_dut_index][src_asic_index][port] + else None for port in qosConfig["hdrm_pool_size"]["src_port_ids"]] + self.updateTestPortIdIp(dutConfig, get_src_dst_asic_and_duts, qosConfig["hdrm_pool_size"]) + + + # begin - collect all available test ports for probing + duthost = get_src_dst_asic_and_duts['src_dut'] + src_dut = get_src_dst_asic_and_duts['src_dut'] + src_mgFacts = src_dut.get_extended_minigraph_facts(tbinfo) + sonicport_to_testport = src_mgFacts.get("minigraph_port_indices", {}) + + sonicport_to_pc = {} + for pc_name, pc_info in src_mgFacts['minigraph_portchannels'].items(): + for member in pc_info['members']: + sonicport_to_pc[member] = pc_name + + bcmport_to_sonicport = {} + cmd = " | ".join(("bcmcmd 'knetctrl netif show'", + "grep Interface", + "awk '{print $4 \"=\" $7}'", + "awk -F= '{print $4 \" \" $2}'")) + result = duthost.shell(cmd)['stdout'].strip('"\'"') + for line in result.split("\n"): + if line: + parts = line.split() + if len(parts) == 2: + bcmport_to_sonicport[parts[0]] = parts[1] + + xpe_to_bcmports = defaultdict(list) + cmd = " | ".join(("bcmcmd 'show pmap'", + "grep -vE 'drivshell|show pmap|===|pipe'", + "awk '{ print $2 \" \" $1}'")) + result = duthost.shell(cmd)['stdout'].strip('"\'"') + for line in result.split("\n"): + if line: + parts = line.split() + if len(parts) == 2: + xpe_to_bcmports[parts[0]].append(parts[1]) + if dutTestParams["basicParams"]["sonic_asic_type"] == "broadcom" and dutConfig["dutAsic"] in ("td2", "td3"): + all_ports = [] + for xpe in list(xpe_to_bcmports.keys()): + all_ports.extend(xpe_to_bcmports[xpe]) + if xpe != '0': + del xpe_to_bcmports[xpe] + xpe_to_bcmports['0'] = all_ports + + cmd = " | ".join(("show int des", + "grep -E 'Ethernet[0-9]+\s+up\s+up'", + " awk ' { print $1 } '")) + sonicports_in_upstate = [intf for intf in duthost.shell(cmd)['stdout'].strip('"\'"').split("\n") if intf] + + xpe_to_sonicports = defaultdict(list) + for xpe, bcmports in xpe_to_bcmports.items(): + for bcmport in bcmports: + if bcmport in bcmport_to_sonicport: + xpe_to_sonicports[xpe].append(bcmport_to_sonicport[bcmport]) + + xpe_to_sonicports_in_upstate = defaultdict(list) + for xpe, bcmports in xpe_to_bcmports.items(): + for bcmport in bcmports: + if bcmport in bcmport_to_sonicport: + if bcmport_to_sonicport[bcmport] in sonicports_in_upstate: + xpe_to_sonicports_in_upstate[xpe].append(bcmport_to_sonicport[bcmport]) + + src_dut_index = get_src_dst_asic_and_duts['src_dut_index'] + src_asic_index = get_src_dst_asic_and_duts['src_asic_index'] + src_testPortIds = dutConfig["testPortIds"][src_dut_index][src_asic_index] + + xpe_to_sonicports_in_testPortIds = defaultdict(list) + for xpe, ports in xpe_to_sonicports_in_upstate.items(): + for port in ports: + if sonicport_to_testport[port] in src_testPortIds: + xpe_to_sonicports_in_testPortIds[xpe].append(port) + + xpe_to_unique_sonicports = defaultdict(list) + for xpe, ports in xpe_to_sonicports_in_testPortIds.items(): + # sort by sonic port number + sorted_ports = sorted(ports, key=lambda port: int(port.replace("Ethernet", ""))) + included_pcs = set() + for port in sorted_ports: + pc = sonicport_to_pc.get(port, None) + if pc is None or pc not in included_pcs: + xpe_to_unique_sonicports[xpe].append(port) + if pc: + included_pcs.add(pc) + + xpe_to_testports = defaultdict(list) + for xpe, sonicports in xpe_to_unique_sonicports.items(): + for sonicport in sonicports: + if sonicport in sonicport_to_testport: + xpe_to_testports[xpe].append(sonicport_to_testport[sonicport]) + + max_ports_xpe = max(xpe_to_testports.keys(), key=lambda xpe: len(xpe_to_testports[xpe])) + probing_port_ids = xpe_to_testports[max_ports_xpe] + + logger.info(f"sonicport_to_testport {sonicport_to_testport},\nsonicport_to_pc {sonicport_to_pc},\nbcmport_to_sonicport {bcmport_to_sonicport},\nxpe_to_bcmports {xpe_to_bcmports},\nsonicports_in_upstate {sonicports_in_upstate},\nxpe_to_sonicports {xpe_to_sonicports},\nxpe_to_sonicports_in_upstate {xpe_to_sonicports_in_upstate},\nxpe_to_unique_sonicports {xpe_to_unique_sonicports},\nxpe_to_testports {xpe_to_testports},\nprobing_port_ids {probing_port_ids},\ntest_port_ids {dutConfig['testPortIds']},\ntestPortIps {dutConfig['testPortIps']}") + # end + + + testParams = dict() + testParams.update(dutTestParams["basicParams"]) + + testParams.update({"test_port_ips": dutConfig["testPortIps"]}) + testParams.update({"probing_port_ids": probing_port_ids}) + + testParams.update({ + "testbed_type": dutTestParams["topo"], + "dscps": qosConfig["hdrm_pool_size"]["dscps"], + "ecn": qosConfig["hdrm_pool_size"]["ecn"], + "pgs": qosConfig["hdrm_pool_size"]["pgs"], + "src_port_ids": qosConfig["hdrm_pool_size"]["src_port_ids"], + "src_port_ips": [testPortIps[src_dut_index][src_asic_index][port]['peer_addr'] + for port in qosConfig["hdrm_pool_size"]["src_port_ids"]], + "dst_port_id": qosConfig["hdrm_pool_size"]["dst_port_id"], + "dst_port_ip": + testPortIps[dst_dut_index][dst_asic_index][qosConfig["hdrm_pool_size"]["dst_port_id"]]['peer_addr'], + "pgs_num": qosConfig["hdrm_pool_size"]["pgs_num"], + "pkts_num_trig_pfc": qosConfig["hdrm_pool_size"]["pkts_num_trig_pfc"], + "pkts_num_leak_out": qosConfig["pkts_num_leak_out"], + "pkts_num_hdrm_full": qosConfig["hdrm_pool_size"]["pkts_num_hdrm_full"], + "pkts_num_hdrm_partial": qosConfig["hdrm_pool_size"]["pkts_num_hdrm_partial"], + "hwsku": dutTestParams['hwsku'], + "src_port_vlan": dutConfig["testPorts"]["src_port_vlan"] + }) + + if "platform_asic" in dutTestParams["basicParams"]: + testParams["platform_asic"] = dutTestParams["basicParams"]["platform_asic"] + else: + testParams["platform_asic"] = None + + pkts_num_trig_pfc_shp = qosConfig["hdrm_pool_size"].get( + "pkts_num_trig_pfc_shp") + if pkts_num_trig_pfc_shp: + testParams["pkts_num_trig_pfc_shp"] = pkts_num_trig_pfc_shp + + packet_size = qosConfig["hdrm_pool_size"].get("packet_size") + if packet_size: + testParams["packet_size"] = packet_size + testParams["cell_size"] = qosConfig["hdrm_pool_size"]["cell_size"] + + margin = qosConfig["hdrm_pool_size"].get("margin") + if margin: + testParams["margin"] = margin + + if "pkts_num_egr_mem" in list(qosConfig.keys()): + testParams["pkts_num_egr_mem"] = qosConfig["pkts_num_egr_mem"] + + if "pkts_num_trig_pfc_multi" in qosConfig["hdrm_pool_size"]: + testParams.update({"pkts_num_trig_pfc_multi": qosConfig["hdrm_pool_size"]["pkts_num_trig_pfc_multi"]}) + + bufferConfig = dutQosConfig["bufferConfig"] + testParams["ingress_lossless_pool_size"] = bufferConfig["BUFFER_POOL"]["ingress_lossless_pool"]["size"] + testParams["egress_lossy_pool_size"] = bufferConfig["BUFFER_POOL"]["egress_lossy_pool"]["size"] + + # Get cell_size with fallback to sub-layers if not found at top level + def find_cell_size(config_dict): + """Recursively find cell_size in config dictionary""" + if isinstance(config_dict, dict): + if 'cell_size' in config_dict: + return config_dict['cell_size'] + for value in config_dict.values(): + result = find_cell_size(value) + if result is not None: + return result + return None + + cell_size = dutQosConfig["param"].get("cell_size", None) + if cell_size is None: + cell_size = find_cell_size(dutQosConfig["param"]) + testParams["cell_size"] = cell_size + + # Get pdb parameter from command line + enable_qos_ptf_pdb = request.config.getoption("--enable_qos_ptf_pdb", default=False) + + if ('platform_asic' in dutTestParams["basicParams"] and + dutTestParams["basicParams"]["platform_asic"] == "broadcom-dnx"): + testParams['src_port_vlan'] = src_port_vlans + + self.runPtfTest( + ptfhost, testCase="headroom_pool_probing.HeadroomPoolProbing", + testParams=testParams, pdb=enable_qos_ptf_pdb, test_subdir='probe') diff --git a/tests/qos/test_qos_sai.py b/tests/qos/test_qos_sai.py index 4af6f184f85..6b714f3a16f 100644 --- a/tests/qos/test_qos_sai.py +++ b/tests/qos/test_qos_sai.py @@ -289,97 +289,6 @@ def _set_speed_and_populate_arp(fanout_host, speed, dut_port_list): logger.info("Restore speed to {}".format(original_speed)) _set_speed_and_populate_arp(fanout_host, original_speed, dut_port_list) - def replaceNonExistentPortId(self, availablePortIds, portIds): - ''' - if port id of availablePortIds/dst_port_ids is not existing in availablePortIds - replace it with correct one, make sure all port id is valid - e.g. - Given below parameter: - availablePortIds: [0, 2, 4, 6, 8, 10, 16, 18, 20, 22, 24, 26, - 28, 30, 32, 34, 36, 38, 44, 46, 48, 50, 52, 54] - portIds: [1, 2, 3, 4, 5, 6, 7, 8, 9] - get result: - portIds: [0, 2, 16, 4, 18, 6, 20, 8, 22] - ''' - if len(portIds) > len(availablePortIds): - logger.info('no enough ports for test') - return False - - # cache available as free port pool - freePorts = [pid for pid in availablePortIds] - - # record invaild port - # and remove valid port from free port pool - invalid = [] - for idx, pid in enumerate(portIds): - if pid not in freePorts: - invalid.append(idx) - else: - freePorts.remove(pid) - - # replace invalid port from free port pool - for idx in invalid: - portIds[idx] = freePorts.pop(0) - - return True - - def updateTestPortIdIp(self, dutConfig, get_src_dst_asic_and_duts, qosParams=None): - src_dut_index = get_src_dst_asic_and_duts['src_dut_index'] - dst_dut_index = get_src_dst_asic_and_duts['dst_dut_index'] - src_asic_index = get_src_dst_asic_and_duts['src_asic_index'] - dst_asic_index = get_src_dst_asic_and_duts['dst_asic_index'] - src_testPortIds = dutConfig["testPortIds"][src_dut_index][src_asic_index] - dst_testPortIds = dutConfig["testPortIds"][dst_dut_index][dst_asic_index] - testPortIds = src_testPortIds + list(set(dst_testPortIds) - set(src_testPortIds)) - - portIdNames = [] - portIds = [] - - for idName in dutConfig["testPorts"]: - if re.match(r'(?:src|dst)_port\S+id', idName): - portIdNames.append(idName) - ipName = idName.replace('id', 'ip') - pytest_assert( - ipName in dutConfig["testPorts"], 'Not find {} for {} in dutConfig'.format(ipName, idName)) - portIds.append(dutConfig["testPorts"][idName]) - pytest_assert(self.replaceNonExistentPortId(testPortIds, set(portIds)), "No enough test ports") - for idx, idName in enumerate(portIdNames): - dutConfig["testPorts"][idName] = portIds[idx] - ipName = idName.replace('id', 'ip') - if 'src' in ipName: - testPortIps = dutConfig["testPortIps"][src_dut_index][src_asic_index] - else: - testPortIps = dutConfig["testPortIps"][dst_dut_index][dst_asic_index] - dutConfig["testPorts"][ipName] = testPortIps[portIds[idx]]['peer_addr'] - - if qosParams is not None: - portIdNames = [] - portNumbers = [] - portIds = [] - for idName in qosParams.keys(): - if re.match(r'(?:src|dst)_port\S+ids?', idName): - portIdNames.append(idName) - ids = qosParams[idName] - if isinstance(ids, list): - portIds += ids - # if it's port list, record number of pots - portNumbers.append(len(ids)) - else: - portIds.append(ids) - # record None to indicate it's just one port - portNumbers.append(None) - pytest_assert(self.replaceNonExistentPortId(testPortIds, portIds), "No enough test ports") - startPos = 0 - for idx, idName in enumerate(portIdNames): - if portNumbers[idx] is not None: # port list - qosParams[idName] = [ - portId for portId in portIds[startPos:startPos + portNumbers[idx]]] - startPos += portNumbers[idx] - else: # not list, just one port - qosParams[idName] = portIds[startPos] - startPos += 1 - logger.debug('updateTestPortIdIp dutConfig["testPorts"]: {}'.format(dutConfig["testPorts"])) - def check_and_set_ecn_status(self, duthost, qosConfig, expected_status='on'): ecn_status = '' if duthost.sonichost.is_multi_asic: From 3c1fda84fbd1b4cfa3e9f9a5172063da947d2d89 Mon Sep 17 00:00:00 2001 From: Xu Chen Date: Tue, 24 Feb 2026 22:37:14 +0800 Subject: [PATCH 02/12] fix pre-commit errors Signed-off-by: Xu Chen --- tests/qos/test_qos_probe.py | 46 +++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/tests/qos/test_qos_probe.py b/tests/qos/test_qos_probe.py index bcd4cfb45ad..338a7d5b8d7 100644 --- a/tests/qos/test_qos_probe.py +++ b/tests/qos/test_qos_probe.py @@ -24,7 +24,6 @@ from tests.common.fixtures.ptfhost_utils import ptf_portmap_file # noqa: F401 from tests.common.fixtures.ptfhost_utils import iptables_drop_ipv6_tx # noqa: F401 from tests.common.dualtor.dual_tor_utils import dualtor_ports, is_tunnel_qos_remap_enabled # noqa: F401 -from tests.common.helpers.assertions import pytest_assert from .qos_sai_base import QosSaiBase from tests.common.helpers.ptf_tests_helper import downstream_links, upstream_links, select_random_link,\ get_stream_ptf_ports, apply_dscp_cfg_setup, apply_dscp_cfg_teardown, fetch_test_logs_ptf # noqa: F401 @@ -38,7 +37,7 @@ class TestQosProbe(QosSaiBase): """TestQosProbe contains probe-based buffer threshold detection tests. - + These tests use advanced algorithms to automatically detect buffer thresholds: - Binary search for PFC XOFF threshold - Ingress drop threshold detection @@ -91,7 +90,9 @@ def testQosPfcXoffProbe( self.updateTestPortIdIp(dutConfig, get_src_dst_asic_and_duts) probing_port_ids = [dutConfig["testPorts"]["src_port_id"], dutConfig["testPorts"]["dst_port_id"]] - logger.info(f"Simplified probing strategy: using src_port_id={dutConfig['testPorts']['src_port_id']}, dst_port_id={dutConfig['testPorts']['dst_port_id']}") + logger.info(f"Simplified probing strategy: using " + f"src_port_id={dutConfig['testPorts']['src_port_id']}, " + f"dst_port_id={dutConfig['testPorts']['dst_port_id']}") testParams = dict() testParams.update(dutTestParams["basicParams"]) @@ -144,7 +145,7 @@ def testQosPfcXoffProbe( bufferConfig = dutQosConfig["bufferConfig"] testParams["ingress_lossless_pool_size"] = bufferConfig["BUFFER_POOL"]["ingress_lossless_pool"]["size"] testParams["egress_lossy_pool_size"] = bufferConfig["BUFFER_POOL"]["egress_lossy_pool"]["size"] - + # Get cell_size with fallback to sub-layers if not found at top level def find_cell_size(config_dict): """Recursively find cell_size in config dictionary""" @@ -156,7 +157,7 @@ def find_cell_size(config_dict): if result is not None: return result return None - + cell_size = dutQosConfig["param"].get("cell_size", None) if cell_size is None: cell_size = find_cell_size(dutQosConfig["param"]) @@ -209,7 +210,9 @@ def testQosIngressDropProbe( self.updateTestPortIdIp(dutConfig, get_src_dst_asic_and_duts) probing_port_ids = [dutConfig["testPorts"]["src_port_id"], dutConfig["testPorts"]["dst_port_id"]] - logger.info(f"Simplified probing strategy: using src_port_id={dutConfig['testPorts']['src_port_id']}, dst_port_id={dutConfig['testPorts']['dst_port_id']}") + logger.info(f"Simplified probing strategy: using " + f"src_port_id={dutConfig['testPorts']['src_port_id']}, " + f"dst_port_id={dutConfig['testPorts']['dst_port_id']}") testParams = dict() testParams.update(dutTestParams["basicParams"]) @@ -289,8 +292,9 @@ def find_cell_size(config_dict): ) def testQosHeadroomPoolProbe( - self, duthosts, get_src_dst_asic_and_duts, ptfhost, dutTestParams, dutConfig, dutQosConfig, - ingressLosslessProfile, iptables_drop_ipv6_tx, change_lag_lacp_timer, tbinfo, request): # noqa: F811 + self, duthosts, get_src_dst_asic_and_duts, ptfhost, dutTestParams, + dutConfig, dutQosConfig, ingressLosslessProfile, iptables_drop_ipv6_tx, + change_lag_lacp_timer, tbinfo, request): # noqa: F811 # NOTE: cisco-8800 will skip this test since there are no headroom pool """ Test QoS Headroom pool size using advanced probing @@ -365,7 +369,6 @@ def testQosHeadroomPoolProbe( else None for port in qosConfig["hdrm_pool_size"]["src_port_ids"]] self.updateTestPortIdIp(dutConfig, get_src_dst_asic_and_duts, qosConfig["hdrm_pool_size"]) - # begin - collect all available test ports for probing duthost = get_src_dst_asic_and_duts['src_dut'] src_dut = get_src_dst_asic_and_duts['src_dut'] @@ -408,7 +411,7 @@ def testQosHeadroomPoolProbe( xpe_to_bcmports['0'] = all_ports cmd = " | ".join(("show int des", - "grep -E 'Ethernet[0-9]+\s+up\s+up'", + r"grep -E 'Ethernet[0-9]+\s+up\s+up'", " awk ' { print $1 } '")) sonicports_in_upstate = [intf for intf in duthost.shell(cmd)['stdout'].strip('"\'"').split("\n") if intf] @@ -453,13 +456,26 @@ def testQosHeadroomPoolProbe( if sonicport in sonicport_to_testport: xpe_to_testports[xpe].append(sonicport_to_testport[sonicport]) - max_ports_xpe = max(xpe_to_testports.keys(), key=lambda xpe: len(xpe_to_testports[xpe])) + max_ports_xpe = max(xpe_to_testports.keys(), + key=lambda xpe: len(xpe_to_testports[xpe])) probing_port_ids = xpe_to_testports[max_ports_xpe] - logger.info(f"sonicport_to_testport {sonicport_to_testport},\nsonicport_to_pc {sonicport_to_pc},\nbcmport_to_sonicport {bcmport_to_sonicport},\nxpe_to_bcmports {xpe_to_bcmports},\nsonicports_in_upstate {sonicports_in_upstate},\nxpe_to_sonicports {xpe_to_sonicports},\nxpe_to_sonicports_in_upstate {xpe_to_sonicports_in_upstate},\nxpe_to_unique_sonicports {xpe_to_unique_sonicports},\nxpe_to_testports {xpe_to_testports},\nprobing_port_ids {probing_port_ids},\ntest_port_ids {dutConfig['testPortIds']},\ntestPortIps {dutConfig['testPortIps']}") + logger.info( + f"sonicport_to_testport {sonicport_to_testport},\n" + f"sonicport_to_pc {sonicport_to_pc},\n" + f"bcmport_to_sonicport {bcmport_to_sonicport},\n" + f"xpe_to_bcmports {xpe_to_bcmports},\n" + f"sonicports_in_upstate {sonicports_in_upstate},\n" + f"xpe_to_sonicports {xpe_to_sonicports},\n" + f"xpe_to_sonicports_in_upstate {xpe_to_sonicports_in_upstate},\n" + f"xpe_to_unique_sonicports {xpe_to_unique_sonicports},\n" + f"xpe_to_testports {xpe_to_testports},\n" + f"probing_port_ids {probing_port_ids},\n" + f"test_port_ids {dutConfig['testPortIds']},\n" + f"testPortIps {dutConfig['testPortIps']}" + ) # end - testParams = dict() testParams.update(dutTestParams["basicParams"]) @@ -514,7 +530,7 @@ def testQosHeadroomPoolProbe( bufferConfig = dutQosConfig["bufferConfig"] testParams["ingress_lossless_pool_size"] = bufferConfig["BUFFER_POOL"]["ingress_lossless_pool"]["size"] testParams["egress_lossy_pool_size"] = bufferConfig["BUFFER_POOL"]["egress_lossy_pool"]["size"] - + # Get cell_size with fallback to sub-layers if not found at top level def find_cell_size(config_dict): """Recursively find cell_size in config dictionary""" @@ -526,7 +542,7 @@ def find_cell_size(config_dict): if result is not None: return result return None - + cell_size = dutQosConfig["param"].get("cell_size", None) if cell_size is None: cell_size = find_cell_size(dutQosConfig["param"]) From d365ea654479274845277e781709f068f59da73a Mon Sep 17 00:00:00 2001 From: Xu Chen Date: Tue, 24 Feb 2026 22:45:07 +0800 Subject: [PATCH 03/12] fix pre-commit errors Signed-off-by: Xu Chen --- tests/qos/qos_sai_base.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/qos/qos_sai_base.py b/tests/qos/qos_sai_base.py index 653200d577a..ac4efa46e2e 100644 --- a/tests/qos/qos_sai_base.py +++ b/tests/qos/qos_sai_base.py @@ -145,7 +145,8 @@ def dutTestParams(self, duthosts, dut_test_params_qos, tbinfo, get_src_dst_asic_ yield dut_test_params_qos - def runPtfTest(self, ptfhost, testCase='', testParams={}, relax=False, pdb=False, skip_pcap=False, test_subdir='py3'): + def runPtfTest(self, ptfhost, testCase='', testParams={}, relax=False, pdb=False, + skip_pcap=False, test_subdir='py3'): """ Runs QoS SAI test case on PTF host @@ -1977,10 +1978,10 @@ def dutQosConfig( portSpeedCableLength = dutQosConfig["portSpeedCableLength"] else: qosParams = qosConfigs['qos_params'][dutAsic][dutTopo] - + # Get buffer config for all devices (not just cisco/broadcom) bufferConfig = dutBufferConfig(duthost, dut_asic) - + yield { "param": qosParams, "portSpeedCableLength": portSpeedCableLength, From 0bfc4affa1c21662c6e99de7d897aad491adf1ff Mon Sep 17 00:00:00 2001 From: Xu Chen Date: Tue, 24 Feb 2026 23:15:46 +0800 Subject: [PATCH 04/12] fix pre-commit errors Signed-off-by: Xu Chen --- tests/qos/test_qos_probe.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qos/test_qos_probe.py b/tests/qos/test_qos_probe.py index 338a7d5b8d7..50067cc4bd0 100644 --- a/tests/qos/test_qos_probe.py +++ b/tests/qos/test_qos_probe.py @@ -292,9 +292,9 @@ def find_cell_size(config_dict): ) def testQosHeadroomPoolProbe( - self, duthosts, get_src_dst_asic_and_duts, ptfhost, dutTestParams, - dutConfig, dutQosConfig, ingressLosslessProfile, iptables_drop_ipv6_tx, - change_lag_lacp_timer, tbinfo, request): # noqa: F811 + self, duthosts, get_src_dst_asic_and_duts, ptfhost, dutTestParams, + dutConfig, dutQosConfig, ingressLosslessProfile, iptables_drop_ipv6_tx, # noqa: F811 + change_lag_lacp_timer, tbinfo, request): # NOTE: cisco-8800 will skip this test since there are no headroom pool """ Test QoS Headroom pool size using advanced probing From df2f8b2bbc6c4b736de348b005a29c6e3ca4ddca Mon Sep 17 00:00:00 2001 From: Xu Chen Date: Wed, 25 Feb 2026 13:29:40 +0800 Subject: [PATCH 05/12] Add conditional mark for test_qos_probe.py pending platform validation Signed-off-by: Xu Chen --- .../tests_mark_conditions.yaml | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml b/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml index 63765fc5e11..5a02b7abe71 100644 --- a/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml +++ b/tests/common/plugins/conditional_mark/tests_mark_conditions.yaml @@ -3995,6 +3995,30 @@ qos/test_qos_masic.py: - "topo_type in ['m0', 'mx', 'm1']" - "asic_type in ['vs']" +qos/test_qos_probe.py: + skip: + reason: "Buffer threshold probing tests need platform-specific validation. Tracked by per-ASIC GitHub issues." + conditions_logical_operator: or + conditions: + # Virtual Switch support + - "asic_type in ['vs'] and https://github.com/sonic-net/sonic-mgmt/issues/22599" + # Cisco-8000 GB series (8102/8101 GB SKUs) + - "hwsku in ['Cisco-8102-C64', 'Cisco-8101-O8C48', 'Cisco-8102-28FH-DPU-O'] and https://github.com/sonic-net/sonic-mgmt/issues/22601" + # Cisco-8000 GR/GR2 series (8101 GR/GR2 SKUs) + - "hwsku in ['Cisco-8101-O32', 'Cisco-8101-O8V48'] and https://github.com/sonic-net/sonic-mgmt/issues/22602" + # Broadcom TD3 (Trident 3) + - "hwsku in ['Arista-7050CX3-32C-C32', 'Arista-7050CX3-32S-C32'] and https://github.com/sonic-net/sonic-mgmt/issues/22603" + # Broadcom TH (Tomahawk) + - "hwsku in ['Arista-7060CX-32S-C32', 'Arista-7060CX-32S-Q32'] and https://github.com/sonic-net/sonic-mgmt/issues/22604" + # Broadcom TH2 (Tomahawk 2) + - "hwsku in ['Arista-7260CX3-C64', 'Arista-7260CX3-D108C8', 'Arista-7260CX3-D108C10'] and https://github.com/sonic-net/sonic-mgmt/issues/22605" + # Broadcom TH5 (Tomahawk 5) + - "hwsku in ['Arista-7060X6-16PE-384C-B-O128S2', 'Arista-7060X6-64PE-B-O128'] and https://github.com/sonic-net/sonic-mgmt/issues/22606" + # Mellanox SPC1 (Spectrum 1) + - "hwsku in ['Mellanox-SN2700'] and https://github.com/sonic-net/sonic-mgmt/issues/22607" + # Mellanox SPC3 (Spectrum 3) + - "hwsku in ['Mellanox-SN4600C-C64'] and https://github.com/sonic-net/sonic-mgmt/issues/22608" + qos/test_qos_sai.py: skip: reason: "qos_sai tests not supported on t1 topo / M* topo does not support qos and It is skipped for '202412' for now" From 453cc95224e85d606d539a9ba0d020e3b4cbd172 Mon Sep 17 00:00:00 2001 From: Xu Chen Date: Tue, 17 Mar 2026 14:34:11 +0800 Subject: [PATCH 06/12] fix: remove leading space typo in breakout config key Fix KeyError in testQosIngressDropProbe for breakout SKUs. Line 206 had [" breakout"] (with leading space) instead of ["breakout"]. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen --- tests/qos/test_qos_probe.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qos/test_qos_probe.py b/tests/qos/test_qos_probe.py index 50067cc4bd0..86bec3e98ec 100644 --- a/tests/qos/test_qos_probe.py +++ b/tests/qos/test_qos_probe.py @@ -203,7 +203,7 @@ def testQosIngressDropProbe( portSpeedCableLength = dutQosConfig["portSpeedCableLength"] if dutTestParams['hwsku'] in self.BREAKOUT_SKUS and 'backend' not in dutTestParams['topo']: - qosConfig = dutQosConfig["param"][portSpeedCableLength][" breakout"] + qosConfig = dutQosConfig["param"][portSpeedCableLength]["breakout"] else: qosConfig = dutQosConfig["param"][portSpeedCableLength] From 96daf8d8bc30e79d41d4b079c9d23081f34f5799 Mon Sep 17 00:00:00 2001 From: Xu Chen Date: Tue, 17 Mar 2026 14:35:43 +0800 Subject: [PATCH 07/12] refactor: extract find_cell_size() to class-level @staticmethod Deduplicate find_cell_size() which was identically defined 3 times as nested functions inside testQosPfcXoffProbe, testQosIngressDropProbe, and testQosHeadroomPoolProbe. Now a single @staticmethod on TestQosProbe. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen --- tests/qos/test_qos_probe.py | 51 +++++++++++-------------------------- 1 file changed, 15 insertions(+), 36 deletions(-) diff --git a/tests/qos/test_qos_probe.py b/tests/qos/test_qos_probe.py index 86bec3e98ec..ff18cf6d38f 100644 --- a/tests/qos/test_qos_probe.py +++ b/tests/qos/test_qos_probe.py @@ -44,6 +44,18 @@ class TestQosProbe(QosSaiBase): - Headroom pool size probing """ + @staticmethod + def find_cell_size(config_dict): + """Recursively find cell_size in config dictionary.""" + if isinstance(config_dict, dict): + if 'cell_size' in config_dict: + return config_dict['cell_size'] + for value in config_dict.values(): + result = TestQosProbe.find_cell_size(value) + if result is not None: + return result + return None + @pytest.fixture(scope="class", autouse=True) def setup(self, disable_voq_watchdog_class_scope): return @@ -147,20 +159,9 @@ def testQosPfcXoffProbe( testParams["egress_lossy_pool_size"] = bufferConfig["BUFFER_POOL"]["egress_lossy_pool"]["size"] # Get cell_size with fallback to sub-layers if not found at top level - def find_cell_size(config_dict): - """Recursively find cell_size in config dictionary""" - if isinstance(config_dict, dict): - if 'cell_size' in config_dict: - return config_dict['cell_size'] - for value in config_dict.values(): - result = find_cell_size(value) - if result is not None: - return result - return None - cell_size = dutQosConfig["param"].get("cell_size", None) if cell_size is None: - cell_size = find_cell_size(dutQosConfig["param"]) + cell_size = self.find_cell_size(dutQosConfig["param"]) testParams["cell_size"] = cell_size # Get pdb parameter from command line @@ -267,20 +268,9 @@ def testQosIngressDropProbe( testParams["egress_lossy_pool_size"] = bufferConfig["BUFFER_POOL"]["egress_lossy_pool"]["size"] # Get cell_size with fallback to sub-layers if not found at top level - def find_cell_size(config_dict): - """Recursively find cell_size in config dictionary""" - if isinstance(config_dict, dict): - if 'cell_size' in config_dict: - return config_dict['cell_size'] - for value in config_dict.values(): - result = find_cell_size(value) - if result is not None: - return result - return None - cell_size = dutQosConfig["param"].get("cell_size", None) if cell_size is None: - cell_size = find_cell_size(dutQosConfig["param"]) + cell_size = self.find_cell_size(dutQosConfig["param"]) testParams["cell_size"] = cell_size # Get pdb parameter from command line @@ -532,20 +522,9 @@ def testQosHeadroomPoolProbe( testParams["egress_lossy_pool_size"] = bufferConfig["BUFFER_POOL"]["egress_lossy_pool"]["size"] # Get cell_size with fallback to sub-layers if not found at top level - def find_cell_size(config_dict): - """Recursively find cell_size in config dictionary""" - if isinstance(config_dict, dict): - if 'cell_size' in config_dict: - return config_dict['cell_size'] - for value in config_dict.values(): - result = find_cell_size(value) - if result is not None: - return result - return None - cell_size = dutQosConfig["param"].get("cell_size", None) if cell_size is None: - cell_size = find_cell_size(dutQosConfig["param"]) + cell_size = self.find_cell_size(dutQosConfig["param"]) testParams["cell_size"] = cell_size # Get pdb parameter from command line From c2ff13fa29c24a99b7658d30fc275151c7eb1f08 Mon Sep 17 00:00:00 2001 From: Xu Chen Date: Tue, 17 Mar 2026 14:52:58 +0800 Subject: [PATCH 08/12] fix: change set(portIds) to list(portIds) in updateTestPortIdIp set() does not support index assignment (portIds[idx] = ...) which replaceNonExistentPortId() uses internally. This is a pre-existing bug from PR #8149 (test_qos_sai.py L345), moved here during refactoring. The set() was likely intended for deduplication but breaks item assignment. In practice it rarely triggered because most testbeds have all valid ports, so the index assignment path was never reached. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen --- tests/qos/qos_sai_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qos/qos_sai_base.py b/tests/qos/qos_sai_base.py index ac4efa46e2e..6093ab8349e 100644 --- a/tests/qos/qos_sai_base.py +++ b/tests/qos/qos_sai_base.py @@ -690,7 +690,7 @@ def updateTestPortIdIp(self, dutConfig, get_src_dst_asic_and_duts, qosParams=Non pytest_assert( ipName in dutConfig["testPorts"], 'Not find {} for {} in dutConfig'.format(ipName, idName)) portIds.append(dutConfig["testPorts"][idName]) - pytest_assert(self.replaceNonExistentPortId(testPortIds, set(portIds)), "No enough test ports") + pytest_assert(self.replaceNonExistentPortId(testPortIds, list(portIds)), "No enough test ports") for idx, idName in enumerate(portIdNames): dutConfig["testPorts"][idName] = portIds[idx] ipName = idName.replace('id', 'ip') From 98db457d6080cf379296f3e7156259c5a193e6d4 Mon Sep 17 00:00:00 2001 From: Xu Chen Date: Tue, 17 Mar 2026 15:00:41 +0800 Subject: [PATCH 09/12] refactor: rename in_py3 to in_subdir in ptf_runner.py The boolean originally indicated whether a test file was in the py3/ subdirectory. With the addition of the probe/ subdirectory for the MMU threshold probing framework, the variable now indicates whether the test is in any subdirectory (py3 or probe). Rename for clarity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen --- tests/ptf_runner.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/ptf_runner.py b/tests/ptf_runner.py index 99a90e43a01..d7a4010bf9f 100644 --- a/tests/ptf_runner.py +++ b/tests/ptf_runner.py @@ -83,7 +83,9 @@ def get_test_path(testdir, testname): """ Returns two values - first: the complete path of the test based on testdir and testname. - - second: True if file is in 'py3' or 'probe' False otherwise + - second: True if file is in a subdirectory ('py3' or 'probe'), False otherwise. + Originally only 'py3' existed; 'probe' was added for the MMU threshold + probing framework. Raises FileNotFoundError if file is not found """ curr_path = os.path.dirname(os.path.abspath(__file__)) @@ -137,8 +139,8 @@ def ptf_runner(host, testdir, testname, platform_dir=None, params={}, cmd = "" ptf_img_type = get_ptf_image_type(host) logger.info('PTF image type: {}'.format(ptf_img_type)) - test_fpath, in_py3 = get_test_path(testdir, testname) - logger.info('Test file path {}, in py3: {}'.format(test_fpath, in_py3)) + test_fpath, in_subdir = get_test_path(testdir, testname) + logger.info('Test file path {}, in subdir: {}'.format(test_fpath, in_subdir)) is_python3 = is_py3_compat(test_fpath) # The logic below automatically chooses the PTF binary to execute a test script @@ -165,7 +167,7 @@ def ptf_runner(host, testdir, testname, platform_dir=None, params={}, err_msg = 'cannot run Python 2 test in a Python 3 only {} {}'.format(testdir, testname) raise Exception(err_msg) - if in_py3: + if in_subdir: tdir = pathlib.Path(testdir).joinpath(test_subdir) cmd = "{} --test-dir {} {}".format(ptf_cmd, tdir, testname) else: From d001e4fe485890ddbcf55a11a9b84c71ef0a9e85 Mon Sep 17 00:00:00 2001 From: Xu Chen Date: Thu, 26 Mar 2026 21:52:01 +0800 Subject: [PATCH 10/12] fix: guard bcmcmd calls with broadcom ASIC type check bcmcmd commands (knetctrl netif show, show pmap) are Broadcom-specific and crash on Mellanox/Cisco with 'command not found'. Wrapped entire XPE port mapping logic in 'if sonic_asic_type == broadcom:' guard. Non-Broadcom platforms use all available test ports directly. Addresses @StormLiangMS review (2026-03-25): bcmcmd runs on all platforms. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen --- tests/qos/test_qos_probe.py | 153 ++++++++++++++++++------------------ 1 file changed, 78 insertions(+), 75 deletions(-) diff --git a/tests/qos/test_qos_probe.py b/tests/qos/test_qos_probe.py index ff18cf6d38f..ed3a32c9d9f 100644 --- a/tests/qos/test_qos_probe.py +++ b/tests/qos/test_qos_probe.py @@ -370,85 +370,88 @@ def testQosHeadroomPoolProbe( for member in pc_info['members']: sonicport_to_pc[member] = pc_name - bcmport_to_sonicport = {} - cmd = " | ".join(("bcmcmd 'knetctrl netif show'", - "grep Interface", - "awk '{print $4 \"=\" $7}'", - "awk -F= '{print $4 \" \" $2}'")) - result = duthost.shell(cmd)['stdout'].strip('"\'"') - for line in result.split("\n"): - if line: - parts = line.split() - if len(parts) == 2: - bcmport_to_sonicport[parts[0]] = parts[1] - - xpe_to_bcmports = defaultdict(list) - cmd = " | ".join(("bcmcmd 'show pmap'", - "grep -vE 'drivshell|show pmap|===|pipe'", - "awk '{ print $2 \" \" $1}'")) - result = duthost.shell(cmd)['stdout'].strip('"\'"') - for line in result.split("\n"): - if line: - parts = line.split() - if len(parts) == 2: - xpe_to_bcmports[parts[0]].append(parts[1]) - if dutTestParams["basicParams"]["sonic_asic_type"] == "broadcom" and dutConfig["dutAsic"] in ("td2", "td3"): - all_ports = [] - for xpe in list(xpe_to_bcmports.keys()): - all_ports.extend(xpe_to_bcmports[xpe]) - if xpe != '0': - del xpe_to_bcmports[xpe] - xpe_to_bcmports['0'] = all_ports - - cmd = " | ".join(("show int des", - r"grep -E 'Ethernet[0-9]+\s+up\s+up'", - " awk ' { print $1 } '")) - sonicports_in_upstate = [intf for intf in duthost.shell(cmd)['stdout'].strip('"\'"').split("\n") if intf] - - xpe_to_sonicports = defaultdict(list) - for xpe, bcmports in xpe_to_bcmports.items(): - for bcmport in bcmports: - if bcmport in bcmport_to_sonicport: - xpe_to_sonicports[xpe].append(bcmport_to_sonicport[bcmport]) - - xpe_to_sonicports_in_upstate = defaultdict(list) - for xpe, bcmports in xpe_to_bcmports.items(): - for bcmport in bcmports: - if bcmport in bcmport_to_sonicport: - if bcmport_to_sonicport[bcmport] in sonicports_in_upstate: - xpe_to_sonicports_in_upstate[xpe].append(bcmport_to_sonicport[bcmport]) - src_dut_index = get_src_dst_asic_and_duts['src_dut_index'] src_asic_index = get_src_dst_asic_and_duts['src_asic_index'] src_testPortIds = dutConfig["testPortIds"][src_dut_index][src_asic_index] - xpe_to_sonicports_in_testPortIds = defaultdict(list) - for xpe, ports in xpe_to_sonicports_in_upstate.items(): - for port in ports: - if sonicport_to_testport[port] in src_testPortIds: - xpe_to_sonicports_in_testPortIds[xpe].append(port) - - xpe_to_unique_sonicports = defaultdict(list) - for xpe, ports in xpe_to_sonicports_in_testPortIds.items(): - # sort by sonic port number - sorted_ports = sorted(ports, key=lambda port: int(port.replace("Ethernet", ""))) - included_pcs = set() - for port in sorted_ports: - pc = sonicport_to_pc.get(port, None) - if pc is None or pc not in included_pcs: - xpe_to_unique_sonicports[xpe].append(port) - if pc: - included_pcs.add(pc) - - xpe_to_testports = defaultdict(list) - for xpe, sonicports in xpe_to_unique_sonicports.items(): - for sonicport in sonicports: - if sonicport in sonicport_to_testport: - xpe_to_testports[xpe].append(sonicport_to_testport[sonicport]) - - max_ports_xpe = max(xpe_to_testports.keys(), - key=lambda xpe: len(xpe_to_testports[xpe])) - probing_port_ids = xpe_to_testports[max_ports_xpe] + sonic_asic_type = dutTestParams["basicParams"].get("sonic_asic_type", "") + + if sonic_asic_type == "broadcom": + # Broadcom: use bcmcmd to map ports to XPEs, select ports from the + # XPE with the most available test ports (required for multi-XPE ASICs + # like TH/TH2 where headroom pool is per-XPE) + bcmport_to_sonicport = {} + cmd = " | ".join(("bcmcmd 'knetctrl netif show'", + "grep Interface", + "awk '{print $4 \"=\" $7}'", + "awk -F= '{print $4 \" \" $2}'")) + result = duthost.shell(cmd)['stdout'].strip('"\'"') + for line in result.split("\n"): + if line: + parts = line.split() + if len(parts) == 2: + bcmport_to_sonicport[parts[0]] = parts[1] + + xpe_to_bcmports = defaultdict(list) + cmd = " | ".join(("bcmcmd 'show pmap'", + "grep -vE 'drivshell|show pmap|===|pipe'", + "awk '{ print $2 \" \" $1}'")) + result = duthost.shell(cmd)['stdout'].strip('"\'"') + for line in result.split("\n"): + if line: + parts = line.split() + if len(parts) == 2: + xpe_to_bcmports[parts[0]].append(parts[1]) + if dutConfig["dutAsic"] in ("td2", "td3"): + all_ports = [] + for xpe in list(xpe_to_bcmports.keys()): + all_ports.extend(xpe_to_bcmports[xpe]) + if xpe != '0': + del xpe_to_bcmports[xpe] + xpe_to_bcmports['0'] = all_ports + + cmd = " | ".join(("show int des", + r"grep -E 'Ethernet[0-9]+\s+up\s+up'", + " awk ' { print $1 } '")) + sonicports_in_upstate = [intf for intf in duthost.shell(cmd)['stdout'].strip('"\'"').split("\n") if intf] + + xpe_to_sonicports_in_upstate = defaultdict(list) + for xpe, bcmports in xpe_to_bcmports.items(): + for bcmport in bcmports: + if bcmport in bcmport_to_sonicport: + if bcmport_to_sonicport[bcmport] in sonicports_in_upstate: + xpe_to_sonicports_in_upstate[xpe].append(bcmport_to_sonicport[bcmport]) + + xpe_to_sonicports_in_testPortIds = defaultdict(list) + for xpe, ports in xpe_to_sonicports_in_upstate.items(): + for port in ports: + if sonicport_to_testport[port] in src_testPortIds: + xpe_to_sonicports_in_testPortIds[xpe].append(port) + + xpe_to_unique_sonicports = defaultdict(list) + for xpe, ports in xpe_to_sonicports_in_testPortIds.items(): + sorted_ports = sorted(ports, key=lambda port: int(port.replace("Ethernet", ""))) + included_pcs = set() + for port in sorted_ports: + pc = sonicport_to_pc.get(port, None) + if pc is None or pc not in included_pcs: + xpe_to_unique_sonicports[xpe].append(port) + if pc: + included_pcs.add(pc) + + xpe_to_testports = defaultdict(list) + for xpe, sonicports in xpe_to_unique_sonicports.items(): + for sonicport in sonicports: + if sonicport in sonicport_to_testport: + xpe_to_testports[xpe].append(sonicport_to_testport[sonicport]) + + max_ports_xpe = max(xpe_to_testports.keys(), + key=lambda xpe: len(xpe_to_testports[xpe])) + probing_port_ids = xpe_to_testports[max_ports_xpe] + else: + # Non-Broadcom (Mellanox, Cisco, etc.): no XPE concept, + # use all available test ports directly + probing_port_ids = src_testPortIds logger.info( f"sonicport_to_testport {sonicport_to_testport},\n" From 3d8746f917ce305cdd807818cc38afa87cb1fb50 Mon Sep 17 00:00:00 2001 From: Xu Chen Date: Thu, 26 Mar 2026 21:52:50 +0800 Subject: [PATCH 11/12] fix: add empty dict guard before max() on xpe_to_testports max() on empty dict raises ValueError. Skip test gracefully when no available test ports are found after XPE mapping. Addresses @StormLiangMS review (2026-03-25): max() on potentially empty dict. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen --- tests/qos/test_qos_probe.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/qos/test_qos_probe.py b/tests/qos/test_qos_probe.py index ed3a32c9d9f..f83764596c5 100644 --- a/tests/qos/test_qos_probe.py +++ b/tests/qos/test_qos_probe.py @@ -445,6 +445,9 @@ def testQosHeadroomPoolProbe( if sonicport in sonicport_to_testport: xpe_to_testports[xpe].append(sonicport_to_testport[sonicport]) + if not xpe_to_testports: + pytest.skip("No available test ports found for probing (empty XPE mapping)") + max_ports_xpe = max(xpe_to_testports.keys(), key=lambda xpe: len(xpe_to_testports[xpe])) probing_port_ids = xpe_to_testports[max_ports_xpe] From 980ec1c1eaf7a6b1ea944e0059a0ca2f23ed882e Mon Sep 17 00:00:00 2001 From: Xu Chen Date: Thu, 26 Mar 2026 21:53:28 +0800 Subject: [PATCH 12/12] fix: remove broadcom-only variables from logger.info After bcmcmd guard, variables like bcmport_to_sonicport, xpe_to_bcmports are only defined inside the broadcom branch. Logger.info must only reference variables available in both branches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen --- tests/qos/test_qos_probe.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/qos/test_qos_probe.py b/tests/qos/test_qos_probe.py index f83764596c5..aec8f421da5 100644 --- a/tests/qos/test_qos_probe.py +++ b/tests/qos/test_qos_probe.py @@ -457,16 +457,8 @@ def testQosHeadroomPoolProbe( probing_port_ids = src_testPortIds logger.info( - f"sonicport_to_testport {sonicport_to_testport},\n" - f"sonicport_to_pc {sonicport_to_pc},\n" - f"bcmport_to_sonicport {bcmport_to_sonicport},\n" - f"xpe_to_bcmports {xpe_to_bcmports},\n" - f"sonicports_in_upstate {sonicports_in_upstate},\n" - f"xpe_to_sonicports {xpe_to_sonicports},\n" - f"xpe_to_sonicports_in_upstate {xpe_to_sonicports_in_upstate},\n" - f"xpe_to_unique_sonicports {xpe_to_unique_sonicports},\n" - f"xpe_to_testports {xpe_to_testports},\n" f"probing_port_ids {probing_port_ids},\n" + f"sonic_asic_type {sonic_asic_type},\n" f"test_port_ids {dutConfig['testPortIds']},\n" f"testPortIps {dutConfig['testPortIps']}" )