HLD for 'Have a deterministic approach in SONiC for Interface Link bring-up sequence' by shyam77git · Pull Request #916 · sonic-net/SONiC

shyam77git · 2021-12-23T02:27:05Z

No description provided.

ghost · 2021-12-23T02:27:17Z

All CLA requirements met.

prgeor · 2021-12-23T14:18:14Z

@keboliu can you review

rlhui · 2021-12-24T01:31:48Z

Is this needed for 400G transceivers only? Is CMIS transceivers support in sonic a pre-requisite of this feature or no?

shyam77git · 2022-01-17T23:23:28Z

Is this needed for 400G transceivers only?

For 400G optics/transceivers its mandatory as per the CMIS spec.
For 100G and other optics type/variant, its highly recommended and followed as well (on existing LCs)
Please refer to "Proposal" section of this HLD for further details and reasonings.

Is CMIS transceivers support in sonic a pre-requisite of this feature or no?

Yes. CMIS transceiver API is expected to be in place & working prior to having this HLD workflow.
This workflow would come on top of the CMIS API i.e.,
xcvrd would invoke/call the CMIS transceiver API on receiving state update of host_tx_ready field in PORT_TABLE of STATE_DB. This API in turn would perform data_path init (on 400G optics) and tx laser enable (for 100G, 40G optics)

doc/sfp-cmis/Interface-Link-bring-up-sequence.md

lguohan · 2022-01-18T17:02:40Z

doc/sfp-cmis/Interface-Link-bring-up-sequence.md

+
+
+
+


how do we plan to test this feature? can we test this in the virtual switch?

@lguohan
At present the plan is to validate on the HW with 400G optical/copper and 100G optical/copper modules.
We have started the discussion with @prgeor 'Prince George' to determine/understand feasibility to validate these scenarios in simulated environment.

doc/sfp-cmis/Interface-Link-bring-up-sequence.md

Address review comments, added more details to 'Proposal' section, added 'Out of Scope' section

doc/sfp-cmis/Interface-Link-bring-up-sequence.md

Update Interface-Link-bring-up-sequence.md

* Update Interface-Link-bring-up-sequence.md * Update Interface-Link-bring-up-sequence.md * Update Interface-Link-bring-up-sequence.md * Update Interface-Link-bring-up-sequence.md * Update Interface-Link-bring-up-sequence.md

rlhui · 2022-01-22T20:21:35Z

@mikeberesford - please help to approve if this looks good to you. Thanks.

prgeor

@shyam77git can you address few comments

doc/sfp-cmis/Interface-Link-bring-up-sequence.md

Further review comments addressed

prgeor · 2022-01-27T11:57:54Z

@liat-grozovik @keboliu @Junchao-Mellanox please review this new link bring up sequence.

liat-grozovik · 2022-01-27T12:19:52Z

@liat-grozovik @keboliu @Junchao-Mellanox please review this new link bring up sequence.

please expect delay in review comments due to Chinese new year vacation.

doc/sfp-cmis/Interface-Link-bring-up-sequence.md

dgsudharsan · 2022-01-27T22:51:41Z

Few questions:

What are the consequences does this have on fast reboot?
How is config reload scenario handled?
Are there any special considerations for warm reboot?
Will there be an option provided to disable it per platform?
Will there be options provided to enable it per transceiver type/speed? Disabling and enabling Tx might be time consuming for certain transceivers and would increase the downtime.
Can you update which platform APIs would be used in xcvrd for tx enable/disable?
Can you brief error handling when Tx settings fail in xcvrd?

liat-grozovik · 2022-01-28T07:14:30Z

please also add a cleat flow of how you are going to do the TX setting. This cannot be a direct call to layer but rather go via SAI. How do you plan to have it? Is there a SAI API / Attribute who reflect it?

jaganbal-a · 2022-01-28T21:18:02Z

@dgsudharsan
1.What are the consequences does this have on fast reboot?
Jagan> As the fast reboot mechanism re-initialize host ASIC (NPU, external PHY) , With this HLD implementation, the optics DP-Init and DP-activate will happen after host device tx enable which is executed in parallel today. So the additional time consumption will be based on optics module and a fixed time for the "host_tx_ready" notification handling.
2. How is config reload scenario handled?
Jagan> Config reload is like system boot-up w.r.t to this feature.
3. Are there any special considerations for warm reboot?
Jagan> Since data path is not disturbed in warm reboot, there shouldn't any impact.
4. Will there be an option provided to disable it per platform?
5. Will there be options provided to enable it per transceiver type/speed? Disabling and enabling Tx might be time consuming for certain transceivers and would increase the downtime.
Jagan> No. W.r.t CMIS spec it is not optional, so there is no plan to enable/disable per platform or per optics basis.
The durations/limitation comes from the optics module and not from SONiC infrastructure.
6. Can you update which platform APIs would be used in xcvrd for tx enable/disable?
Jagan> It will be based on the transceiver that is present, CMIS/SFF.
@prgeor Can you please comment on API availability/Signature.
7. Can you brief error handling when Tx settings fail in xcvrd?
Jagan> Optics Tx settings failure is not in scope of this HLD.
Please check CMIS/SFP refactor HLD.
https://github.com/Azure/SONiC/blob/master/doc/sfp-cmis/cmis-init.md
https://github.com/Azure/SONiC/blob/master/doc/sfp-refactor/sfp-refactor.md

Addressed further review comments

shyam77git · 2022-01-28T22:28:27Z

@liat-grozovik

please also add a cleat flow of how you are going to do the TX setting. This cannot be a direct call to layer but rather go via SAI. How do you plan to have it? Is there a SAI API / Attribute who reflect it?

SAI_PORT_ATTR_ADMIN_STATE is the existing attribute and its already being used to program host ASIC (NPU/PHY) Tx.
Also, same is mentioned in this HLD's 2nd & 3rd workflows. Please refer to syncd-libSAI and gbsyncd-libSAI verticals in these workflows.

rlhui · 2022-02-01T16:46:53Z

Can we describe how this can be backward compatible or how it can allow gradual migration for different platforms.

Added additional work-flow (Ability to enable/disable this feature)

Added Sequence diagram (Workflow) to enable/disable this feature

Added breakout handling section.

Updated 'Table of Contents' with 'Breakout handling ' item

shyam77git · 2022-02-03T05:04:41Z

Can we describe how this can be backward compatible or how it can allow gradual migration for different platforms.

@rlhui
We would be adding a mechanism to enable this feature on per platform basis.

I've added 'Feature enablement' section to the HLD describing the required workflow

doc/sfp-cmis/Interface-Link-bring-up-sequence.md

liat-grozovik · 2022-02-08T16:36:42Z

@shyam77git once PRs are available please include them in the PR description and please wait for @keboliu to review and approve them as well.

Are there any sonic-mgmt tests that should be modified or the existing ones will for sure cover the new flows?

zhangyanzhao · 2022-05-12T15:40:27Z

Can the contributor of this feature add the code PRs list into this HLD? Thanks.

orchagent changes for sonic-net/SONiC#916

shyam77git · 2022-06-01T19:36:09Z

Can the contributor of this feature add the code PRs list into this HLD? Thanks.

@zhangyanzhao
PRs for this HLD are:
sonic-net/sonic-platform-daemons#254
sonic-net/sonic-swss#2277
Both are under active review

I'd add them to this HLD as well

orchagent changes for sonic-net/SONiC#916

…h before enabling transceiver<CMIS compliant> Tx. (#2277) * Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com> orchagent changes for sonic-net/SONiC#916 * Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com> Addressing PR comment * Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com> Addressing PR comments-cosmetic * Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com> fixed typo * Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com> VS test code and addressing PR comment * Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com> set host_tx_ready to false if gbsyncd SAI API fails. Co-authored-by: Jaganathan Anbalagan <jaganbal@cisco.com>

…h before enabling transceiver<CMIS compliant> Tx. (sonic-net#2277) * Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com> orchagent changes for sonic-net/SONiC#916 * Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com> Addressing PR comment * Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com> Addressing PR comments-cosmetic * Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com> fixed typo * Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com> VS test code and addressing PR comment * Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com> set host_tx_ready to false if gbsyncd SAI API fails. Co-authored-by: Jaganathan Anbalagan <jaganbal@cisco.com>

shyam77git added 2 commits December 22, 2021 12:44

Create Interface-Link-bring-up-sequence.md

dc05062

Update Interface-Link-bring-up-sequence.md

323b161

shyam77git changed the title ~~HLD to have determistic approach for Itnerface Link bring-up sequence~~ HLD to have determistic approach for Interface Link bring-up sequence Dec 23, 2021

prgeor self-requested a review December 23, 2021 14:16

shyam77git changed the title ~~HLD to have determistic approach for Interface Link bring-up sequence~~ HLD for 'Have a deterministic approach in SONiC for Interface Link bring-up sequence' Dec 23, 2021

prgeor mentioned this pull request Jan 17, 2022

[BFN] Updated platform plugins sonic-net/sonic-buildimage#9540

Merged

6 tasks

mikeberesford reviewed Jan 18, 2022

View reviewed changes

doc/sfp-cmis/Interface-Link-bring-up-sequence.md Outdated Show resolved Hide resolved

doc/sfp-cmis/Interface-Link-bring-up-sequence.md Outdated Show resolved Hide resolved

doc/sfp-cmis/Interface-Link-bring-up-sequence.md Show resolved Hide resolved

lguohan reviewed Jan 18, 2022

View reviewed changes

mikeberesford reviewed Jan 18, 2022

View reviewed changes

doc/sfp-cmis/Interface-Link-bring-up-sequence.md Show resolved Hide resolved

andywongarista reviewed Jan 18, 2022

View reviewed changes

doc/sfp-cmis/Interface-Link-bring-up-sequence.md Show resolved Hide resolved

shyam77git added 3 commits January 19, 2022 11:15

Update Interface-Link-bring-up-sequence.md

736b9a8

Update Interface-Link-bring-up-sequence.md

28a67ee

Update Interface-Link-bring-up-sequence.md

b802395

Address review comments, added more details to 'Proposal' section, added 'Out of Scope' section

mikeberesford reviewed Jan 19, 2022

View reviewed changes

jaganbal-a and others added 3 commits January 20, 2022 20:56

Update Interface-Link-bring-up-sequence.md

4c735ca

Merge pull request #1 from jaganbal-a/patch-2

17f1e86

Update Interface-Link-bring-up-sequence.md

Update Interface-Link-bring-up-sequence.md (#2)

d9d1ba6

* Update Interface-Link-bring-up-sequence.md * Update Interface-Link-bring-up-sequence.md * Update Interface-Link-bring-up-sequence.md * Update Interface-Link-bring-up-sequence.md * Update Interface-Link-bring-up-sequence.md

prgeor requested changes Jan 24, 2022

View reviewed changes

Update Interface-Link-bring-up-sequence.md

87ae10f

Further review comments addressed

prgeor self-assigned this Jan 27, 2022

liat-grozovik requested a review from keboliu January 27, 2022 12:20

mikeberesford reviewed Jan 27, 2022

View reviewed changes

doc/sfp-cmis/Interface-Link-bring-up-sequence.md Show resolved Hide resolved

doc/sfp-cmis/Interface-Link-bring-up-sequence.md Show resolved Hide resolved

doc/sfp-cmis/Interface-Link-bring-up-sequence.md Show resolved Hide resolved

Update Interface-Link-bring-up-sequence.md

c272efa

Addressed further review comments

shyam77git and others added 4 commits February 2, 2022 12:44

Update Interface-Link-bring-up-sequence.md

6c85773

Added additional work-flow (Ability to enable/disable this feature)

Update Interface-Link-bring-up-sequence.md

4a39c08

Added Sequence diagram (Workflow) to enable/disable this feature

Update Interface-Link-bring-up-sequence.md (#3)

fdc4e15

Added breakout handling section.

Update Interface-Link-bring-up-sequence.md

1bdd505

Updated 'Table of Contents' with 'Breakout handling ' item

keboliu reviewed Feb 7, 2022

View reviewed changes

doc/sfp-cmis/Interface-Link-bring-up-sequence.md Show resolved Hide resolved

keboliu reviewed Feb 7, 2022

View reviewed changes

doc/sfp-cmis/Interface-Link-bring-up-sequence.md Show resolved Hide resolved

keboliu reviewed Feb 7, 2022

View reviewed changes

doc/sfp-cmis/Interface-Link-bring-up-sequence.md Show resolved Hide resolved

prgeor approved these changes Feb 8, 2022

View reviewed changes

prgeor merged commit 6a344c5 into sonic-net:master Feb 8, 2022

prgeor added the enhancement label Feb 8, 2022

jaganbal-a pushed a commit to jaganbal-a/sonic-swss_link_det that referenced this pull request May 12, 2022

Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com>

b825336

orchagent changes for sonic-net/SONiC#916

jaganbal-a mentioned this pull request May 16, 2022

Orchagent changes for synchronizing npu/phy device Tx in the data path before enabling transceiver<CMIS compliant> Tx. sonic-net/sonic-swss#2277

Merged

jaganbal-a pushed a commit to jaganbal-a/sonic-swss_link_det that referenced this pull request Jul 1, 2022

Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com>

086c58a

orchagent changes for sonic-net/SONiC#916

jaganbal-a pushed a commit to jaganbal-a/sonic-swss_link_det that referenced this pull request Jul 8, 2022

Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com>

31533c4

orchagent changes for sonic-net/SONiC#916

jaganbal-a pushed a commit to jaganbal-a/sonic-swss_link_det that referenced this pull request Jul 13, 2022

Signed-off-by: Jaganathan Anbalagan <jaganbal@cisco.com>

2c8da6b

orchagent changes for sonic-net/SONiC#916

mdanish-kh mentioned this pull request Jul 25, 2022

Incorrect HLD Review Meeting URL in OCP Website #1038

Open

Conversation

shyam77git commented Dec 23, 2021

Uh oh!

ghost commented Dec 23, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prgeor commented Dec 23, 2021

Uh oh!

rlhui commented Dec 24, 2021

Uh oh!

shyam77git commented Jan 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lguohan Jan 18, 2022

Choose a reason for hiding this comment

Uh oh!

jaganbal-a Jan 21, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rlhui commented Jan 22, 2022

Uh oh!

prgeor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

prgeor commented Jan 27, 2022

Uh oh!

liat-grozovik commented Jan 27, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgsudharsan commented Jan 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liat-grozovik commented Jan 28, 2022

Uh oh!

jaganbal-a commented Jan 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shyam77git commented Jan 28, 2022

Uh oh!

rlhui commented Feb 1, 2022

Uh oh!

shyam77git commented Feb 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

liat-grozovik commented Feb 8, 2022

Uh oh!

zhangyanzhao commented May 12, 2022

Uh oh!

shyam77git commented Jun 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

ghost commented Dec 23, 2021 •

edited by ghost

Loading

shyam77git commented Jan 17, 2022 •

edited

Loading

dgsudharsan commented Jan 27, 2022 •

edited

Loading

jaganbal-a commented Jan 28, 2022 •

edited

Loading

shyam77git commented Feb 3, 2022 •

edited

Loading