[PCBB] Add testcases to verify separated `DSCP_TO_TC_MAP` on leaf router #6393

bingwang-ms · 2022-09-22T09:50:24Z

Description of PR

Summary:
This PR is to add two new test cases testQosSaiSeparatedDscpQueueMapping and testQosSaiSeparatedDscpToPgMapping to verify separated DSCP_TO_TC_MAP for uplink and downlink on T1.

Code change in sonic-buildimage sonic-net/sonic-buildimage#11569

Type of change

Bug fix
Testbed and Framework(new/improvement)
Test case(new/improvement)

Back port request

201911
202012

Approach

What is the motivation for this PR?

This PR is to add two new test cases testQosSaiSeparatedDscpQueueMapping and testQosSaiSeparatedDscpToPgMapping to verify separated DSCP_TO_TC_MAP for uplink and downlink on T1.

How did you do it?

Swap syncd docker to syncd-rpc
Generate traffic with various DSCP value, and send the packet to DUT from uplink ports or downlink ports
Check the increment in each PG and Queue with SAI rpc call.

How did you verify/test it?

Verified on a TH2 device

qos/test_qos_sai.py::TestQosSai::testQosSaiSeparatedDscpToPgMapping[str-7260cx3-acs-7-None-downstream] PASSED                                                                                                                                            [ 50%]
qos/test_qos_sai.py::TestQosSai::testQosSaiSeparatedDscpToPgMapping[str-7260cx3-acs-7-None-upstream]  PASSED      

qos/test_qos_sai.py::TestQosSai::testQosSaiSeparatedDscpQueueMapping[str-7260cx3-acs-7-None-downstream] PASSED                                                                                                                                           [ 50%]
qos/test_qos_sai.py::TestQosSai::testQosSaiSeparatedDscpQueueMapping[str-7260cx3-acs-7-None-upstream] PASSED                                                                                                                                             [100%]

Any platform specific information?

No.

Supported testbed topology if it's a new test case?

T1 specific test cases.

Documentation

lgtm-com · 2022-09-22T10:19:58Z

This pull request introduces 3 alerts when merging 3c33c95 into 0558e61 - view on LGTM.com

new alerts:

3 for Except block handles 'BaseException'

bingwang-ms · 2022-11-02T07:46:25Z

@stephenxs Can you please help review this PR?

…_tc_map

azure-pipelines · 2022-11-02T07:57:37Z

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed - hook id: trailing-whitespace - exit code: 1 - files were modified by this hook Fixing tests/qos/qos_sai_base.py Fixing tests/common/fixtures/duthost_utils.py Fixing tests/qos/test_qos_sai.py fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/qos/qos_sai_base.py:7:1: F401 'tests.common.fixtures.ptfhost_utils.ptf_portmap_file' imported but unused tests/qos/qos_sai_base.py:10:1: F401 'tests.common.dualtor.dual_tor_utils.upper_tor_host' imported but unused tests/qos/qos_sai_base.py:10:1: F401 'tests.common.dualtor.dual_tor_utils.lower_tor_host' imported but unused ... [truncated extra lines, please run pre-commit locally to view full check results]

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

lgtm-com · 2022-11-02T08:21:37Z

This pull request introduces 3 alerts when merging 68880b8 into b3623be - view on LGTM.com

new alerts:

3 for Except block handles 'BaseException'

stephenxs

We plan to have different DSCP_TO_TC mapping on downlink/uplink ports in t0 as well. so hardcoding AZURE_DOWNLINK is probably not good.
The rest LGTM.

stephenxs · 2022-11-04T09:49:20Z

tests/common/fixtures/duthost_utils.py

+
+
+def separated_dscp_to_tc_map_on_uplink(duthost):
+    """


Maybe we can make it more generic by adding an argument for the expected map name?
Eg., in dual ToR t0 scenario, maybe AZURE_DOWNLINK will be introduced for downlink ports (sonic-net/sonic-buildimage#12605).

Updated. Now the code counts the dscp_to_tc_map for each port.

tests/common/fixtures/duthost_utils.py

…_tc_map

azure-pipelines · 2022-11-07T10:51:59Z

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed - hook id: trailing-whitespace - exit code: 1 - files were modified by this hook Fixing tests/qos/qos_sai_base.py Fixing tests/qos/test_qos_sai.py fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/qos/qos_sai_base.py:7:1: F401 'tests.common.fixtures.ptfhost_utils.ptf_portmap_file' imported but unused tests/qos/qos_sai_base.py:10:1: F401 'tests.common.dualtor.dual_tor_utils.upper_tor_host' imported but unused tests/qos/qos_sai_base.py:10:1: F401 'tests.common.dualtor.dual_tor_utils.lower_tor_host' imported but unused tests/qos/qos_sai_base.py:10:1: F401 'tests.common.dualtor.dual_tor_utils.dualtor_ports' imported but unused ... [truncated extra lines, please run pre-commit locally to view full check results]

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

lgtm-com · 2022-11-07T11:12:23Z

This pull request introduces 5 alerts when merging c5fa8af into a1c1ddb - view on LGTM.com

new alerts:

3 for Except block handles 'BaseException'
2 for Unused import

tests/qos/qos_sai_base.py

stephenxs

LGTM

wangxin · 2022-11-11T13:16:24Z

@bingwang-ms There are conflicts cherry-picking this PR to 202012 branch. Can you help create a separate PR to the 202012 branch?

…ter (#6393) * Add testcases to verify separated DSCP_TO_TC_MAP on T1

bingwang-ms · 2022-11-14T02:06:52Z

Open another PR #6817 to backport the change into 202012 branch.

What is the motivation for this PR? This PR is to backport PR #6393 into 202012 branch. This PR is to add two new test cases testQosSaiSeparatedDscpQueueMapping and testQosSaiSeparatedDscpToPgMapping to verify separated DSCP_TO_TC_MAP for uplink and downlink on T1. Code change in sonic-buildimage sonic-net/sonic-buildimage#11569 How did you do it? Swap syncd docker to syncd-rpc Generate traffic with various DSCP value, and send the packet to DUT from uplink ports or downlink ports Check the increment in each PG and Queue with SAI rpc call. How did you verify/test it? Verified on a TH2 device ``` collected 2 items qos/test_qos_sai.py::TestQosSai::testQosSaiSeparatedDscpQueueMapping[str-7260cx3-acs-7-None-downstream] PASSED [ 50%] qos/test_qos_sai.py::TestQosSai::testQosSaiSeparatedDscpQueueMapping[str-7260cx3-acs-7-None-upstream] PASSED [100%] collected 2 items qos/test_qos_sai.py::TestQosSai::testQosSaiSeparatedDscpToPgMapping[str-7260cx3-acs-7-None-downstream] PASSED [ 50%] qos/test_qos_sai.py::TestQosSai::testQosSaiSeparatedDscpToPgMapping[str-7260cx3-acs-7-None-upstream] PASSED [100%] ``` Any platform specific information? No. Supported testbed topology if it's a new test case? T1 specific test cases.

bingwang-ms added 2 commits September 22, 2022 02:43

Add testcases to verify separated DSCP_TO_TC_MAP on T1

9618400

Add empty line

3c33c95

bingwang-ms requested a review from stephenxs November 2, 2022 07:45

bingwang-ms added 2 commits November 2, 2022 00:56

Sync code with master branch

1af0734

Merge remote-tracking branch 'upstream/master' into separated_dscp_to…

68880b8

…_tc_map

stephenxs reviewed Nov 4, 2022

View reviewed changes

bingwang-ms added 3 commits November 7, 2022 01:37

Cache qos maps

af9b73f

Merge remote-tracking branch 'upstream/master' into separated_dscp_to…

fd70142

…_tc_map

Always enable dscp_to_pg_mapping test

c5fa8af

bingwang-ms mentioned this pull request Nov 8, 2022

Update WRR test to support qos remapping #6737

Merged

6 tasks

stephenxs reviewed Nov 8, 2022

View reviewed changes

tests/qos/qos_sai_base.py Show resolved Hide resolved

stephenxs approved these changes Nov 8, 2022

View reviewed changes

lolyu approved these changes Nov 8, 2022

View reviewed changes

bingwang-ms merged commit 7afbb6d into sonic-net:master Nov 8, 2022

neethajohn added Request for 202012 branch Request for 202205 branch labels Nov 8, 2022

wangxin added the Cherry Pick Conflict_202012 label Nov 11, 2022

wangxin pushed a commit that referenced this pull request Nov 11, 2022

[PCBB] Add testcases to verify separated DSCP_TO_TC_MAP on leaf rou…

b460d23

…ter (#6393) * Add testcases to verify separated DSCP_TO_TC_MAP on T1

wangxin added the Included in 202205 branch label Nov 11, 2022

bingwang-ms mentioned this pull request Nov 14, 2022

[PCBB] [202012] Add testcases to verify separated DSCP_TO_TC_MAP on leaf router #6817

Merged

5 tasks

wangxin added the Included in 202012 branch label Nov 14, 2022

wangxin removed the Cherry Pick Conflict_202012 label Nov 14, 2022

vcheketx mentioned this pull request Nov 21, 2022

[qos_sai] portName referenced before assignment on t1-lag topo #6864

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PCBB] Add testcases to verify separated `DSCP_TO_TC_MAP` on leaf router #6393

[PCBB] Add testcases to verify separated `DSCP_TO_TC_MAP` on leaf router #6393

bingwang-ms commented Sep 22, 2022

lgtm-com bot commented Sep 22, 2022

bingwang-ms commented Nov 2, 2022

azure-pipelines bot commented Nov 2, 2022

lgtm-com bot commented Nov 2, 2022

stephenxs left a comment

stephenxs Nov 4, 2022

bingwang-ms Nov 7, 2022

azure-pipelines bot commented Nov 7, 2022

lgtm-com bot commented Nov 7, 2022

stephenxs left a comment

wangxin commented Nov 11, 2022

bingwang-ms commented Nov 14, 2022

[PCBB] Add testcases to verify separated DSCP_TO_TC_MAP on leaf router #6393

[PCBB] Add testcases to verify separated DSCP_TO_TC_MAP on leaf router #6393

Conversation

bingwang-ms commented Sep 22, 2022

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

lgtm-com bot commented Sep 22, 2022

bingwang-ms commented Nov 2, 2022

azure-pipelines bot commented Nov 2, 2022

lgtm-com bot commented Nov 2, 2022

stephenxs left a comment

Choose a reason for hiding this comment

stephenxs Nov 4, 2022

Choose a reason for hiding this comment

bingwang-ms Nov 7, 2022

Choose a reason for hiding this comment

azure-pipelines bot commented Nov 7, 2022

lgtm-com bot commented Nov 7, 2022

stephenxs left a comment

Choose a reason for hiding this comment

wangxin commented Nov 11, 2022

bingwang-ms commented Nov 14, 2022

[PCBB] Add testcases to verify separated `DSCP_TO_TC_MAP` on leaf router #6393

[PCBB] Add testcases to verify separated `DSCP_TO_TC_MAP` on leaf router #6393