Skip to content

Fix show int transceiver EEPROM crash for for Backplane cartridge + enhance EEPROM CLI output#4020

Merged
prgeor merged 5 commits intosonic-net:masterfrom
mihirpat1:backplane-cartridge-eeprom-crash-enhance-output
Aug 13, 2025
Merged

Fix show int transceiver EEPROM crash for for Backplane cartridge + enhance EEPROM CLI output#4020
prgeor merged 5 commits intosonic-net:masterfrom
mihirpat1:backplane-cartridge-eeprom-crash-enhance-output

Conversation

@mihirpat1
Copy link
Contributor

@mihirpat1 mihirpat1 commented Aug 7, 2025

Summary

This PR fixes a crash in the show interface transceiver eeprom command when dealing with Backplane cartridge modules and enhances the overall EEPROM CLI (show + sfputil) output consistency and organization.

This change should be merged along with sonic-net/sonic-platform-common#590

Following traceback is seen with the Backplane cartridge cables. Hence, we need to resolve the traceback.

root@admin:/home/admin# show int transceiver info Ethernet128
Traceback (most recent call last):
  File "/usr/local/bin/sfpshow", line 824, in <module>
    cli()
  File "/usr/local/lib/python3.11/dist-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/click/core.py", line 1137, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/bin/sfpshow", line 764, in info
    sfp.get_eeprom()
  File "/usr/local/lib/python3.11/dist-packages/utilities_common/multi_asic.py", line 156, in wrapped_run_on_all_asics
    func(self,  *args, **kwargs)
  File "/usr/local/bin/sfpshow", line 641, in get_eeprom
    self.intf_eeprom[self.intf_name] = self.convert_interface_sfp_info_to_cli_output_string(
                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/bin/sfpshow", line 473, in convert_interface_sfp_info_to_cli_output_string
    sfp_info_output = self.convert_sfp_info_to_output_string(sfp_info_dict, sfp_firmware_info_dict)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/bin/sfpshow", line 354, in convert_sfp_info_to_output_string
    output += '{}{}: {}\n'.format(indent, data_map[key], sfp_info_dict[key])
                                                         ~~~~~~~~~~~~~^^^^^
KeyError: 'active_apsel_hostlane1'

🐛 Primary Bug Fix

  • Fixed crash for Backplane cartridge modules: Resolved issue where show interface transceiver eeprom command would crash when processing Backplane cartridge transceivers
    • Using the sfp_info_dict rather than a pre-defined dictionary (CMIS_DATA_MAP, C_CMIS_DATA_MAP, QSFP_DATA_MAP, QSFP_DD_DATA_MAP, QSFP_DATA_MAP) to display the EEPROM fields. This helps to avoid modifying the pre-defined dictionary upon introducing new fields in sfp_info_dict for a new transceiver

🚀 EEPROM CLI Enhancements

Code Refactoring & Shared Utilities

  • Created shared utility functions in utilities_common/sfp_helper.py:
    • get_data_map_sort_key(): Provides consistent sorting logic for SFP info keys
    • get_transceiver_data_map(): Automatically selects appropriate data map based on transceiver type

Improved Field Organization

  • Consistent Field Ordering: Both sfputil and sfpshow now display transceiver fields in logical, sorted order

Code Quality Improvements

  • Removed Code Duplication: Eliminated redundant sorting and display logic between utilities
  • Better Maintainability: Shared functions make future updates and bug fixes easier

🧪 Testing

  • Verified fix resolves Backplane cartridge crash issue
  • Confirmed consistent field ordering across both CLIs

Field differences between nightly (left) and current changes (right) for CLI output of different types of transceivers
(the red and greyed out portion depict differences)

  1. CMIS transceiver
    show int transceiver info CLI
image

sfputil show eeprom CLI
image

image
  1. CMIS transceiver with flat memory
    show int transceiver info CLI
image

sfputil show eeprom CLI
image

image
  1. C-CMIS transceiver
    show int transceiver info CLI
image

sfputil show eeprom CLI
image

image
  1. SFF-8636 with non-flat memory
    show int transceiver info CLI
image

sfputil show eeprom CLI
image

  1. SFF-8636 with flat memory
    show int transceiver info CLI
image

sfputil show eeprom CLI
image

  1. SFF-8436 with flat memory (DAC)
    show int transceiver info CLI
image

sfputil show eeprom CLI
image

  1. SFF-8472 transceiver
    show int transceiver info CLI
image

sfputil show eeprom CLI
image

…nhance EEPROM CLI output

Signed-off-by: Mihir Patel <patelmi@microsoft.com>
@mihirpat1 mihirpat1 requested a review from Copilot August 7, 2025 00:03
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

This comment was marked as outdated.

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mihirpat1 mihirpat1 requested a review from Copilot August 7, 2025 00:26

This comment was marked as outdated.

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mihirpat1 mihirpat1 requested a review from Copilot August 7, 2025 00:33

This comment was marked as outdated.

@mihirpat1
Copy link
Contributor Author

@az-pz Can you please help in reviewing this?

mssonicbld added a commit to mssonicbld/sonic-platform-common that referenced this pull request Aug 9, 2025
<!-- Provide a general summary of your changes in the Title above -->

#### Description
<!--
     Describe your changes in detail
-->
This PR updates the `get_transceiver_info()` API in the CMIS module to remove fields from the returned `XCVR_INFO` dictionary that are dependent on the active application ID. These fields include:
- host_electrical_interface
- media_interface_code
- host_lane_assignment_option
- media_lane_assignment_option

#### Motivation and Context
<!--
     Why is this change required? What problem does it solve?
     If this pull request closes/resolves an open Issue, make sure you
     include the text "fixes #xxxx", "closes #xxxx" or "resolves #xxxx" here
-->
These fields are only meaningful when the active application ID is explicitly shown. Including them without context can lead to confusion or misinterpretation of transceiver capabilities. This change ensures the API response reflects only valid and context-aware data.

#### How Has This Been Tested?
<!--
     Please describe in detail how you tested your changes.
     Include details of your testing environment, and the tests you ran to
     see how your change affects other areas of the code, etc.
-->
Please refer to the test section of sonic-net/sonic-utilities#4020

#### Additional Information (Optional)
MSFT ADO - 34325583
mssonicbld added a commit to sonic-net/sonic-platform-common that referenced this pull request Aug 9, 2025
…593)

<!-- Provide a general summary of your changes in the Title above -->

#### Description
<!--
 Describe your changes in detail
-->
This PR updates the `get_transceiver_info()` API in the CMIS module to remove fields from the returned `XCVR_INFO` dictionary that are dependent on the active application ID. These fields include:
- host_electrical_interface
- media_interface_code
- host_lane_assignment_option
- media_lane_assignment_option

#### Motivation and Context
<!--
 Why is this change required? What problem does it solve?
 If this pull request closes/resolves an open Issue, make sure you
 include the text "fixes #xxxx", "closes #xxxx" or "resolves #xxxx" here
-->
These fields are only meaningful when the active application ID is explicitly shown. Including them without context can lead to confusion or misinterpretation of transceiver capabilities. This change ensures the API response reflects only valid and context-aware data.

#### How Has This Been Tested?
<!--
 Please describe in detail how you tested your changes.
 Include details of your testing environment, and the tests you ran to
 see how your change affects other areas of the code, etc.
-->
Please refer to the test section of sonic-net/sonic-utilities#4020

#### Additional Information (Optional)
MSFT ADO - 34325583
return QSFP_DATA_MAP # Default fallback

is_sfp_cmis = is_transceiver_cmis(sfp_info_dict)
is_sfp_c_cmis = 'supported_max_tx_power' in sfp_info_dict
Copy link
Contributor

Choose a reason for hiding this comment

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

@mihirpat1 use optoe API is_coherent_module() ?

Copy link
Contributor Author

@mihirpat1 mihirpat1 Aug 9, 2025

Choose a reason for hiding this comment

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

@prgeor I think invoking the is_coherent_module API may not be the ideal thing to do since is_coherent_module API accesses the transceiver EEPROM directly. However, the current caller of get_transceiver_data_map API is meant to retrieve data from the redis-db rather than accessing the transceiver EEPROM directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mihirpat1 in that case should we define is_transceiver_coherent(sfp_dict) ? And use that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@prgeor Addressed this now

Copy link
Contributor

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@mihirpat1 can you check the copilot review comments?

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mihirpat1 mihirpat1 requested a review from Copilot August 12, 2025 03:58
Copy link
Contributor

Copilot AI left a comment

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 fixes a crash in the show interface transceiver eeprom command when dealing with Backplane cartridge modules and enhances EEPROM CLI output consistency between sfputil and sfpshow commands.

  • Fixed KeyError crash for Backplane cartridge modules by using dynamic field iteration instead of hardcoded data maps
  • Created shared utility functions for consistent field ordering and transceiver type detection
  • Improved code maintainability by eliminating duplicate sorting logic between CLI utilities

Reviewed Changes

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

Show a summary per file
File Description
utilities_common/sfp_helper.py Added shared utility functions for transceiver type detection and consistent field sorting
sfputil/main.py Updated to use shared utilities and dynamic field iteration to prevent KeyError crashes
scripts/sfpshow Updated to use shared utilities and handle unknown fields gracefully
tests/sfputil_test.py Updated test expectations for new field ordering and output format
tests/sfp_test.py Updated test expectations for additional fields in output
tests/mock_tables/state_db.json Updated mock data to reflect new field handling

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202505: #4032

YairRaviv pushed a commit to YairRaviv/sonic-utilities that referenced this pull request Jan 12, 2026
…nhance EEPROM CLI output (sonic-net#4020)

* Fix show int transceiver EEPROM crash for for Backplane cartridge + enhance EEPROM CLI output

Signed-off-by: Mihir Patel <patelmi@microsoft.com>

* Fixed SA warning

* Added None check for sfp_firmware_info_dict

* Added is_transceiver_c_cmis

* Added new line before get_data_map_sort_key

---------

Signed-off-by: Mihir Patel <patelmi@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.

6 participants