From c5d0c23e966ed973c71916309b31d1529f638222 Mon Sep 17 00:00:00 2001 From: Longyin Huang Date: Mon, 11 Nov 2024 20:21:41 -0800 Subject: [PATCH 1/3] Add high power class handling in sff_mgr --- sonic-xcvrd/xcvrd/sff_mgr.py | 18 +++++++++++++++++- sonic-xcvrd/xcvrd/xcvrd.py | 10 +++------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/sonic-xcvrd/xcvrd/sff_mgr.py b/sonic-xcvrd/xcvrd/sff_mgr.py index 0940b6b54..933d86bc1 100644 --- a/sonic-xcvrd/xcvrd/sff_mgr.py +++ b/sonic-xcvrd/xcvrd/sff_mgr.py @@ -78,9 +78,10 @@ class SffManagerTask(threading.Thread): SFF_LOGGER_PREFIX = "SFF-MAIN: " - def __init__(self, namespaces, main_thread_stop_event, platform_chassis, helper_logger): + def __init__(self, enable_sff_mgr_controlled_tx, namespaces, main_thread_stop_event, platform_chassis, helper_logger): threading.Thread.__init__(self) self.name = "SffManagerTask" + self.enable_sff_mgr_controlled_tx = enable_sff_mgr_controlled_tx self.exc = None self.task_stopping_event = threading.Event() self.main_thread_stop_event = main_thread_stop_event @@ -96,6 +97,9 @@ def __init__(self, namespaces, main_thread_stop_event, platform_chassis, helper_ self.xcvr_table_helper = XcvrTableHelper(namespaces) self.namespaces = namespaces + if not self.enable_sff_mgr_controlled_tx: + self.log_notice("enable_sff_mgr_controlled_tx is False, skipping controlling module TX based on host_tx_ready") + def log_notice(self, message): self.helper_logger.log_notice("{}{}".format(self.SFF_LOGGER_PREFIX, message)) @@ -435,6 +439,18 @@ def task_worker(self): # Skip if these essential routines are not available continue + if xcvr_inserted: + try: + if api.handle_high_power_class(): + self.log_notice("{}: done handling high power class".format(lport)) + else: + self.log_error("{}: failed to enable high power class".format(lport)) + except (AttributeError, NotImplementedError): + pass + + if not self.enable_sff_mgr_controlled_tx: + continue + if active_lanes is None: active_lanes = self.get_active_lanes_for_lport(lport, subport_idx, len(lanes_list), diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 0c6e9dded..4e273c846 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -2222,13 +2222,9 @@ def run(self): port_mapping_data = self.init() # Start the SFF manager - sff_manager = None - if self.enable_sff_mgr: - sff_manager = SffManagerTask(self.namespaces, self.stop_event, platform_chassis, helper_logger) - sff_manager.start() - self.threads.append(sff_manager) - else: - self.log_notice("Skipping SFF Task Manager") + sff_manager = SffManagerTask(self.enable_sff_mgr, self.namespaces, self.stop_event, platform_chassis, helper_logger) + sff_manager.start() + self.threads.append(sff_manager) # Start the CMIS manager cmis_manager = None From 20cba369409a46f589f1c2e7a7aa975c03921300 Mon Sep 17 00:00:00 2001 From: Longyin Huang Date: Mon, 11 Nov 2024 20:29:51 -0800 Subject: [PATCH 2/3] Fix test_xcvrd --- sonic-xcvrd/tests/test_xcvrd.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index e5b7d3180..1a8ce5789 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -200,7 +200,7 @@ class TestXcvrdThreadException(object): @patch('xcvrd.sff_mgr.PortChangeObserver', MagicMock(side_effect=NotImplementedError)) def test_SffManagerTask_task_run_with_exception(self): stop_event = threading.Event() - sff_mgr = SffManagerTask(DEFAULT_NAMESPACE, stop_event, MagicMock(), helper_logger) + sff_mgr = SffManagerTask(True, DEFAULT_NAMESPACE, stop_event, MagicMock(), helper_logger) exception_received = None trace = None try: @@ -1432,7 +1432,7 @@ def test_DaemonXcvrd_run(self, mock_task_stop1, mock_task_stop2, mock_task_run1, def test_SffManagerTask_handle_port_change_event(self): stop_event = threading.Event() - task = SffManagerTask(DEFAULT_NAMESPACE, stop_event, MagicMock(), helper_logger) + task = SffManagerTask(True, DEFAULT_NAMESPACE, stop_event, MagicMock(), helper_logger) port_change_event = PortChangeEvent('PortConfigDone', -1, 0, PortChangeEvent.PORT_SET) task.on_port_update_event(port_change_event) @@ -1470,7 +1470,7 @@ def test_SffManagerTask_handle_port_change_event(self): assert len(task.port_dict) == 0 def test_SffManagerTask_get_active_lanes_for_lport(self): - sff_manager_task = SffManagerTask(DEFAULT_NAMESPACE, + sff_manager_task = SffManagerTask(True, DEFAULT_NAMESPACE, threading.Event(), MagicMock(), helper_logger) @@ -1524,7 +1524,7 @@ def test_SffManagerTask_get_active_lanes_for_lport(self): assert result == expected_result def test_SffManagerTask_get_active_lanes_for_lport_with_invalid_input(self): - sff_manager_task = SffManagerTask(DEFAULT_NAMESPACE, + sff_manager_task = SffManagerTask(True, DEFAULT_NAMESPACE, threading.Event(), MagicMock(), helper_logger) @@ -1547,7 +1547,7 @@ def test_SffManagerTask_get_active_lanes_for_lport_with_invalid_input(self): def test_SffManagerTask_get_host_tx_status(self, mock_get_state_port_tbl): mock_get_state_port_tbl.return_value.hget.return_value = (True, 'true') - sff_manager_task = SffManagerTask(DEFAULT_NAMESPACE, + sff_manager_task = SffManagerTask(True, DEFAULT_NAMESPACE, threading.Event(), MagicMock(), helper_logger) @@ -1561,7 +1561,7 @@ def test_SffManagerTask_get_host_tx_status(self, mock_get_state_port_tbl): def test_SffManagerTask_get_admin_status(self, mock_get_cfg_port_tbl): mock_get_cfg_port_tbl.return_value.hget.return_value = (True, 'up') - sff_manager_task = SffManagerTask(DEFAULT_NAMESPACE, + sff_manager_task = SffManagerTask(True, DEFAULT_NAMESPACE, threading.Event(), MagicMock(), helper_logger) @@ -1587,7 +1587,7 @@ def test_SffManagerTask_task_worker(self, mock_chassis): mock_chassis.get_all_sfps = MagicMock(return_value=[mock_sfp]) mock_chassis.get_sfp = MagicMock(return_value=mock_sfp) - task = SffManagerTask(DEFAULT_NAMESPACE, + task = SffManagerTask(True, DEFAULT_NAMESPACE, threading.Event(), mock_chassis, helper_logger) From c01b5495a064480c39aaa3f8ea5cb90ca369f42c Mon Sep 17 00:00:00 2001 From: Longyin Huang Date: Thu, 12 Dec 2024 15:26:53 -0800 Subject: [PATCH 3/3] Add UT coverage and rename sff_mgr enable flag to enable_sff_mgr_controlled_tx --- sonic-xcvrd/tests/test_xcvrd.py | 38 +++++++++++++++++++++++++++++++++ sonic-xcvrd/xcvrd/sff_mgr.py | 4 ++-- sonic-xcvrd/xcvrd/xcvrd.py | 10 ++++----- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 1a8ce5789..3a5a3a1a2 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -1673,6 +1673,44 @@ 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() diff --git a/sonic-xcvrd/xcvrd/sff_mgr.py b/sonic-xcvrd/xcvrd/sff_mgr.py index 933d86bc1..f627c638c 100644 --- a/sonic-xcvrd/xcvrd/sff_mgr.py +++ b/sonic-xcvrd/xcvrd/sff_mgr.py @@ -441,8 +441,8 @@ def task_worker(self): if xcvr_inserted: try: - if api.handle_high_power_class(): - self.log_notice("{}: done handling high power class".format(lport)) + 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): diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 4e273c846..202a630fa 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -2052,12 +2052,12 @@ def retry_eeprom_reading(self): class DaemonXcvrd(daemon_base.DaemonBase): - def __init__(self, log_identifier, skip_cmis_mgr=False, enable_sff_mgr=False): + def __init__(self, log_identifier, skip_cmis_mgr=False, enable_sff_mgr_controlled_tx=False): super(DaemonXcvrd, self).__init__(log_identifier, enable_runtime_log_config=True) self.stop_event = threading.Event() self.sfp_error_event = threading.Event() self.skip_cmis_mgr = skip_cmis_mgr - self.enable_sff_mgr = enable_sff_mgr + self.enable_sff_mgr_controlled_tx = enable_sff_mgr_controlled_tx self.namespaces = [''] self.threads = [] @@ -2222,7 +2222,7 @@ def run(self): port_mapping_data = self.init() # Start the SFF manager - sff_manager = SffManagerTask(self.enable_sff_mgr, self.namespaces, self.stop_event, platform_chassis, helper_logger) + sff_manager = SffManagerTask(self.enable_sff_mgr_controlled_tx, self.namespaces, self.stop_event, platform_chassis, helper_logger) sff_manager.start() self.threads.append(sff_manager) @@ -2304,10 +2304,10 @@ def run(self): def main(): parser = argparse.ArgumentParser() parser.add_argument('--skip_cmis_mgr', action='store_true') - parser.add_argument('--enable_sff_mgr', action='store_true') + parser.add_argument('--enable_sff_mgr_controlled_tx', action='store_true') args = parser.parse_args() - xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER, args.skip_cmis_mgr, args.enable_sff_mgr) + xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER, args.skip_cmis_mgr, args.enable_sff_mgr_controlled_tx) xcvrd.run()