CMIS Module Management Enhancement HLD #1453

noaOrMlnx · 2023-08-29T08:33:57Z

Add design document to enhance host_tx_ready set process to State DB, to have full synchronization between asic and module configuration.

PR title	state	context
Add host_tx_ready enhancements
Add STATE_TRANSCEIVER_INFO_TABLE_NAME to shcema.h
Add SAI Notification support for host_tx_ready

Quality: GA Level

madhupalu · 2023-09-05T15:23:56Z

doc/cmis-module-enhancement/cmis-module-enhancement.md

+
+#### 6.1.2. Host TX Ready signal
+As mentioned earlier, currently SONIC assumes that as soon as the Admin Status is set to UP and the corresponding SAI call returns with OK/SUCCESS status, the "host_tx_ready" flag can be set in the STATE DB to trigger the CMIS State Machine for specific port. 
+But it is not always a truth. ASIC port initialization process takes some time and this time can increase with move to new transceiver technologies (e.g. BiDi). 


SONiC taken (~ 10-20 sec )to bring up the ports and complete the FSM, do you see any performance impact with this design tx_ready_signal ?

no. This signal provides better sync between ASIC port init and start of CMIS state machine. The CMIS FSM requires the high-speed signal from ASIC prior to entering the DataPathInit state. So, waiting for this high-speed signal is inevitable to succeed in CMIS state machine.

Wondering if 10-20 sec is related to init of all ports ?

madhupalu · 2023-09-05T15:26:37Z

doc/cmis-module-enhancement/cmis-module-enhancement.md

+#### 6.3.3. Handling of HW-based “host_tx_ready” event
+Per description above, on the platform supporting new enhancements, the SDK/FW shall send an asynchronous notification of “host_tx_ready” to SWSS when the ASIC starts/stops sending high-speed signal to a module.
+
+The vendor SDK shall internally support this indication from ASIC (via the trap mechanism) and shall call the notification callback registered by Ports OA on initialization .


How does SONiC be notified the whethernew tx_trap_signal trap supported or not? I believe it must be vendor CMIS implemetation must support this trap otherwise correct us!

SONIC (Ports OA) will perform the query capability for support of HW-based host_tx_ready notification. Then it will initializes the consumer for this notification only if Capability Query returned the indication of support on the platform. The vendor SAI will indicate its support of this notification ONLY if the trap from ASIC about start/stop of host_tx_ready signal is supported by ASIC/FW on the platform

zhangyanzhao · 2023-09-05T15:35:23Z

SONiC Community review recording https://zoom.us/rec/share/IHXaeq-KLSq8Ig0HGoeIRvsbireqGh0xpVhwFaTNRdKYZKyZ7mUSgIijp-XxM3VA.kKt4Kh5LkkQk_5Ii

jaganbal-a · 2023-10-19T23:46:11Z

doc/cmis-module-enhancement/cmis-module-enhancement.md

+With move to SW-based management of CMIS modules - when the ASIC side is configured by vendor SDK, and module side by SONIC, some problems have been identified with the current approach.
+
+#### 6.1.2. Host TX Ready signal
+As mentioned earlier, currently SONIC assumes that as soon as the Admin Status is set to UP and the corresponding SAI call returns with OK/SUCCESS status, the "host_tx_ready" flag can be set in the STATE DB to trigger the CMIS State Machine for specific port. 


There is no assumption on host_tx_ready flag being set, the ASIC vendor SW have to make sure the valid transmission/tuning etc while processing/handling the attribute 'SAI_PORT_ATTR_ADMIN_STATE'.

What is the advantage of returning from SAI_PORT_ATTR_ADMIN_STATE for Up case without enabling the proper transmission ?

Can you please share the benchmark on completion of SAI_PORT_ATTR_ADMIN_STATE and reaching the transmission of high speed signal from your setup? IMO this will be couple of hundred ms.

@jaganbal-a

What is the advantage of returning from SAI_PORT_ATTR_ADMIN_STATE for Up case without enabling the proper transmission ?

Before transmitting to CMIS module mgmt, admin state UP was enough for FW to start transmitting high speed signal.
Then, we faces the unterminated transmition issue, and added a new criteria, to know that the module is plugged.

So from now on, the high speed signal will be sent only when admin=up and allow flag = true. means, the module is plugged in.
We need to do it to make sure that the high speed signal is not sent when module is not even plugged in.

Can you please share the benchmark on completion of SAI_PORT_ATTR_ADMIN_STATE and reaching the transmission of high speed signal from your setup? IMO this will be couple of hundred ms.

A benchmark is not relevan there, since it is per SerDes design, which is different between any vendor, and depends on module type, SerDes which are advanced or not etc.

jaganbal-a · 2023-10-19T23:52:56Z

doc/cmis-module-enhancement/cmis-module-enhancement.md

+
+#### 6.1.3. Unterminated Transmission
+With move to the SW-based module management, the module presence is handled by SONIC, and FW might be unaware of the module presence status. 
+In this case, when the Admin status of a port is set to UP, FW can start transmitting the high-speed signal even without a plugged-in module. Such unterminated transmission can cause cross-talks to adjacent ports, high EMI, high BER and eventually shorten the transceiver lifetime so it is recommended that ASIC will not start sending the high-speed signal before a module is plugged.


I see a usecase where the some amount power is saved when the transmission is not enabled in ASIC.

Can you please elaborate on unterminated transmission ? and how it cause cross-talks to adjacent ports?

@jaganbal-a

I see a usecase where the some amount power is saved when the transmission is not enabled in ASIC.

The reason for this enhancement is not power. it is a side affect.
The main reason here is to make sure the SerDes will not be damaged when high-speed signal will be sent without having a module plugged in to cage.

Can you please elaborate on unterminated transmission ? and how it cause cross-talks to adjacent ports?

Sending high-speed signal without having a plugged in module, can cause interruptions. for adjacent cables, it will cause more BER and again, can damage the SerDes.

prgeor · 2023-11-14T08:33:16Z

@noaOrMlnx @liat-grozovik some open comments in this HLD to be taken care

prgeor · 2023-11-14T08:41:41Z

doc/cmis-module-enhancement/cmis-module-enhancement.md

+{
+    ...
+    ...
+    if (saiTxReadyNotifySupported && saiHwTxSignalEnabled)


@noaOrMlnx what about platform that does not support this hw notification mechanism? Can we have parity in behavior for both?

@prgeor
If a platform does not support notification mechanism, but does support the enable signal, then following situation can happen:

host tx ready will not set to true when admin status is set to up.

"allow" signal will be sent to lower layers saying the module is plugged in.

host_tx_ready will never be set to TRUE since a notification that high-speed signal has been transmitted will never arrive.

That's the reason we need to make sure those 2 queries are supported.

@noaOrMlnx I was referring to host_tx_ready which gets deleted from STATE_DB table after a warm-reboot. With your proposal, for platform that support notification mechanis, the host_tx_ready will be restored after warm-reboot. But for other platforms, host_tx_ready will remain deleted from STATE_DB table causing Xcvrd to bring the link down. See my previous comment above: We should not delete PORT_TABLE from STATE_DB table during warm-reboot that way we can keep the behavior same for all platforms.

Hi @prgeor
Can you explain why do you expect link-flap when host_tx_ready is deleted with the deletion of PORT_TABLE? It is the exact same behavior as before.

IMO reserving PORT_TABLE will not be the right solution.
In PORT_TABLE there are some other keys as 'oper_status'. We do not want to have this key reserved between boots, but we need to know the real status - if the port link is up or not.
That's why we get the real oper status after boot is done via query to SAI.

@noaOrMlnx oper_status i agree. I was talking about host_tx_ready. Have you tested if PORT_TABLE's host_tx_ready is not deleted ? The impact of host_tx_ready deleted during warm-reboot is Xcvrd re-initializing the CMIS optics causing link flap

prgeor · 2023-11-14T08:43:57Z

doc/cmis-module-enhancement/cmis-module-enhancement.md

+
+
+##### 6.3.3.1. Warm-boot handling
+After warm-boot, SONiC will not receive a new “host_tx_ready” notification from SAI (because nothing has changed in ASIC/module). 


@noaOrMlnx I am not sure what is the usecase to have host_tx_ready updated during warm reboot? Xcvrd is not going to act upon this host_tx_ready notification anyways during warm reboot

@prgeor The only reason here is to have STATE DB updated with lower layers status.
This was also discussed during HLD review, it is not mandatory but nice to have it all updated with the same status.

@noaOrMlnx can we preserve the STATE_DB's TRANSCEIVER_INFO table and PORT_TABLE (host_tx_ready) across warm-reboot so that it does not cause link flap.

@prgeor Hi Prince, As far as I checked, TRANSCEIVER_INFO table does not deleted when doing warm-reboot.
only PORT_TABLE.
For PORT_TABLE, please check my above comment, which explains why I think it will not be the solution.

@noaOrMlnx it seems except few TABLES mentioned here other tables get deleted before warm-reboot

@noaOrMlnx is your confirmation verified on a real switch? i.e if on warm-reboot host_tx_ready does not get deleted from STATE_DB and hence does not need to be preserved and restored before and after warm-reboot?

my bad, both tables are deleted during warm reboot

prgeor · 2023-11-16T17:42:23Z

doc/cmis-module-enhancement/cmis-module-enhancement.md

+#### 6.2.2. host_tx_ready
+When the ASIC starts transmitting the high-speed signal toward a plugged module the vendor SAI should notify the SONIC (SWSS) of that via a new notification - SAI_SWITCH_ATTR_PORT_HOST_TX_READY_NOTIFY.
+
+The SWSS shall use this notification to set "host_tx_ready" flag in STATE DB which will trigger the CMIS initialization of the module.


Suggested change

The SWSS shall use this notification to set "host_tx_ready" flag in STATE DB which will trigger the CMIS initialization of the module.

The OA shall use this notification to set "host_tx_ready" flag in STATE DB which will trigger the CMIS initialization of the module.

prgeor · 2023-11-16T18:19:11Z

doc/cmis-module-enhancement/cmis-module-enhancement.md

+When a module is plugged in/out the PMON adds/deletes a per-port entry in the TRANSCIEVER_TABLE in STATE DB.
+This event shall be used by Ports OA as a trigger for enabling or disabling the host_tx_signal (using setting true/false to SAI_PORT_ATTR_HOST_TX_SIGNAL_ENABLE on the Port Object).
+
+To detect this insertion/removal event the Ports Orch Agent shall register for changes in the TRANSCIEVER_TABLE in STATE DB and will do the following:


@noaOrMlnx AFAIK, STATE_DB TRANSCEIVER_TABLE is not preserved across warm-reboot. If so, my concern is during warm-reboot if the TRANSCEIVER_TABLE gets deleted, then SAI_PORT_ATTR_HOST_TX_SIGNAL_ENABLE could become FALSE after warm-reboot. Could you please check this.?

@prgeor
Hi,
When we execute warm-reboot, the whole table is deleted. Since we are performing a reboot, it will happen after OA will stop, so no notification is received.
When OA is up again after reboot, the table is clean, so notification will be received only upon new info inserted to the table.
So for me it looks like we should not worry about it.

New PORT_HOST_TX_READY notification logic was added to sonic-sairedis and syncd in order to support host_tx_ready synchronization enhancements. HLD: sonic-net/SONiC#1453

in order to have the host_tx_ready enhancements, the table name is added to schema.h. HLD: sonic-net/SONiC#1453

liat-grozovik · 2023-12-03T13:21:34Z

@prgeor kindly reminder to the HLD PR review. do we still have open issue?

prgeor · 2023-12-06T01:26:35Z

@prgeor kindly reminder to the HLD PR review. do we still have open issue?

@liat-grozovik i have some concern, on warm-reboot, rest all looks good. have put in the HLD comment and PR comment as well. next will sync up with @noaOrMlnx offline

skg-net · 2024-02-05T20:03:19Z

@noaOrMlnx Can you please update the Quality Metric (Alpha/Beta/GA) for the feature either in this PR comments or in HLD itself based on https://github.com/sonic-net/SONiC/blob/master/doc/SONiC%20feature%20quality%20definition.md
Thanks

Add CMIS Module Management Enhacement design document

a8bd862

liat-grozovik changed the title ~~Add CMIS Module Management Enhancement design document~~ CMIS Module Management Enhancement HLD Aug 29, 2023

zhangyanzhao assigned noaOrMlnx Sep 5, 2023

madhupalu reviewed Sep 5, 2023

View reviewed changes

jaganbal-a reviewed Oct 19, 2023

View reviewed changes

This was referenced Oct 23, 2023

Add host_tx_ready enhancements sonic-net/sonic-swss#2930

Merged

Add STATE_TRANSCEIVER_INFO_TABLE_NAME to shcema.h sonic-net/sonic-swss-common#824

Merged

Add SAI Notification support for host_tx_ready sonic-net/sonic-sairedis#1307

Merged

prgeor reviewed Nov 14, 2023

View reviewed changes

prgeor reviewed Nov 16, 2023

View reviewed changes

kcudnik pushed a commit to sonic-net/sonic-swss-common that referenced this pull request Nov 27, 2023

Add STATE_TRANSCEIVER_INFO_TABLE_NAME to shcema.h (#824)

8dc6218

in order to have the host_tx_ready enhancements, the table name is added to schema.h. HLD: sonic-net/SONiC#1453

prgeor approved these changes Dec 8, 2023

View reviewed changes

eddyk-nvidia approved these changes Dec 10, 2023

View reviewed changes

liat-grozovik merged commit ac6beef into sonic-net:master Dec 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CMIS Module Management Enhancement HLD #1453

CMIS Module Management Enhancement HLD #1453

noaOrMlnx commented Aug 29, 2023 •

edited

Loading

madhupalu Sep 5, 2023

eddyk-nvidia Sep 6, 2023

madhupalu Sep 5, 2023

eddyk-nvidia Sep 6, 2023

zhangyanzhao commented Sep 5, 2023

jaganbal-a Oct 19, 2023

noaOrMlnx Nov 14, 2023 •

edited

Loading

jaganbal-a Oct 19, 2023

noaOrMlnx Nov 14, 2023

prgeor commented Nov 14, 2023

prgeor Nov 14, 2023

noaOrMlnx Nov 14, 2023

prgeor Nov 16, 2023

noaOrMlnx Nov 19, 2023

prgeor Dec 5, 2023

prgeor Nov 14, 2023

noaOrMlnx Nov 14, 2023

prgeor Nov 16, 2023

noaOrMlnx Nov 19, 2023

prgeor Dec 5, 2023

prgeor Dec 6, 2023

noaOrMlnx Dec 6, 2023

prgeor Nov 16, 2023

prgeor Nov 16, 2023

noaOrMlnx Nov 19, 2023

liat-grozovik commented Dec 3, 2023

prgeor commented Dec 6, 2023

skg-net commented Feb 5, 2024



		##### 6.3.3.1. Warm-boot handling
		After warm-boot, SONiC will not receive a new “host_tx_ready” notification from SAI (because nothing has changed in ASIC/module).

	The SWSS shall use this notification to set "host_tx_ready" flag in STATE DB which will trigger the CMIS initialization of the module.
	The OA shall use this notification to set "host_tx_ready" flag in STATE DB which will trigger the CMIS initialization of the module.

CMIS Module Management Enhancement HLD #1453

CMIS Module Management Enhancement HLD #1453

Conversation

noaOrMlnx commented Aug 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhangyanzhao commented Sep 5, 2023

Choose a reason for hiding this comment

noaOrMlnx Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prgeor commented Nov 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liat-grozovik commented Dec 3, 2023

prgeor commented Dec 6, 2023

skg-net commented Feb 5, 2024

noaOrMlnx commented Aug 29, 2023 •

edited

Loading

noaOrMlnx Nov 14, 2023 •

edited

Loading