Disable Cisco VOQ WD for dualtor QOS, improve PG test debuggability (sonic-net#20502) (sonic-net#20870)

Double commit sonic-net#20502

Disable VOQ WD during test_tunnel_qos_remap.py
Commonize some aspects of the VOQ WD disabling from QOS SAI to avoid major code duplication.
Improve performance of test for cisco by reducing unnecessary 8-second per loop wait time to 1 second. (Test passed with 0.5 seconds as well, since this is a SAI bypass operation the updated stat should be near-instant).
Improve debuggability of PG tunnel decap test by logging all failures and summarizing a report at the end.
Fix up test_voq_watchdog.py test with new commonization. Rename parametrization to avoid shadowing.

Author: rbpittman

tests/qos/qos_helpers.py

@@ -1,5 +1,7 @@
+from contextlib import contextmanager
 from netaddr import IPNetwork
 from .qos_fixtures import lossless_prio_dscp_map, leaf_fanouts      # noqa F401
+from tests.common.cisco_data import is_cisco_device, copy_set_voq_watchdog_script_cisco_8000, run_dshell_command
 import re
 import os
 import json
@@ -259,3 +261,58 @@ def dutBufferConfig(duthost, dut_asic=None):
     except Exception as err:
         logger.info(err)
     return bufferConfig
+
+
+def voq_watchdog_enabled(get_src_dst_asic_and_duts):
+    dst_dut = get_src_dst_asic_and_duts['dst_dut']
+    if not is_cisco_device(dst_dut):
+        return False
+    namespace_option = "-n asic0" if dst_dut.facts.get("modular_chassis") else ""
+    show_command = "show platform npu global {}".format(namespace_option)
+    result = run_dshell_command(dst_dut, show_command)
+    pattern = r"voq_watchdog_enabled +: +True"
+    match = re.search(pattern, result["stdout"])
+    return match
+
+
+def modify_voq_watchdog(duthosts, get_src_dst_asic_and_duts, enable):
+    # Skip if voq watchdog is not enabled.
+    if not voq_watchdog_enabled(get_src_dst_asic_and_duts):
+        logger.info("voq_watchdog is not enabled, skipping modify voq watchdog")
+        return
+
+    dst_dut = get_src_dst_asic_and_duts['dst_dut']
+    dst_asic = get_src_dst_asic_and_duts['dst_asic']
+    dut_list = [dst_dut]
+    asic_index_list = [dst_asic.asic_index]
+
+    if not get_src_dst_asic_and_duts["single_asic_test"]:
+        src_dut = get_src_dst_asic_and_duts['src_dut']
+        src_asic = get_src_dst_asic_and_duts['src_asic']
+        dut_list.append(src_dut)
+        asic_index_list.append(src_asic.asic_index)
+        # fabric card asics
+        for rp_dut in duthosts.supervisor_nodes:
+            for asic in rp_dut.asics:
+                dut_list.append(rp_dut)
+                asic_index_list.append(asic.asic_index)
+
+    # Modify voq watchdog.
+    for (dut, asic_index) in zip(dut_list, asic_index_list):
+        copy_set_voq_watchdog_script_cisco_8000(
+            dut=dut,
+            asic=asic_index,
+            enable=enable)
+        cmd_opt = "-n asic{}".format(asic_index)
+        if not dst_dut.sonichost.is_multi_asic:
+            cmd_opt = ""
+        dut.shell("sudo show platform npu script {} -s set_voq_watchdog.py".format(cmd_opt))
+
+
+@contextmanager
+def disable_voq_watchdog(duthosts, get_src_dst_asic_and_duts):
+    # Disable voq watchdog.
+    modify_voq_watchdog(duthosts, get_src_dst_asic_and_duts, enable=False)
+    yield
+    # Enable voq watchdog.
+    modify_voq_watchdog(duthosts, get_src_dst_asic_and_duts, enable=True)

tests/qos/qos_sai_base.py

@@ -11,14 +11,12 @@
 import copy
 import time
 import collections
-from contextlib import contextmanager
 
 from tests.common.fixtures.ptfhost_utils import ptf_portmap_file  # noqa F401
 from tests.common.helpers.assertions import pytest_assert, pytest_require
 from tests.common.helpers.multi_thread_utils import SafeThreadPoolExecutor
 from tests.common.mellanox_data import is_mellanox_device as isMellanoxDevice
-from tests.common.cisco_data import is_cisco_device, copy_set_voq_watchdog_script_cisco_8000, \
-    copy_dshell_script_cisco_8000, run_dshell_command
+from tests.common.cisco_data import is_cisco_device, copy_dshell_script_cisco_8000, run_dshell_command
 from tests.common.dualtor.dual_tor_common import active_standby_ports  # noqa F401
 from tests.common.dualtor.dual_tor_utils import upper_tor_host, lower_tor_host, dualtor_ports, is_tunnel_qos_remap_enabled  # noqa F401
 from tests.common.dualtor.mux_simulator_control \
@@ -35,7 +33,7 @@
 from tests.common.snappi_tests.qos_fixtures import get_pfcwd_config, reapply_pfcwd
 from tests.common.snappi_tests.common_helpers import \
         stop_pfcwd, disable_packet_aging, enable_packet_aging
-from .qos_helpers import dutBufferConfig
+from .qos_helpers import dutBufferConfig, disable_voq_watchdog
 
 logger = logging.getLogger(__name__)
 
@@ -2783,66 +2781,14 @@ def set_cir_change(self, get_src_dst_asic_and_duts, dutConfig):
         yield
         return
 
-    def voq_watchdog_enabled(self, get_src_dst_asic_and_duts):
-        dst_dut = get_src_dst_asic_and_duts['dst_dut']
-        if not is_cisco_device(dst_dut):
-            return False
-        namespace_option = "-n asic0" if dst_dut.facts.get("modular_chassis") else ""
-        show_command = "show platform npu global {}".format(namespace_option)
-        result = run_dshell_command(dst_dut, show_command)
-        pattern = r"voq_watchdog_enabled +: +True"
-        match = re.search(pattern, result["stdout"])
-        return match
-
-    def modify_voq_watchdog(self, duthosts, get_src_dst_asic_and_duts, dutConfig, enable):
-        # Skip if voq watchdog is not enabled.
-        if not self.voq_watchdog_enabled(get_src_dst_asic_and_duts):
-            logger.info("voq_watchdog is not enabled, skipping modify voq watchdog")
-            return
-
-        dst_dut = get_src_dst_asic_and_duts['dst_dut']
-        dst_asic = get_src_dst_asic_and_duts['dst_asic']
-        dut_list = [dst_dut]
-        asic_index_list = [dst_asic.asic_index]
-
-        if not get_src_dst_asic_and_duts["single_asic_test"]:
-            src_dut = get_src_dst_asic_and_duts['src_dut']
-            src_asic = get_src_dst_asic_and_duts['src_asic']
-            dut_list.append(src_dut)
-            asic_index_list.append(src_asic.asic_index)
-            # fabric card asics
-            for rp_dut in duthosts.supervisor_nodes:
-                for asic in rp_dut.asics:
-                    dut_list.append(rp_dut)
-                    asic_index_list.append(asic.asic_index)
-
-        # Modify voq watchdog.
-        for (dut, asic_index) in zip(dut_list, asic_index_list):
-            copy_set_voq_watchdog_script_cisco_8000(
-                dut=dut,
-                asic=asic_index,
-                enable=enable)
-            cmd_opt = "-n asic{}".format(asic_index)
-            if not dst_dut.sonichost.is_multi_asic:
-                cmd_opt = ""
-            dut.shell("sudo show platform npu script {} -s set_voq_watchdog.py".format(cmd_opt))
-
-    @contextmanager
-    def disable_voq_watchdog(self, duthosts, get_src_dst_asic_and_duts, dutConfig):
-        # Disable voq watchdog.
-        self.modify_voq_watchdog(duthosts, get_src_dst_asic_and_duts, dutConfig, enable=False)
-        yield
-        # Enable voq watchdog.
-        self.modify_voq_watchdog(duthosts, get_src_dst_asic_and_duts, dutConfig, enable=True)
-
     @pytest.fixture(scope='function')
-    def disable_voq_watchdog_function_scope(self, duthosts, get_src_dst_asic_and_duts, dutConfig):
-        with self.disable_voq_watchdog(duthosts, get_src_dst_asic_and_duts, dutConfig) as result:
+    def disable_voq_watchdog_function_scope(self, duthosts, get_src_dst_asic_and_duts):
+        with disable_voq_watchdog(duthosts, get_src_dst_asic_and_duts) as result:
             yield result
 
     @pytest.fixture(scope='class')
-    def disable_voq_watchdog_class_scope(self, duthosts, get_src_dst_asic_and_duts, dutConfig):
-        with self.disable_voq_watchdog(duthosts, get_src_dst_asic_and_duts, dutConfig) as result:
+    def disable_voq_watchdog_class_scope(self, duthosts, get_src_dst_asic_and_duts):
+        with disable_voq_watchdog(duthosts, get_src_dst_asic_and_duts) as result:
             yield result
 
     def oq_watchdog_enabled(self, get_src_dst_asic_and_duts):

tests/qos/test_tunnel_qos_remap.py

@@ -29,7 +29,8 @@
     dut_config, qos_config, tunnel_qos_maps, run_ptf_test, toggle_mux_to_host,\
     setup_module, update_docker_services, swap_syncd, counter_poll_config                               # noqa F401
 from .tunnel_qos_remap_base import leaf_fanout_peer_info, start_pfc_storm, \
-    stop_pfc_storm, get_queue_counter, get_queue_watermark, disable_packet_aging                        # noqa F401
+    stop_pfc_storm, get_queue_counter, get_queue_watermark, disable_packet_aging, \
+    disable_voq_watchdog_dualtor  # noqa F401
 from ptf import testutils
 from ptf.testutils import simple_tcp_packet
 from tests.common.fixtures.conn_graph_facts import conn_graph_facts, fanout_graph_facts     # noqa F401

tests/qos/test_voq_watchdog.py

@@ -26,6 +26,7 @@
 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 .qos_helpers import voq_watchdog_enabled, modify_voq_watchdog
 from .qos_sai_base import QosSaiBase
 
 logger = logging.getLogger(__name__)
@@ -55,13 +56,13 @@ class TestVoqWatchdog(QosSaiBase):
     """
     @pytest.fixture(scope="class", autouse=True)
     def check_skip_voq_watchdog_test(self, get_src_dst_asic_and_duts):
-        if not self.voq_watchdog_enabled(get_src_dst_asic_and_duts):
+        if not voq_watchdog_enabled(get_src_dst_asic_and_duts):
             pytest.skip("Voq watchdog test is skipped since voq watchdog is not enabled.")
 
-    @pytest.mark.parametrize("voq_watchdog_enabled", [True, False])
+    @pytest.mark.parametrize("enable_voq_watchdog", [True, False])
     def testVoqWatchdog(
             self, ptfhost, dutTestParams, dutConfig, dutQosConfig,
-            duthosts, get_src_dst_asic_and_duts, voq_watchdog_enabled
+            duthosts, get_src_dst_asic_and_duts, enable_voq_watchdog
     ):
         """
             Test VOQ watchdog
@@ -71,16 +72,16 @@ def testVoqWatchdog(
                 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
-                voq_watchdog_enabled (bool): if VOQ watchdog is enabled or not
+                enable_voq_watchdog (bool): Whether to enable or disable VOQ watchdog during the test
             Returns:
                 None
             Raises:
                 RunAnsibleModuleFail if ptf test fails
         """
 
         try:
-            if not voq_watchdog_enabled:
-                self.modify_voq_watchdog(duthosts, get_src_dst_asic_and_duts, dutConfig, enable=False)
+            if not enable_voq_watchdog:
+                modify_voq_watchdog(duthosts, get_src_dst_asic_and_duts, enable=False)
 
             testParams = dict()
             testParams.update(dutTestParams["basicParams"])
@@ -94,7 +95,7 @@ def testVoqWatchdog(
                 "src_port_vlan": dutConfig["testPorts"]["src_port_vlan"],
                 "packet_size": 1350,
                 "pkts_num": PKTS_NUM,
-                "voq_watchdog_enabled": voq_watchdog_enabled,
+                "voq_watchdog_enabled": enable_voq_watchdog,
                 "dutInterfaces": dutConfig["dutInterfaces"],
                 "testPorts": dutConfig["testPorts"],
             })
@@ -108,5 +109,5 @@ def testVoqWatchdog(
                 testParams=testParams)
 
         finally:
-            if not voq_watchdog_enabled:
-                self.modify_voq_watchdog(duthosts, get_src_dst_asic_and_duts, dutConfig, enable=True)
+            if not enable_voq_watchdog:
+                modify_voq_watchdog(duthosts, get_src_dst_asic_and_duts, enable=True)

tests/qos/tunnel_qos_remap_base.py

@@ -15,7 +15,7 @@
 from tests.common.fixtures.duthost_utils import dut_qos_maps_module                                 # noqa F401
 from tests.common.fixtures.ptfhost_utils import ptf_portmap_file_module                             # noqa F401
 from tests.common.utilities import get_iface_ip
-from .qos_helpers import dutBufferConfig
+from .qos_helpers import dutBufferConfig, disable_voq_watchdog
 from .files.cisco.qos_param_generator import QosParamCisco
 
 logger = logging.getLogger(__name__)
@@ -530,9 +530,7 @@ def run_ptf_test(ptfhost, test_case='', test_params={}):
     A helper function to run test script on ptf host
     """
     logger.info("Start running {} on ptf host".format(test_case))
-    pytest_assert(ptfhost.shell(
-        argv=[
-            "/root/env-python3/bin/ptf",
+    argv = ["/root/env-python3/bin/ptf",
             "--test-dir",
             "saitests/py3",
             test_case,
@@ -552,7 +550,16 @@ def run_ptf_test(ptfhost, test_case='', test_params={}):
             "--log-file",
             "/tmp/{0}.log".format(test_case),
             "--test-case-timeout",
-            "1200"
-        ],
-        chdir="/root",
-    )["rc"] == 0, "Failed when running test '{0}'".format(test_case))
+            "1200"]
+    result = ptfhost.shell(argv=argv, chdir="/root")
+    pytest_assert(result["rc"] == 0, "Failed when running test '{0}'".format(test_case))
+
+
+@pytest.fixture(scope='module', autouse=True)
+def disable_voq_watchdog_dualtor(duthosts, rand_selected_dut):
+    get_src_dst_asic_and_duts = {}
+    get_src_dst_asic_and_duts["single_asic_test"] = True
+    get_src_dst_asic_and_duts["dst_dut"] = rand_selected_dut
+    get_src_dst_asic_and_duts["dst_asic"] = rand_selected_dut.asics[0]
+    with disable_voq_watchdog(duthosts, get_src_dst_asic_and_duts) as result:
+        yield result

tests/saitests/py3/sai_qos_tests.py

@@ -1587,6 +1587,13 @@ def runTest(self):
             # Disable tx on EGRESS port so that headroom buffer cannot be free
             self.sai_thrift_port_tx_disable(self.dst_client, asic_type, [dst_port_id])
 
+            if 'cisco-8000' in asic_type:
+                PKT_NUM = 50
+                cntr_wait_time = 1
+            else:
+                PKT_NUM = 100
+                cntr_wait_time = 8
+
             # There are packet leak even port tx is disabled (18 packets leak on TD3 found)
             # Hence we send some packet to fill the leak before testing
             if asic_type != 'mellanox':
@@ -1618,11 +1625,9 @@ def runTest(self):
                     else:
                         send_packet(self, src_port_id, pkt, 20)
                 assert not leakout_failed, "Failed filling leakout"
-                time.sleep(10)
-            if 'cisco-8000' in asic_type:
-                PKT_NUM = 50
-            else:
-                PKT_NUM = 100
+                time.sleep(cntr_wait_time)
+
+            fails = []
             for inner_dscp, pg in dscp_to_pg_map.items():
                 logging.info("Iteration: inner_dscp:{}, pg: {}".format(inner_dscp, pg))
                 # Build and send packet to active tor.
@@ -1655,7 +1660,7 @@ def runTest(self):
 
                 send_packet(self, src_port_id, pkt, PKT_NUM)
                 # validate pg counters increment by the correct pkt num
-                time.sleep(8)
+                time.sleep(cntr_wait_time)
 
                 pg_shared_wm_res = sai_thrift_read_pg_shared_watermark(self.src_client, asic_type,
                                                                        port_list['src'][src_port_id])
@@ -1664,9 +1669,14 @@ def runTest(self):
                 logging.info(f"pg_wm_inc: {pg_wm_inc}")
                 lower_bounds = (PKT_NUM - ERROR_TOLERANCE[pg]) * cell_size * cell_occupancy
                 upper_bounds = (PKT_NUM + ERROR_TOLERANCE[pg]) * cell_size * cell_occupancy
-                print("DSCP {}, PG {}, expectation: {} <= {} <= {}".format(
-                    inner_dscp, pg, lower_bounds, pg_wm_inc, upper_bounds), file=sys.stderr)
-                assert lower_bounds <= pg_wm_inc <= upper_bounds
+                msg = "DSCP {}, PG {}, expectation: {} <= {} <= {}, base wmk {}, new wmk {}".format(
+                    inner_dscp, pg, lower_bounds, pg_wm_inc, upper_bounds,
+                    pg_shared_wm_res_base[pg], pg_shared_wm_res[pg])
+                print(msg, file=sys.stderr)
+                if not (lower_bounds <= pg_wm_inc <= upper_bounds):
+                    print("FAILED CHECK", file=sys.stderr)
+                    fails.append(msg)
+            assert len(fails) == 0, "Some PG watermark checks failed ({}): \n{}".format(len(fails), "\n".join(fails))
 
         finally:
             # Enable tx on dest port