[202511] [xcvrd] Support mixed application codes in CMIS decommission logic (#794)#860
Merged
vmittal-msft merged 1 commit intoJul 22, 2026
Conversation
…onic-net#794) * [xcvrd] Support mixed application codes in CMIS decommission logic Signed-off-by: Mihir Patel <patelmi@microsoft.com> * Added support for gearbox host lane count Signed-off-by: Mihir Patel <patelmi@microsoft.com> * Created dict for gearbox_lanes_dict Signed-off-by: Mihir Patel <patelmi@microsoft.com> * Added error logs Signed-off-by: Mihir Patel <patelmi@microsoft.com> * Replaced get_application with get_active_apsel_hostlane Signed-off-by: Mihir Patel <patelmi@microsoft.com> * Optimized usage of gearbox_lanes_dict and created functions split getting desired application map Signed-off-by: Mihir Patel <patelmi@microsoft.com> --------- Signed-off-by: Mihir Patel <patelmi@microsoft.com>
Collaborator
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR cherry-picks the fix from #794 into the 202511 branch to prevent unnecessary CMIS decommission cycles on OSFP-8X modules configured with mixed breakout by computing a per-lane “desired app code” map across sibling logical ports and comparing against each lane’s active AppSel.
Changes:
- Add CONFIG_DB-backed sibling port discovery and build a per-lane desired application-code map (
get_sibling_port_configs(),get_desired_app_map()). - Update
is_decommission_required()to acceptlport, compare lane-by-lane against the desired map, and ignore unused lanes (AppSel=0). - Cache gearbox lane-count data once per
task_workeriteration and update call sites/tests accordingly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| sonic-xcvrd/xcvrd/cmis/cmis_manager_task.py | Adds sibling-port CONFIG_DB lookup and per-lane desired app mapping; updates decommission decision logic; introduces per-iteration gearbox lane-count caching. |
| sonic-xcvrd/tests/test_xcvrd.py | Updates and expands unit tests to cover mixed-mode per-lane decommission logic and the new gearbox-lane cache usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+493
to
+495
| desired_map = self.get_desired_app_map(api, lport) | ||
| active_apsel = api.get_active_apsel_hostlane() | ||
| current_map = [] |
vmittal-msft
approved these changes
Jul 22, 2026
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.
Cherry-pick of #794 to 202511.
Description
The original
is_decommission_required()compared all 8 module lanes against asingle application code (
app_new) — the app code of whichever logical porttriggered the check. On an OSFP-8X module configured with mixed breakout
(e.g., 1x400G on lanes 0-3 + 4x100G on lanes 4-7), lanes belonging to sibling
ports have a different valid app code. The old logic saw
current_app != app_newon those sibling lanes and returnedTrue,triggering an unnecessary decommission cycle.
Motivation and Context
Fix — Two changes in
cmis_manager_task.py:get_desired_app_map(api, lport)— reads CONFIG_DB PORT tableto find all sibling logical ports on the same physical port (matched by
index). For each sibling, resolves the desired CMIS application code andlane mask, building a per-lane desired app map (e.g.
[3,3,3,3,1,1,1,1]).is_decommission_required(api, lport)— acceptslportinsteadof
app_new, callsget_desired_app_map(), and compares each lane'scurrent app against its per-lane desired app. Lanes currently unused
(
AppSel=0) are ignored.How Has This Been Tested?
Cherry-pick conflict resolution notes: 202511 differs from master in two
pre-existing ways unrelated to #794 (kept unchanged here):
self.is_fast_reboot_enabled = Falseinstance attribute(master refactored to lazy accessor
_is_fast_reboot_enabled).process_single_lport; master split it intohandle_cmis_inserted_state/handle_cmis_dp_pre_init_check_state/handle_cmis_dp_deinit_state/process_cmis_state_machine.The functional PR #794 changes (
get_host_lane_count,get_sibling_port_configs,get_desired_app_map,is_decommission_required)are byte-for-byte identical to master. Call-site updates in the monolithic
process_single_lport(dropgearbox_lanes_dictparam; callis_decommission_required(api, lport)) applied.Original PR testing (from #794): non-breakout baseline, sfputil reset + SNS,
xcvrd/pmon/swss restart, config reload, device reboot, loopback,
admin shut/no-shut, and DPB (
1x800G ↔ 2x400G ↔ 4x200G ↔ 8x100G, 50iterations each direction).
Additional Information (Optional)
MSFT ADO - 37489480