Skip to content

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

Open
madhanmellanox wants to merge 10 commits into201911from
201911MellanoxSN3800D100C12S2SKU
Open

[201911]: Adding SKU Mellanox-SN3800-D100C12S2#15
madhanmellanox wants to merge 10 commits into201911from
201911MellanoxSN3800D100C12S2SKU

Conversation

@madhanmellanox
Copy link
Copy Markdown
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.
Copy link
Copy Markdown

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' %}
Copy link
Copy Markdown

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' %}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The same.

Copy link
Copy Markdown
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' %}
Copy link
Copy Markdown

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' %}
Copy link
Copy Markdown

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
Copy Markdown

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' %}
Copy link
Copy Markdown

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' %}
Copy link
Copy Markdown

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
Copy Markdown

@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
Copy Markdown

@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
Copy Markdown
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
Copy Markdown

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

Copy link
Copy Markdown

@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
Copy Markdown

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
Copy Markdown

@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