Skip to content

Add high power class support for non-CMIS modules in xcvrd#574

Merged
prgeor merged 8 commits intosonic-net:masterfrom
longhuan-cisco:high_power_handling
Jul 10, 2025
Merged

Add high power class support for non-CMIS modules in xcvrd#574
prgeor merged 8 commits intosonic-net:masterfrom
longhuan-cisco:high_power_handling

Conversation

@longhuan-cisco
Copy link
Contributor

@longhuan-cisco longhuan-cisco commented Dec 13, 2024

HLD: sonic-net/SONiC#1867
sonic-platform-common PR: sonic-net/sonic-platform-common#521

Description

  • Add xcvrd/sff_mgr support for enabling high power class if module's power class is greater or equal to 5 (Refer to SFF-8636 spec 6.2.6 Control Functions (Page 00h, Bytes 86-99) for byte 93 definition)

Motivation and Context

Without enabling high power class for those modules, the module won't operate properly with power output and link won't come up.

How Has This Been Tested?

Verified the module operate with proper power output.

Additional Information (Optional)

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prgeor prgeor requested a review from Copilot March 24, 2025 23:17
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 introduces support for high power class enabling for non‐CMIS modules and updates the SFF manager configuration by renaming the flag from enable_sff_mgr to enable_sff_mgr_controlled_tx. The key changes include:

  • Updating the constructor signature and related test cases to reflect the new enable_sff_mgr_controlled_tx parameter.
  • Adding a new method (handle_high_power_class) in SffManagerTask to perform high power class enabling.
  • Adjusting the daemon initialization and argument parser in xcvrd.py to use the updated parameter name.

Reviewed Changes

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

File Description
sonic-xcvrd/tests/test_xcvrd.py Updated constructor arguments and added tests for high power class handling.
sonic-xcvrd/xcvrd/sff_mgr.py Renamed the constructor parameter and added the handle_high_power_class method with corresponding logic updates.
sonic-xcvrd/xcvrd/xcvrd.py Updated DaemonXcvrd and argparse handling to use enable_sff_mgr_controlled_tx.
Comments suppressed due to low confidence (1)

sonic-xcvrd/xcvrd/sff_mgr.py:203

  • The word "occured" is misspelled. It should be corrected to "occurred".
self.helper_logger.log_error("Exception occured at {} thread due to {}".format(threading.current_thread().getName(), repr(e)))

Copy link
Collaborator

@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.

@longhuan-cisco How are you taking care of the breakout scenario where 4x10G some logical ports are admin down and some UP?

@prgeor
Copy link
Collaborator

prgeor commented Mar 24, 2025

@longhuan-cisco Why the SFF manager is calling lpmode for SFF8472 when these SFPs do'nt suppot? See
sonic-net/sonic-platform-common#512

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@longhuan-cisco
Copy link
Contributor Author

@longhuan-cisco Why the SFF manager is calling lpmode for SFF8472 when these SFPs do'nt suppot? See sonic-net/sonic-platform-common#512

The below code was added via #565:

sfp.set_lpmode(False)
if isinstance(api, Sff8472Api)

@longhuan-cisco
Copy link
Contributor Author

@longhuan-cisco How are you taking care of the breakout scenario where 4x10G some logical ports are admin down and some UP?

admin_status for subports is taken care of by below tx_disable/enable:

if api.tx_disable_channel(mask, target_tx_disable_flag):

@longhuan-cisco longhuan-cisco requested a review from prgeor July 9, 2025 23:19
prgeor
prgeor previously approved these changes Jul 10, 2025
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prgeor prgeor merged commit fd863c1 into sonic-net:master Jul 10, 2025
5 checks passed
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.

4 participants