Fix testQosSaiDwrr issue

tests/qos/args/qos_sai_args.py

@@ -53,3 +53,14 @@ def add_qos_sai_args(parser):
         default=False,
         help="Test QoS on dual ToR ports"
     )
+
+    qos_group.addoption(
+        "--port_target_speed",
+        action="store",
+        type=str,
+        default=None,
+        help="port_target_speed is only for testQosSaiDwrr."
+             "When it is None, test will do nothing,"
+             "When it is set a value of port speed, the tested dst port and the corresponding fanout port"
+             "will be changed to the set value. It can be set to like 50000, 10000"
+    )

tests/qos/qos_sai_base.py

@@ -1509,42 +1509,10 @@ def populateArpEntries(
             Raises:
                 RunAnsibleModuleFail if ptf test fails
         """
+        self.populate_arp_entries(
+            get_src_dst_asic_and_duts, ptfhost, dutTestParams,
+            dutConfig, releaseAllPorts, handleFdbAging, tbinfo, lower_tor_host)
 
-        dut_asic = get_src_dst_asic_and_duts['src_asic']
-        duthost = get_src_dst_asic_and_duts['src_dut']
-
-        # This is not needed in T2.
-        if "t2" in dutTestParams["topo"]:
-            yield
-            return
-
-        dut_asic.command('sonic-clear fdb all')
-        dut_asic.command('sonic-clear arp')
-
-        saiQosTest = None
-        if dutTestParams["topo"] in self.SUPPORTED_T0_TOPOS:
-            saiQosTest = "sai_qos_tests.ARPpopulate"
-        elif dutTestParams["topo"] in self.SUPPORTED_PTF_TOPOS:
-            saiQosTest = "sai_qos_tests.ARPpopulatePTF"
-        else:
-            for dut_asic in get_src_dst_asic_and_duts['all_asics']:
-                result = dut_asic.command("arp -n")
-                pytest_assert(result["rc"] == 0, "failed to run arp command on {0}".format(dut_asic.sonichost.hostname))
-                if result["stdout"].find("incomplete") == -1:
-                    saiQosTest = "sai_qos_tests.ARPpopulate"
-
-        if saiQosTest:
-            testParams = dutTestParams["basicParams"]
-            testParams.update(dutConfig["testPorts"])
-            testParams.update({
-                "testPortIds": dutConfig["testPortIds"],
-                "testPortIps": dutConfig["testPortIps"]
-            })
-            self.runPtfTest(
-                ptfhost, testCase=saiQosTest, testParams=testParams
-            )
-        yield
-        return
 
     @pytest.fixture(scope='class', autouse=True)
     def dut_disable_ipv6(self, duthosts, get_src_dst_asic_and_duts, tbinfo, lower_tor_host):
@@ -2083,3 +2051,45 @@ def select_port_ids_for_mellnaox_device(self, duthost, mgFacts, testPortIds):
                 max_port_num = len(port_list)
         logger.info("Test ports ids is{}".format(test_port_ids))
         return test_port_ids
+
+    def populate_arp_entries(
+            self, get_src_dst_asic_and_duts,
+            ptfhost, dutTestParams, dutConfig, releaseAllPorts, handleFdbAging, tbinfo, lower_tor_host  # noqa F811
+    ):
+        """
+        Update ARP entries of QoS SAI test ports
+        """
+        dut_asic = get_src_dst_asic_and_duts['src_asic']
+
+        # This is not needed in T2.
+        if "t2" in dutTestParams["topo"]:
+            yield
+            return
+
+        dut_asic.command('sonic-clear fdb all')
+        dut_asic.command('sonic-clear arp')
+
+        saiQosTest = None
+        if dutTestParams["topo"] in self.SUPPORTED_T0_TOPOS:
+            saiQosTest = "sai_qos_tests.ARPpopulate"
+        elif dutTestParams["topo"] in self.SUPPORTED_PTF_TOPOS:
+            saiQosTest = "sai_qos_tests.ARPpopulatePTF"
+        else:
+            for dut_asic in get_src_dst_asic_and_duts['all_asics']:
+                result = dut_asic.command("arp -n")
+                pytest_assert(result["rc"] == 0, "failed to run arp command on {0}".format(dut_asic.sonichost.hostname))
+                if result["stdout"].find("incomplete") == -1:
+                    saiQosTest = "sai_qos_tests.ARPpopulate"
+
+        if saiQosTest:
+            testParams = dutTestParams["basicParams"]
+            testParams.update(dutConfig["testPorts"])
+            testParams.update({
+                "testPortIds": dutConfig["testPortIds"],
+                "testPortIps": dutConfig["testPortIps"]
+            })
+            self.runPtfTest(
+                ptfhost, testCase=saiQosTest, testParams=testParams
+            )
+        yield
+        return

tests/qos/test_qos_sai.py

@@ -37,6 +37,8 @@
 from tests.common.helpers.pfc_storm import PFCStorm
 from tests.pfcwd.files.pfcwd_helper import set_pfc_timers, start_wd_on_ports
 from qos_sai_base import QosSaiBase
+from tests.common.platform.device_utils import list_dut_fanout_connections
+from tests.common.utilities import wait_until
 
 logger = logging.getLogger(__name__)
 
@@ -50,7 +52,11 @@
 def ignore_expected_loganalyzer_exception(get_src_dst_asic_and_duts, loganalyzer):
     """ignore the syslog ERR syncd0#syncd: [03:00.0] brcm_sai_set_switch_attribute:1920 updating switch mac addr failed with error -2"""
     ignore_regex = [
-            ".*ERR syncd[0-9]*#syncd.*brcm_sai_set_switch_attribute.*updating switch mac addr failed with error.*"
+            ".*ERR syncd[0-9]*#syncd.*brcm_sai_set_switch_attribute.*updating switch mac addr failed with error.*",
+            # The following error log is related to
+            # the bug of https://github.com/sonic-net/sonic-buildimage/issues/13265
+            ".*ERR lldp#lldpmgrd.*Command failed.*lldpcli.*configure.*ports.*lldp.*unknown command from argument"
+            ".*configure.*command was failed.*times, disabling retry.*"
     ]
 
     if loganalyzer:
@@ -120,6 +126,87 @@ class TestQosSai(QosSaiBase):
 
     BREAKOUT_SKUS = ['Arista-7050-QX-32S']
 
+    @pytest.fixture(scope='function')
+    def change_port_speed(
+            self, request, ptfhost, duthosts, dutTestParams, fanouthosts, dutConfig, tbinfo,
+            get_src_dst_asic_and_duts, releaseAllPorts, handleFdbAging, lower_tor_host):
+        """When port_target_speed is not None, change dut dst port speed and the corresponding port speed,
+           and then recover them.
+        """
+        target_speed = request.config.getoption("--port_target_speed")
+        is_change_sport_speed = False
+
+        if target_speed:
+            logger.info("target speed is {}".format(target_speed))
+            duthost = get_src_dst_asic_and_duts['src_dut']
+            dut_dst_port = dutConfig['dutInterfaces'][dutConfig["testPorts"]["dst_port_id"]]
+            dut_int_status = duthost.get_interfaces_status()
+            original_speed = dut_int_status[dut_dst_port]["speed"].replace("G", "000")
+
+            if int(target_speed) < int(original_speed):
+                dut_port_list = [dut_dst_port]
+                fanout_port_list = []
+
+                def _get_dut_change_speed_port_list():
+                    src_mgFacts = duthost.get_extended_minigraph_facts(tbinfo)
+                    portchannels = src_mgFacts["minigraph_portchannels"]
+                    for po, po_info in portchannels.items():
+                        if dut_dst_port in po_info['members']:
+                            dut_port_list = po_info['members']
+                            break
+                    logger.info("dut port list :{}".format(dut_port_list))
+
+                def _get_fanout_and_fanout_change_speed_port_list():
+                    for dut_port, fanout, fanout_port in list_dut_fanout_connections(duthost, fanouthosts):
+                        if dut_port in dut_port_list:
+                            fanout_port_list.append(fanout_port)
+                            fanout_host = fanout
+                    logger.info("fanout port list :{}".format(fanout_port_list))
+                    return fanout_host
+
+                def _update_target_speed(fanout_host, target_speed):
+                    logger.info("Get one speed that is smaller or equal than target speed")
+                    all_dut_speeds = duthost.get_supported_speeds(dut_port_list[0])
+                    all_fanout_speeds = fanout_host.get_supported_speeds(fanout_port_list[0])
+                    common_speeds = list(set(all_dut_speeds).intersection(set(all_fanout_speeds)))
+                    if target_speed not in common_speeds:
+                        min_speed = common_speeds[0]
+                        for speed in common_speeds:
+                            if int(min_speed) > int(speed):
+                                min_speed = speed
+                            target_speed = min_speed
+                    logger.info("Updated target_speed is {}".format(target_speed))
+                    return target_speed
+
+                def _set_speed_and_populate_arp(fanout_host, speed):
+                    for dut_port in dut_port_list:
+                        logger.info('DUT: Set {} speed to {}'.format(dut_port, speed))
+                        duthost.shell("sudo config interface speed {} {}".format(dut_port, speed))
+                    for fanout_port in fanout_port_list:
+                        logger.info('Fanout: Set {} speed to {}'.format(fanout_host, speed))
+                        fanout_host.set_speed(fanout_port, speed)
+                    wait_until(60, 1, 0, duthost.links_status_up, dut_port_list)
+
+                    logger.info("populate arp, because change speed will cause port flap")
+                    self.populate_arp_entries(
+                        get_src_dst_asic_and_duts, ptfhost, dutTestParams, dutConfig,
+                        releaseAllPorts, handleFdbAging, tbinfo, lower_tor_host)
+
+                _get_dut_change_speed_port_list()
+                fanout_host = _get_fanout_and_fanout_change_speed_port_list()
+                target_speed = _update_target_speed(fanout_host, target_speed)
+
+                if int(target_speed) < int(original_speed):
+                    logger.info("Change speed to {}".format(target_speed))
+                    is_change_sport_speed = True
+                    _set_speed_and_populate_arp(fanout_host, target_speed)
+
+        yield
+
+        if is_change_sport_speed:
+            logger.info("Restore speed to {}".format(original_speed))
+            _set_speed_and_populate_arp(fanout_host, original_speed)
+
     def replaceNonExistentPortId(self, availablePortIds, portIds):
         '''
         if port id of availablePortIds/dst_port_ids is not existing in availablePortIds
@@ -1335,7 +1422,7 @@ def testQosSaiDot1pPgMapping(
         )
 
     def testQosSaiDwrr(
-        self, ptfhost, duthosts, get_src_dst_asic_and_duts, dutTestParams, dutConfig, dutQosConfig,
+        self, ptfhost, duthosts, get_src_dst_asic_and_duts, dutTestParams, dutConfig, dutQosConfig, change_port_speed
     ):
         """
             Test QoS SAI DWRR
@@ -1399,6 +1486,18 @@ def testQosSaiDwrr(
         else:
             testParams["ecn"] = qosConfig["lossy_queue_1"]["ecn"]
 
+        # To overcome this case:
+        # When the previous test case just sends a large of packets only by one queue such as queue1,
+        # then Dwrr test might fail, because queue1 has got much chance to send packets before,
+        # so it will get less chance to send packets than expected.
+        # Therefore the first run is a dry run, and will not check Dwrr function.
+        # After the dry run, all tested queues can be scheduled so that all queues are at the same start.
+        testParams["dry_run"] = True
+        self.runPtfTest(
+            ptfhost, testCase="sai_qos_tests.WRRtest", testParams=testParams
+        )
+
+        testParams["dry_run"] = False
         self.runPtfTest(
             ptfhost, testCase="sai_qos_tests.WRRtest", testParams=testParams
         )

tests/saitests/py3/sai_qos_tests.py

@@ -3049,6 +3049,7 @@ def runTest(self):
         src_port_vlan = self.test_params['src_port_vlan']
         src_port_mac = self.dataplane.get_mac(0, src_port_id)
         qos_remap_enable = bool(self.test_params.get('qos_remap_enable', False))
+        dry_run = bool(self.test_params.get('dry_run', False))
         print("dst_port_id: %d, src_port_id: %d qos_remap_enable: %d" %
               (dst_port_id, src_port_id, qos_remap_enable))
         print("dst_port_mac: %s, src_port_mac: %s, src_port_ip: %s, dst_port_ip: %s" % (
@@ -3206,9 +3207,9 @@ def runTest(self):
         print(diff_list, file=sys.stderr)
 
         for dscp, diff in diff_list:
-           if platform_asic and platform_asic == "broadcom-dnx":
+            if platform_asic and platform_asic == "broadcom-dnx":
                 logging.info("On J2C+ can't control how packets are dequeued (CS00012272267) - so ignoring diff check now")
-           else:
+            elif not dry_run:
                 assert diff < limit, "Difference for %d is %d which exceeds limit %d" % (dscp, diff, limit)
 
         # Read counters