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

[chassis][voq] 400g to100g speed changes for chassis linecards #13935

Merged
merged 2 commits into from
Feb 24, 2023

Conversation

arlakshm
Copy link
Contributor

@arlakshm arlakshm commented Feb 22, 2023

Why I did it

On SONiC VoQ chassis, the speed changes are done from 400G to 100G needs to be supported on 400G linecards.
To enable this, along with speed change the port lanes need to be changed. This PR has the changes to update the port lanes when such speed change happens.

This PR is intended only for VoQ chassis linecards. These platforms today have 400g port with 8 serdes lines, and 100g will operate with 4 serdes lane. When the port speed changes from 400G to 100G the first 4 lanes will be used for 100G port.

Platforms which support 2x50g PAM4 or support 100G PAM4 serdes or other combinations are not handled in the PR.

How I did it

Updated the port lanes when the port speed is changed from 400g to 100g.

How to verify it

UT and test on sonic chassis

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • [x ] 202205
  • [x ] 202211

Description for the changelog

Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.

Link to config_db schema for YANG module changes

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

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
Copy link
Contributor

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@arlakshm approved with comments if you an address

src/sonic-config-engine/minigraph.py Outdated Show resolved Hide resolved
@arlakshm
Copy link
Contributor Author

Copy link
Contributor

@judyjoseph judyjoseph left a comment

Choose a reason for hiding this comment

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

LGTM

@abdosi
Copy link
Contributor

abdosi commented Feb 22, 2023

cc @anamehra for review

@abdosi
Copy link
Contributor

abdosi commented Feb 22, 2023

added one comment,

@anamehra
Copy link
Contributor

Thanks for tagging @abdosi !
@shyam77git , @rajann FYI, please check.

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
@anamehra
Copy link
Contributor

Does this support the speed change only via minigraph or CLI as well?

Copy link
Contributor

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@arlakshm @abdosi please note that in future when we use 100G PAM4 serdes this change won't be able to handle 400G speed(4 lanes) to 100G speed(1 lane). There is an inherent assumption here that 400G speed will always have 8 lanes and 100G will always have 4 lanes. Can we think of a better generic solution? This solution looks only specific to a particular platform and won't stand future platforms that are going to be based on 100G PAM4.

@gechiang
Copy link
Collaborator

100G PAM4 serdes

Excellent point. I understand some vendors have started to use this but I would suggest we take on this 100G PAM4 serdes as a phase 2 change. Also, the adaptation to this new optic which inherently has "higher" chance of noise can causing bad data at corrupting 2 bits at the same time may require additional time to see if it will be "widely" adopted by the community...

@arlakshm
Copy link
Contributor Author

@arlakshm @abdosi please note that in future when we use 100G PAM4 serdes this change won't be able to handle 400G speed(4 lanes) to 100G speed(1 lane). There is an inherent assumption here that 400G speed will always have 8 lanes and 100G will always have 4 lanes. Can we think of a better generic solution? This solution looks only specific to a particular platform and won't stand future platforms that are going to be based on 100G PAM4.

@prgeor, as discussed offline, the scope of change to solve a specific use case: Support changing speed from 400g to 100g for voq linecards. These platforms today have 400g port with 8 serdes lines, and 100g will operate with 4 serdes lane.
Currently we don't have platforms with 400g port with 4 lanes. When such platforms are introduced, we need to enhance this change in the future

@arlakshm
Copy link
Contributor Author

Does this support the speed change only via minigraph or CLI as well?

The current use case is to support this speed change via minigraph

@arlakshm
Copy link
Contributor Author

100G PAM4 serdes

Excellent point. I understand some vendors have started to use this but I would suggest we take on this 100G PAM4 serdes as a phase 2 change. Also, the adaptation to this new optic which inherently has "higher" chance of noise can causing bad data at corrupting 2 bits at the same time may require additional time to see if it will be "widely" adopted by the community...

@gechiang, yes, The current PR is not intended for all the platforms. We can enhance this change on other platforms, based testing and other use-cases which might arise in the future.

@prgeor
Copy link
Contributor

prgeor commented Feb 24, 2023

@lguohan please help merge

@prgeor
Copy link
Contributor

prgeor commented Feb 24, 2023

100G PAM4 serdes

Excellent point. I understand some vendors have started to use this but I would suggest we take on this 100G PAM4 serdes as a phase 2 change. Also, the adaptation to this new optic which inherently has "higher" chance of noise can causing bad data at corrupting 2 bits at the same time may require additional time to see if it will be "widely" adopted by the community...

@gechiang not just that, we dont have means in minigraph to know whether the 100G speed is from 2x100G, 4x100G or 8x100G. We can come back later

Copy link
Contributor

@abdosi abdosi left a comment

Choose a reason for hiding this comment

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

LGTM for current usecase

@lguohan
Copy link
Collaborator

lguohan commented Feb 24, 2023

@prgeor , we should test this and see if pizza box works with this lane selection.

@lguohan
Copy link
Collaborator

lguohan commented Feb 24, 2023

the pr looks good, however, description is a little bit concise/brief. i suggest to pull some of the discussions into the pr description and give people a full story. for example, there is some limitation of this pr, like 2x50PAM 100g mode is not supported.

@arlakshm
Copy link
Contributor Author

the pr looks good, however, description is a little bit concise/brief. i suggest to pull some of the discussions into the pr description and give people a full story. for example, there is some limitation of this pr, like 2x50PAM 100g mode is not supported.

@lguohan updated the description.

@lguohan lguohan merged commit da1526c into sonic-net:master Feb 24, 2023
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Feb 24, 2023
…-net#13935)

On SONiC VoQ chassis, the speed changes are done from 400G to 100G needs to be supported on 400G linecards.
To enable this, along with speed change the port lanes need to be changed. This PR has the changes to update the port lanes when such speed change happens.

This PR is intended only for VoQ chassis linecards. These platforms today have 400g port with 8 serdes lines, and 100g will operate with 4 serdes lane. When the port speed changes from 400G to 100G the first 4 lanes will be used for 100G port.

Platforms which support 2x50g PAM4 or support 100G PAM4 serdes or other combinations are not handled in the PR.

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202205: #13982

@sanmalho-git
Copy link

@arlakshm - was this tested against a multi-asic linecard?

@arlakshm
Copy link
Contributor Author

arlakshm commented Mar 3, 2023

@sanmalho-git, yes I am testing on a multi asic linecard

@sanmalho-git
Copy link

@arlakshm - thanks - please let us know once you are done with testing, and I can try it out on our cards as well. As is, it is failing on multi-asic cards.

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Mar 19, 2023
…-net#13935)

On SONiC VoQ chassis, the speed changes are done from 400G to 100G needs to be supported on 400G linecards.
To enable this, along with speed change the port lanes need to be changed. This PR has the changes to update the port lanes when such speed change happens.

This PR is intended only for VoQ chassis linecards. These platforms today have 400g port with 8 serdes lines, and 100g will operate with 4 serdes lane. When the port speed changes from 400G to 100G the first 4 lanes will be used for 100G port.

Platforms which support 2x50g PAM4 or support 100G PAM4 serdes or other combinations are not handled in the PR.

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202211: #14322

mssonicbld pushed a commit that referenced this pull request Mar 19, 2023
On SONiC VoQ chassis, the speed changes are done from 400G to 100G needs to be supported on 400G linecards.
To enable this, along with speed change the port lanes need to be changed. This PR has the changes to update the port lanes when such speed change happens.

This PR is intended only for VoQ chassis linecards. These platforms today have 400g port with 8 serdes lines, and 100g will operate with 4 serdes lane. When the port speed changes from 400G to 100G the first 4 lanes will be used for 100G port.

Platforms which support 2x50g PAM4 or support 100G PAM4 serdes or other combinations are not handled in the PR.

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
StormLiangMS pushed a commit to StormLiangMS/sonic-buildimage that referenced this pull request Mar 28, 2023
Related work items: sonic-net#276, sonic-net#305, sonic-net#332, sonic-net#338, sonic-net#339, sonic-net#1188, sonic-net#1192, sonic-net#1197, sonic-net#1206, sonic-net#1685, sonic-net#1690, sonic-net#1696, sonic-net#1699, sonic-net#1709, sonic-net#1727, sonic-net#1737, sonic-net#1741, sonic-net#1742, sonic-net#2511, sonic-net#2512, sonic-net#2532, sonic-net#2559, sonic-net#2626, sonic-net#2638, sonic-net#2645, sonic-net#2649, sonic-net#2660, sonic-net#2669, sonic-net#2670, sonic-net#2678, sonic-net#10084, sonic-net#11442, sonic-net#11873, sonic-net#12047, sonic-net#12110, sonic-net#12207, sonic-net#12529, sonic-net#12678, sonic-net#13235, sonic-net#13287, sonic-net#13372, sonic-net#13395, sonic-net#13456, sonic-net#13497, sonic-net#13522, sonic-net#13545, sonic-net#13547, sonic-net#13552, sonic-net#13569, sonic-net#13572, sonic-net#13578, sonic-net#13591, sonic-net#13611, sonic-net#13647, sonic-net#13649, sonic-net#13660, sonic-net#13710, sonic-net#13716, sonic-net#13724, sonic-net#13726, sonic-net#13732, sonic-net#13735, sonic-net#13739, sonic-net#13757, sonic-net#13786, sonic-net#13792, sonic-net#13800, sonic-net#13801, sonic-net#13802, sonic-net#13805, sonic-net#13806, sonic-net#13812, sonic-net#13814, sonic-net#13822, sonic-net#13831, sonic-net#13834, sonic-net#13847, sonic-net#13870, sonic-net#13882, sonic-net#13884, sonic-net#13885, sonic-net#13894, sonic-net#13895, sonic-net#13926, sonic-net#13932, sonic-net#13935, sonic-net#13942, sonic-net#13951, sonic-net#13953, sonic-net#13964
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.