Skip to content
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

[201911]: Adding SKU Mellanox-SN3800-D100C12S2 #15

Open
wants to merge 10 commits into
base: 201911
Choose a base branch
from

Conversation

madhanmellanox
Copy link
Owner

Why I did it

To create a new SKU Mellanox-SN3800-D100C12S2

How I did it

I arrived at the SKU configuration values based on the following SKU template, Port mapping and number of uplinks and downlinks.

SKU template:
Port configuration
• Breakout mode for each port - Defined in port mapping
• Speed of the port Defined in Port mapping
• Auto-negotiation enable/disable No setting required
• FEC mode No setting required
• Type of transceiver used Not needed
Buffer configuration
• Shared headroom enable
• If shared headroom enabled what is the over-subscription ratio as in SN3800
• Dynamic Buffer disable
• In static buffer scenario how many uplinks and downlinks? as in SN3800
• 2km cable support required? no
Switch configuration
• Warmboot enabled? yes
• Should warmboot be added to SAI profile when enabled? yes
• Is VxLAN source port range set? yes
• Should Vxlan source port range be added to SAI profile when set. as in SN3800
• Is Static Policy Based Hashing enabled? no

Port Mapping
etp1 to etp36 split into 50G
etp37 and etp40 is 10G
etp38 and etp39 splint into 50G
etp41 to etp52 split into 50G
etp53 to etp64 is 100G

Number of Uplinks / Downlinks:
TO topology: 12 100G uplinks and rest all downlinks.
T1 topology: (SKU will not be used in T1 topology), so same 12 100G uplinks and rest all downlinks is used to arrive at buffer config values.

How to verify it

Build the image, install it on the 3800 switch and set the SKU and verify the ports come up with proper speeds.

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

  • 201811
  • 201911
  • 202006
  • 202012

Description for the changelog

Changes are in sonic-buildimage/device/mellanox/x86_64-mlnx_msn3800-r0/Mellanox-SN3800-D100C12S2/ folder.

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

@@ -0,0 +1,17 @@
# PG lossless profiles.

Choose a reason for hiding this comment

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

We don't need to add a new file. We can just point it to the one in Mellanox-SN3800-D112C8

@@ -0,0 +1,100 @@
{% set default_cable = '5m' %}
{% set ingress_lossless_pool_size = '47194112' %}
{% set ingress_lossless_pool_xoff = '47194112' %}

Choose a reason for hiding this comment

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

The xoff stands for shared headroom pool size. It should not be equal to pool size.
The buffer pool size is not correct. It's too large for Spectrum 2. You probably used Spectrum 3.
My result is 20664320 xoff 3321856

  • 10G 5m * 2 headroom 44kb
  • 50G 5m * 100 headroom 51kb
  • 100G 40m * 12 headroom 66kb

@@ -0,0 +1,100 @@
{% set default_cable = '5m' %}
{% set ingress_lossless_pool_size = '46133248' %}
{% set ingress_lossless_pool_xoff = '46133248' %}

Choose a reason for hiding this comment

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

The same.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks Stephen, I will incorporate these changes.

@@ -0,0 +1,100 @@
{% set default_cable = '5m' %}
{% set ingress_lossless_pool_size = '20664320' %}
{% set ingress_lossless_pool_xoff = '20664320' %}

Choose a reason for hiding this comment

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

Suggested change
{% set ingress_lossless_pool_xoff = '20664320' %}
{% set ingress_lossless_pool_xoff = '3321856' %}

{% set default_cable = '5m' %}
{% set ingress_lossless_pool_size = '20664320' %}
{% set ingress_lossless_pool_xoff = '20664320' %}
{% set egress_lossless_pool_size = '3321856' %}

Choose a reason for hiding this comment

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

Suggested change
{% set egress_lossless_pool_size = '3321856' %}
{% set egress_lossless_pool_size = '34287552' %}

@stephenxs
Copy link

ingress_lossless_pool_xoff represents the shared headroom pool size.
egress_lossless_pool_size should be the MMUSize

@@ -0,0 +1,100 @@
{% set default_cable = '5m' %}
{% set ingress_lossless_pool_size = '19601408' %}
{% set ingress_lossless_pool_xoff = '19601408' %}

Choose a reason for hiding this comment

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

Suggested change
{% set ingress_lossless_pool_xoff = '19601408' %}
{% set ingress_lossless_pool_xoff = '4384768' %}

{% set default_cable = '5m' %}
{% set ingress_lossless_pool_size = '19601408' %}
{% set ingress_lossless_pool_xoff = '19601408' %}
{% set egress_lossless_pool_size = '4384768' %}

Choose a reason for hiding this comment

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

Suggested change
{% set egress_lossless_pool_size = '4384768' %}
{% set egress_lossless_pool_size = '34287552' %}

Copy link

@stephenxs stephenxs left a comment

Choose a reason for hiding this comment

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

Approved for buffer confurations

Ethernet0 0,1 etp1a 1 50000
Ethernet2 2,3 etp1b 1 50000
Ethernet4 4,5 etp2a 2 50000
Ethernet4 6,7 etp2b 2 50000
Copy link

@dgsudharsan dgsudharsan Jun 24, 2021

Choose a reason for hiding this comment

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

Why do we have two Ethernet4 defined here? I believe it should be Ethernet6

Copy link
Owner Author

Choose a reason for hiding this comment

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

got it @dgsudharsan, will fix it.

@volodymyrsamotiy
Copy link

@madhanmellanox, please fix comment provided by @dgsudharsan.
Everything else looks good to me.

Copy link

@dgsudharsan dgsudharsan left a comment

Choose a reason for hiding this comment

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

Approving it for SONiC port level changes.

@stephenxs
Copy link

Currently, we don't take port admin state into account when calculating buffer pool size.
There is a request about reclaiming the reserved buffer of admin down port from MSFT, which is under internal discussion.
I suggest taking the number of all ports instead of admin up ports into account when calculating the buffer sizes for now, which means we don't need to update the buffer pool size.

@dgsudharsan
Copy link

@madhanmellanox Can you please close this?

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.

4 participants