Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Keep set_high_power_class raw and put handling logic in sff_mgr
longhuan-cisco committed Mar 9, 2025
commit db2a06f02b3a966f75b5e730e1cfa4eddfe86dc8
81 changes: 43 additions & 38 deletions sonic-xcvrd/tests/test_xcvrd.py
Original file line number Diff line number Diff line change
@@ -1571,6 +1571,48 @@ def test_SffManagerTask_get_admin_status(self, mock_get_cfg_port_tbl):
mock_get_cfg_port_tbl.assert_called_once_with(0)
mock_get_cfg_port_tbl.return_value.hget.assert_called_once_with(lport, 'admin_status')

def test_SffManagerTask_handle_high_power_class(self):
mock_xcvr_api = MagicMock()
mock_xcvr_api.get_power_class = MagicMock(return_value=5)
mock_xcvr_api.set_high_power_class = MagicMock(return_value=True)
lport = 'Ethernet0'

sff_manager_task = SffManagerTask(True, DEFAULT_NAMESPACE,
threading.Event(),
MagicMock(),
helper_logger)

# Test with normal case
sff_manager_task.handle_high_power_class(mock_xcvr_api, lport)
assert mock_xcvr_api.get_power_class.call_count == 1
assert mock_xcvr_api.set_high_power_class.call_count == 1

# Test with get_power_class failed
mock_xcvr_api.get_power_class.return_value = None
sff_manager_task.handle_high_power_class(mock_xcvr_api, lport)
assert mock_xcvr_api.get_power_class.call_count == 2
assert mock_xcvr_api.set_high_power_class.call_count == 1

# Test for no need to set high power class
mock_xcvr_api.get_power_class.return_value = 4
sff_manager_task.handle_high_power_class(mock_xcvr_api, lport)
assert mock_xcvr_api.get_power_class.call_count == 3
assert mock_xcvr_api.set_high_power_class.call_count == 1

# Test for set_high_power_class failed
mock_xcvr_api.get_power_class.return_value = 5
mock_xcvr_api.set_high_power_class.return_value = False
sff_manager_task.handle_high_power_class(mock_xcvr_api, lport)
assert mock_xcvr_api.get_power_class.call_count == 4
assert mock_xcvr_api.set_high_power_class.call_count == 2

# Test for set_high_power_class not supported
mock_xcvr_api.get_power_class.return_value = 5
mock_xcvr_api.set_high_power_class = MagicMock(side_effect=AttributeError("Attribute not found"))
sff_manager_task.handle_high_power_class(mock_xcvr_api, lport)
assert mock_xcvr_api.get_power_class.call_count == 5
assert mock_xcvr_api.set_high_power_class.call_count == 1

@patch('xcvrd.xcvrd.platform_chassis')
@patch('xcvrd.sff_mgr.PortChangeObserver', MagicMock(handle_port_update_event=MagicMock()))
def test_SffManagerTask_task_worker(self, mock_chassis):
@@ -1579,6 +1621,7 @@ def test_SffManagerTask_task_worker(self, mock_chassis):
mock_xcvr_api.is_flat_memory = MagicMock(return_value=False)
mock_xcvr_api.is_copper = MagicMock(return_value=False)
mock_xcvr_api.get_tx_disable_support = MagicMock(return_value=True)
mock_xcvr_api.get_power_class = MagicMock(return_value=1)

mock_sfp = MagicMock()
mock_sfp.get_presence = MagicMock(return_value=True)
@@ -1673,44 +1716,6 @@ def test_SffManagerTask_task_worker(self, mock_chassis):
assert mock_xcvr_api.tx_disable_channel.call_count == 2
mock_sfp.get_presence = MagicMock(return_value=True)

# high power enabling failure case and enable_sff_mgr_controlled_tx is False
mock_xcvr_api.tx_disable_channel.call_count = 0
mock_xcvr_api.set_high_power_class.call_count = 0
task.enable_sff_mgr_controlled_tx = False
task.get_host_tx_status = MagicMock(return_value='true')
task.get_admin_status = MagicMock(return_value='up')
mock_xcvr_api.set_high_power_class = MagicMock(return_value=False)
port_change_event = PortChangeEvent('Ethernet4', 2, 0, PortChangeEvent.PORT_SET, {
'type': 'QSFP28',
'subport': '0',
'lanes': '1,2,3,4',
})
task.on_port_update_event(port_change_event)
assert len(task.port_dict) == 2
task.task_stopping_event.is_set = MagicMock(side_effect=[False] + [False] * len(task.port_dict) + [True])
task.task_worker()
assert mock_xcvr_api.tx_disable_channel.call_count == 0
assert mock_xcvr_api.set_high_power_class.call_count == 1

# high power enabling exception case
mock_xcvr_api.tx_disable_channel.call_count = 0
mock_xcvr_api.set_high_power_class.call_count = 0
task.enable_sff_mgr_controlled_tx = False
task.get_host_tx_status = MagicMock(return_value='true')
task.get_admin_status = MagicMock(return_value='up')
mock_xcvr_api.set_high_power_class = MagicMock(side_effect=AttributeError("Attribute not found"))
port_change_event = PortChangeEvent('Ethernet8', 3, 0, PortChangeEvent.PORT_SET, {
'type': 'QSFP28',
'subport': '0',
'lanes': '1,2,3,4',
})
task.on_port_update_event(port_change_event)
assert len(task.port_dict) == 3
task.task_stopping_event.is_set = MagicMock(side_effect=[False] + [False] * len(task.port_dict) + [True])
task.task_worker()
assert mock_xcvr_api.tx_disable_channel.call_count == 0
assert mock_xcvr_api.set_high_power_class.call_count == 1

def test_CmisManagerTask_update_port_transceiver_status_table_sw_cmis_state(self):
port_mapping = PortMapping()
stop_event = threading.Event()
34 changes: 27 additions & 7 deletions sonic-xcvrd/xcvrd/sff_mgr.py
Original file line number Diff line number Diff line change
@@ -288,6 +288,32 @@ def convert_bool_array_to_bit_mask(self, bool_array):
mask += (1 << i if flag else 0)
return mask

def handle_high_power_class(self, xcvr_api, lport):
"""
Enable high power class for the transceiver.

Args:
xcvr_api (XcvrApi): The XcvrApi instance for the transceiver.
lport (str): Logical port name.
"""
try:
power_class = xcvr_api.get_power_class()
if power_class is None:
self.log_error("{}: failed to get power class".format(lport))
return

if power_class < 5:
# No action needed for power class < 5
return

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
Continue the discussion of sonic-net/sonic-platform-common#521 (comment) here at sff_mgr PR, regarding whether setting power_override bit should be included as part of handle_high_power_class:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

high power class enabling (added by this PR) and lpmode turning-off are two different independent steps. Not all modules need high power class enabling(only for power_class>=5), while most of the optical modules need lpmode turning-off.

Today, power_override bit setting has already been done by sff_mgr's lpmode turning-off (as part of which, xcvr API set_lpmode() gets called setting power_override bit). There's no need to do extra setting for that bit as part of high power class enabling.

image
Also from functionality POV, would it make more sense to decouple power_override bit from handling of high_power_class? Because, according to the truth table 6-11, once high power class (byte 93 bit 2 or/and bit 3) gets set properly, the module will function in the desired power class after the module is brought out of lpmode by either HW control or SW control.

if xcvr_api.set_high_power_class(power_class, True):
self.log_notice("{}: done enabling high power class".format(lport))
else:
self.log_error("{}: failed to enable high power class".format(lport))

except (AttributeError, NotImplementedError):
pass

def task_worker(self):
'''
The main goal of sff_mgr is to make sure SFF compliant modules are
@@ -440,13 +466,7 @@ def task_worker(self):
continue

if xcvr_inserted:
try:
if api.set_high_power_class(True):
self.log_notice("{}: done enabling high power class".format(lport))
else:
self.log_error("{}: failed to enable high power class".format(lport))
except (AttributeError, NotImplementedError):
pass
self.handle_high_power_class(api, lport)

if not self.enable_sff_mgr_controlled_tx:
continue