Skip to content

[QoS]Adding support to read and verify the VOQ counters in LossyQueueTest#11000

Merged
yxieca merged 4 commits intosonic-net:masterfrom
ansrajpu-git:LossyQ_voq
Mar 20, 2024
Merged

[QoS]Adding support to read and verify the VOQ counters in LossyQueueTest#11000
yxieca merged 4 commits intosonic-net:masterfrom
ansrajpu-git:LossyQ_voq

Conversation

@ansrajpu-git
Copy link
Copy Markdown
Contributor

Description of PR

Summary:
Adding support to read and verify the VOQ counters in LossyQueueTest for the dnx platform.
Below are the changes made to achieve this:

  • Collect all the system ports associated with the destination port of the DUT (qos_sai_base.py)
  • Get the voq_id for all for the corresponding system ports and read the voq_counters( switch.py)
  • Read & verify the voq_counters in Test (sai_qos_tests.py)

Fixes # (issue)

Type of change

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

Back port request

  • 201911
  • 202012
  • 202205
  • 202305

Approach

What is the motivation for this PR?

How did you do it?

Below are the changes made to support voq counters in LossyQueueTest:

  • Collect all the system ports associated with the destination port of the DUT (qos_sai_base.py)
  • Get the voq_id for all for the corresponding system ports and read the voq_counters( switch.py)
  • Read & verify the voq_counters in Test (sai_qos_tests.py)

How did you verify/test it?

Executed the Qos tests

Any platform specific information?

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

Documentation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we add vendor/chip specific check as it system_port may not be applicable for all ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please share more info on this logic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As we discussed, the pg value is set by adding 2 (pg = int(self.test_params['pg']) + 2) in the initialization. Hence reducing 2 here to get the correct voq_index

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need queue1 here ? Can't we work with VoQ id directly ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed the naming convention to 'counter'. It is just a counter value.

@arlakshm
Copy link
Copy Markdown
Contributor

arlakshm commented Mar 7, 2024

@kenneth-arista and @ysmanman for viz..

@vmittal-msft
Copy link
Copy Markdown
Contributor

@ansrajpu-git please fix conflicts.
@yxieca @wangxin please help merge.

@mssonicbld
Copy link
Copy Markdown
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...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Failed
- hook id: check-ast
- exit code: 1

tests/qos/qos_sai_base.py: failed parsing with CPython 3.8.10:

Traceback (most recent call last):
File "/home/AzDevOps/.cache/pre-commit/repogovuhxqo/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/qos/qos_sai_base.py", line 794
"dst_sys_ports": dst_all_sys_port
^
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>

@ansrajpu-git
Copy link
Copy Markdown
Contributor Author

@ansrajpu-git please fix conflicts. @yxieca @wangxin please help merge.

@vmittal-msft ,resolved conflicts.

@vmittal-msft
Copy link
Copy Markdown
Contributor

@yxieca @wangxin please help merge

@yxieca yxieca merged commit ccca897 into sonic-net:master Mar 20, 2024
@ansrajpu-git
Copy link
Copy Markdown
Contributor Author

Decided to postpone this PR merge to 202205 branch now, as this introduces new SAI modules.

@XuChen-MSFT
Copy link
Copy Markdown
Contributor

@ansrajpu-git @vmittal-msft need to cherry-pick to 202311 ?

@rlhui rlhui added the Test gap label Jun 19, 2024
kenneth-arista added a commit to kenneth-arista/sonic-mgmt that referenced this pull request Jul 17, 2024
sonic-net#11000 only can handle
multi-asic linecards. This fixes the limitation foro single ASIC
linecards.
yxieca pushed a commit that referenced this pull request Jul 24, 2024
#11000 only can handle
multi-asic linecards. This fixes the limitation foro single ASIC
linecards.
arista-hpandya pushed a commit to arista-hpandya/sonic-mgmt that referenced this pull request Oct 2, 2024
sonic-net#11000 only can handle
multi-asic linecards. This fixes the limitation foro single ASIC
linecards.
vikshaw-Nokia pushed a commit to vikshaw-Nokia/sonic-mgmt that referenced this pull request Oct 23, 2024
sonic-net#11000 only can handle
multi-asic linecards. This fixes the limitation foro single ASIC
linecards.
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Nov 22, 2024
sonic-net#11000 only can handle
multi-asic linecards. This fixes the limitation foro single ASIC
linecards.
mssonicbld pushed a commit that referenced this pull request Nov 22, 2024
#11000 only can handle
multi-asic linecards. This fixes the limitation foro single ASIC
linecards.
patrickmacarthur pushed a commit to patrickmacarthur/sonic-mgmt that referenced this pull request Nov 22, 2024
sonic-net#11000 only can handle
multi-asic linecards. This fixes the limitation
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.

9 participants