Update link up sequence with SI settings sync between OA and xcvrd by arpit-nexthop · Pull Request #2301 · sonic-net/SONiC

arpit-nexthop · 2026-04-20T00:43:52Z

Updated Interface link up sequence with SI settings sync between orchagent and xcvrd

PR title	state	context
[sonic-swss] [orchagent]: SI settings sync protocol between xcvrd and orchagent
SI settings sync protocol between xcvrd and orchagent

Signed-off-by: arpit-nexthop <arpit@nexthop.ai>

mssonicbld · 2026-04-20T00:44:00Z

/azp run

azure-pipelines · 2026-04-20T00:44:06Z

No pipelines are associated with this pull request.

mihirpat1

@arpit-nexthop Can you please update the PR description with the table similar to the below so that we can maintain the list of relevant code changes for this HLD change?
#1432 (comment)

benle7 · 2026-04-20T18:20:05Z

Hi @arpit-nexthop

With this new design, do we prevent the admin state (and host TX signal) from coming up
until the host SI settings are configured?
If so, did you verify it in the ‘swss restart’ flow as well?

Thanks

Copilot

Pull request overview

Updates the documented interface link bring-up sequence to describe SI settings synchronization between orchagent and xcvrd using a two-field handshake with a monotonic counter.

Changes:

Replaces the prior NPU SI settings sync description with a two-field protocol (si_sync_status in APPL_DB and si_settings_sync_status in STATE_DB) including sequence counter semantics.
Updates workflows/mermaid diagrams and CMIS state machine to use CMIS_STATE_SI_SETTINGS_WAIT and new timeout behavior.
Refreshes crash/restart and OIR test-plan tables to reflect the new SI settings sync fields/values.

mihirpat1 · 2026-04-20T19:54:24Z

+
+
+3. The OA upon receiving SI settings (via `si_sync_status` update in APPL_DB) will  
+	- Disable port admin status (without updating host_tx_ready, to avoid triggering xcvrd)


@arpit-nexthop Wouldn't disabling port admin status also cause the CMIS SM to start from CMIS_STATE_INSERTED?

No, that happens because we set the host_tx_ready to false. Here we are not setting it to false when SI is present. This avoids the xcvrd going to CMIS_INSERTED state.

@arpit-nexthop I agree that the CMIS reinit happens due to host_tx_ready toggling.
However, what do you mean by Disable port admin status? Wouldn't this cause the CMIS SM to be notified which would eventually result in a CMIS reinit per the below?
https://github.com/sonic-net/sonic-platform-daemons/blob/3bb3540f10f9efbaadfd98f424efad96d561682d/sonic-xcvrd/xcvrd/xcvrd_utilities/port_event_helper.py#L7
https://github.com/sonic-net/sonic-platform-daemons/blob/3bb3540f10f9efbaadfd98f424efad96d561682d/sonic-xcvrd/xcvrd/cmis/cmis_manager_task.py#L1196

This is not admin status in config_db, this is the call from OA to syncd/gbsyncd for disabling or enabling interface.

Disable port admin status becomes Disable host port

@arpit-nexthop Can you please address this comment?

mihirpat1 · 2026-04-20T20:33:12Z

-4. CMIS_STATE_NPU_SI_SETTINGS_WAIT state will wait for NPU_SI_SETTINGS_DONE and upon reaching to NPU_SI_SETTINGS_DONE, CMIS SM will transition to CMIS_STATE_DP_INIT state.  
-There will be a timeout of 5s for every retry
+4. CMIS_STATE_SI_SETTINGS_WAIT state will wait for `SI_SYNC_DONE:<N>` in STATE_DB `si_settings_sync_status`, where N must match the notification number sent. Upon match, CMIS SM transitions to CMIS_STATE_DP_INIT state.  
+There will be a timeout of 10s; upon timeout, CMIS SM proceeds to CMIS_STATE_DP_INIT with a warning (no reinit)


@arpit-nexthop Why are we allowing CMIS initialization to progress without applying SI settings in case of timeout?

I believe this is what we discussed in the meeting, where we wanted to wait for some time, reading the settings being applied. If not we should proceed rather than failing the state machine since SI notification does not happen in the cmis task.

We agreed not to add a notification for settings being programmed.

mihirpat1 · 2026-04-20T20:44:29Z


 5. The CmisManagerTask thread will set “CMIS_REINIT_REQUIRED" to false after CMIS SM reaches to a steady state (CMIS_STATE_UNKNOWN, CMIS_STATE_FAILED, CMIS_STATE_READY and CMIS_STATE_REMOVED) for the corresponding port

 6. XCVRD will subscribe to PORT_TABLE in STATE_DB and trigger self-restart if the PORT_TABLE|Ethernet* is deleted for the namespace.  


@prgeor @arpit-nexthop
Given that swss.sh already restarts xcvrd when media_settings.json is present swss.sh#L429-L440, there's now redundancy between that workaround and this STATE_DB subscription mechanism.
Should the swss.sh workaround be removed as part of this change's implementation PRs?

arpit-nexthop · 2026-04-20T21:30:18Z

Hi @arpit-nexthop

With this new design, do we prevent the admin state (and host TX signal) from coming up until the host SI settings are configured? If so, did you verify it in the ‘swss restart’ flow as well?

Thanks

@benle7 Do you mean warm restart here? swss restart in generally leads to restarting syncd and reprogramming the ASIC as well.

mihirpat1 · 2026-04-20T21:54:54Z

@arpit-nexthop Please help in addressing the comments provided by Copilot as well.

…TATE_SI_SETTINGS_WAIT state' Signed-off-by: arpit-nexthop <arpit@nexthop.ai>

mssonicbld · 2026-04-20T22:03:59Z

/azp run

azure-pipelines · 2026-04-20T22:04:05Z

No pipelines are associated with this pull request.

benle7 · 2026-04-21T05:30:24Z

Hi @arpit-nexthop
With this new design, do we prevent the admin state (and host TX signal) from coming up until the host SI settings are configured? If so, did you verify it in the ‘swss restart’ flow as well?
Thanks

@benle7 Do you mean warm restart here? swss restart in generally leads to restarting syncd and reprogramming the ASIC as well.

I mean to "systemctl restart swss".
Today, PortOrch can set admin True (and open the host TX signal) before configuring the SI settings -
This order can lead to some module issues.
So I would like to understand if your design will change it and enforce configuring SI settings before opening
the host TX signal (set admin True).

Signed-off-by: arpit-nexthop <arpit@nexthop.ai>

mssonicbld · 2026-04-21T22:09:39Z

/azp run

azure-pipelines · 2026-04-21T22:09:44Z

No pipelines are associated with this pull request.

bmridul · 2026-04-22T17:33:09Z

@longhuan-cisco , pls review

Signed-off-by: arpit-nexthop <arpit@nexthop.ai>

mssonicbld · 2026-04-29T16:07:24Z

/azp run

azure-pipelines · 2026-04-29T16:07:31Z

No pipelines are associated with this pull request.

bmridul · 2026-04-29T17:00:13Z

+          Write SI params + si_settings_notification = SI_SETTINGS_NOTIFIED[N] to APPL_DB
+          Reset notify_si_settings = False
+    end note
+    if_state --> CMIS_STATE_READY : if host_tx_ready != True or admin_status != up - Action disable TX


Is this correct? How do SI settings get applied when interface us made admin up?

OA turns shuts the port, applies SI settings and enables port and then sends the notification

Question is whether state transition to CMIS_STATE_READY is correct here? (if host_tx_ready is not True or Admin status is Down)

This section of the mermaid template refers to the admin_down state transition

mihirpat1 · 2026-05-04T18:57:58Z

+
+
+3. The OA upon receiving SI settings (via `si_sync_status` update in APPL_DB) will  
+	- Disable port admin status (without updating host_tx_ready, to avoid triggering xcvrd)


@arpit-nexthop Can you please address this comment?

mihirpat1 · 2026-05-04T21:12:03Z


+| Value                        | Written by                                                                                                                                                                          | Purpose                                                                                   |
+| ---------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- |
+| `SI_SETTINGS_DEFAULT:<N>`    | 1\. SfpStateUpdateTask during transceiver removal (increments N)<br>2. xcvrd during boot-up initialization                                                                          | Signals to OA that transceiver was removed or SI settings are in default state             |


@arpit-nexthop Do you mean the xcvrd main process will perform this action?
xcvrd during boot-up initialization

This is no longer required as initial removal / insertion notification will take care of this

mihirpat1 · 2026-05-04T21:18:17Z

+	- If `si_settings_notification == SI_SETTINGS_DEFAULT:<N>` is received (transceiver removed), OA writes `si_settings_ack = SI_SETTINGS_DEFAULT:<N>` to STATE_DB (no serdes programming)
+
+4. CMIS_STATE_SI_SETTINGS_WAIT state will wait for `SI_SYNC_DONE:<N>` in STATE_DB `si_settings_ack`, where N must match the notification number sent. Upon match, CMIS SM transitions to CMIS_STATE_DP_INIT state.
+There will be a timeout of 10s; upon timeout, CmisManagerTask resets `si_settings_notification` to `SI_SETTINGS_DEFAULT:<N+1>` in APPL_DB, sets `notify_si_settings=True`, and calls `force_cmis_reinit(retries+1)`, transitioning back to CMIS_STATE_INSERTED. This consumes a retry (max 3) and causes re-notification on the next INSERTED entry. After exhausting retries, the SM transitions to CMIS_STATE_FAILED.


@arpit-nexthop Just to clarify, wouldn't calling force_cmis_reinit(retries+1) automatically trigger SI settings notification?

We would not want to keep setting the SI settings everytime we do a force cmis reinit, (called in shut/start flow as well). SI settings application will be re-triggered only if we fail to get the notification or an OIR event happens.

mihirpat1 · 2026-05-04T22:01:55Z


 ```mermaid
-stateDiagram
+stateDiagram-v2


@arpit-nexthop Can you please go through this diagram and remove the legacy content?

@mihirpat1 Added the PRE_INIT check as well. I would recommend viewing the file to see the diagram, the git unified view is not really showing the new diagram, just the old one being deleted.

https://github.com/nexthop-ai/SONiC/blob/7fce6219a6de03d03143e4e1e29ce05b1a9da83a/doc/sfp-cmis/Interface-Link-bring-up-sequence.md#cmis-state-machine-with-introduction-of-cmis_state_si_settings_wait-state

Signed-off-by: arpit-nexthop <arpit@nexthop.ai>

mssonicbld · 2026-05-27T15:25:48Z

/azp run

azure-pipelines · 2026-05-27T15:25:55Z

No pipelines are associated with this pull request.

snider-nokia · 2026-05-27T17:56:13Z

@arpit-nexthop, all,
Strongly suggest considering a slight alteration to this refactoring whereby orchagent does not actually bring the ASIC port admin up until such time as the xcvrd provided SI settings have actually arrived at Redis for orchagent use. Even if SI settings are not in the mix for a given configuration on a given port, orchagent can still wait until the Redis notification flag explicitly indicates that the extant SI configuration provided by xcvrd (or not) is good-to-go.

Introducing this type of gate for orchagent, in advance of orchagent actually bringing the ASIC port admin up will prevent the churn that is currently seen when an ASIC port has already been enabled but then SI settings arrive at Redis thus causing orchagent to modify ASIC SerDes settings and consequently flap the ASIC port.

Waiting at orchagent until such time as the SI settings are confirmed to have arrived from xcvrd (even if there were no changes actually provided by xcvrd with the exception of Redis confirmation flag) would prevent ASIC port from being brought up while only partially configured (in advance of SI settings arrival). Preventing this ASIC port churn/flap will help to clean up the critical enable path such that the ASIC port is only brought up once all provisioning has necessarily been completed and thus commensurately serve to minimize SI related flux within the machinery (as ASIC port(s) are transitioning down-then-up during provisioning time).

Am also copying this same comment to orchagent PR #4497.

snider-nokia · 2026-05-28T19:47:16Z

For clarity, @benle7's suggestion/request earlier is essentially the same ask I made in my previous comment.

benle7 · 2026-05-31T07:28:40Z

@snider-nokia Correct.

@prgeor FYI
As I remember, you mentioned this HLD should resolve the issues we found with the AOI module:

Open host TX before SI configured -> unstable host signal -> CDR LoL.
When swss get restarted while optics datapath is already DpActivated, the module is not re-initialized.

longhuan-cisco · 2026-06-24T19:42:39Z

+5. The CmisManagerTask thread will set “CMIS_REINIT_REQUIRED” to false after CMIS SM reaches to a steady state (CMIS_STATE_UNKNOWN, CMIS_STATE_FAILED, CMIS_STATE_READY and CMIS_STATE_REMOVED) for the corresponding port

-5. The CmisManagerTask thread will set “CMIS_REINIT_REQUIRED" to false after CMIS SM reaches to a steady state (CMIS_STATE_UNKNOWN, CMIS_STATE_FAILED, CMIS_STATE_READY and CMIS_STATE_REMOVED) for the corresponding port
+5a. **`can_skip_cmis_init_after_restart`**: When xcvrd restarts (process restart or warm reboot), CmisManagerTask checks whether full CMIS re-initialization can be skipped for a port. Skipping avoids an unnecessary link flap on already-configured ports. All four conditions must hold:


This section describes can_skip_cmis_init_after_restart, including SI notification/ACK matching, but I do not see this helper or equivalent SI-ack based restart-skip logic in the current xcvrd implementation PR.

Current xcvrd appears to rely on is_cmis_application_update_required(), and SI notification is done earlier in INSERTED when notify_si_settings is set. Please either add the described restart-skip logic to the implementation or update the HLD to match the actual behavior.

longhuan-cisco · 2026-08-20T19:13:48Z

-  Also, this key helps in preventing additional link flap which can happen if CMIS state machine initializes a port before NPU SI settings are applied (since application of NPU SI settings involves disable followed by enable of it).  
-In case of continuous restart of xcvrd, both the keys will still hold the same value as before the restart. This would ensure that the port re-initialization is resumed from the last known state.  
+  - SI settings synchronization uses a two-field protocol to communicate between xcvrd and Orchagent (OA):
+    - `si_settings_notification` in PORT_TABLE (APPL_DB): written by xcvrd to signal SI settings state. OA reads this via its normal ConsumerStateTable flow. Values: `SI_SETTINGS_NOTIFIED:<N>` (xcvrd has notified SI settings for sequence N), `SI_SETTINGS_DEFAULT:<N>` (transceiver removed or no SI settings needed for sequence N).


Medium: document SI_SETTINGS_UNAVAIL as a third terminal protocol value

The implementation PRs no longer use SI_SETTINGS_DEFAULT:<N> for a present port that has no applicable SI settings. xcvrd publishes the counterless value SI_SETTINGS_UNAVAIL; OA treats it as terminal, does not program SerDes, releases the admin-up gate, and writes si_settings_ack = SI_SETTINGS_UNAVAIL.

Notification Meaning OA response

SI_SETTINGS_NOTIFIED:<N> SI settings for this insertion are available Program them and acknowledge SI_SYNC_DONE:<N>

SI_SETTINGS_DEFAULT:<N> Removal/reset of the numbered transaction Do not program; acknowledge SI_SETTINGS_DEFAULT:<N>

SI_SETTINGS_UNAVAIL A present port has no applicable SI settings Do not program; acknowledge SI_SETTINGS_UNAVAIL and release the gate

The HLD currently lists only NOTIFIED and DEFAULT, and says DEFAULT can also mean "no SI settings needed." That no longer matches either implementation. It also hides the terminal-notification invariant: every gated present external port must eventually receive either NOTIFIED:<N> or UNAVAIL.

Could you add UNAVAIL to both field tables and the relevant flow/state diagrams, and reserve DEFAULT:<N> for removal/reset?

rebuild-source: sonic-net#2301 @ nexthop-ai/SONiC 7fce621 [case: upstream:open]

Update link up sequence with SI settings sync between OA and xcvrd

21747e7

Signed-off-by: arpit-nexthop <arpit@nexthop.ai>

This was referenced Apr 20, 2026

[orchagent] SI settings sync protocol between xcvrd and orchagent sonic-net/sonic-swss#4497

Open

[xcvrd] SI settings sync protocol between xcvrd and orchagent sonic-net/sonic-platform-daemons#799

Open

arpit-nexthop marked this pull request as ready for review April 20, 2026 14:55

mihirpat1 reviewed Apr 20, 2026

View reviewed changes

mihirpat1 requested a review from Copilot April 20, 2026 18:58

Copilot started reviewing on behalf of mihirpat1 April 20, 2026 18:58 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

mihirpat1 reviewed Apr 20, 2026

View reviewed changes

Fix mermaid render on 'CMIS State machine with introduction of CMIS_S…

992e3d3

…TATE_SI_SETTINGS_WAIT state' Signed-off-by: arpit-nexthop <arpit@nexthop.ai>

kenneth-arista added this to SONiC Chassis Apr 21, 2026

Updated review comments

7323847

Signed-off-by: arpit-nexthop <arpit@nexthop.ai>

rlhui requested a review from bmridul April 22, 2026 17:22

Update review comments

a45147f

Signed-off-by: arpit-nexthop <arpit@nexthop.ai>

bmridul reviewed Apr 29, 2026

View reviewed changes

prsunny requested review from judyjoseph and prgeor May 1, 2026 18:09

mihirpat1 reviewed May 4, 2026

View reviewed changes

snider-nokia mentioned this pull request May 19, 2026

Bug: Host_tx_ready and Serdes SI settings race sonic-net/sonic-buildimage#24109

Open

Update review comments

7fce621

Signed-off-by: arpit-nexthop <arpit@nexthop.ai>

longhuan-cisco reviewed Jun 24, 2026

View reviewed changes

rlhui assigned arpit-nexthop Aug 10, 2026

longhuan-cisco reviewed Aug 20, 2026

View reviewed changes

nh-grecs Bot pushed a commit to nexthop-ai/SONiC that referenced this pull request Aug 23, 2026

Update link up sequence with SI settings sync between OA and xcvrd

842483a

rebuild-source: sonic-net#2301 @ nexthop-ai/SONiC 7fce621 [case: upstream:open]

nh-grecs Bot pushed a commit to nexthop-ai/SONiC that referenced this pull request Aug 24, 2026

Update link up sequence with SI settings sync between OA and xcvrd

7238d95

rebuild-source: sonic-net#2301 @ nexthop-ai/SONiC 7fce621 [case: upstream:open]

nh-grecs Bot pushed a commit to nexthop-ai/SONiC that referenced this pull request Aug 25, 2026

Update link up sequence with SI settings sync between OA and xcvrd

35ad892

rebuild-source: sonic-net#2301 @ nexthop-ai/SONiC 7fce621 [case: upstream:open]

nh-grecs Bot pushed a commit to nexthop-ai/SONiC that referenced this pull request Aug 25, 2026

Update link up sequence with SI settings sync between OA and xcvrd

21469b8

rebuild-source: sonic-net#2301 @ nexthop-ai/SONiC 7fce621 [case: upstream:open]

nh-grecs Bot pushed a commit to nexthop-ai/SONiC that referenced this pull request Aug 26, 2026

Update link up sequence with SI settings sync between OA and xcvrd

c6a3681

rebuild-source: sonic-net#2301 @ nexthop-ai/SONiC 7fce621 [case: upstream:open]

nh-grecs Bot pushed a commit to nexthop-ai/SONiC that referenced this pull request Aug 27, 2026

Update link up sequence with SI settings sync between OA and xcvrd

52910c4

rebuild-source: sonic-net#2301 @ nexthop-ai/SONiC 7fce621 [case: upstream:open]

nh-grecs Bot pushed a commit to nexthop-ai/SONiC that referenced this pull request Aug 27, 2026

Update link up sequence with SI settings sync between OA and xcvrd

3e255a5

rebuild-source: sonic-net#2301 @ nexthop-ai/SONiC 7fce621 [case: upstream:open]

nh-grecs Bot pushed a commit to nexthop-ai/SONiC that referenced this pull request Aug 28, 2026

Update link up sequence with SI settings sync between OA and xcvrd

69e2798

rebuild-source: sonic-net#2301 @ nexthop-ai/SONiC 7fce621 [case: upstream:open]

nh-grecs Bot pushed a commit to nexthop-ai/SONiC that referenced this pull request Aug 29, 2026

Update link up sequence with SI settings sync between OA and xcvrd

2089a57

rebuild-source: sonic-net#2301 @ nexthop-ai/SONiC 7fce621 [case: upstream:open]

nh-grecs Bot pushed a commit to nexthop-ai/SONiC that referenced this pull request Aug 29, 2026

Update link up sequence with SI settings sync between OA and xcvrd

0951131

rebuild-source: sonic-net#2301 @ nexthop-ai/SONiC 7fce621 [case: upstream:open]



		3. The OA upon receiving SI settings (via `si_sync_status` update in APPL_DB) will
		- Disable port admin status (without updating host_tx_ready, to avoid triggering xcvrd)


		5. The CmisManagerTask thread will set “CMIS_REINIT_REQUIRED" to false after CMIS SM reaches to a steady state (CMIS_STATE_UNKNOWN, CMIS_STATE_FAILED, CMIS_STATE_READY and CMIS_STATE_REMOVED) for the corresponding port

		6. XCVRD will subscribe to PORT_TABLE in STATE_DB and trigger self-restart if the PORT_TABLE\|Ethernet* is deleted for the namespace.

Notification	Meaning	OA response
`SI_SETTINGS_NOTIFIED:<N>`	SI settings for this insertion are available	Program them and acknowledge `SI_SYNC_DONE:<N>`
`SI_SETTINGS_DEFAULT:<N>`	Removal/reset of the numbered transaction	Do not program; acknowledge `SI_SETTINGS_DEFAULT:<N>`
`SI_SETTINGS_UNAVAIL`	A present port has no applicable SI settings	Do not program; acknowledge `SI_SETTINGS_UNAVAIL` and release the gate

Uh oh!

Conversation

arpit-nexthop commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mssonicbld commented Apr 20, 2026

Uh oh!

azure-pipelines Bot commented Apr 20, 2026

Uh oh!

mihirpat1 left a comment

Choose a reason for hiding this comment

Uh oh!

benle7 commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arpit-nexthop commented Apr 20, 2026

Uh oh!

mihirpat1 commented Apr 20, 2026

Uh oh!

mssonicbld commented Apr 20, 2026

Uh oh!

azure-pipelines Bot commented Apr 20, 2026

Uh oh!

benle7 commented Apr 21, 2026

Uh oh!

mssonicbld commented Apr 21, 2026

Uh oh!

azure-pipelines Bot commented Apr 21, 2026

Uh oh!

bmridul commented Apr 22, 2026

Uh oh!

mssonicbld commented Apr 29, 2026

Uh oh!

azure-pipelines Bot commented Apr 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arpit-nexthop commented Apr 20, 2026 •

edited

Loading

benle7 commented Apr 20, 2026 •

edited

Loading

arpit-nexthop May 27, 2026 •

edited

Loading

snider-nokia commented May 27, 2026 •

edited

Loading