Skip to content

[sonic_xcvr] Restore Abort advertisement in FW mgmt features - #722

Merged
prgeor merged 1 commit into
sonic-net:masterfrom
pnakka28:cdb_abort_support
Jul 17, 2026
Merged

[sonic_xcvr] Restore Abort advertisement in FW mgmt features#722
prgeor merged 1 commit into
sonic-net:masterfrom
pnakka28:cdb_abort_support

Conversation

@pnakka28

@pnakka28 pnakka28 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Description

Restore the "Abort CMD102h supported" advertisement in the CDB firmware management features return text.

When get_module_fw_mgmt_feature() was refactored to route all firmware-management features through CdbFwHandler.get_fw_mgmt_features(). This dropped the Abort CMD102h supported line that the previous implementation emitted, and the abort advertisement was not captured in return text.

Changes:

  • cdb/cdb_fw.py: capture is_abort_supported from the reply and include it in get_fw_mgmt_features().
  • Restore Abort CMD102h supported <bool> in the info text.
  • Updated test_cdb_fw.py and test_cmis.py based on the changes.

Motivation and Context

How Has This Been Tested?

  • Tested on 202605 image version.
  • Unit tests: test_cdb_fw.py and test_cmis.py passed -> 484 passed
  • Tested on hardware with 202605 image get_module_fw_mgmt_feature() API now reports Abort CMD102h supported <bool>.

Additional Information (Optional)

Signed-off-by: Pavan Kalyan Nakka <pnakka@microsoft.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@pnakka28 pnakka28 changed the title Restore Abort advertisement in FW mgmt features [sonic_xcvr] Restore Abort advertisement in FW mgmt features Jul 17, 2026
@prgeor
prgeor requested review from Copilot and prgeor July 17, 2026 21:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR restores the “Abort CMD102h supported” advertisement in the CMIS CDB firmware-management features info text returned by the public transceiver API, after a prior refactor moved feature collection into CdbFwHandler.get_fw_mgmt_features().

Changes:

  • Decode and propagate the “abort supported” bit from the firmware management advertisement field through CdbFwHandler.get_fw_mgmt_features().
  • Re-add Abort CMD102h supported <bool> to the get_module_fw_mgmt_feature() returned info text.
  • Update unit tests to reflect the updated get_fw_mgmt_features() tuple shape.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/sonic_xcvr/test_cmis.py Updates mocked firmware-feature tuples to include abort support bit.
tests/sonic_xcvr/test_cdb_fw.py Updates expected get_fw_mgmt_features() return tuple to include abort support.
sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page9f_cdb.py Ensures firmware-mgmt advertisement field decodes into per-bit values (bitdecode=True).
sonic_platform_base/sonic_xcvr/cdb/cdb_fw.py Captures abort support bit and returns it via get_fw_mgmt_features().
sonic_platform_base/sonic_xcvr/api/public/cdb_fw.py Restores abort support advertisement line in returned info text.

Comment thread sonic_platform_base/sonic_xcvr/api/public/cdb_fw.py
mgmt_features_adv = reply.get(cdb_consts.CDB_FIRMWARE_MGMT_ADV, {})
self.start_payload_size = reply[cdb_consts.CDB_START_CMD_PAYLOAD_SIZE]
self.is_lpl_only = reply[cdb_consts.CDB_WRITE_MECHANISM] == "LPL"
self.rw_length_ext = reply[cdb_consts.CDB_READ_WRITE_LENGTH_EXT] + 8
@prgeor
prgeor merged commit 95e4796 into sonic-net:master Jul 17, 2026
6 checks passed
@mssonicbld

Copy link
Copy Markdown
Collaborator

This PR has backport request label(s) for branch(es): 202605, but is missing required test information. Please make sure you tick the tested branch(es) in the Tested branch section and provide test evidence (e.g., 202605: <test result>) in the Test result section as well in your PR description.

---Powered by SONiC BuildBot

@rookie-who

Copy link
Copy Markdown

@pnakka28 — This PR is requested for the 202605 branch but is missing test evidence. To move forward with cherry-pick approval, please:

  1. Test on the 202605 branch and add results (image version + test output) to the PR description
  2. Check the 202605 box in the "Tested Branches" section of the PR description
  3. Add the "Tested for 202605 branch" label once testing is complete

Also, to help us prioritize correctly, could you clarify: Is this PR a bug fix / regression fix, a new feature, a platform config change, or a dependency/infra update? Understanding the nature of the change helps us triage the cherry-pick queue.

Thank you!

Review by @vaibhavhd via automated tooling

@pnakka28

pnakka28 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

@pnakka28 — This PR is requested for the 202605 branch but is missing test evidence. To move forward with cherry-pick approval, please:

  1. Test on the 202605 branch and add results (image version + test output) to the PR description
  2. Check the 202605 box in the "Tested Branches" section of the PR description
  3. Add the "Tested for 202605 branch" label once testing is complete

Also, to help us prioritize correctly, could you clarify: Is this PR a bug fix / regression fix, a new feature, a platform config change, or a dependency/infra update? Understanding the nature of the change helps us triage the cherry-pick queue.

Thank you!

Review by @vaibhavhd via automated tooling

This is a regression fix. A previous refactor dropped the existing "Abort CMD102h supported" advertisement from the returned info text. This PR restores that behavior.

I tested the change against the 202605 branch and updated the PR description with the image version and test results.

Note: This PR needs to be cherry picked after #681 is merged into 202605. The conflict label is due to this.

@vaibhavhd

@mssonicbld

Copy link
Copy Markdown
Collaborator

The change is not in 202605 yet. @pnakka28, please manually create the cherry pick PR for branch 202605.
You can ping the release branch owner(github account: vaibhavhd) to approve your cherry pick PR.
If this change is already in 202605, please comment "already in 202605". Thanks!

---Powered by SONiC BuildBot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants