[Qos]HeadroomPoolSize test with dynamic_threshold based buffer allocation by ansrajpu-git · Pull Request #16885 · sonic-net/sonic-mgmt

ansrajpu-git · 2025-02-10T21:01:49Z

Description of PR

The existing HeadroomPool size test is based on traditional way of buffer allocation to calculate the headroompoolsize.
For Broadcom chassis, this new test calculates the buffer allocation using dynamic threshold to fill shared pool & headroom and verify the headroom size per vsq.

Skip test :-testQosSaiHeadroomPoolWatermark - SAI_BUFFER_POOL_STAT_XOFF_ROOM_WATERMARK_BYTES is not supported on broadcom-dnx
Summary:
For Broadcom chassis, since the headroom per vsq is huge, 4 src port are used to fill the headroom.

Fixes # (issue)
Issue : #13503

Type of change

Back port request

Approach

What is the motivation for this PR?

Verifying HeadroomPoolsize test for broadcom-dnx chassis

How did you do it?

How did you verify/test it?

Executed qos test for headroomPool size and verify the results
Resource allocation

max headroom per vsq

Any platform specific information?

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

Documentation

mssonicbld · 2025-02-10T21:01:53Z

/azp run

azure-pipelines · 2025-02-10T21:02:05Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-02-10T21:06:06Z

/azp run

azure-pipelines · 2025-02-10T21:06:20Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-02-10T21:39:14Z

/azp run

azure-pipelines · 2025-02-10T21:39:26Z

Azure Pipelines successfully started running 1 pipeline(s).

ansrajpu-git · 2025-02-10T21:57:59Z

@vmittal-msft , please review.

mssonicbld · 2025-02-13T18:08:40Z

/azp run

azure-pipelines · 2025-02-13T18:08:47Z

Pull request contains merge conflicts.

arista-nwolfe · 2025-02-19T18:30:12Z

tests/qos/files/qos_params.j2c.yaml

-                    pkts_num_trig_pfc: 657523
-                    pkts_num_hdrm_full: 622850
-                    pkts_num_hdrm_partial: 622750
-                    margin: 300
+                    pkts_num_trig_pfc: 542448
+                    pkts_num_hdrm_full: 713397
+                    pkts_num_hdrm_partial: 713250
+                    margin: 100


Could you share details on how you calculated these values so we can do a similar calculation for qos_params.j2.yaml?

Could you share details on how you calculated these values so we can do a similar calculation for qos_params.j2.yaml?

@ansrajpu-git if you might help reply? thanks.

Could you share details on how you calculated these values so we can do a similar calculation for qos_params.j2.yaml?

@arista-nwolfe , the calculation is based on the Pool_size and the alpha value (dynamic threshold) used.
The pkts_num_trig_pfc is calculated as = 2** (alpha value) x R , where R is the remaining pool_ size
The pkts_num_headroom_full is the same value as buffer allocation per vsq.
Above value calculated has to be divided by the packet size used in test, to place in qos_yaml file.

arista-nwolfe · 2025-02-27T05:08:24Z

Also note that on a T2 full topology with all J2c+ LCs I saw the tests skipped due to Insufficient number of ports to fill the headroom. Is this test meant for a different topology with more ports?

qos/test_qos_sai.py::TestQosSai::testQosSaiHeadroomPoolSize[single_asic] SKIPPED (Insufficient number of ports to fill the headroom)                                       [ 20%]
qos/test_qos_sai.py::TestQosSai::testQosSaiHeadroomPoolSize[single_dut_multi_asic] SKIPPED (Insufficient number of ports to fill the headroom)                             [ 40%]
qos/test_qos_sai.py::TestQosSai::testQosSaiHeadroomPoolSize[multi_dut_longlink_to_shortlink] SKIPPED (Insufficient number of ports to fill the headroom)                   [ 60%]
qos/test_qos_sai.py::TestQosSai::testQosSaiHeadroomPoolSize[multi_dut_shortlink_to_shortlink] SKIPPED (Insufficient number of ports to fill the headroom)                  [ 80%]
qos/test_qos_sai.py::TestQosSai::testQosSaiHeadroomPoolSize[multi_dut_shortlink_to_longlink] SKIPPED (Insufficient number of ports to fill the headroom)                   [100%]

vmittal-msft

@ansrajpu-git It looks good to me. Please handle minor comments as discussed on call. Also, please re-base.

mssonicbld · 2025-02-27T21:18:44Z

/azp run

azure-pipelines · 2025-02-27T21:18:50Z

Pull request contains merge conflicts.

ansrajpu-git · 2025-02-27T22:35:57Z

Also note that on a T2 full topology with all J2c+ LCs I saw the tests skipped due to Insufficient number of ports to fill the headroom. Is this test meant for a different topology with more ports?

qos/test_qos_sai.py::TestQosSai::testQosSaiHeadroomPoolSize[single_asic] SKIPPED (Insufficient number of ports to fill the headroom)                                       [ 20%]
qos/test_qos_sai.py::TestQosSai::testQosSaiHeadroomPoolSize[single_dut_multi_asic] SKIPPED (Insufficient number of ports to fill the headroom)                             [ 40%]
qos/test_qos_sai.py::TestQosSai::testQosSaiHeadroomPoolSize[multi_dut_longlink_to_shortlink] SKIPPED (Insufficient number of ports to fill the headroom)                   [ 60%]
qos/test_qos_sai.py::TestQosSai::testQosSaiHeadroomPoolSize[multi_dut_shortlink_to_shortlink] SKIPPED (Insufficient number of ports to fill the headroom)                  [ 80%]
qos/test_qos_sai.py::TestQosSai::testQosSaiHeadroomPoolSize[multi_dut_shortlink_to_longlink] SKIPPED (Insufficient number of ports to fill the headroom)                   [100%]

@arista-nwolfe , For broadcom-dnx, T2 topology ,buffer profile except "400000_120000m" has a smaller headroom per vsq , which is not sufficient to fill the Nominal headroom with available ports.For ex., 400000_2000m has a nominal headroom of ~791MB but the headroom per vsq is ~1.44MB. Upon calculating, 273 ports are needed approximately to fill the headroom.

mssonicbld · 2025-02-27T22:52:19Z

/azp run

azure-pipelines · 2025-02-27T22:52:30Z

Azure Pipelines successfully started running 1 pipeline(s).

ansrajpu-git · 2025-02-28T04:08:32Z

Required

@vmittal-msft , comments update and rebase done.

mssonicbld · 2025-02-28T15:36:35Z

/azp run

azure-pipelines · 2025-02-28T15:36:47Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-03-04T22:32:07Z

/azp run

mssonicbld · 2025-04-14T19:12:33Z

/azp run

azure-pipelines · 2025-04-14T19:12:46Z

Azure Pipelines successfully started running 1 pipeline(s).

ansrajpu-git · 2025-04-14T21:41:38Z

Cherry-pick -PR-Azure/sonic-mgmt.msft#191 raised for msft-202405 branch

XuChen-MSFT · 2025-04-16T01:33:18Z

@ansrajpu-git
original "HdrmPoolSizeTest" is also support device which is dynamically allocate buffer. and I think dynamic buffer allocation should be default behaviors.
why need add new case ? I just quicky go throught "HdrmPoolSizeTest_withDynamicBufferCacl", I don't found some big difference with original one "HdrmPoolSizeTest" .
The original case is to automatically select the ports and PGs required for the hdrm test in qos_param_generator.py, calculate how much buffer each PG needs to fill the shared buffer according to the dynamic threshold, and then start the consumption test of the headroom pool.

ansrajpu-git · 2025-04-16T14:24:06Z

@ansrajpu-git original "HdrmPoolSizeTest" is also support device which is dynamically allocate buffer. and I think dynamic buffer allocation should be default behaviors. why need add new case ? I just quicky go throught "HdrmPoolSizeTest_withDynamicBufferCacl", I don't found some big difference with original one "HdrmPoolSizeTest" . The original case is to automatically select the ports and PGs required for the hdrm test in qos_param_generator.py, calculate how much buffer each PG needs to fill the shared buffer according to the dynamic threshold, and then start the consumption test of the headroom pool.

@XuChen-MSFT , The new test has similar test steps to previous test 'HdrmPoolSizeTest' but it was working with multiple check condition for broadcom-dnx platform. With the dynamic buffer allocation, the test was intermittently failing, and we see that there is some adjustment need between the shared pool almost filled and trigger pfc frames ,precisely with few more packets.
As suggested by Vineet, we came up with new test specifically to broadcom-dnx platform for a clean look. We will be raising another PR for removing the broadcom-dnx check from the older test, once this PR is merged.
@vmittal-msft,please comment

vmittal-msft · 2025-04-21T22:12:08Z

@yxieca @rlhui please help merge.

vmittal-msft · 2025-04-21T23:06:30Z

@XuChen-MSFT we can discuss this if you have any concern. please ping.

ansrajpu-git · 2025-04-30T15:01:51Z

Cherry-pick for msft-202503 -Azure/sonic-mgmt.msft#236

…tion (sonic-net#16885) What is the motivation for this PR? Verifying HeadroomPoolsize test for broadcom-dnx chassis How did you verify/test it? Executed qos test for headroomPool size and verify the results

…hold based buffer allocation (sonic-net#236) Cherry-pick (sonic-net#16885) What is the motivation for this PR? Verifying HeadroomPoolsize test for broadcom-dnx chassis How did you verify/test it? Executed qos test for headroomPool size and verify the results

…tion (sonic-net#16885) What is the motivation for this PR? Verifying HeadroomPoolsize test for broadcom-dnx chassis How did you verify/test it? Executed qos test for headroomPool size and verify the results Signed-off-by: opcoder0 <110003254+opcoder0@users.noreply.github.com>

…tion (sonic-net#16885) What is the motivation for this PR? Verifying HeadroomPoolsize test for broadcom-dnx chassis How did you verify/test it? Executed qos test for headroomPool size and verify the results Signed-off-by: Aharon Malkin <amalkin@nvidia.com>

…tion (sonic-net#16885) What is the motivation for this PR? Verifying HeadroomPoolsize test for broadcom-dnx chassis How did you verify/test it? Executed qos test for headroomPool size and verify the results Signed-off-by: Guy Shemesh <gshemesh@nvidia.com>

vmittal-msft added the Request for 202405 branch label Feb 18, 2025

rlhui requested a review from vmittal-msft February 19, 2025 18:18

rlhui added Request for 202411 branch Request for msft-202405 Branch Chassis Test Gap New or existing test cases that need to be added for chassis platforms and removed Request for 202405 branch labels Feb 19, 2025

rlhui requested a review from arista-nwolfe February 19, 2025 18:24

arista-nwolfe reviewed Feb 19, 2025

View reviewed changes

vmittal-msft approved these changes Feb 27, 2025

View reviewed changes

ansrajpu-git force-pushed the HdrmPool_dyBuffer branch from 394e15c to 65812d7 Compare February 27, 2025 22:51

mssonicbld added the Cherry Pick Conflict_msft-202405 label Feb 27, 2025

ansrajpu-git added 8 commits April 14, 2025 15:11

flake8 issue

97d88fc

Flake8 fix

d9db910

[QoS]Hdrmpooltest update for src_vlan

40abc2d

[Qos]400G_2k profile descoped ofr HdrmPool

5c3b42a

[Qos]Misc correction in comments

cb3c57b

Flake8_fix

a42eb5c

flake8

b19bcb1

[Qos]qos_param update

f753878

ansrajpu-git force-pushed the HdrmPool_dyBuffer branch from 830a959 to f753878 Compare April 14, 2025 19:12

This was referenced Apr 14, 2025

[Cherry-pick][Qos]HeadroomPoolSize test with dynamic_threshold based buffer allocation #17982

Closed

[Cherry-pick][Qos]HeadroomPoolSize test with dynamic_threshold based buffer allocation Azure/sonic-mgmt.msft#191

Open

tjchadaga added the Request for msft-202503 Branch label Apr 17, 2025

mssonicbld added the Cherry Pick Conflict_msft-202503 label Apr 17, 2025

yxieca merged commit 0247297 into sonic-net:master Apr 22, 2025
20 checks passed

github-project-automation bot moved this to Done in SONiC Chassis Apr 22, 2025

vmittal-msft requested a review from XuChen-MSFT April 22, 2025 18:55

Conversation

ansrajpu-git commented Feb 10, 2025 • edited by rlhui Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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 Feb 10, 2025

Uh oh!

azure-pipelines bot commented Feb 10, 2025

Uh oh!

mssonicbld commented Feb 10, 2025

Uh oh!

azure-pipelines bot commented Feb 10, 2025

Uh oh!

mssonicbld commented Feb 10, 2025

Uh oh!

azure-pipelines bot commented Feb 10, 2025

Uh oh!

ansrajpu-git commented Feb 10, 2025

Uh oh!

mssonicbld commented Feb 13, 2025

Uh oh!

azure-pipelines bot commented Feb 13, 2025

Uh oh!

arista-nwolfe Feb 19, 2025

Choose a reason for hiding this comment

Uh oh!

rlhui Feb 27, 2025

Choose a reason for hiding this comment

Uh oh!

ansrajpu-git Feb 27, 2025

Choose a reason for hiding this comment

Uh oh!

arista-nwolfe commented Feb 27, 2025

Uh oh!

vmittal-msft left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Feb 27, 2025

Uh oh!

azure-pipelines bot commented Feb 27, 2025

Uh oh!

ansrajpu-git commented Feb 27, 2025

Uh oh!

mssonicbld commented Feb 27, 2025

Uh oh!

azure-pipelines bot commented Feb 27, 2025

Uh oh!

ansrajpu-git commented Feb 28, 2025

Uh oh!

mssonicbld commented Feb 28, 2025

Uh oh!

azure-pipelines bot commented Feb 28, 2025

Uh oh!

mssonicbld commented Mar 4, 2025

Uh oh!

mssonicbld commented Apr 14, 2025

Uh oh!

azure-pipelines bot commented Apr 14, 2025

Uh oh!

ansrajpu-git commented Apr 14, 2025

Uh oh!

XuChen-MSFT commented Apr 16, 2025

Uh oh!

ansrajpu-git commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vmittal-msft commented Apr 21, 2025

Uh oh!

vmittal-msft commented Apr 21, 2025

Uh oh!

ansrajpu-git commented Feb 10, 2025 •

edited by rlhui

Loading

vmittal-msft left a comment •

edited

Loading

ansrajpu-git commented Apr 16, 2025 •

edited

Loading