Skip to content

Fix Handle non-responsive SFP modules gracefully in sfputil#4654

Open
nhilderm-ciena wants to merge 1 commit into
sonic-net:masterfrom
nhilderm-ciena:fix/sfputil-handle-non-responsive-modules
Open

Fix Handle non-responsive SFP modules gracefully in sfputil#4654
nhilderm-ciena wants to merge 1 commit into
sonic-net:masterfrom
nhilderm-ciena:fix/sfputil-handle-non-responsive-modules

Conversation

@nhilderm-ciena

Copy link
Copy Markdown

Handle non-responsive SFP modules gracefully in sfputil show eeprom and show fwversion commands.

What I did

Added None guards in sfputil show eeprom and show fwversion commands to prevent crashes when transceiver modules are non-responsive or return None from platform API calls.

When an SFP module is physically present but non-responsive (e.g. due to I2C timeout), the platform API calls get_transceiver_info() and get_xcvr_api() return None. The existing code does not check for None before attempting to use the return values, causing unhandled exceptions.

The Problem:

  • In sfputil show eeprom, get_transceiver_info() returns None for a non-responsive module but the code immediately passes the result to convert_sfp_info_to_output_string(), which crashes.
  • In sfputil show fwversion, get_xcvr_api() returns None but the code immediately calls api.get_module_fw_info(), which crashes with AttributeError.

How I did it

Added explicit None checks:

  • In eeprom command: check xcvr_info before processing, print error and continue to next port if None
  • In fwversion command: check api before calling get_module_fw_info(), print error and exit gracefully if None

How to verify it

  • Test with a non-responsive SFP module present:
    sfputil show eeprom -p EthernetX
    (should print error message instead of crashing)

  • Test firmware version with non-responsive module:
    sfputil show fwversion -p EthernetX
    (should print error message instead of crashing)

  • Test with responsive modules (regression):
    sfputil show eeprom
    sfputil show fwversion -p EthernetX
    (should work normally)

Previous command output (if the output of a command-line utility has changed)

$ sfputil show eeprom -p Ethernet0
# (crashes with traceback when module is non-responsive)

New command output (if the output of a command-line utility has changed)

$ sfputil show eeprom -p Ethernet0
Ethernet0: SFP EEPROM detected
        Error: Failed to read EEPROM for this port

@linux-foundation-easycla

linux-foundation-easycla Bot commented Jun 29, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: nhilderm-ciena / name: nhilderm (61f3823)

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

Add None guards in sfputil show eeprom and show fwversion commands to
prevent crashes when transceiver modules are non-responsive or return
None from platform API calls.

sfputil commands crash on non-responsive SFP modules.

When an SFP module is physically present but non-responsive (e.g. due to
I2C timeout), the platform API calls get_transceiver_info() and
get_xcvr_api() return None. The existing code does not check for None
before attempting to use the return values, causing unhandled exceptions.

The Problem:
In sfputil show eeprom, get_transceiver_info() returns None for a
non-responsive module but the code immediately passes the result to
convert_sfp_info_to_output_string(), which crashes.
In sfputil show fwversion, get_xcvr_api() returns None but the code
immediately calls api.get_module_fw_info(), which crashes with
AttributeError.

Added explicit None checks:
- In eeprom command: check xcvr_info before processing, print error and
  continue to next port if None
- In fwversion command: check api before calling get_module_fw_info(),
  print error and exit gracefully if None

- Test with a non-responsive SFP module present:
sfputil show eeprom -p EthernetX
(should print error message instead of crashing)

- Test firmware version with non-responsive module:
sfputil show fwversion -p EthernetX
(should print error message instead of crashing)

- Test with responsive modules (regression):
sfputil show eeprom
sfputil show fwversion -p EthernetX
(should work normally)

Signed-off-by: nhilderm <nhilderm@ciena.com>
@nhilderm-ciena
nhilderm-ciena force-pushed the fix/sfputil-handle-non-responsive-modules branch from 61f3823 to 4fd1682 Compare June 30, 2026 13:46
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

@nhilderm-ciena
nhilderm-ciena force-pushed the fix/sfputil-handle-non-responsive-modules branch from ddbd82c to aa530bf Compare June 30, 2026 14:47
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

@nhilderm-ciena
nhilderm-ciena force-pushed the fix/sfputil-handle-non-responsive-modules branch from aa530bf to 4fd1682 Compare June 30, 2026 15:13
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

@mssonicbld

Copy link
Copy Markdown
Collaborator

Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks!

---Powered by SONiC BuildBot

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants