-
Notifications
You must be signed in to change notification settings - Fork 177
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
[sonic_xcvr/cmis] Tune to set_lpmode status for 400G-ZR #327
base: master
Are you sure you want to change the base?
[sonic_xcvr/cmis] Tune to set_lpmode status for 400G-ZR #327
Conversation
Signed-off-by: jostar-yang <jostar_yang@edge-core.com>
@jostar-yang |
Yes. The PR is for sonic-net/sonic-buildimage#12675 |
@@ -975,7 +979,8 @@ def set_lpmode(self, lpmode): | |||
self.xcvr_eeprom.write(consts.MODULE_LEVEL_CONTROL, lpmode_val) | |||
time.sleep(1) | |||
mstate = self.get_module_state() | |||
return True if mstate == 'ModuleReady' else False | |||
return True if mstate == 'ModuleReady' or mstate=='ModulePwrUp' else False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ModulePwrUp is a transient state. Why do we need to check this state here?.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we try to do retry, it seems need 3-4sec until status=True on ZR. Or can you advise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait for ModuleReady on ZR will need 3-4sec on ZR, so we try to use ModulePwrUp. And it can get status=True.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We must check the steady state of the module. i.e. We will have to wait till the module reaches "ModuleReady" state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I retest and check spend time for set_lpmode=on. It spend time from 0.24 to 0.4sec on 400G-ZR until it get status=True. For set off, it speed time about 55sec until get ModuleReady.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jostar-yang please try using get_datapath_init_duration() timeout to know how much time module can takes to transition from LowPwr to ModuleReady
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call self.get_datapath_deinit_duration() after self.get_lpmode()==True. API,get_datapath_init_duration, return "5000". What is unit for the API return code?
root@sonic:~# sfputil lpmode on Ethernet248
Enabling low-power mode for port Ethernet248 ... spend set_lpmdoe=on ,use 0.248994sec
CC:get dp_deinit_time=5000.000000
OK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jostar-yang Thanks. Please use this timeout value to wait upto max timeout duration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you means use "dp_deinit_time=5000" to be timeout value? What is it's unit? Is unit ms or us?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I Check sonic_xcvr/codes/public/cmis.py and CMIS SPEC. Unit seems is ms. "5000" is from "0111b" that mapping to "1 s <= Tstate < 5 s"
@@ -965,7 +965,11 @@ def set_lpmode(self, lpmode): | |||
# Force module transition to LowPwr under SW control | |||
lpmode_val = lpmode_val | (1 << CmisApi.LowPwrRequestSW) | |||
self.xcvr_eeprom.write(consts.MODULE_LEVEL_CONTROL, lpmode_val) | |||
time.sleep(0.1) | |||
for retries in range(50): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please get the exact time taken by the module to get into LP mode?.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We test set lpmode to on, need 2sec and can get status=True.
For lpmode=off, it need 3-4sec and can get get status=True. Otherwise , they will get false in current code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jostar-yang please use get_datapath_deinit_duration() to know how much time it take for module to transition to LowPwr mode i.e ModuleDeactivatedT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call get_datapath_init_duration until mstate == 'ModuleReady'. It get "60000".
root@sonic:~# sfputil lpmode off Ethernet248
Disabling low-power mode for port Ethernet248 ... 1:spend set_lpmdoe=off ,use 54.217353sec
BB:get dp_init_time=60000.000000
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jostar-yang please update the code to read the timeout value and wait max duration for the advertised timeout
@prince, the suggestion is to wait for 5sec instead of 0.1 sec to collect the status of the lpmode? |
Why I did it: Update submodule commit-id due to sync PRs: - sonic-net/sonic-platform-common#327 - sonic-net/sonic-platform-common#334 How I dit it: Update sonic-platform-common to latest commit-id due to sync PRs. How to verify it: Git-clone to check if download the correct codes.
…onic-net#355) This change onboards refactor of various redis DB opens in y_cable_table-helper.py file and is required to onboard y_cable_async_client PR sonic-net#327 Basically it has changes for all the table to be now exclusively opened once per class instance . It also accommodates refactor for getting async channels open given the params to open it. Description Motivation and Context refactor ycabled How Has This Been Tested? UT and deploy changes on test DUT Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
…or ycable tasks for change events (sonic-net#327) Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com This PR adds an enhancement to ycabled when if and whenever a notification arrives from SoC or server that it is going to be serviced. The notification can come at anytime and hence it must be listened to or awaited by the client at all times. For this we use asyncio lib and avail the async functionality in python. the added code defines a GracefulRestartClient class that can be used to send requests to a gRPC server for graceful restart. The class has three async methods: send_request, receive_response, and notify_graceful_restart_start. send_request method retrieves tor from request_queue, creates a request with the ToR, and sends it to the server using the stub's NotifyGracefulRestartStart method. It then reads the response from the server and prints the details of the response. receive_response method retrieves response from response_queue, prints/puts the the response in DB, sleeps for the period mentioned in the response, and then puts the tor of the response back in the request_queue We follow the existing ycabled pattern and instantiate the task_worker which contains all these tasks in a seperate thread Description Motivation and Context How Has This Been Tested? UT and using this server to validate
Signed-off-by: jostar-yang jostar_yang@edge-core.com
Description
We add retry for set_lpmode=on for 400G-ZR. Let check more time to get status. When get status=true, break checking.
Add "mstate=='ModulePwrUp" for set_lpmode=off. Because we find it need more time. So we can use check mstate is PWrUP to return True. Otherwise, ZR maybe need 3 or 4 sec to get mstate=='ModulePwrUp.
Motivation and Context
Motivation and Context
We test 400-ZR to machine(as9716-32d) and cmd below to set lpmode on or off. But get status is failed. From show cmd, "on/off" value can be set to port eeprom.
root@sonic:~# sfputil lpmode on Ethernet248
Enabling low-power mode for port Ethernet248 ... Failed
root@sonic:~# sfputil show lpmode
Port Low-power Mode
....................
Ethernet248 On
root@sonic:~# sfputil lpmode off Ethernet248
Disabling low-power mode for port Ethernet248 ... Failed
root@sonic:~# sfputil show lpmode
Port Low-power Mode
Ethernet248 Off
How Has This Been Tested?
After change code, test result is ok.
root@sonic:# sfputil lpmode on Ethernet248
Enabling low-power mode for port Ethernet248 ... OK
root@sonic:#
root@sonic:~# sfputil show lpmode
Port Low-power Mode
....................
Ethernet248 On
..................
Ethernet240 Off
root@sonic:~# sfputil lpmode off Ethernet248
Disabling low-power mode for port Ethernet248 ... OK
root@sonic:~# sfputil show lpmode
Port Low-power Mode
Ethernet248 Off
Additional Information (Optional)