Extract serdes programming logic into dedicated function#4285
Merged
prsunny merged 18 commits intosonic-net:masterfrom Mar 20, 2026
Merged
Extract serdes programming logic into dedicated function#4285prsunny merged 18 commits intosonic-net:masterfrom
prsunny merged 18 commits intosonic-net:masterfrom
Conversation
In the below pull request, Copilot suggested two improvements that were not resolved before the PR was merged. Addressing those two suggestions with this commit. sonic-net#4113 Signed-off-by: Brian Gallagher <[email protected]>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
6 tasks
prgeor
previously approved these changes
Mar 3, 2026
There was a problem hiding this comment.
Pull request overview
This PR follows up on prior review feedback by refactoring serdes (signal integrity) programming in PortsOrch::doPortTask() to reuse a single helper for ASIC and gearbox cases, and by fixing a unit-test SAI stub to correctly clear port→serdes state on serdes removal.
Changes:
- Refactor serdes programming into a local helper lambda to share admin-down + programming logic across ASIC and gearbox line/system sides.
- Update the unit-test SAI
remove_port_serdesstub to erase any port→serdes mappings referencing the removed serdes OID.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
orchagent/portsorch.cpp |
Introduces a reusable helper lambda to program serdes attributes consistently across ASIC/gearbox paths. |
tests/mock_tests/portsorch_ut.cpp |
Fixes the mock SAI behavior so serdes removal is reflected in subsequent SAI_PORT_ATTR_PORT_SERDES_ID queries. |
Signed-off-by: Brian Gallagher <[email protected]>
Signed-off-by: Brian Gallagher <[email protected]>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Brian Gallagher <[email protected]>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
@bgallagher-nexthop , could you provide title to reflect the changes in PR? you can add copilot review etc in description. |
Signed-off-by: Brian Gallagher <[email protected]>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
bgallagher-nexthop
changed the title
Signed-off-by: Brian Gallagher <[email protected]>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Brian Gallagher <[email protected]>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Brian Gallagher <[email protected]>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Brian Gallagher <[email protected]>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
@prsunny do you think this is good to merge? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the below pull request, Copilot suggested two improvements that were not resolved before the PR was merged.
Addressing those two suggestions with this commit.
#4113
What I did
Why I did it
Copilot's comments were valid.
How I verified it
Ran the portsorch unit tests in
tests/mock_tests/portsorch_ut.cpp.