sonic-sairedis
sonic-sairedis copied to clipboard
Manage LANES mapping on multi-ASIC platform
This issue is related to the dynamic port breakout mode support on multi-ASIC platforms. When the port breakout mode changes, SWSS removes ports from LANES mapping and adds new ports to LANES mapping. This logic works properly on the single-chip platform (NPU switch type) but it does not work properly on the multi-ASIC platform (VOQ switch type) because LANES mapping is not created on the multi-ASIC platform.
To fix this issue, my proposal is to create LANES mapping on the multi-ASIC platform. Though the fix is straightforward, the comment in the existing codebase is a bit concerning to me. I am not sure if creating LANES mapping on the multi-ASIC platform could affect backward compatibility. @arlakshm
Hi @kcudnik , Could you take another look at the latest commit and see if it looks good to you? The code analysis fails but I don't think it's related to my change.
/Azp run Azure.sonic-sairedis
Azure Pipelines successfully started running 1 pipeline(s).
/AzurePipelines run Azure.sonic-sairedis
Commenter does not have sufficient privileges for PR 1127 in repo sonic-net/sonic-sairedis
/azpw run Azure.sonic-sairedis
/AzurePipelines run Azure.sonic-sairedis
Azure Pipelines successfully started running 1 pipeline(s).
/azpw run LGTM analysis: C/C++
/AzurePipelines run LGTM analysis: C/C++
No pipelines are associated with this pull request.
@shyam77git, @anamehra - would you like to review this as well? thanks
Hi @abohanyang How is this change tested? Could you please add any details on the test and logs?
@abohanyang Don't see mention of External-PHY/ gbsyncd in this. Did this change consider boards/LCs with External-PHY (gbsyncd vertical)? be it single-ASIC or multi-ASIC? If not, how to assess impact of this change to boards/LCs having External-PHY?
@abohanyang Don't see mention of External-PHY/ gbsyncd in this. Did this change consider boards/LCs with External-PHY (gbsyncd vertical)? be it single-ASIC or multi-ASIC? If not, how to assess impact of this change to boards/LCs having External-PHY?
@shyam77git Actually, we just found a new issue last week when we test this change against a system with BCM54182 PHY (using broncos SAI). I submit the new change and you can see that helperCheckLaneMap is called only for NPU and VOQ. This change won't have any impact on SAI_SWITCH_TYPE_PHY.
@anamehra @shyam77git , can you please review this PR and signoff if no more comments?
@anamehra @shyam77git , can you please review this PR again and signoff if no more comments?
LGTM! @abdosi , anything specific you want to check for chassis-packet?
can we update PR title to
Manage LANES mapping on VOQ system. VOQ can be single/multi-asic.
@rlhui The change looks ok to me.
can we update PR title to
Manage LANES mapping on VOQ system. VOQ can be single/multi-asic.
@abohanyang , if you could please just update the title to reflect the exact scope of this, then we can merge this, thanks.
can we update PR title to
Manage LANES mapping on VOQ system. VOQ can be single/multi-asic.@abohanyang , if you could please just update the title to reflect the exact scope of this, then we can merge this, thanks.
Done