Skip to content

Generate PFC storm using PFC backpressure on additional Arista devices#17971

Closed
veronica-arista wants to merge 7 commits intosonic-net:masterfrom
veronica-arista:pfc_gen_backpressure_th_th2
Closed

Generate PFC storm using PFC backpressure on additional Arista devices#17971
veronica-arista wants to merge 7 commits intosonic-net:masterfrom
veronica-arista:pfc_gen_backpressure_th_th2

Conversation

@veronica-arista
Copy link
Contributor

@veronica-arista veronica-arista commented Apr 12, 2025

Description of PR

Add a pfc_gen script for Arista fanout devices to generate pfc pause frames by setting PFC backpressure status in hardware. Enable the storm generation to use this script on interfaces that are connected to Arista fanout devices running EOS or Sonic. Currently supports the following Arista devices:
Arista-7060X6
Arista-7060DX5
Arista-7060PX5
Arista-7060CX
Arista-7260CX3
Arista-7260QX3

This is implemented on top of PR #15594

Summary:
Fixes # (issue)

Type of change

  • 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?

Adds more reliable PFC storm generation for interfaces connected to Arista fanout devices

How did you do it?

How did you verify/test it?

Ran sonic-mgmt pfcwd tests on testbed with Arista fanout switches of the listed SKUs running EOS.
Ran the pfc_gen script manually on Arista switches running Sonic.

Any platform specific information?

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

Documentation

Add a pfc_gen script that generates storm by directly setting PFC
backpressure status in hardware (using broadcom shell commands) and
modify pfc_storm to use this new script if the port is connected to th5
MPC. Intended to upstream for MSFT.
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@veronica-arista veronica-arista changed the title Pfc gen backpressure th th2 Generate PFC storm using PFC backpressure on additional Arista devices Apr 12, 2025
@lipxu
Copy link
Contributor

lipxu commented Apr 15, 2025

Thanks @veronica-arista , can you please run the test_pfcwd_timer_accuracy case and share the logs of "sorted all detect time", thanks a lot

bash
cd {{pfc_gen_dir}}
{% if (pfc_asym is defined) and (pfc_asym == True) %}
{% if pfc_storm_defer_time is defined %}sleep {{pfc_storm_defer_time}} &&{% endif %} sudo python3 {{pfc_gen_file}} -c {{pfc_gen_chip_name}} -p {{pfc_queue_index}} -i {{pfc_fanout_interface | replace("Ethernet", "et") | replace("/", "_")}} > /dev/null 2>&1 &
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this new feature only support python3? Are there any limitations, thanks

Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no testing on python2. Python2 is long out of support.

@XuChen-MSFT
Copy link
Contributor

@veronica-arista Nice change. May I ask how much delay there is when enabling and disabling PFC via hardware?
Also, do you have a test report?

Copy link
Contributor

@XuChen-MSFT XuChen-MSFT left a comment

Choose a reason for hiding this comment

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

approved

@XuChen-MSFT
Copy link
Contributor

@veronica-arista need to address below pre-commit failure

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/common/helpers/pfc_gen_brcm_xgs.py:7:1: F401 'binascii' imported but unused
tests/common/helpers/pfc_gen_brcm_xgs.py:46:121: E501 line too long (132 > 120 characters)
tests/common/helpers/pfc_gen_brcm_xgs.py:79:31: F541 f-string is missing placeholders
tests/common/helpers/pfc_gen_brcm_xgs.py:82:52: W605 invalid escape sequence '\S'
tests/common/helpers/pfc_gen_brcm_xgs.py:82:83: W605 invalid escape sequence '\S'
tests/common/helpers/pfc_gen_brcm_xgs.py:82:98: W605 invalid escape sequence '['
tests/common/helpers/pfc_gen_brcm_xgs.py:82:106: W605 invalid escape sequence '\d'
tests/common/helpers/pfc_gen_brcm_xgs.py:82:109: W605 invalid escape sequence ']'
tests/common/helpers/pfc_gen_brcm_xgs.py:82:121: E501 line too long (137 > 120 characters)
tests/common/helpers/pfc_gen_brcm_xgs.py:82:126: W605 invalid escape sequence '\S'
tests/common/helpers/pfc_gen_brcm_xgs.py:100:20: E111 indentation is not a multiple of 4
tests/common/helpers/pfc_gen_brcm_xgs.py:117:20: E111 indentation is not a multiple of 4
tests/common/helpers/pfc_gen_brcm_xgs.py:221:5: F841 local variable 'fsCleanup' is assigned to but never used
tests/common/helpers/pfc_gen_brcm_xgs.py:232:8: E111 indentation is not a multiple of 4
tests/common/helpers/pfc_gen_brcm_xgs.py:234:1: E302 expected 2 blank lines, found 1
tests/common/helpers/pfc_gen_brcm_xgs.py:235:4: E111 indentation is not a multiple of 4
tests/common/helpers/pfc_storm.py:15:1: E302 expected 2 blank lines, found 1
tests/common/helpers/pfc_storm.py:34:1: E302 expected 2 blank lines, found 1
tests/common/helpers/pfc_storm.py:177:15: E271 multiple spaces after keyword
tests/common/helpers/pfc_storm.py:266:121: E501 line too long (128 > 120 characters)
tests/common/helpers/pfc_storm.py:267:121: E501 line too long (126 > 120 characters)
tests/common/helpers/pfc_storm.py:289:121: E501 line too long (128 > 120 characters)
tests/common/helpers/pfc_storm.py:290:121: E501 line too long (126 > 120 characters)

flake8...............................................(no files to check)Skipped
check conditional mark sort..........................(no files to check)Skipped

##[error]Bash exited with code '1'.
Finishing: Run pre-commit check

@andywongarista
Copy link
Contributor

@XuChen-MSFT Since @veronica-arista is away I have duplicated this PR with pre-commit errors fixed, please review: #18033

mssonicbld added a commit to mssonicbld/sonic-mgmt.msft that referenced this pull request Apr 24, 2025
<!--
Please make sure you've read and understood our contributing guidelines;
https://github.com/sonic-net/SONiC/blob/gh-pages/CONTRIBUTING.md

Please provide following information to help code review process a bit easier:
-->
### Description of PR
NOTE: This is a duplicate of sonic-net/sonic-mgmt#17971, as @veronica-arista is away.

Add a pfc_gen script for Arista fanout devices to generate pfc pause frames by setting PFC backpressure status in hardware. Enable the storm generation to use this script on interfaces that are connected to Arista fanout devices running EOS or Sonic. Currently supports the following Arista devices:
Arista-7060X6
Arista-7060DX5
Arista-7060PX5
Arista-7060CX
Arista-7260CX3
Arista-7260QX3

This is implemented on top of PR sonic-net/sonic-mgmt#15594

Summary:
Fixes # (issue)

### 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
- [x] Test case improvement

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

### Approach
#### What is the motivation for this PR?
Adds more reliable PFC storm generation for interfaces connected to Arista fanout devices

#### How did you do it?

#### How did you verify/test it?
Ran sonic-mgmt pfcwd tests on testbed with Arista fanout switches of the listed SKUs running EOS.
Ran the pfc_gen script manually on Arista switches running Sonic.

#### 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?
-->
mssonicbld added a commit to Azure/sonic-mgmt.msft that referenced this pull request Apr 24, 2025
…tional Arista devices (#222)

<!--
Please make sure you've read and understood our contributing guidelines;
https://github.com/sonic-net/SONiC/blob/gh-pages/CONTRIBUTING.md

Please provide following information to help code review process a bit easier:
-->
### Description of PR
NOTE: This is a duplicate of sonic-net/sonic-mgmt#17971, as @veronica-arista is away.

Add a pfc_gen script for Arista fanout devices to generate pfc pause frames by setting PFC backpressure status in hardware. Enable the storm generation to use this script on interfaces that are connected to Arista fanout devices running EOS or Sonic. Currently supports the following Arista devices:
Arista-7060X6
Arista-7060DX5
Arista-7060PX5
Arista-7060CX
Arista-7260CX3
Arista-7260QX3

This is implemented on top of PR sonic-net/sonic-mgmt#15594

Summary:
Fixes # (issue)

### 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
- [x] Test case improvement

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

### Approach
#### What is the motivation for this PR?
Adds more reliable PFC storm generation for interfaces connected to Arista fanout devices

#### How did you do it?

#### How did you verify/test it?
Ran sonic-mgmt pfcwd tests on testbed with Arista fanout switches of the listed SKUs running EOS.
Ran the pfc_gen script manually on Arista switches running Sonic.

#### 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?
-->
@lipxu
Copy link
Contributor

lipxu commented Apr 28, 2025

@veronica-arista Could we close this PR since it should duplicate with #18033, thanks

@veronica-arista
Copy link
Contributor Author

Closing since this change was merged as #18033

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants