Skip to content

Adding default QoS configurations for Arista-7050CX3-32S-C28S4.#22117

Merged
kperumalbfn merged 2 commits intosonic-net:masterfrom
r12f:user/r12f/fix-c28s4
Mar 25, 2025
Merged

Adding default QoS configurations for Arista-7050CX3-32S-C28S4.#22117
kperumalbfn merged 2 commits intosonic-net:masterfrom
r12f:user/r12f/fix-c28s4

Conversation

@r12f
Copy link
Contributor

@r12f r12f commented Mar 22, 2025

Why I did it

The default QoS configuration for Arista-7050CX3-32S-C28S4 is not added. This change is fixing the issues.

Work item tracking
  • Microsoft ADO (number only):

How I did it

Added the default QoS configurations for Arista-7050CX3-32S-C28S4.

How to verify it

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

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

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@r12f r12f requested a review from kperumalbfn March 24, 2025 16:53
@@ -0,0 +1,52 @@
{%- set default_cable = '300m' %}
Copy link
Contributor

Choose a reason for hiding this comment

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

please copy buffers.json.j2 for this new SKU

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like they are exactly the same:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mean another file?

{%- macro generate_port_lists(PORT_ALL) %}
{# Generate list of ports #}
{%- for port_idx in range(0,32) %}
{%- if PORT_ALL.append("Ethernet%d" % (port_idx * 4)) %}{%- endif %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have the same port name mappings for 10G, like port_idx * 4? Please check.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch, we use the whole port, so it worked out to be the same port names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I have confirmed that before. Since these 10G ports uses only 1 lane from the physical port, so the port number still jumps by 4.

Here is the port config ini:

image

"size": "32599040",
"type": "egress",
"mode": "static"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

In this config, egress_lossless has less buffer compared to ingress lossless. So, there could be egress drop before PFC gets generated.

For now, we could go with buffer configs in device/arista/x86_64-arista_7050cx3_32s/Arista-7050CX3-32S-D48C8/BALANCED/buffers_defaults_t0.j2

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, default should be files under balanced folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Ying, the default is already in balance folder. But the problem is even that is not working right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to switch from C32 to D48C8.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated now.

yxieca
yxieca previously approved these changes Mar 25, 2025
@yxieca yxieca self-requested a review March 25, 2025 00:48
@r12f
Copy link
Contributor Author

r12f commented Mar 25, 2025

@kperumalbfn , all comments are addressed now. do you mind to help take another look?

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@kperumalbfn kperumalbfn left a comment

Choose a reason for hiding this comment

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

Thanks @r12f

@kperumalbfn kperumalbfn merged commit f92eddd into sonic-net:master Mar 25, 2025
21 checks passed
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #22141

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202411: #22744

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