Skip to content

[sonic_xcvr] Fix firmware download TypeError - #744

Merged
mihirpat1 merged 1 commit into
sonic-net:masterfrom
pnakka28:firmware_download_bug
Aug 24, 2026
Merged

[sonic_xcvr] Fix firmware download TypeError#744
mihirpat1 merged 1 commit into
sonic-net:masterfrom
pnakka28:firmware_download_bug

Conversation

@pnakka28

@pnakka28 pnakka28 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Description

The CDB firmware block size fields come back from the module as floats. That float made it all to file.read() during firmware download, which only accepts an integer, so the download crashed with a TypeError on any EPL-capable module that didn't advertise the maximum block size.

Motivation and Context

Motivation and Context

Firmware download fails on any module that doesn't advertise the maximum block size:

    CDB: Starting firmware download
    Downloading ...  [------------------------------------]    0%
    Traceback (most recent call last):
      File "/usr/local/bin/sfputil", line 8, in <module>
        sys.exit(cli())
      ...
      File "/usr/local/lib/python3.13/dist-packages/sfputil/main.py", line 1725, in download
        status = download_firmware(port_name, filepath)
      File "/usr/local/lib/python3.13/dist-packages/sfputil/main.py", line 1580, in download_firmware
        data = fd.read(count)
    TypeError: argument should be integer or None, not 'float'
    CDB: Starting firmware download
    CDB: Start firmware download failed - status 67
    CDB: Starting firmware download
    CDB: Start firmware download failed - status 67

How Has This Been Tested?

  • Modified unit tests and made sure they are passing
  • With the fix, verified firmware downloaded successfully

On version 202605:
Hardware: Reproduced the TypeError on the unpatched code, then confirmed firmware download proceeds past block-write with the fix applied.
Unit tests: pytest tests/sonic_xcvr/test_cdb_fw.py

Additional Information (Optional)

MSFT ADO - 39388348

Signed-off-by: Pavan Kalyan Nakka <pnakka@microsoft.com>
@pnakka28
pnakka28 requested a balanced review from Copilot August 24, 2026 18:34
@azure-pipelines

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

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

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

Fixes CDB firmware downloads by normalizing module-reported block sizes to integers before file reads.

Changes:

  • Converts firmware payload and block sizes to integers.
  • Adds coverage for floating-point EPL block sizes.

Reviewed changes

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

File Description
sonic_platform_base/sonic_xcvr/cdb/cdb_fw.py Normalizes firmware block-size fields.
tests/sonic_xcvr/test_cdb_fw.py Verifies EPL block-size conversion.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@mihirpat1 mihirpat1 added the CMIS label Aug 24, 2026
@mihirpat1
mihirpat1 merged commit c07f074 into sonic-net:master Aug 24, 2026
6 checks passed
@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

vaibhavhd pushed a commit that referenced this pull request Aug 28, 2026
(cherry picked from commit c07f074)

Signed-off-by: Pavan Kalyan Nakka <pnakka@microsoft.com>
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.

4 participants