-
Notifications
You must be signed in to change notification settings - Fork 543
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
[orchagent]Modifying NPU_SI_SETTINGS_SYNC_STATUS based on NPU SI settings application status #2951
base: master
Are you sure you want to change the base?
Conversation
…tings application status Signed-off-by: Mihir Patel <[email protected]>
@prgeor @shyam77git @jaganbal-a - It will be great if you can help in reviewing this. |
string value; | ||
bool foundNPUSiSettingsSyncStatus = m_portStateTable.hget(p.m_alias, "NPU_SI_SETTINGS_SYNC_STATUS", value); | ||
if (foundNPUSiSettingsSyncStatus) { | ||
m_portStateTable.hset(p.m_alias, "NPU_SI_SETTINGS_SYNC_STATUS", "NPU_SI_SETTINGS_DONE"); |
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.
Shall we set DONE
after port admin_status is restored?
Because according to CMIS spec, only after HOST TX is good, we can move to CMIS_DP_INIT.
|
||
string value; | ||
bool foundNPUSiSettingsSyncStatus = m_portStateTable.hget(p.m_alias, "NPU_SI_SETTINGS_SYNC_STATUS", value); | ||
if (foundNPUSiSettingsSyncStatus) { |
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.
Before bringing down port admin_status and applying serdes settings to SAI, Shall we also check if the currently programed serdes settings in SAI/SDK/NPU (via querying SAI or via maintaining a local cache for current serdes settings obtained from SAI) are the same with the ones we are going to apply?
This can serves as extra protection to eliminate unnecessary serdes setting re-applying (with interface shut/start), especially useful if the serdes attributes had already been set properly in SAI/SDK/NPU.
e.g. During sai_port_api->create_swtich call, some vendors creates ports inside create_switch phase with all the proper serdes settings, for static port creation case.
What I did
Add change to update NPU_SI_SETTINGS_SYNC_STATUS based on NPU SI settings application status.
Why I did it
The change set is part of implementation of the HLD sonic-net/SONiC#1432
How I verified it
Please refer to sonic-net/sonic-platform-daemons#403 for the test results.
Details if related