Skip to content

Improvising qos tests by tunning the qos params for single_asic, single_dut_multi_asic and multi_dut#8222

Merged
vmittal-msft merged 10 commits intosonic-net:masterfrom
ansrajpu-git:qos_fix
Aug 22, 2023
Merged

Improvising qos tests by tunning the qos params for single_asic, single_dut_multi_asic and multi_dut#8222
vmittal-msft merged 10 commits intosonic-net:masterfrom
ansrajpu-git:qos_fix

Conversation

@ansrajpu-git
Copy link
Contributor

@ansrajpu-git ansrajpu-git commented May 5, 2023

Description of PR

This PR is in continuation of PR# #8149
which was originally part of PR# #6946

The existing QoS (test_qos_sai.py) is written to accommodate a single asic on a single Dut. But, we require the same tests to be executed against a T2 chassis (with single/multi-asic linecards) and multi-asic pizza boxes.

Summary:
This PR is in continuation to above PR's mentioned

Fixes # (issue)

Type of change

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

Back port request

  • 201911
  • 202012
  • 202205

Approach

What is the motivation for this PR?

1.Qos test cases failed with intermittent errors

How did you do it?

Two issues are addressed here :

1.The dscp queue mapping for LossyQueue Test changed in config file to map to queue 1 of traffic-class instead of  0  since disabling the tx and filling up the queue 0  prevents the lacp packets going out and port channel goes down

2.During Qos test on transmission disable and enable, sometimes on test failure the port dangles in a transmission disable state and did not recover. Switching the step to enable the transmission port before the BCMSAI credit-watchdog enable , eradicate the test failure due to bad port state.

How did you verify/test it?

Executed qos testcases on for single_asic ,single_dut_multi_asic & multi_dut

Any platform specific information?

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

Documentation

@mssonicbld
Copy link
Collaborator

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check for added large files..............................................Passed
check python ast.........................................................Failed
- hook id: check-ast
- exit code: 1

tests/saitests/py3/sai_base_test.py: failed parsing with CPython 3.8.10:

Traceback (most recent call last):
File "/home/AzDevOps/.cache/pre-commit/repoojeenau5/py_env-python3/lib/python3.8/site-packages/pre_commit_hooks/check_ast.py", line 21, in main
ast.parse(f.read(), filename=filename)
File "/usr/lib/python3.8/ast.py", line 47, in parse
return compile(source, filename, mode, flags,
File "tests/saitests/py3/sai_base_test.py", line 12
<<<<<<< HEAD
^
SyntaxError: invalid syntax

...
[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:

  1. 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.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

@mssonicbld
Copy link
Collaborator

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/saitests/py3/sai_base_test.py:12:1: F401 'json' imported but unused
tests/saitests/py3/sai_base_test.py:24:1: F811 redefinition of unused 'socket' from line 13
tests/saitests/py3/sai_base_test.py:138:5: E303 too many blank lines (3)
tests/saitests/py3/sai_qos_tests.py:3115:12: E114 indentation is not a multiple of 4 (comment)

check conditional mark sort..............................................Passed

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

  1. 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.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

@xwjiang-ms
Copy link
Contributor

pre-commit check is a mandatory check, please fix detected issues

@ansrajpu-git ansrajpu-git changed the title Enhance qos tests to support single-asic, multi-asic, and multi-dut testing Improvising qos tests by tunning the qos params for single_asic, single_dut_multi_asic and multi_dut May 8, 2023
@mssonicbld
Copy link
Collaborator

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/saitests/py3/sai_base_test.py:12:1: F401 'json' imported but unused
tests/saitests/py3/sai_base_test.py:24:1: F811 redefinition of unused 'socket' from line 13
tests/saitests/py3/sai_base_test.py:138:5: E303 too many blank lines (3)
tests/saitests/py3/sai_qos_tests.py:442:17: E265 block comment should start with '# '
tests/saitests/py3/sai_qos_tests.py:3121:12: E114 indentation is not a multiple of 4 (comment)

check conditional mark sort..............................................Passed

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

  1. 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.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

@mssonicbld
Copy link
Collaborator

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/saitests/py3/sai_qos_tests.py:451:17: E265 block comment should start with '# '

check conditional mark sort..............................................Passed

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

  1. 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.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

@vmittal-msft
Copy link
Contributor

This PR is dependent on #8149. Once we have #8149 pushed in, we can push in this as well.

@judyjoseph
Copy link
Contributor

@vmittal-msft Could you check and review this PR ?

Copy link
Contributor

@vmittal-msft vmittal-msft Aug 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets remove it. This will be fixed by BRCM CSP.

@ansrajpu-git ansrajpu-git force-pushed the qos_fix branch 2 times, most recently from c0dc5c8 to a67cc10 Compare August 8, 2023 14:46
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please keep existing values as we will have fix for this.

@mssonicbld
Copy link
Collaborator

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/saitests/py3/sai_base_test.py:183:21: F821 undefined name 'time'
tests/saitests/py3/sai_base_test.py:190:93: E231 missing whitespace after ','
tests/saitests/py3/sai_base_test.py:193:5: E303 too many blank lines (2)
tests/saitests/py3/sai_base_test.py:206:21: F821 undefined name 'time'
tests/saitests/py3/sai_qos_tests.py:3094:12: E114 indentation is not a multiple of 4 (comment)

flake8...............................................(no files to check)Skipped
check conditional mark sort..............................................Passed

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

  1. 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.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

vmittal-msft
vmittal-msft previously approved these changes Aug 11, 2023
@ansrajpu-git
Copy link
Contributor Author

@XuChen-MSFT ,@wsycqyz ,kindly review & approve

@mssonicbld
Copy link
Collaborator

@ansrajpu-git PR conflicts with 202205 branch

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #9703

mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Aug 28, 2023
…le_dut_multi_asic and multi_dut (sonic-net#8222)

* QoS tunning for LossyQueueTest
* Making assert specific to dnx for DscpQueueMapping qos_test
fix flake8 errors
flake8 issue resolved
* Recommiting the changes of qos.yml to qos_params_j2c.yml
* Reverting dscp to 7 as this is fixed in BRCM CSP
* Revert "Making assert specific to dnx for DscpQueueMapping qos_test"

This reverts commit dcbf90fa1bbdb3778d9ef86a202c30d86bd08114.
* changing dscp value back to 8
* tx_enable/disable retry check corrected
* Flake8 errore fixed
* Making check to run only for chassis in LossyQueue
* Adding check for sai_base for tx_enable/disable
wangxin pushed a commit that referenced this pull request Aug 29, 2023
…le_dut_multi_asic and multi_dut (#8222) (#9703)

This PR is in continuation of PR# #8149
which was originally part of PR# #6946

The existing QoS (test_qos_sai.py) is written to accommodate a single asic on a single Dut. But, we require the same tests to be executed against a T2 chassis (with single/multi-asic linecards) and multi-asic pizza boxes.

What is the motivation for this PR?
1.Qos test cases failed with intermittent errors

How did you do it?
Two issues are addressed here :

1.The dscp queue mapping for LossyQueue Test changed in config file to map to queue 1 of traffic-class instead of  0  since disabling the tx and filling up the queue 0  prevents the lacp packets going out and port channel goes down

2.During Qos test on transmission disable and enable, sometimes on test failure the port dangles in a transmission disable state and did not recover. Switching the step to enable the transmission port before the BCMSAI credit-watchdog enable , eradicate the test failure due to bad port state.

How did you verify/test it?
Executed qos testcases on for single_asic ,single_dut_multi_asic & multi_dut

Co-authored-by: ansrajpu-git <[email protected]>
ansrajpu-git added a commit to ansrajpu-git/sonic-mgmt that referenced this pull request Oct 19, 2023
… single_asic, single_dut_multi_asic and multi_dut (sonic-net#8222) from master to 202205

* QoS tunning for LossyQueueTest
* Making assert specific to dnx for DscpQueueMapping qos_test
fix flake8 errors
flake8 issue resolved
* Recommiting the changes of qos.yml to qos_params_j2c.yml
* Reverting dscp to 7 as this is fixed in BRCM CSP
* Revert "Making assert specific to dnx for DscpQueueMapping qos_test"

This reverts commit dcbf90fa1bbdb3778d9ef86a202c30d86bd08114.
* changing dscp value back to 8
* tx_enable/disable retry check corrected
* Flake8 errore fixed
* Making check to run only for chassis in LossyQueue
* Adding check for sai_base for tx_enable/disable
wangxin pushed a commit that referenced this pull request Oct 23, 2023
… single_asic, single_dut_multi_asic and multi_dut (#8222) from master to 202205 (#10209)

Master PR #8222

QoS tunning for LossyQueueTest
Recommiting the changes of qos.yml to qos_params_j2c.yml
tx_enable/disable retry check corrected
Making check to run only for chassis in LossyQueue
Adding check for sai_base for tx_enable/disable
AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Jan 25, 2024
…le_dut_multi_asic and multi_dut (sonic-net#8222)

* QoS tunning for LossyQueueTest
* Making assert specific to dnx for DscpQueueMapping qos_test
fix flake8 errors
flake8 issue resolved
* Recommiting the changes of qos.yml to qos_params_j2c.yml
* Reverting dscp to 7 as this is fixed in BRCM CSP
* Revert "Making assert specific to dnx for DscpQueueMapping qos_test"

This reverts commit dcbf90fa1bbdb3778d9ef86a202c30d86bd08114.
* changing dscp value back to 8
* tx_enable/disable retry check corrected
* Flake8 errore fixed
* Making check to run only for chassis in LossyQueue
* Adding check for sai_base for tx_enable/disable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

7 participants