Skip to content

Fix QoS configuations for Quicksilver #19315

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 25, 2024

Conversation

rick-arista
Copy link
Contributor

@rick-arista rick-arista commented Jun 14, 2024

Adds egress_lossy_pool and fixes SAI error caused by TH5 default config

Why I did it

This change is needed to fix a SAI crash on boot. These values zero out the reserved buffer space that by default would cause the pools defined in config_db.json to fail to create due to insufficient remaining room.

Work item tracking
  • Microsoft ADO (number only):

How I did it

The buffer pool values were chosen by simply dividing the total available MMU memory into 3 pools.

How to verify it

Without this change device fails to boot correctly.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305
  • 202311
  • 202405

Description for the changelog

Fix QoS configuations for Quicksilver

Adds egress_lossy_pool and fixes SAI error caused by TH5 default config
"egress_lossless_pool": {
"size": "41300000",
Copy link
Collaborator

Choose a reason for hiding this comment

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

in the sai definition, we do not have ITM concept. we treat whole device as pool, and therefore i am not sure we need to cut this in half is satisfying the sai definition.

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 think you're right, but these are just placeholder values to fix a SAI crash anyways. We'll be updating these with tuned values shortly.

@r12f
Copy link
Contributor

r12f commented Jun 16, 2024

hi @rick-arista , thanks a lot for sending this PR - somehow with this fix, the PFC frame still cannot be sent out with ixia test. I can see the new pg profile is created now and the ports are associated with it, but no PFC frame will be generated:

image

@bingwang-ms
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@r12f r12f left a comment

Choose a reason for hiding this comment

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

lgtm!

@yxieca yxieca merged commit d23f4e3 into sonic-net:master Jun 25, 2024
19 of 20 checks passed
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Jun 25, 2024
Adds egress_lossy_pool and fixes SAI error caused by TH5 default config

Why I did it
This change is needed to fix a SAI crash on boot. These values zero out the reserved buffer space that by default would cause the pools defined in config_db.json to fail to create due to insufficient remaining room.

How I did it
The buffer pool values were chosen by simply dividing the total available MMU memory into 3 pools.

How to verify it
Without this change device fails to boot correctly.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #19393

yxieca pushed a commit that referenced this pull request Jun 26, 2024
Adds egress_lossy_pool and fixes SAI error caused by TH5 default config

Why I did it
This change is needed to fix a SAI crash on boot. These values zero out the reserved buffer space that by default would cause the pools defined in config_db.json to fail to create due to insufficient remaining room.

How I did it
The buffer pool values were chosen by simply dividing the total available MMU memory into 3 pools.

How to verify it
Without this change device fails to boot correctly.

Co-authored-by: rick-arista <[email protected]>
arun1355492 pushed a commit to arun1355492/sonic-buildimage that referenced this pull request Jul 26, 2024
Adds egress_lossy_pool and fixes SAI error caused by TH5 default config

Why I did it
This change is needed to fix a SAI crash on boot. These values zero out the reserved buffer space that by default would cause the pools defined in config_db.json to fail to create due to insufficient remaining room.

How I did it
The buffer pool values were chosen by simply dividing the total available MMU memory into 3 pools.

How to verify it
Without this change device fails to boot correctly.
liushilongbuaa pushed a commit to liushilongbuaa/sonic-buildimage that referenced this pull request Aug 1, 2024
Adds egress_lossy_pool and fixes SAI error caused by TH5 default config

Why I did it
This change is needed to fix a SAI crash on boot. These values zero out the reserved buffer space that by default would cause the pools defined in config_db.json to fail to create due to insufficient remaining room.

How I did it
The buffer pool values were chosen by simply dividing the total available MMU memory into 3 pools.

How to verify it
Without this change device fails to boot correctly.
@bingwang-ms
Copy link
Contributor

@rick-arista Please file another PR for 202405 branch to address cherry-pick conflict.

yanjundeng pushed a commit to yanjundeng/sonic-buildimage that referenced this pull request Apr 23, 2025
Adds egress_lossy_pool and fixes SAI error caused by TH5 default config

Why I did it
This change is needed to fix a SAI crash on boot. These values zero out the reserved buffer space that by default would cause the pools defined in config_db.json to fail to create due to insufficient remaining room.

How I did it
The buffer pool values were chosen by simply dividing the total available MMU memory into 3 pools.

How to verify it
Without this change device fails to boot correctly.
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