Adding new ecn testcases for cisco-8000 platform.

[rraghav-cisco]

New test case for cisco-8000 to demonstrate accuracy of ECN marking under continuous traffic when a single XOFF is injected.

Test is parameterized across several variables.

While waiting for IXIA to get number of ECN marked packets (Issue: open-traffic-generator/snappi-ixnetwork#541), we are using the counters from DUT, using the extension added via (#6040).

[lgtm-com]

This pull request introduces 2 alerts and fixes 7 when merging 828f6c4 into 63c0375 - view on LGTM.com

new alerts:

• 1 for Unused local variable
• 1 for Unused import

fixed alerts:

• 4 for Module is imported more than once
• 2 for Module is imported with 'import' and 'import from'
• 1 for Comparison using is when operands support `__eq__`

[lgtm-com]

This pull request introduces 2 alerts when merging b5e4f3f into 45b20e6 - view on LGTM.com

new alerts:

• 1 for Unused local variable
• 1 for Unused import

[baiwei0427]

My overall feeling is that current run_ecn_test is too complex. It has a mix of two mechanisms. Can we implement another run_ecn_test for Cisco only?

[rraghav-cisco]

My overall feeling is that current run_ecn_test is too complex. It has a mix of two mechanisms. Can we implement another run_ecn_test for Cisco only?

@baiwei0427 : Thanks for your comments. I started with seperate functions one each for 1-1 and 3-1 cases, and then combined them all into the original function. There was a lot of similarity between them and the original function, so it was decided to merge them all.

[lgtm-com]

This pull request introduces 4 alerts when merging 29999c5 into 709d503 - view on LGTM.com

new alerts:

• 4 for Wrong name for an argument in a call

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: rraghav-cisco (815d067, 20193df, 828f6c4, b5e4f3f, 580b48b, 07b18e2, 8a56ab2, 71062fd, 8961d6d, 1fd7567, f3e3268, 29999c5, cea16d7)

[lgtm-com]

This pull request introduces 4 alerts when merging cea16d7 into 6738170 - view on LGTM.com

new alerts:

• 4 for Wrong name for an argument in a call

[lgtm-com]

This pull request introduces 4 alerts when merging fc69572 into 6738170 - view on LGTM.com

new alerts:

• 4 for Wrong name for an argument in a call

[rraghav-cisco]

@baiwei0427 : Pls let me know if you have comments on the latest code.

[baiwei0427]

@baiwei0427 : Pls let me know if you have comments on the latest code.

@rraghav-cisco I will add comments asap.

[baiwei0427]

It seems that there is no logic to judge if the test passes.

[rraghav-cisco]

The test is passed if run_ecn_test is successfully done. That function itself has the required asserts to check.

[baiwei0427]

The test is passed if run_ecn_test is successfully done. That function itself has the required asserts to check.

Where are the required asserts in run_ecn_test?

[rraghav-cisco]

@baiwei0427 , I was referring to the following checks. If any of them fails, the test will fail.

grep -n assert files/helper.py
4:from tests.common.helpers.assertions import pytest_assert
89: pytest_assert(testbed_config is not None, 'Failed to get L2/3 testbed config')
99: pytest_assert(config_result is True, 'Failed to configure WRED/ECN at the DUT')
108: pytest_assert(config_result is True, 'Failed to configure PFC threshold to 8')
116: pytest_assert(port_id is not None,
221: pytest_assert(len(tx_port_id_list) > 0, "Cannot find any TX ports")
224: pytest_assert(tx_port_id_list != [], "Cannot find a suitable TX port")
396: pytest_assert(attempts < max_attempts,

[lgtm-com]

This pull request introduces 4 alerts when merging 3ae5665 into fc2108a - view on LGTM.com

new alerts:

• 4 for Wrong name for an argument in a call

[rraghav-cisco]

/azp run

[azure-pipelines]

Commenter does not have sufficient privileges for PR 6041 in repo sonic-net/sonic-mgmt

[rraghav-cisco]

/easycla

[kevinskwang]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[rraghav-cisco]

This needs to be merged after 6329 is merged.

[rraghav-cisco]

@kevinskwang Any update on azp failures ?

[kevinskwang]

Could you fix the conflict by first?

[azure-pipelines]

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/ixia/ecn/files/helper.py
Fixing tests/ixia/ecn/test_dequeue_ecn.py

fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing tests/ixia/ecn/test_red_accuracy.py

check yaml...............................................................Failed
- hook id: check-yaml
- exit code: 1

while constructing a mapping
...
[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

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

pre-commit install

5. Use pre-commit to check staged file:

pre-commit

6. Alternatively, you can check committed files using:

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

[rraghav-cisco]

Could you fix the conflict by first?

@kevinskwang : Conflict is resolved. Pls check.

[lgtm-com]

This pull request introduces 4 alerts when merging a8879ce into f5cca01 - view on LGTM.com

new alerts:

• 4 for Wrong name for an argument in a call

[kevinskwang]

Reverted as some import errors, please fix it and create a new PR for this. @rraghav-cisco

[rraghav-cisco]

Reverted as some import errors, please fix it and create a new PR for this. @rraghav-cisco

@kevinskwang , sure. I will take it up after current priorities. Thanks.

[developfast]

@rraghav-cisco Currently, we are not running any test cases in the ixia directory. All of our RDMA related test cases have been migrated to the snappi directory here. The new ecn test would go in this directory - https://github.com/sonic-net/sonic-mgmt/tree/master/tests/snappi/ecn. Please make the new code changes to this directory when you bring up the new PR. Thanks!