[orchagent]: admin-disable port before setPortSerdesAttribute()

[amnsinghal]

What I did
Admin-disable port before applying media-based NPU serdes attributes from media_settings.json.

Why I did it
This fix is needed along with xcvrd changes #377.
Maintain deterministic behavior of interface bring-up, by toggling host_tx_ready flag, which will trigger CMIS reinit for the module once NPU serdes params have been applied.

How I verified it
Validated media_settings being notified and applied on Cisco 8111 with subject changes combined with diffs from #360, #377 and #15453.
Will update final results once #377 is frozen.

Details if related
Proposal for xcvrd changes: #356

[amnsinghal]

@prgeor @mihirpat1 for your perusal.

[eddyk-nvidia]

Looks OK to me

[prgeor]

@amnsinghal waiting for your UT test: Try this change in master without any other in-flight changes from Mihir's

[prgeor]

@amnsinghal @mihirpat1 we are seeing Xcvrd is missing the event host_tx_ready=False. Xcvrd is busy doing something else while OA sets the host_tx_ready = False and when finally OA sets the host_tx_ready=True, Xcvrd do get the notification but the filtering logic filters this event. Need to see a better way to notify change of media setting to Xcvrd.

[amnsinghal]

@amnsinghal waiting for your UT test: Try this change in master without any other in-flight changes from Mihir's

@prgeor Here is a summary of UT results without Mihir's diff:

1. Reboot, config reload: All dockers/processes start clean, xcvrd starts CMIS FSM, notifies NPU media_settings to OA/syncd/SAI-SDK, link comes up as expected.
2. xcvrd crash, xcvrd graceful restart: xcvrd renotifies media_settings, causing link-flap, this should be taken care by #377.
3. syncd process crash, syncd docker restart: xcvrd did not re-notify media_settings, nor did module DP reinit, this should be taken care by #377. Link went down (due to NPU reset) and came up with default NPU params.
4. syncd process graceful restart: xcvrd did not re-notify media_settings, nor did module DP reinit, this should be taken care by #377. Link went down (due to NPU reset) and came up with media-based NPU params from ASIC-DB.
5. OA process crash: xcvrd did not re-notify media_settings, nor did module DP reinit, this should be taken care by #377. Link went down (due to NPU reset) and came up with default NPU params.

[amnsinghal]

@prgeor As discussed today, can you pls approve and merge this fix, if no further comments. Thanks.

[prgeor]

@amnsinghal approved with minor comments

[amnsinghal]

/AzurePipelines run

[azure-pipelines]

Commenter does not have sufficient privileges for PR 2831 in repo sonic-net/sonic-swss

[prgeor]

@prsunny please help approve and merge.

[prgeor]

@amnsinghal re-submitted the test pipeline

[amnsinghal]

@amnsinghal re-submitted the test pipeline

Thanks @prgeor

[amnsinghal]

@prgeor @prsunny Can you pls rerun Azure Pipeline, prev rerun failed in early build stage, apparently due to env/pipeline issue: https://dev.azure.com/mssonic/build/_build/results?buildId=323644&view=results. This changeset had passed all sanities yesterday. Thanks.

[kevinskwang]

/azp run

[azure-pipelines]

Commenter does not have sufficient privileges for PR 2831 in repo sonic-net/sonic-swss

[lguohan]

test are failing, also new code needs to add new unit test code.

[lguohan]

test are failing, also new code needs to add new unit test code.

[lguohan]

new code path must be protected by new unit test code, adding new logics without new code unit test is not a good practice. @prsunny

[amnsinghal]

test are failing, also new code needs to add new unit test code.

@lguohan All tests had passed 2 days back before Update syslog in orchagent/portsorch.cpp , which was just a syslog rephrase, no functional change.

[amnsinghal]

new code path must be protected by new unit test code, adding new logics without new code unit test is not a good practice. @prsunny

@lguohan This is existing code flow with only addition of admin-disable before applying NPU serdes attributes.

[prsunny]

@amnsinghal , can you please add unit test for coverage of this change?

[amnsinghal]

@amnsinghal , can you please add unit test for coverage of this change?

@prsunny @lguohan Added UT in mock_tests/portsorch_ut.cpp, kindly review and approve.

[amnsinghal]

Thanks @prgeor @prsunny @lguohan!
Can you pls request cherry-pick to 202205 and/or 202305.

[amnsinghal]

@prsunny @prgeor Can you pls request cherry-pick to 202305, thanks.

[mihirpat1]

@StormLiangMS - Can you please help with cherry-pick to 202305?
ADO - 24988722

[StormLiangMS]

@amnsinghal there is conflict, could you help to file separate PR for 202305?