[QoS] PFC test cases by baiwei0427 · Pull Request #2599 · sonic-net/sonic-mgmt

baiwei0427 · 2020-11-29T09:51:50Z

Signed-off-by: Wei Bai [email protected]

Description of PR

Summary: Add PFC test cases implemented using Tgen API & Update Tgen sample script

Fixes # (issue)

Type of change

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

Approach

What is the motivation for this PR?

We need PFC test cases

How did you do it?

Add pre-tests to get lossless and lossy priority information of each DUT in the testbed (see tests/test_pretest.py and tests/conftest.py)
Add IXIA fixtures, e.g., ixia_api and ixia_t0_testbed (L2/L3 config of a T0 testbed).
Add QoS fixtures and other helper functions
Add general PFC test helper functions in tests/pfc/files/helper.py. Currently, we reuse an IXIA session (ixia_api, module-level fixture) to run a series of tests. Different tests use the same testbed configuration (ixia_t0_testbed) but have different flow configurations and expected results.

How did you verify/test it?

I did test using SONiC switches and IXIA chassis. The Tgen API version is 0.0.64.

Any platform specific information?

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

T0 topology using IXIA chassis as the fanout

Documentation

https://github.com/Azure/sonic-mgmt/blob/master/docs/pfc/GLOBAL_PAUSE_README.md
https://github.com/Azure/sonic-mgmt/blob/master/docs/pfc/PFC_PAUSE_LOSSLESS_README.md
https://github.com/Azure/sonic-mgmt/blob/master/docs/pfc/PFC_PAUSE_LOSSY_README.md

lgtm-com · 2020-11-29T10:09:57Z

This pull request introduces 21 alerts and fixes 6 when merging 9462b8f into 345323c - view on LGTM.com

new alerts:

18 for Unused import
3 for Result of integer division may be truncated

fixed alerts:

3 for Unused local variable
2 for Wrong name for an argument in a call
1 for Result of integer division may be truncated

lgtm-com · 2020-11-30T01:49:09Z

This pull request introduces 3 alerts and fixes 7 when merging bcde48b into 345323c - view on LGTM.com

new alerts:

3 for Result of integer division may be truncated

fixed alerts:

3 for Unused local variable
2 for Wrong name for an argument in a call
1 for Unused import
1 for Result of integer division may be truncated

baiwei0427 · 2020-11-30T06:07:14Z

retest this please

tests/common/ixia/ixia_fixtures.py

tests/pfc/test_global_pause.py

tests/pfc/test_pfc_pause_lossless.py

tests/pfc/test_pfc_pause_lossy.py

tests/pfc/test_pfc_pause_lossless.py

tests/ixia/test_tgen.py

yxieca · 2020-12-03T17:24:05Z

There are quite some LGTM warnings, can you address these?

lgtm-com · 2020-12-11T05:32:45Z

This pull request introduces 3 alerts and fixes 7 when merging 4f02571 into 4cdc296 - view on LGTM.com

new alerts:

3 for Result of integer division may be truncated

fixed alerts:

3 for Unused local variable
2 for Wrong name for an argument in a call
1 for Unused import
1 for Result of integer division may be truncated

lgtm-com · 2020-12-11T06:14:39Z

This pull request fixes 7 alerts when merging 6ce66d6 into 4cdc296 - view on LGTM.com

fixed alerts:

3 for Unused local variable
2 for Wrong name for an argument in a call
1 for Unused import
1 for Result of integer division may be truncated

baiwei0427 · 2020-12-11T07:56:48Z

There are quite some LGTM warnings, can you address these?

fixed

baiwei0427 · 2020-12-11T07:56:55Z

retest this please

ashutshkumr · 2020-12-14T20:11:15Z

Hello, I'm a part of team working on stabilizing tgen and have been going through this PR to understand the use cases being targeted with regards to PFC (using Ixia Test Ports).

I looked at the test plan and TCs, and felt there were few areas where we could've achieved (IMHO) more complete validation:

keep tx traffic flows active, longer than rx traffic flows (pause frames) so we can receive packets with lossless priorities (i.e. after pause storm is over)
use exact packet counts for tx traffic flows and use flow stats to validate packet count for each priority before and after pause storm
configure tx flows with all 8 priorities and send pause storm enabled for all priorities

I have uploaded a test script for reference (works against back-to-back connected Ixia Ports): https://github.com/open-traffic-generator/ixnetwork/blob/test-pfc/tests/pfc/test_pfc_pause_e2e.py

I'm also curious if you think there's value in covering following separate cases:

tx traffic flows should resume sending packets with lossless priorities when pause quanta for all pause frames originating from rx change from non-zero to zero
accuracy of DUT's reaction to being overwhelmed by lossless flows and duration for which the reaction persists

neethajohn · 2020-12-14T22:15:43Z

tests/common/ixia/common_helpers.py


+def get_egress_lossless_buffer_size(host_ans):
+    """
+    Get egress losless buffer size of a switch


neethajohn · 2020-12-14T22:16:26Z

tests/common/ixia/ixia_fixtures.py

+                    fanout_graph_facts,
+                    duthosts,
+                    rand_one_dut_hostname):


can you fix the alignment?

neethajohn · 2020-12-14T22:24:20Z

@baiwei0427 , can you also include the check to ensure that the DUT sends PFC to the Tx port? That can be addressed in a separate PR

baiwei0427 · 2020-12-15T02:36:08Z

n rx traffic flows (pause frames) so we can receive packets with lossless priorities (i.e. after pause storm is over)

use exact packet counts for tx traffic flows and use flow stats to validate packet count for each priority before and after pause storm

I agree with you. Our current test plan and implementation cannot fully verify all the functionalities of PFC. Let's update the test plan and add some PFC tests in the separate PRs.

This reverts commit 49625a1.

Signed-off-by: Neetha John <[email protected]> Fixes the error seen during test case collection introduced by #2599. Initialize the variable as a list https://sonic-jenkins.westus2.cloudapp.azure.com/job/common/job/sonic-mgmt-testing/job/sonic-mgmt-pr/2462/consoleFull How did you verify/test it? Tried test case collection with the fix. No longer seeing the error

baiwei0427 added 10 commits November 25, 2020 02:24

Add priority fixtures

9eaa24b

Merge changes to qos_fixtures.py

e5a8a5a

Add PFC test case

7cdb097

Use duthosts and rand_one_dut_hostname to replace duthost

0df0150

Remove sleep

4b96deb

Update PFC pause lossless test case

c3ae2f4

Add PFC pause lossy test case

7d0027b

Update tgen sample script

9c1c1fe

Change function name

212d164

Update PFC test cases

9462b8f

baiwei0427 added 2 commits November 29, 2020 22:53

Update conftest.py

0d24852

Remove unused modules & Update result verification

bcde48b

neethajohn requested review from neethajohn and yxieca November 30, 2020 22:00

neethajohn reviewed Dec 2, 2020

View reviewed changes

Update the code to address review comments

4f02571

Address alerts

6ce66d6

baiwei0427 changed the title ~~Add PFC test cases~~ [QoS] PFC test cases Dec 11, 2020

neethajohn approved these changes Dec 14, 2020

View reviewed changes

neethajohn merged commit 49625a1 into sonic-net:master Dec 15, 2020

neethajohn added a commit that referenced this pull request Dec 15, 2020

Revert "[QoS] PFC test cases (#2599)"

a8f8f7b

This reverts commit 49625a1.

This was referenced Dec 15, 2020

Revert "[QoS] PFC test cases" #2671

Closed

[pfc] Fix variable return type #2672

Merged

Conversation

baiwei0427 commented Nov 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

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

Uh oh!

lgtm-com bot commented Nov 29, 2020

Uh oh!

lgtm-com bot commented Nov 30, 2020

Uh oh!

baiwei0427 commented Nov 30, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yxieca commented Dec 3, 2020

Uh oh!

lgtm-com bot commented Dec 11, 2020

Uh oh!

lgtm-com bot commented Dec 11, 2020

Uh oh!

baiwei0427 commented Dec 11, 2020

Uh oh!

baiwei0427 commented Dec 11, 2020

Uh oh!

ashutshkumr commented Dec 14, 2020

Uh oh!

neethajohn Dec 14, 2020

Choose a reason for hiding this comment

Uh oh!

neethajohn Dec 14, 2020

Choose a reason for hiding this comment

Uh oh!

neethajohn commented Dec 14, 2020

Uh oh!

baiwei0427 commented Dec 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

baiwei0427 commented Nov 29, 2020 •

edited

Loading