Leverage the fanout's ASIC to send PFC storm by nhe-NV · Pull Request #13242 · sonic-net/sonic-mgmt

nhe-NV · 2024-06-12T02:45:14Z

Currently, the PFC storm is sent using packet socket on the host CPU, which causes some drawbacks

There is strict timing requirement, which can not be guaranteed
The more ports under PFC storm, the higher probability that the timing requirement is not satisfied

We will leverage the fanout's ASIC to send PFC storm

Use SDK API to enable lossless traffic, set xoff to 0, and setup mappings for the PGs and the corresponding IEEE priorities

After the change, the pfc related test case which is using onxy as fanout will send the pfc frame with ASIC instead of using the CPU.
New docker storm docker need to be build with the Docker file provided in this PR.

Description of PR

Summary:
Fixes # (issue)

Type of change

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

Back port request

Approach

What is the motivation for this PR?

To make the pfc related test case much more stable.

How did you do it?

Change sending pfc frame with CPU to ASIC on the onxy fanout.

How did you verify/test it?

Run the pfc related test case on the setup which has onxy fanout, test case could pass stablly.

Any platform specific information?

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

Documentation

mssonicbld · 2024-06-12T02:57:09Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

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 ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:13:1: F403 'from python_sdk_api.sx_api import *' used; unable to detect undefined names ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:18:1: E302 expected 2 blank lines, found 1 ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:21:1: E302 expected 2 blank lines, found 1 ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:32:1: E302 expected 2 blank lines, found 1 ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:33:10: F405 'sx_api_port_pfc_enable_set' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:34:14: F405 'SX_STATUS_SUCCESS' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:38:17: F405 'new_sx_cos_priority_t_arr' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:39:15: F405 'new_sx_cos_ieee_prio_t_arr' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:40:18: F405 'new_sx_cos_port_prio_buff_t_p' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:42:1: E302 expected 2 blank lines, found 1 ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:43:5: F405 'sx_cos_priority_t_arr_setitem' may be undefined, or defined from star imports: python_sdk_api.sx_api ... [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:

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.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

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

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

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

wsycqyz · 2024-06-13T03:11:19Z

The pre-commit check is mandatory. Can you help fix it?

wsycqyz · 2024-06-13T03:12:23Z

@bingwang-ms This seems to be a big PR. FYI.

nhe-NV · 2024-06-15T09:40:01Z

/azpw run

mssonicbld · 2024-06-15T09:40:03Z

/AzurePipelines run

azure-pipelines · 2024-06-15T09:40:14Z

Azure Pipelines successfully started running 1 pipeline(s).

bingwang-ms · 2024-07-18T21:26:57Z

@kperumalbfn Can you please help review this PR?

bingwang-ms · 2024-07-22T21:44:28Z

@nhe-NV Does this change require the leaf-fanout switch running Onyx? If that's the case, the PR may not help us because we are not using Onyx as leaf-fanout OS.
BTW, I didn't see code for building and importing the new container. Is that done manually?

bingwang-ms · 2024-07-22T22:18:43Z

The new PFCWD logic requires queue not empty. sonic-net/sonic-swss#3036
How is that condition satisfied?

nhe-NV · 2024-07-23T01:34:01Z

Hi @bingwang-ms the logic is implemented in #12733.
If run the test case with the PR #12733 only, and not include this PR, you will have the "queue not empty" requirement met, but due to the pfc frame is sent by the CPU, the traffic is not stable, and espicially for test_pfcwd_timer_accuracy, the test could pass with very low possiblity.

bingwang-ms · 2024-07-23T17:58:36Z

I see. Thanks for the clarification. Then it makes sense to me. I missed #12733 as there is no cherry-pick request. I just added the cherry-pick label for it.

bingwang-ms · 2024-07-23T17:59:09Z

Can you please check this question as well?

bingwang-ms · 2024-07-23T17:59:45Z

Related PR for Non-Onyx leaf-fanout #13768

nhe-NV · 2024-07-23T23:39:42Z

I see you found the answer for nononxy one: "Related PR for Non-Onyx leaf-fanout #13768"

What do you mean this" I didn't see code for building and importing the new container. Is that done manually?"
In the Code, we do have the code to build the container.

nhe-NV · 2024-07-23T23:41:07Z

bingwang-ms · 2024-07-24T01:01:37Z

I see. So the code for building the docker is not changed right? Can you please point me the code for building the docker?

nhe-NV · 2024-07-24T01:25:28Z

Hi @bingwang-ms In this PR, you can find ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/Makefile, this is a new file for build the new docker. but the docker name that build is same as prev

bingwang-ms · 2024-07-24T02:07:51Z

Yes, I saw the makefile. But can you point me the code for building the new docker image? I didn't see that code in this PR.

nhe-NV · 2024-07-24T02:17:39Z

Is this that you are looking for?

bingwang-ms · 2024-07-26T20:55:25Z

Hi @nhe-NV , I understand we have Makefile and Dockerfile. I'm looking for the code to call the make command or the docker build command as I didn't see that code in this change. Can you point it to me?

nhe-NV · 2024-07-29T01:14:19Z

Hi @bingwang-ms , same as prev pfc_wd docker, it dose not have the build command in the script, user will build their own docker and save it locally on demand. In the relevent test case, it will use it directly

bingwang-ms · 2024-07-29T16:54:20Z

I see. Thanks for the clarification. Please address the conflict.

mssonicbld · 2024-07-30T00:18:50Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

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 ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:119:18: F405 'new_uint32_t_p' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:120:5: F405 'uint32_t_p_assign' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:121:28: F405 'new_sx_port_attributes_t_arr' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:122:10: F405 'sx_api_port_device_get' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:123:14: F405 'SX_STATUS_SUCCESS' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:126:16: F405 'uint32_t_p_value' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:129:28: F405 'new_sx_port_attributes_t_arr' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:130:10: F405 'sx_api_port_device_get' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:131:15: F405 'SX_STATUS_SUCCESS' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:141:27: F405 'sx_port_attributes_t_arr_getitem' may be undefined, or defined from star imports: python_sdk_api.sx_api ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pfc_gen.py:156:27: F405 'sx_port_attributes_t_arr_getitem' may be undefined, or defined from star imports: python_sdk_api.sx_api ... [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:

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.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

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

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

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

Currently, the PFC storm is sent using packet socket on the host CPU, which causes some drawbacks - There is strict timing requirement, which can not be guaranteed - The more ports under PFC storm, the higher probability that the timing requirement is not satisfied We will leverage the fanout's ASIC to send PFC storm - Use SDK API to enable lossless traffic, set xoff to 0, and setup mappings for the PGs and the corresponding IEEE priorities Limitation Still keep sending pfc frame with cpu in the storm docker, but rename it to pfc_gen_cpu.py in the docker. If use the ASIC to send the pfc frame, ASIC could not send specified count of the pfc frame. need to still use the CPU to send the pfc frame.such as the test_pfc_counter test Signed-off-by: Stephen Sun <[email protected]>

Currently, the PFC storm is sent using packet socket on the host CPU, which causes some drawbacks - There is strict timing requirement, which can not be guaranteed - The more ports under PFC storm, the higher probability that the timing requirement is not satisfied We will leverage the fanout's ASIC to send PFC storm - Use SDK API to enable lossless traffic, set xoff to 0, and setup mappings for the PGs and the corresponding IEEE priorities Limitation Still keep sending pfc frame with cpu in the storm docker, but rename it to pfc_gen_cpu.py in the docker. If use the ASIC to send the pfc frame, ASIC could not send specified count of the pfc frame. need to still use the CPU to send the pfc frame.such as the test_pfc_counter test Signed-off-by: Stephen Sun <[email protected]> Co-authored-by: Stephen Sun <[email protected]>

mssonicbld · 2024-07-30T16:08:12Z

Cherry-pick PR to 202405: #13885

Currently, the PFC storm is sent using packet socket on the host CPU, which causes some drawbacks - There is strict timing requirement, which can not be guaranteed - The more ports under PFC storm, the higher probability that the timing requirement is not satisfied We will leverage the fanout's ASIC to send PFC storm - Use SDK API to enable lossless traffic, set xoff to 0, and setup mappings for the PGs and the corresponding IEEE priorities Limitation Still keep sending pfc frame with cpu in the storm docker, but rename it to pfc_gen_cpu.py in the docker. If use the ASIC to send the pfc frame, ASIC could not send specified count of the pfc frame. need to still use the CPU to send the pfc frame.such as the test_pfc_counter test Signed-off-by: Stephen Sun <[email protected]> Co-authored-by: Stephen Sun <[email protected]>

nhe-NV force-pushed the sneding_pfc_with_asic branch from 899de5a to 7537e39 Compare June 12, 2024 02:53

nhe-NV force-pushed the sneding_pfc_with_asic branch from 7537e39 to 30c303c Compare June 13, 2024 04:02

nhe-NV added Request for 202305 branch Request for 202311 branch Request for 202405 branch labels Jun 15, 2024

roy-sror approved these changes Jun 30, 2024

View reviewed changes

stephenxs approved these changes Jul 13, 2024

View reviewed changes

bingwang-ms assigned kperumalbfn Jul 18, 2024

nhe-NV force-pushed the sneding_pfc_with_asic branch from 30c303c to 5debb87 Compare July 30, 2024 00:14

nhe-NV force-pushed the sneding_pfc_with_asic branch 3 times, most recently from f4fa608 to 05bb1e8 Compare July 30, 2024 00:24

nhe-NV force-pushed the sneding_pfc_with_asic branch from 05bb1e8 to 5547d5d Compare July 30, 2024 00:27

bingwang-ms approved these changes Jul 30, 2024

View reviewed changes

bingwang-ms merged commit 17a20ab into sonic-net:master Jul 30, 2024

bingwang-ms added Approved for 202405 branch and removed Request for 202305 branch labels Jul 30, 2024

mssonicbld added the Created PR to 202405 branch label Jul 30, 2024

mssonicbld mentioned this pull request Jul 30, 2024

[action] [PR:13242] Leverage the fanout's ASIC to send PFC storm #13885

Merged

9 tasks

mssonicbld added Included in 202405 branch and removed Created PR to 202405 branch labels Jul 30, 2024

Conversation

nhe-NV commented Jun 12, 2024

Description of PR

Type of change

Back port request

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!

mssonicbld commented Jun 12, 2024

Uh oh!

wsycqyz commented Jun 13, 2024

Uh oh!

wsycqyz commented Jun 13, 2024

Uh oh!

nhe-NV commented Jun 15, 2024

Uh oh!

mssonicbld commented Jun 15, 2024

Uh oh!

azure-pipelines bot commented Jun 15, 2024

Uh oh!

bingwang-ms commented Jul 18, 2024

Uh oh!

bingwang-ms commented Jul 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bingwang-ms commented Jul 22, 2024

Uh oh!

nhe-NV commented Jul 23, 2024

Uh oh!

bingwang-ms commented Jul 23, 2024

Uh oh!

bingwang-ms commented Jul 23, 2024

Uh oh!

bingwang-ms commented Jul 23, 2024

Uh oh!

nhe-NV commented Jul 23, 2024

Uh oh!

nhe-NV commented Jul 23, 2024

Uh oh!

bingwang-ms commented Jul 24, 2024

Uh oh!

nhe-NV commented Jul 24, 2024

Uh oh!

bingwang-ms commented Jul 24, 2024

Uh oh!

nhe-NV commented Jul 24, 2024

Uh oh!

bingwang-ms commented Jul 26, 2024

Uh oh!

nhe-NV commented Jul 29, 2024

Uh oh!

bingwang-ms commented Jul 29, 2024

Uh oh!

mssonicbld commented Jul 30, 2024

Uh oh!

mssonicbld commented Jul 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

bingwang-ms commented Jul 22, 2024 •

edited

Loading