Skip to content

[Snappi] Modified pfcwd tests to add warm-up traffic#7712

Merged
developfast merged 2 commits intosonic-net:masterfrom
developfast:dojha/warm_up_traffic_pfcwd
Mar 14, 2023
Merged

[Snappi] Modified pfcwd tests to add warm-up traffic#7712
developfast merged 2 commits intosonic-net:masterfrom
developfast:dojha/warm_up_traffic_pfcwd

Conversation

@developfast
Copy link
Contributor

Description of PR

Summary: During the pfc watch dog (pfcwd) basic tests, pfc watch dog was being triggered by fake alerts i.e. when it was not supposed to be triggered. This was causing unnecessary test failures. To curb this issue, warm-up data traffic is initially sent before any other traffic (pfc pause storm and data traffic) to prevent pfcwd fake alerts caused by non-incremented packet counters during the pfcwd polling interval. This PR is in addition to #7499.

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?

Existing issue with certain switches where PFC watch dog was being triggered by fake alerts.

How did you do it?

Added warm-up data traffic before any other traffic (pfc pause storm and data traffic) to prevent pfcwd fake alerts caused by non-incremented packet counters during the pfcwd polling interval.

How did you verify/test it?

The related PFCWD tests pass on a lab switch.

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 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/snappi/pfcwd/files/pfcwd_burst_storm_helper.py
Fixing tests/snappi/pfcwd/files/pfcwd_multi_node_helper.py
Fixing tests/snappi/pfcwd/files/pfcwd_runtime_traffic_helper.py

fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/snappi/pfcwd/files/pfcwd_multi_node_helper.py:6:1: F401 'tests.common.fixtures.conn_graph_facts.conn_graph_facts' imported but unused
tests/snappi/pfcwd/files/pfcwd_multi_node_helper.py:6:1: F401 'tests.common.fixtures.conn_graph_facts.fanout_graph_facts' imported but unused
tests/snappi/pfcwd/files/pfcwd_multi_node_helper.py:93:64: E502 the backslash is redundant between brackets
...
[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>

@neethajohn
Copy link
Contributor

LGTM. @baiwei0427, please take a look

pause_flow_dur_sec = poll_interval_sec * 0.5
pause_flow_gap_sec = burst_cycle_sec - pause_flow_dur_sec

""" Warm up traffic is initially sent before any other traffic to prevent pfcwd
Copy link
Contributor

Choose a reason for hiding this comment

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

prevent pfcwd fake alerts due to idle links

pause_pkt.pause_class_7.value = pause_time[7]

pause_flow_start_time = id * (pause_flow_dur_sec + pause_flow_gap_sec)
pause_flow_start_time = id * (pause_flow_dur_sec + pause_flow_gap_sec) + WARM_UP_TRAFFIC_DUR
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should introduce a new argument "pause_flow_start_delay_sec" for __gen_traffic function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The id value is generated within the function itself (as well as pause_flow_dur_sec + pause_flow_gap_sec), so I prefer to keep it like it is for now. We can rename the variable to pause_flow_start_delay_sec?

Comment on lines +252 to +253

__gen_data_flows(testbed_config=testbed_config,
Copy link
Contributor

Choose a reason for hiding this comment

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

Add comments here "Generate warm up traffic"

fake alerts caused by non-incremented packet counters during pfcwd detection periods"""
warm_up_traffic_dur_sec = WARM_UP_TRAFFIC_DUR
warm_up_traffic_delay_sec = 0

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of generating "warm up traffic", "data flows" and "PFC storm" in a function, how about using two functions __gen_data_flows and __gen_pause_flow to generate different flows, just like what we did in pfcwd_multi_node_helper.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking the same - let me have a look at code re-usability and move around some components

asic_type=asic_type)


def sec_to_nanosec(secs):
Copy link
Contributor

Choose a reason for hiding this comment

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

This function has been defined, implemented and used multiple times.

Copy link
Contributor Author

@developfast developfast Mar 13, 2023

Choose a reason for hiding this comment

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

Will move to a separate helper function file

@mssonicbld
Copy link
Collaborator

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.................................................Passed
fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/snappi/pfcwd/files/pfcwd_basic_helper.py:6:1: F401 'tests.common.fixtures.conn_graph_facts.conn_graph_facts' imported but unused
tests/snappi/pfcwd/files/pfcwd_basic_helper.py:6:1: F401 'tests.common.fixtures.conn_graph_facts.fanout_graph_facts' imported but unused
tests/snappi/pfcwd/files/pfcwd_basic_helper.py:26:1: E302 expected 2 blank lines, found 1
tests/snappi/pfcwd/files/pfcwd_basic_helper.py:90:32: E225 missing whitespace around operator
tests/snappi/pfcwd/files/pfcwd_basic_helper.py:359:23: E122 continuation line missing indentation or outdented
tests/snappi/pfcwd/files/pfcwd_multi_node_helper.py:6:1: F401 'tests.common.fixtures.conn_graph_facts.conn_graph_facts' imported but unused
tests/snappi/pfcwd/files/pfcwd_multi_node_helper.py:6:1: F401 'tests.common.fixtures.conn_graph_facts.fanout_graph_facts' imported but unused
tests/snappi/pfcwd/files/pfcwd_multi_node_helper.py:93:64: E502 the backslash is redundant between brackets
tests/snappi/pfcwd/files/pfcwd_multi_node_helper.py:94:52: E502 the backslash is redundant between brackets
tests/snappi/pfcwd/files/pfcwd_multi_node_helper.py:97:60: E502 the backslash is redundant between brackets
tests/snappi/pfcwd/files/pfcwd_multi_node_helper.py:98:50: E502 the backslash is redundant between brackets
...
[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>

@developfast developfast requested a review from baiwei0427 March 14, 2023 07:28
@developfast developfast requested a review from neethajohn March 14, 2023 07:32
@developfast developfast merged commit 0f53991 into sonic-net:master Mar 14, 2023
wangxin pushed a commit that referenced this pull request Mar 15, 2023
* added warm-up traffic to all pfcwd tests
wangxin pushed a commit that referenced this pull request Mar 15, 2023
* added warm-up traffic to all pfcwd tests
auspham added a commit to auspham/sonic-mgmt that referenced this pull request Jan 21, 2025
Signed-off-by: Austin Pham <austinpham@microsoft.com>
auspham added a commit to auspham/sonic-mgmt that referenced this pull request Jan 21, 2025
Signed-off-by: Austin Pham <austinpham@microsoft.com>
auspham added a commit to auspham/sonic-mgmt that referenced this pull request Jan 22, 2025
fix: migrate sonic-net#7370 to multidut code

Signed-off-by: Austin Pham <austinpham@microsoft.com>

fix: migrate sonic-net#9567 to multidut code

Signed-off-by: Austin Pham <austinpham@microsoft.com>

fix: migrate sonic-net#7367 to multidut code

Signed-off-by: Austin Pham <austinpham@microsoft.com>

fix: migrate sonic-net#7712

Signed-off-by: Austin Pham <austinpham@microsoft.com>

fix: migrate sonic-net#7712 to pfcwd_multidut_runtime_traffic_helper

Signed-off-by: Austin Pham <austinpham@microsoft.com>

chore: update a2a parameterize

Signed-off-by: Austin Pham <austinpham@microsoft.com>
auspham added a commit to auspham/sonic-mgmt that referenced this pull request Jan 22, 2025
fix: migrate sonic-net#7370 to multidut code

Signed-off-by: Austin Pham <austinpham@microsoft.com>

fix: migrate sonic-net#9567 to multidut code

Signed-off-by: Austin Pham <austinpham@microsoft.com>

fix: migrate sonic-net#7367 to multidut code

Signed-off-by: Austin Pham <austinpham@microsoft.com>

fix: migrate sonic-net#7712

Signed-off-by: Austin Pham <austinpham@microsoft.com>

fix: migrate sonic-net#7712 to pfcwd_multidut_runtime_traffic_helper

Signed-off-by: Austin Pham <austinpham@microsoft.com>

chore: update a2a parameterize

Signed-off-by: Austin Pham <austinpham@microsoft.com>
yejianquan pushed a commit that referenced this pull request Jan 29, 2025
Summary: Update the missing T0 T1 tests feature in multidut

Fixes # (issue) 30929279

The following PR were migrated for pfcwd

[Snappi] Increase number of calls for ARP wait #7370

[Snappi] PFCWD Tolerance Fixes #9567

[Snappi] Modifications to pfcwd a2a test design for mlnx 4600c #7367 to multidut code

[Snappi] Modified pfcwd tests to add warm-up traffic #7712

Signed-off-by: Austin Pham <austinpham@microsoft.com>
mssonicbld added a commit to mssonicbld/sonic-mgmt.msft that referenced this pull request Jan 29, 2025
Summary: Update the missing T0 T1 tests feature in multidut

Fixes # (issue) 30929279

The following PR were migrated for pfcwd

- sonic-net/sonic-mgmt#7370

- sonic-net/sonic-mgmt#9567

- sonic-net/sonic-mgmt#7367 to multidut code

- sonic-net/sonic-mgmt#7712

### Type of change

<!--
- Fill x for your type of change.
- e.g.
- [x] Bug fix
-->

- [ ] Bug fix
- [ ] Testbed and Framework(new/improvement)
- [ ] New Test case
    - [ ] Skipped for non-supported platforms
- [ ] Test case improvement

### Back port request
- [ ] 202012
- [ ] 202205
- [ ] 202305
- [ ] 202311
- [ ] 202405
- [ ] 202411

### 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
<!--
(If it's a new feature, new test case)
Did you update documentation/Wiki relevant to your implementation?
Link to the wiki page?
-->

snappi_tests/multidut/pfcwd/test_multidut_pfcwd_m2o_with_snappi.py
```

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
- generated xml file: /var/src/sonic-mgmt_vmsvc5-t2-8800-ixia_669de75fbbfea63e8b73b319/tests/logs/snappi_tests/multidut/pfcwd/test_multidut_pfcwd_m2o_with_snappi.xml -
================== 4 passed, 7 warnings in 1235.72s (0:20:35) ==================
```

snappi_tests/multidut/pfcwd/test_multidut_pfcwd_burst_storm_with_snappi.py
```
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
- generated xml file: /var/src/sonic-mgmt_vmsvc5-t2-8800-ixia_669de75fbbfea63e8b73b319/tests/logs/snappi_tests/multidut/pfcwd/test_multidut_pfcwd_burst_storm_with_snappi.xml -
================== 2 passed, 5 warnings in 858.60s (0:14:18) ===================

```

snappi_tests/multidut/pfcwd/test_multidut_pfcwd_basic_with_snappi.py (known failure on T2, fail as expected)

```
SKIPPED [4] snappi_tests/multidut/pfcwd/test_multidut_pfcwd_basic_with_snappi.py:190: Reboot type fast is not supported on cisco-8000 switches
FAILED snappi_tests/multidut/pfcwd/test_multidut_pfcwd_basic_with_snappi.py::test_pfcwd_basic_multi_lossless_prio_restart_service[multidut_port_info0-True-swss]
===== 1 failed, 23 passed, 16 skipped, 27 warnings in 27779.49s (7:42:59) ======
```

snappi_tests/multidut/pfcwd/test_multidut_pfcwd_a2a_with_snappi.py
```
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
- generated xml file: /var/src/sonic-mgmt_vmsvc5-t2-8800-ixia_669de75fbbfea63e8b73b319/tests/logs/snappi_tests/multidut/pfcwd/test_multidut_pfcwd_a2a_with_snappi.xml -
================== 4 passed, 7 warnings in 1157.40s (0:19:17) ==================
```

snappi_tests/multidut/pfcwd/test_multidut_pfcwd_runtime_traffic_with_snappi.py
```
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
- generated xml file: /var/src/sonic-mgmt_vmsvc5-t2-8800-ixia_669de75fbbfea63e8b73b319/tests/logs/snappi_tests/multidut/pfcwd/test_multidut_pfcwd_runtime_traffic_with_snappi.xml -
================== 2 passed, 6 warnings in 1162.35s (0:19:22) ==================
```
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Jan 29, 2025
Summary: Update the missing T0 T1 tests feature in multidut

Fixes # (issue) 30929279

The following PR were migrated for pfcwd

[Snappi] Increase number of calls for ARP wait sonic-net#7370

[Snappi] PFCWD Tolerance Fixes sonic-net#9567

[Snappi] Modifications to pfcwd a2a test design for mlnx 4600c sonic-net#7367 to multidut code

[Snappi] Modified pfcwd tests to add warm-up traffic sonic-net#7712

Signed-off-by: Austin Pham <austinpham@microsoft.com>
mssonicbld pushed a commit that referenced this pull request Jan 29, 2025
Summary: Update the missing T0 T1 tests feature in multidut

Fixes # (issue) 30929279

The following PR were migrated for pfcwd

[Snappi] Increase number of calls for ARP wait #7370

[Snappi] PFCWD Tolerance Fixes #9567

[Snappi] Modifications to pfcwd a2a test design for mlnx 4600c #7367 to multidut code

[Snappi] Modified pfcwd tests to add warm-up traffic #7712

Signed-off-by: Austin Pham <austinpham@microsoft.com>
nnelluri-cisco pushed a commit to nnelluri-cisco/sonic-mgmt that referenced this pull request Mar 15, 2025
Summary: Update the missing T0 T1 tests feature in multidut

Fixes # (issue) 30929279

The following PR were migrated for pfcwd

[Snappi] Increase number of calls for ARP wait sonic-net#7370

[Snappi] PFCWD Tolerance Fixes sonic-net#9567

[Snappi] Modifications to pfcwd a2a test design for mlnx 4600c sonic-net#7367 to multidut code

[Snappi] Modified pfcwd tests to add warm-up traffic sonic-net#7712

Signed-off-by: Austin Pham <austinpham@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants