-
Notifications
You must be signed in to change notification settings - Fork 727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add qos_params.th5.yaml #14250
base: master
Are you sure you want to change the base?
Add qos_params.th5.yaml #14250
Conversation
Rick is going to update this PR with latest parameter and test changes here. Then we will get it reviewed and merged. |
tests/saitests/py3/sai_qos_tests.py
Outdated
@@ -2609,6 +2615,12 @@ def runTest(self): | |||
(pkts_num_egr_mem + pkts_num_leak_out + pkts_num_dismiss_pfc + | |||
hysteresis) // cell_occupancy + margin - 1 | |||
) | |||
elif hwsku == 'Arista-7060X6-64PE-256x200G': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sync'ed offline. let's check TH5 for better change in the next PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm.
Moved this back to draft for a moment. There's an intermittent failure in one of the test cases due to the margin being 0. I'll update this PR after running the tests locally a few times to verify adding a small margin fixes this. |
@Janetxxx , do you mind to cherry pick this PR to our test branch? This should get the qos tests fixed. |
@Janetxxx I've pushed another small fix that fixes flaky failures in testQosSaiHeadroomPoolWatermark |
tests/qos/files/qos_params.th5.yaml
Outdated
@@ -0,0 +1,199 @@ | |||
qos_params: | |||
th5: | |||
topo-t0-standalone-256: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rick-arista , I believe we should use topo-t0-standalone-32 here, as it is consistent with our internal setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These params are generated for the 256 interface config. If you're testing with 32 interfaces I think they should still work, but you'll need to modify this ad hoc.
pg: 3 | ||
pkts_num_margin: 2 | ||
pkts_num_trig_ingr_drp: 121302 | ||
pkts_num_trig_pfc: 120117 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rick-arista How do you get pkts_num_trig_pfc: 120117?
The pre-commit check detected issues in the files touched by this pull request. Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
Description of PR
Adds static parameters for TH5 QoS tests
Summary:
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
Adds a baseline of parameters, which fixes the headroom pool size test.
How did you do it?
Values were generated by updated version of the generator script.
How did you verify/test it?
Manual test runs.