From 27ad1704bece4fa023cd2a099153adaa41b5b776 Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Thu, 2 Nov 2023 00:46:31 +0000 Subject: [PATCH 1/7] Changes for port reinitialization in case of syncd/swss/oa crash and NPU SI settings update for CMIS transceivers --- sonic-xcvrd/tests/test_xcvrd.py | 152 ++++++++-- sonic-xcvrd/xcvrd/xcvrd.py | 497 ++++++++++++++++++++++++++------ 2 files changed, 533 insertions(+), 116 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index a3adf5823..d67398728 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -25,6 +25,7 @@ swsscommon.ProducerStateTable = MagicMock() swsscommon.SubscriberStateTable = MagicMock() swsscommon.SonicDBConfig = MagicMock() +swsscommon.SonicV2Connector = MagicMock() #swsscommon.Select = MagicMock() test_path = os.path.dirname(os.path.abspath(__file__)) @@ -53,11 +54,11 @@ class TestXcvrdThreadException(object): @patch('xcvrd.xcvrd.platform_chassis', MagicMock()) + @patch('xcvrd.xcvrd_utilities.port_mapping.subscribe_port_update_event', MagicMock(side_effect=NotImplementedError)) def test_CmisManagerTask_task_run_with_exception(self): port_mapping = PortMapping() stop_event = threading.Event() cmis_manager = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event) - cmis_manager.wait_for_port_config_done = MagicMock(side_effect = NotImplementedError) exception_received = None trace = None try: @@ -71,7 +72,7 @@ def test_CmisManagerTask_task_run_with_exception(self): assert(type(exception_received) == NotImplementedError) assert("NotImplementedError" in str(trace) and "effect" in str(trace)) assert("sonic-xcvrd/xcvrd/xcvrd.py" in str(trace)) - assert("wait_for_port_config_done" in str(trace)) + assert("subscribe_port_update_event" in str(trace)) @patch('xcvrd.xcvrd_utilities.port_mapping.subscribe_port_config_change', MagicMock(side_effect = NotImplementedError)) def test_DomInfoUpdateTask_task_run_with_exception(self): @@ -123,6 +124,7 @@ def test_SfpStateUpdateTask_task_run_with_exception(self): @patch('xcvrd.xcvrd.DomInfoUpdateTask.start', MagicMock()) @patch('xcvrd.xcvrd.SfpStateUpdateTask.start', MagicMock()) @patch('xcvrd.xcvrd.DaemonXcvrd.deinit', MagicMock()) + @patch('xcvrd.xcvrd.DaemonXcvrd.subscribe_appl_port_table', MagicMock(return_value = (None, None))) @patch('os.kill') @patch('xcvrd.xcvrd.DaemonXcvrd.init') @patch('xcvrd.xcvrd.DomInfoUpdateTask.join') @@ -130,7 +132,7 @@ def test_SfpStateUpdateTask_task_run_with_exception(self): def test_DaemonXcvrd_run_with_exception(self, mock_task_join1, mock_task_join2, mock_init, mock_os_kill): mock_init.return_value = (PortMapping(), set()) xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER) - xcvrd.stop_event.wait = MagicMock() + xcvrd.stop_event.is_set = MagicMock(return_value=True) xcvrd.run() assert len(xcvrd.threads) == 3 @@ -402,7 +404,9 @@ def test_post_port_sfp_info_to_db(self): 'tx6power': '0.7', 'tx7power': '0.7', 'tx8power': '0.7', })) - @patch('swsscommon.swsscommon.WarmStart', MagicMock()) + @patch('xcvrd.xcvrd.is_module_cmis_sm_driven', MagicMock(return_value=False)) + @patch('xcvrd.xcvrd.is_npu_si_settings_update_required', MagicMock(return_value=True)) + @patch('xcvrd.xcvrd.post_npu_si_settings_to_appl_db', MagicMock()) def test_post_port_sfp_info_and_dom_thr_to_db_once(self): port_mapping = PortMapping() port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD) @@ -452,16 +456,16 @@ def test_get_media_settings_key(self): @patch('xcvrd.xcvrd.g_dict', media_settings_dict) @patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=True)) - def test_notify_media_setting(self): - self._check_notify_media_setting(1) + def test_post_npu_si_settings_to_appl_db(self): + self._check_post_npu_si_settings_to_appl_db_setting(1) @patch('xcvrd.xcvrd.g_dict', media_settings_with_comma_dict) @patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=True)) - def test_notify_media_setting_with_comma(self): - self._check_notify_media_setting(1) - self._check_notify_media_setting(6) + def test_post_npu_si_settings_to_appl_db_with_comma(self): + self._check_post_npu_si_settings_to_appl_db_setting(1) + self._check_post_npu_si_settings_to_appl_db_setting(6) - def _check_notify_media_setting(self, index): + def _check_post_npu_si_settings_to_appl_db_setting(self, index): logical_port_name = 'Ethernet0' xcvr_info_dict = { index: { @@ -473,11 +477,11 @@ def _check_notify_media_setting(self, index): 'type_abbrv_name': 'QSFP+' } } - app_port_tbl = Table("APPL_DB", 'PORT_TABLE') port_mapping = PortMapping() port_change_event = PortChangeEvent('Ethernet0', index, 0, PortChangeEvent.PORT_ADD) port_mapping.handle_port_change_event(port_change_event) - notify_media_setting(logical_port_name, xcvr_info_dict, app_port_tbl, port_mapping) + xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE) + post_npu_si_settings_to_appl_db(logical_port_name, xcvr_info_dict, xcvr_table_helper, port_mapping) @patch('xcvrd.xcvrd_utilities.optics_si_parser.g_optics_si_dict', optics_si_settings_dict) @patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=True)) @@ -512,6 +516,64 @@ def test_get_module_vendor_key(self): result = get_module_vendor_key(1, mock_sfp) assert result == ('CREDO-CAC82X321HW','CREDO') + @patch('xcvrd.xcvrd.platform_chassis') + def test_is_module_cmis_sm_driven(self, mock_platform_chassis): + port_mapping = PortMapping() + port_mapping.get_logical_to_physical = MagicMock(return_value=[0]) + mock_sfp = MagicMock() + mock_platform_chassis.get_sfp = MagicMock(return_value=mock_sfp) + mock_sfp.get_presence = MagicMock(return_value=False) + assert not is_module_cmis_sm_driven(port_mapping, "Ethernet0") + + mock_sfp.get_presence = MagicMock(return_value=True) + mock_xcvr_api = MagicMock() + mock_sfp.get_xcvr_api = MagicMock(return_value=None) + assert not is_module_cmis_sm_driven(port_mapping, "Ethernet0") + + mock_sfp.get_xcvr_api = MagicMock(return_value=mock_xcvr_api) + mock_xcvr_api.is_flat_memory = MagicMock(return_value=True) + assert not is_module_cmis_sm_driven(port_mapping, "Ethernet0") + + mock_xcvr_api.is_flat_memory = MagicMock(return_value=False) + mock_xcvr_api.get_module_type_abbreviation = MagicMock(return_value='QSFP28') + assert not is_module_cmis_sm_driven(port_mapping, "Ethernet0") + + mock_xcvr_api.get_module_type_abbreviation = MagicMock(return_value='QSFP-DD') + assert is_module_cmis_sm_driven(port_mapping, "Ethernet0") + + mock_xcvr_api.get_module_type_abbreviation = MagicMock(side_effect=AttributeError) + assert not is_module_cmis_sm_driven(port_mapping, "Ethernet0") + + def test_get_app_port_table_val_by_key(self): + mock_xcvr_table_helper = MagicMock() + port_mapping = PortMapping() + assert get_app_port_table_val_by_key(None, port_mapping, "Ethernet0", "CMIS_REINIT_REQUIRED") == None + + assert get_app_port_table_val_by_key(mock_xcvr_table_helper, None, "Ethernet0", "CMIS_REINIT_REQUIRED") == None + + port_mapping.get_asic_id_for_logical_port = MagicMock(return_value=0) + mock_xcvr_table_helper.get_non_producer_app_port_tbl = MagicMock(return_value=None) + assert get_app_port_table_val_by_key(mock_xcvr_table_helper, port_mapping, "Ethernet0", "CMIS_REINIT_REQUIRED") == None + + mock_app_port_tbl = MagicMock() + mock_xcvr_table_helper.get_non_producer_app_port_tbl = MagicMock(return_value=mock_app_port_tbl) + mock_app_port_tbl.get_all = MagicMock(return_value=None) + assert get_app_port_table_val_by_key(mock_xcvr_table_helper, port_mapping, "Ethernet0", "CMIS_REINIT_REQUIRED") == None + + mock_app_port_tbl.get_all = MagicMock(return_value=({'CMIS_REINIT_REQUIRED': 'true'})) + assert get_app_port_table_val_by_key(mock_xcvr_table_helper, port_mapping, "Ethernet0", "CMIS_REINIT_REQUIRED") == "true" + + # @patch('xcvrd.xcvrd.get_app_port_table_val_by_key', MagicMock(side_effect=["true", "NPU_SI_SETTINGS_DONE", "true"])) + @patch('xcvrd.xcvrd.get_app_port_table_val_by_key', MagicMock(side_effect=[None, "NPU_SI_SETTINGS_DEFAULT"])) + @patch('xcvrd.xcvrd.get_npu_si_settings_dict', MagicMock(return_value={'key1': 'value1'})) + def test_is_npu_si_settings_update_required(self): + mock_xcvr_table_helper = MagicMock() + port_mapping = PortMapping() + traceiver_dict = {} + + assert not is_npu_si_settings_update_required(mock_xcvr_table_helper, port_mapping, "Ethernet0", traceiver_dict) + assert is_npu_si_settings_update_required(mock_xcvr_table_helper, port_mapping, "Ethernet0", traceiver_dict) + def test_detect_port_in_error_status(self): class MockTable: def get(self, key): @@ -605,6 +667,7 @@ def test_get_port_mapping(self, mock_swsscommon_table): assert port_mapping.get_logical_to_physical('Ethernet-IB0') == None @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('swsscommon.swsscommon.Select.removeSelectable', MagicMock()) @patch('swsscommon.swsscommon.SubscriberStateTable') @patch('swsscommon.swsscommon.Select.select') def test_DaemonXcvrd_wait_for_port_config_done(self, mock_select, mock_sub_table): @@ -617,6 +680,25 @@ def test_DaemonXcvrd_wait_for_port_config_done(self, mock_select, mock_sub_table xcvrd.wait_for_port_config_done('') assert swsscommon.Select.select.call_count == 2 + def test_DaemonXcvrd_initialize_port_init_fields_in_port_table(self): + port_mapping = PortMapping() + xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER) + + port_mapping.logical_port_list = ['Ethernet0'] + port_mapping.get_asic_id_for_logical_port = MagicMock(return_value=0) + mock_xcvrd_table_helper = MagicMock() + mock_xcvrd_table_helper.get_non_producer_app_port_tbl = MagicMock(return_value=None) + xcvrd.xcvr_table_helper = mock_xcvrd_table_helper + xcvrd.initialize_port_init_fields_in_port_table(port_mapping) + + mock_appl_db = MagicMock() + mock_xcvrd_table_helper.get_non_producer_app_port_tbl = MagicMock(return_value=mock_appl_db) + mock_appl_db.get_all = MagicMock(return_value={}) + + xcvrd.initialize_port_init_fields_in_port_table(port_mapping) + mock_appl_db.set.call_count = 2 + + @patch('xcvrd.xcvrd.DaemonXcvrd.subscribe_appl_port_table', MagicMock(return_value = (None, None))) @patch('xcvrd.xcvrd.DaemonXcvrd.init') @patch('xcvrd.xcvrd.DaemonXcvrd.deinit') @patch('xcvrd.xcvrd.DomInfoUpdateTask.start') @@ -626,7 +708,7 @@ def test_DaemonXcvrd_wait_for_port_config_done(self, mock_select, mock_sub_table def test_DaemonXcvrd_run(self, mock_task_stop1, mock_task_stop2, mock_task_run1, mock_task_run2, mock_deinit, mock_init): mock_init.return_value = (PortMapping(), set()) xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER) - xcvrd.stop_event.wait = MagicMock() + xcvrd.stop_event.is_set = MagicMock(return_value=True) xcvrd.run() assert mock_task_stop1.call_count == 1 assert mock_task_stop2.call_count == 1 @@ -696,7 +778,6 @@ def test_CmisManagerTask_task_run_stop(self, mock_chassis): port_mapping = PortMapping() stop_event = threading.Event() cmis_manager = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event) - cmis_manager.wait_for_port_config_done = MagicMock() cmis_manager.start() cmis_manager.join() assert not cmis_manager.is_alive() @@ -893,7 +974,11 @@ def test_CmisManagerTask_post_port_active_apsel_to_db(self): @patch('xcvrd.xcvrd_utilities.port_mapping.subscribe_port_update_event', MagicMock(return_value=(None, None))) @patch('xcvrd.xcvrd_utilities.port_mapping.handle_port_update_event', MagicMock()) @patch('xcvrd.xcvrd._wrapper_get_sfp_type', MagicMock(return_value='QSFP_DD')) - @patch('xcvrd.xcvrd.CmisManagerTask.wait_for_port_config_done', MagicMock()) + @patch('xcvrd.xcvrd.is_module_cmis_sm_driven', MagicMock(return_value=True)) + @patch('xcvrd.xcvrd.get_app_port_table_val_by_key', MagicMock(side_effect=["true", "NPU_SI_SETTINGS_DONE", "true"])) + @patch('xcvrd.xcvrd.is_npu_si_settings_update_required', MagicMock(return_value=True)) + @patch('xcvrd.xcvrd.XcvrTableHelper.get_app_port_tbl', MagicMock()) + @patch('xcvrd.xcvrd.post_npu_si_settings_to_appl_db', MagicMock()) def test_CmisManagerTask_task_worker(self, mock_chassis): mock_xcvr_api = MagicMock() mock_xcvr_api.set_datapath_deinit = MagicMock(return_value=True) @@ -950,6 +1035,9 @@ def test_CmisManagerTask_task_worker(self, mock_chassis): 'ConfigStatusLane8': 'ConfigSuccess' }) mock_xcvr_api.get_datapath_state = MagicMock(side_effect=[ + {}, + {}, + {}, { 'DP1State': 'DataPathDeactivated', 'DP2State': 'DataPathDeactivated', @@ -960,6 +1048,9 @@ def test_CmisManagerTask_task_worker(self, mock_chassis): 'DP7State': 'DataPathDeactivated', 'DP8State': 'DataPathDeactivated' }, + {}, + {}, + {}, { 'DP1State': 'DataPathInitialized', 'DP2State': 'DataPathInitialized', @@ -1023,15 +1114,20 @@ def test_CmisManagerTask_task_worker(self, mock_chassis): task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True]) task.task_worker() assert mock_xcvr_api.set_application.call_count == 1 + assert task.port_dict['Ethernet0']['cmis_state'] == 'NPU_SI_SETTINGS_WAIT' + + # Case 3: AP Configured --> NPU_SI_SETTINGS_WAIT + task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True]) + task.task_worker() assert task.port_dict['Ethernet0']['cmis_state'] == 'DP_INIT' - # Case 3: AP Configured --> DP_INIT + # Case 4: NPU_SI_SETTINGS_WAIT --> DP_INIT task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True]) task.task_worker() assert mock_xcvr_api.set_datapath_init.call_count == 1 assert task.port_dict['Ethernet0']['cmis_state'] == 'DP_TXON' - # Case 4: DP_INIT --> DP_TXON + # Case 5: DP_INIT --> DP_TXON task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True]) task.task_worker() assert mock_xcvr_api.tx_disable_channel.call_count == 2 @@ -1167,6 +1263,7 @@ def test_SfpStateUpdateTask_task_run_stop(self): assert wait_until(5, 1, lambda: task.is_alive() is False) @patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock()) + @patch('xcvrd.xcvrd.is_module_cmis_sm_driven', MagicMock()) @patch('xcvrd.xcvrd.post_port_sfp_info_to_db') def test_SfpStateUpdateTask_retry_eeprom_reading(self, mock_post_sfp_info): mock_table = MagicMock() @@ -1224,17 +1321,19 @@ def test_SfpStateUpdateTask_mapping_event_from_change_event(self): @patch('xcvrd.xcvrd_utilities.port_mapping.subscribe_port_config_change', MagicMock(return_value=(None, None))) @patch('xcvrd.xcvrd_utilities.port_mapping.handle_port_config_change', MagicMock()) @patch('xcvrd.xcvrd.SfpStateUpdateTask.init', MagicMock()) + @patch('xcvrd.xcvrd.is_module_cmis_sm_driven', MagicMock(return_value=False)) + @patch('xcvrd.xcvrd.is_npu_si_settings_update_required', MagicMock(return_value=True)) @patch('os.kill') @patch('xcvrd.xcvrd.SfpStateUpdateTask._mapping_event_from_change_event') @patch('xcvrd.xcvrd._wrapper_get_transceiver_change_event') @patch('xcvrd.xcvrd.del_port_sfp_dom_info_from_db') - @patch('xcvrd.xcvrd.notify_media_setting') + @patch('xcvrd.xcvrd.post_npu_si_settings_to_appl_db') @patch('xcvrd.xcvrd.post_port_dom_threshold_info_to_db') @patch('xcvrd.xcvrd.post_port_sfp_info_to_db') @patch('xcvrd.xcvrd.update_port_transceiver_status_table_sw') @patch('xcvrd.xcvrd.delete_port_from_status_table_hw') def test_SfpStateUpdateTask_task_worker(self, mock_del_status_hw, - mock_update_status, mock_post_sfp_info, mock_post_dom_th, mock_update_media_setting, + mock_update_status, mock_post_sfp_info, mock_post_dom_th, mock_post_npu_si_setting, mock_del_dom, mock_change_event, mock_mapping_event, mock_os_kill): port_mapping = PortMapping() stop_event = threading.Event() @@ -1286,7 +1385,7 @@ def test_SfpStateUpdateTask_task_worker(self, mock_del_status_hw, assert mock_update_status.call_count == 1 assert mock_post_sfp_info.call_count == 2 # first call and retry call assert mock_post_dom_th.call_count == 0 - assert mock_update_media_setting.call_count == 0 + assert mock_post_npu_si_setting.call_count == 0 assert 'Ethernet0' in task.retry_eeprom_set task.retry_eeprom_set.clear() @@ -1299,7 +1398,7 @@ def test_SfpStateUpdateTask_task_worker(self, mock_del_status_hw, assert mock_update_status.call_count == 1 assert mock_post_sfp_info.call_count == 1 assert mock_post_dom_th.call_count == 1 - assert mock_update_media_setting.call_count == 1 + assert mock_post_npu_si_setting.call_count == 1 stop_event.is_set = MagicMock(side_effect=[False, True]) mock_change_event.return_value = (True, {1: SFP_STATUS_REMOVED}, {}) @@ -1322,14 +1421,16 @@ def test_SfpStateUpdateTask_task_worker(self, mock_del_status_hw, assert mock_del_dom.call_count == 1 assert mock_del_status_hw.call_count == 1 + @patch('xcvrd.xcvrd.is_module_cmis_sm_driven', MagicMock(return_value=False)) + @patch('xcvrd.xcvrd.is_npu_si_settings_update_required', MagicMock(return_value=True)) @patch('xcvrd.xcvrd.XcvrTableHelper') @patch('xcvrd.xcvrd._wrapper_get_presence') - @patch('xcvrd.xcvrd.notify_media_setting') + @patch('xcvrd.xcvrd.post_npu_si_settings_to_appl_db') @patch('xcvrd.xcvrd.post_port_dom_threshold_info_to_db') @patch('xcvrd.xcvrd.post_port_sfp_info_to_db') @patch('xcvrd.xcvrd.update_port_transceiver_status_table_sw') def test_SfpStateUpdateTask_on_add_logical_port(self, mock_update_status, mock_post_sfp_info, - mock_post_dom_th, mock_update_media_setting, mock_get_presence, mock_table_helper): + mock_post_dom_th, mock_post_npu_si_setting, mock_get_presence, mock_table_helper): class MockTable: pass @@ -1367,7 +1468,7 @@ class MockTable: assert mock_post_sfp_info.call_count == 1 mock_post_sfp_info.assert_called_with('Ethernet0', task.port_mapping, int_tbl, {}) assert mock_post_dom_th.call_count == 0 - assert mock_update_media_setting.call_count == 0 + assert mock_post_npu_si_setting.call_count == 0 assert 'Ethernet0' in task.retry_eeprom_set task.retry_eeprom_set.clear() @@ -1382,7 +1483,7 @@ class MockTable: mock_post_sfp_info.assert_called_with('Ethernet0', task.port_mapping, int_tbl, {}) assert mock_post_dom_th.call_count == 1 mock_post_dom_th.assert_called_with('Ethernet0', task.port_mapping, dom_threshold_tbl) - assert mock_update_media_setting.call_count == 1 + assert mock_post_npu_si_setting.call_count == 1 assert 'Ethernet0' not in task.retry_eeprom_set mock_get_presence.return_value = False @@ -1611,6 +1712,7 @@ def test_get_media_val_str(self): media_str = get_media_val_str(num_logical_ports, lane_dict, logical_idx) assert media_str == '3,4' + @patch('xcvrd.xcvrd.platform_chassis', MagicMock()) @patch('xcvrd.xcvrd.DaemonXcvrd.load_platform_util', MagicMock()) @patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs', MagicMock(return_value=('/tmp', None))) @patch('swsscommon.swsscommon.WarmStart', MagicMock()) diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index ab06b9ab1..701a19f76 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -48,6 +48,8 @@ TRANSCEIVER_STATUS_TABLE_SW_FIELDS = ["status", "error"] +CMIS_MODULE_TYPES = ['QSFP-DD', 'QSFP_DD', 'OSFP', 'QSFP+C'] + # Mgminit time required as per CMIS spec MGMT_INIT_TIME_DELAY_SECS = 2 @@ -631,7 +633,7 @@ def get_media_settings_value(physical_port, key): if len(default_dict) != 0: return default_dict else: - helper_logger.log_error("Error: No values for physical port '{}'".format(physical_port)) + helper_logger.log_info("Error: No values for physical port '{}'".format(physical_port)) return {} if key[0] in media_dict: @@ -740,58 +742,197 @@ def get_media_val_str(num_logical_ports, lane_dict, logical_idx): return media_val_str -def notify_media_setting(logical_port_name, transceiver_dict, - app_port_tbl, port_mapping): +""" + This function returns the NPU SI settings for a given logical port + + Args: + lport: + logical port name + transceiver_dict: + A dictionary containing the vendor specific transceiver information + port_mapping: + A PortMapping object + + Returns: + A dictionary containing the NPU SI settings for the logical port + Returns empty dictionary if the NPU SI settings are not found +""" +def get_npu_si_settings_dict(lport, transceiver_dict, port_mapping): if not g_dict: + return {} + + physical_port = port_mapping.get_logical_to_physical(lport)[0] + if not _wrapper_get_presence(physical_port): + helper_logger.log_info("Get NPU SI settings: Port {} presence not detected".format(physical_port)) + return {} + + if physical_port not in transceiver_dict: + helper_logger.log_error("Get NPU SI settings: Port {} eeprom not populated in transceiver dict".format(physical_port)) + return {} + + key = get_media_settings_key(physical_port, transceiver_dict) + media_dict = get_media_settings_value(physical_port, key) + if len(media_dict) == 0: + helper_logger.log_debug("Get NPU SI settings: Error in obtaining media setting for {}".format(lport)) + return {} + else: + return media_dict + +""" + This function updates the NPU SI settings for a given logical port to APPL_DB + It then sets the value of NPU_SI_SETTINGS_SYNC_STATUS to NPU_SI_SETTINGS_NOTIFIED + + Args: + lport: + logical port name + transceiver_dict: + A dictionary containing the vendor specific transceiver information + xcvr_table_helper: + A XcvrTableHelper object + port_mapping: + A PortMapping object +""" +def post_npu_si_settings_to_appl_db(lport, transceiver_dict, + xcvr_table_helper, port_mapping): + media_dict = get_npu_si_settings_dict(lport, transceiver_dict, port_mapping) + if len(media_dict) == 0: + helper_logger.log_error("NPI SI settings post: Media dict is empty for {}".format(lport)) return - ganged_port = False - ganged_member_num = 1 + asic_index = port_mapping.get_asic_id_for_logical_port(lport) + physical_port = port_mapping.get_logical_to_physical(lport)[0] + logical_port_list = port_mapping.get_physical_to_logical(physical_port) + num_logical_ports = len(logical_port_list) + logical_idx = logical_port_list.index(lport) - physical_port_list = port_mapping.logical_port_name_to_physical_port_list(logical_port_name) - if physical_port_list is None: - helper_logger.log_error("Error: No physical ports found for logical port '{}'".format(logical_port_name)) - return PHYSICAL_PORT_NOT_EXIST + fvs = swsscommon.FieldValuePairs(len(media_dict)) - if len(physical_port_list) > 1: - ganged_port = True + index = 0 + for media_key in media_dict: + if type(media_dict[media_key]) is dict: + media_val_str = get_media_val_str(num_logical_ports, + media_dict[media_key], + logical_idx) + else: + media_val_str = media_dict[media_key] + fvs[index] = (str(media_key), str(media_val_str)) + index += 1 - for physical_port in physical_port_list: - logical_port_list = port_mapping.get_physical_to_logical(physical_port) - num_logical_ports = len(logical_port_list) - logical_idx = logical_port_list.index(logical_port_name) - if not _wrapper_get_presence(physical_port): - helper_logger.log_info("Media {} presence not detected during notify".format(physical_port)) - continue - if physical_port not in transceiver_dict: - helper_logger.log_error("Media {} eeprom not populated in transceiver dict".format(physical_port)) - continue + xcvr_table_helper.get_app_port_tbl(asic_index).set(lport, fvs) + app_port_table_non_producer = xcvr_table_helper.get_non_producer_app_port_tbl(asic_index) + app_port_table_non_producer.set(app_port_table_non_producer.APPL_DB, 'PORT_TABLE:{}'.format(lport), "NPU_SI_SETTINGS_SYNC_STATUS", "NPU_SI_SETTINGS_NOTIFIED") + helper_logger.log_notice("{}: Posted NPU settings to APPL_DB".format(lport)) - port_name = get_physical_port_name(logical_port_name, - ganged_member_num, ganged_port) - ganged_member_num += 1 - key = get_media_settings_key(physical_port, transceiver_dict) - media_dict = get_media_settings_value(physical_port, key) +""" + Checks if the module is CMIS SM driven - if len(media_dict) == 0: - helper_logger.log_error("Error in obtaining media setting for {}".format(logical_port_name)) - return + Args: + port_mapping: + A PortMapping object + lport: + logical port name - fvs = swsscommon.FieldValuePairs(len(media_dict)) + Returns: + True if the module is CMIS SM driven + False otherwise +""" +def is_module_cmis_sm_driven(port_mapping, lport): + pport = port_mapping.get_logical_to_physical(lport)[0] - index = 0 - for media_key in media_dict: - if type(media_dict[media_key]) is dict: - media_val_str = get_media_val_str(num_logical_ports, - media_dict[media_key], - logical_idx) - else: - media_val_str = media_dict[media_key] - fvs[index] = (str(media_key), str(media_val_str)) - index += 1 + # double-check the HW presence before moving forward + sfp = platform_chassis.get_sfp(pport) + if not sfp.get_presence(): + return False + + try: + # Skip if XcvrApi is not supported + api = sfp.get_xcvr_api() + if api is None: + helper_logger.log_error("{}: No xcvr api found while checking for module CMIS SM driven!".format(lport)) + return False + + if api.is_flat_memory(): + helper_logger.log_notice("{}: Flat memory module found while checking for module CMIS SM driven!".format(lport)) + return False + + type = api.get_module_type_abbreviation() + if (type is None) or (type not in CMIS_MODULE_TYPES): + helper_logger.log_notice("{}: Module type {} not supported for module CMIS SM driven!".format(lport, type)) + return False + + except AttributeError: + # Return False if these essential routines are not available + return False + + return True + +""" + Retrieves the value of a key from APP_DB PORT_TABLE for a given logical port + + Args: + xcvr_table_helper: + A XcvrTableHelper object + port_mapping: + A PortMapping object + lport: + logical port name + key: + key for the corresponding value to be retrieved + + Returns: + The value of the key if the key is found in APP_PORT_TABLE + None otherwise +""" +def get_app_port_table_val_by_key(xcvr_table_helper, port_mapping, lport, key): + if xcvr_table_helper is None: + helper_logger.log_error("xcvr_table_helper is None while getting " + "app port table value by key for lport {}".format(lport)) + return None + + if port_mapping is None: + helper_logger.log_error("port_mapping is None while getting " + "app port table value by key for lport {}".format(lport)) + return None + + asic_index = port_mapping.get_asic_id_for_logical_port(lport) + appl_db = xcvr_table_helper.get_non_producer_app_port_tbl(asic_index) + if appl_db is None: + helper_logger.log_error("appl_db is None for lport {} while getting app port table value by key".format(lport)) + return None + + app_port_table_fvs_dict = appl_db.get_all(appl_db.APPL_DB, 'PORT_TABLE:{}'.format(lport)) + if (app_port_table_fvs_dict is None) or (key not in app_port_table_fvs_dict): + helper_logger.log_error("Unable to find key {} from APP_PORT_TABLE for lport {}".format(key, lport)) + return None + + return app_port_table_fvs_dict[key] - app_port_tbl.set(port_name, fvs) +""" + Checks if NPU SI settings update is required for a module + + Args: + lport: + logical port name + xcvr_table_helper: + A XcvrTableHelper object + port_mapping: + A PortMapping object + transceiver_dict: + A dictionary containing the vendor specific transceiver information + Returns true if both the below conditions are true + - npu_si_settings_sync_val == "NPU_SI_SETTINGS_DEFAULT" + - NPU SI settings are available for the module + Returns false otherwise +""" +def is_npu_si_settings_update_required(lport, xcvr_table_helper, port_mapping, transceiver_dict): + npu_si_settings_sync_val = get_app_port_table_val_by_key( + xcvr_table_helper, port_mapping, lport, "NPU_SI_SETTINGS_SYNC_STATUS") + if npu_si_settings_sync_val is None: + helper_logger.log_error("NPU SI settings update required: npu_si_settings_sync_val is None for {}".format(lport)) + return False + return (npu_si_settings_sync_val == "NPU_SI_SETTINGS_DEFAULT") and \ + (get_npu_si_settings_dict(lport, transceiver_dict, port_mapping) != {}) def waiting_time_compensation_with_sleep(time_start, time_to_wait): time_now = time.time() @@ -871,13 +1012,14 @@ class CmisManagerTask(threading.Thread): CMIS_MAX_RETRIES = 3 CMIS_DEF_EXPIRED = 60 # seconds, default expiration time - CMIS_MODULE_TYPES = ['QSFP-DD', 'QSFP_DD', 'OSFP', 'QSFP+C'] CMIS_MAX_HOST_LANES = 8 + CMIS_NPU_SI_SETTINGS_WAIT_DURATION = 5 # seconds CMIS_STATE_UNKNOWN = 'UNKNOWN' CMIS_STATE_INSERTED = 'INSERTED' CMIS_STATE_DP_DEINIT = 'DP_DEINIT' CMIS_STATE_AP_CONF = 'AP_CONFIGURED' + CMIS_STATE_NPU_SI_SETTINGS_WAIT = 'NPU_SI_SETTINGS_WAIT' CMIS_STATE_DP_ACTIVATE = 'DP_ACTIVATION' CMIS_STATE_DP_INIT = 'DP_INIT' CMIS_STATE_DP_TXON = 'DP_TXON' @@ -1366,34 +1508,70 @@ def post_port_active_apsel_to_db(self, api, lport, host_lanes_mask): intf_tbl.set(lport, fvs) self.log_notice("{}: updated TRANSCEIVER_INFO_TABLE {}".format(lport, tuple_list)) - def wait_for_port_config_done(self, namespace): - # Connect to APPL_DB and subscribe to PORT table notifications - appl_db = daemon_base.db_connect("APPL_DB", namespace=namespace) + """ + Creates a dictionary of various fields of transceiver specific information + which will be used to find the NPU SI settings for the port. + + Args: + pport: + Integer, physical port index + lport: + String, logical port name + api: + XcvrApi object + + Returns: + Dictionary, a dictionary of various fields of transceiver specific information + which will be used to find the NPU SI settings for the port. + """ + def get_transceiver_info_dict_for_npu_si_settings(self, pport, lport, api): + transceiver_info_dict = {} + if api is None: + helper_logger.log_error("Module {} api is None".format(lport)) + return None - sel = swsscommon.Select() - port_tbl = swsscommon.SubscriberStateTable(appl_db, swsscommon.APP_PORT_TABLE_NAME) - sel.addSelectable(port_tbl) + vendor_name = api.get_manufacturer() + if vendor_name is None: + helper_logger.log_info("Module {} vendor name not found".format(lport)) + return None - # Make sure this daemon started after all port configured - while not self.task_stopping_event.is_set(): - (state, c) = sel.select(port_mapping.SELECT_TIMEOUT_MSECS) - if state == swsscommon.Select.TIMEOUT: - continue - if state != swsscommon.Select.OBJECT: - self.log_warning("sel.select() did not return swsscommon.Select.OBJECT") - continue + model_name = api.get_model() + if model_name is None: + helper_logger.log_info("Module {} model name not found".format(lport)) + return None - (key, op, fvp) = port_tbl.pop() - if key in ["PortConfigDone", "PortInitDone"]: - break + media_type = api.get_module_media_type() + if media_type is None: + helper_logger.log_info("Module {} media type not found".format(lport)) + return None + + cable_type = api.get_cable_type() + if cable_type is None: + helper_logger.log_info("Module {} cable type not found".format(lport)) + return None + + cable_length = api.get_cable_length() + if cable_length is None: + helper_logger.log_info("Module {} cable length not found".format(lport)) + return None + + type_abbrv_name = api.get_module_type_abbreviation() + if type_abbrv_name is None: + helper_logger.log_info("Module {} type abbrv name not found".format(lport)) + return None + + transceiver_info_dict[pport] = {"manufacturer": vendor_name, + "model": model_name, + "cable_type": cable_type, + "cable_length": cable_length, + "specification_compliance": media_type, + "type_abbrv_name": type_abbrv_name} + + return transceiver_info_dict def task_worker(self): self.xcvr_table_helper = XcvrTableHelper(self.namespaces) - self.log_notice("Waiting for PortConfigDone...") - for namespace in self.namespaces: - self.wait_for_port_config_done(namespace) - # APPL_DB for CONFIG updates, and STATE_DB for insertion/removal sel, asic_context = port_mapping.subscribe_port_update_event(self.namespaces, helper_logger) while not self.task_stopping_event.is_set(): @@ -1416,6 +1594,16 @@ def task_worker(self): self.CMIS_STATE_FAILED, self.CMIS_STATE_READY, self.CMIS_STATE_REMOVED]: + asic_id = self.port_mapping.get_asic_id_for_logical_port(lport) + cmis_reinit_rqd_val = get_app_port_table_val_by_key( + self.xcvr_table_helper, self.port_mapping, lport, "CMIS_REINIT_REQUIRED") + if cmis_reinit_rqd_val is None: + self.log_error("{}: CMIS terminal states: cmis_reinit_rqd_val is None".format(lport)) + else: + if cmis_reinit_rqd_val == "true": + app_port_table = self.xcvr_table_helper.get_non_producer_app_port_tbl(asic_id) + app_port_table.set(app_port_table.APPL_DB, 'PORT_TABLE:{}'.format(lport), "CMIS_REINIT_REQUIRED", "false") + self.log_notice("{}: CMIS terminal states: CMIS_REINIT_REQUIRED set to false".format(lport)) if state != self.CMIS_STATE_READY: self.port_dict[lport]['appl'] = 0 self.port_dict[lport]['host_lanes_mask'] = 0 @@ -1454,15 +1642,8 @@ def task_worker(self): self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_READY continue - # Skip if it's not a paged memory device - if api.is_flat_memory(): - self.log_notice("{}: skipping CMIS state machine for flat memory xcvr".format(lport)) - self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_READY - continue - - # Skip if it's not a CMIS module - type = api.get_module_type_abbreviation() - if (type is None) or (type not in self.CMIS_MODULE_TYPES): + if not is_module_cmis_sm_driven(self.port_mapping, lport): + self.log_notice("{}: Skipping CMIS state machine as module not CMIS SM driven".format(lport)) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_READY continue @@ -1492,9 +1673,9 @@ def task_worker(self): self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_FAILED continue - self.log_notice("{}: {}G, lanemask=0x{:x}, state={}, appl {} host_lane_count {} " - "retries={}".format(lport, int(speed/1000), host_lanes_mask, - state, appl, host_lane_count, retries)) + self.log_notice("{}: {}G, lanemask=0x{:x}, CMIS state={}, Module state={}, DP state={}, appl {} host_lane_count {} " + "retries={}".format(lport, int(speed/1000), host_lanes_mask, state, + api.get_module_state(), api.get_datapath_state(), appl, host_lane_count, retries)) if retries > self.CMIS_MAX_RETRIES: self.log_error("{}: FAILED".format(lport)) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_FAILED @@ -1565,6 +1746,14 @@ def task_worker(self): if 0 != freq and freq != api.get_laser_config_freq(): need_update = True + cmis_reinit_rqd_val = get_app_port_table_val_by_key( + self.xcvr_table_helper, self.port_mapping, lport, "CMIS_REINIT_REQUIRED") + if cmis_reinit_rqd_val is None: + self.log_error("{}: cmis_reinit_rqd_val is None".format(lport)) + elif cmis_reinit_rqd_val == "true": + need_update = True + self.log_notice("{}: Forcing CMIS reinit as CMIS_REINIT_REQUIRED is true".format(lport)) + if not need_update: # No application updates self.log_notice("{}: no CMIS application update required...READY".format(lport)) @@ -1637,6 +1826,15 @@ def task_worker(self): # Set Explicit control bit to apply Custom Host SI settings ec = 1 + npu_si_settings_wait_required = False + transceiver_info_for_npu_si_settings = self.get_transceiver_info_dict_for_npu_si_settings(pport, lport, api) + if transceiver_info_for_npu_si_settings is not None: + if is_npu_si_settings_update_required(lport, self.xcvr_table_helper, self.port_mapping, transceiver_info_for_npu_si_settings): + post_npu_si_settings_to_appl_db(lport, transceiver_info_for_npu_si_settings, self.xcvr_table_helper, self.port_mapping) + npu_si_settings_wait_required = True + else: + self.log_error("{}: transceiver_info_for_npu_si_settings is None in CMIS_STATE_AP_CONF state".format(lport)) + # D.1.3 Software Configuration and Initialization api.set_application(host_lanes_mask, appl, ec) if not api.scs_apply_datapath_init(host_lanes_mask): @@ -1644,7 +1842,25 @@ def task_worker(self): self.force_cmis_reinit(lport, retries + 1) continue - self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_INIT + if npu_si_settings_wait_required: + self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_NPU_SI_SETTINGS_WAIT + self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds = self.CMIS_NPU_SI_SETTINGS_WAIT_DURATION) + else: + self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_INIT + elif state == self.CMIS_STATE_NPU_SI_SETTINGS_WAIT: + npu_si_settings_sync_val = get_app_port_table_val_by_key(self.xcvr_table_helper, self.port_mapping, lport, "NPU_SI_SETTINGS_SYNC_STATUS") + if npu_si_settings_sync_val is None: + self.log_error("{}: npu_si_settings_sync_val is None in CMIS_STATE_NPU_SI_SETTINGS_WAIT state".format(lport)) + self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_FAILED + continue + if npu_si_settings_sync_val == "NPU_SI_SETTINGS_DONE": + self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_INIT + self.log_notice("{}: NPU SI settings is done now".format(lport)) + else: + if (expired is not None) and (expired <= now): + self.log_notice("{}: timeout for CMIS_STATE_NPU_SI_SETTINGS_WAIT".format(lport)) + self.force_cmis_reinit(lport, retries + 1) + continue elif state == self.CMIS_STATE_DP_INIT: if not self.check_config_error(api, host_lanes_mask, ['ConfigSuccess']): if (expired is not None) and (expired <= now): @@ -1696,6 +1912,20 @@ def task_worker(self): self.log_notice("{}: READY".format(lport)) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_READY + + # Setting CMIS_REINIT_REQUIRED to false to avoid CMIS re-init caused by TRANSCEIVER_INFO table + # update through post_port_active_apsel_to_db + asic_id = self.port_mapping.get_asic_id_for_logical_port(lport) + cmis_reinit_rqd_val = get_app_port_table_val_by_key( + self.xcvr_table_helper, self.port_mapping, lport, "CMIS_REINIT_REQUIRED") + if cmis_reinit_rqd_val is None: + self.log_error("{}: CMIS_STATE_DP_ACTIVATE state: cmis_reinit_rqd_val is None".format(lport)) + else: + if cmis_reinit_rqd_val == "true": + app_port_table = self.xcvr_table_helper.get_non_producer_app_port_tbl(asic_id) + app_port_table.set(app_port_table.APPL_DB, 'PORT_TABLE:{}'.format(lport), "CMIS_REINIT_REQUIRED", "false") + self.log_notice("{}: CMIS_STATE_DP_ACTIVATE state: CMIS_REINIT_REQUIRED set to false".format(lport)) + self.post_port_active_apsel_to_db(api, lport, host_lanes_mask) except (NotImplementedError, AttributeError) as e: @@ -1897,11 +2127,6 @@ def _post_port_sfp_info_and_dom_thr_to_db_once(self, port_mapping, xcvr_table_he transceiver_dict = {} retry_eeprom_set = set() - warmstart = swsscommon.WarmStart() - warmstart.initialize("xcvrd", "pmon") - warmstart.checkWarmStart("xcvrd", "pmon", False) - is_warm_start = warmstart.isWarmStart() - # Post all the current interface sfp/dom threshold info to STATE_DB logical_port_list = port_mapping.logical_port_list for logical_port_name in logical_port_list: @@ -1917,10 +2142,10 @@ def _post_port_sfp_info_and_dom_thr_to_db_once(self, port_mapping, xcvr_table_he if rc != SFP_EEPROM_NOT_READY: post_port_dom_threshold_info_to_db(logical_port_name, port_mapping, xcvr_table_helper.get_dom_threshold_tbl(asic_index), stop_event) - # Do not notify media settings during warm reboot to avoid dataplane traffic impact - if is_warm_start == False: - notify_media_setting(logical_port_name, transceiver_dict, xcvr_table_helper.get_app_port_tbl(asic_index), port_mapping) - transceiver_dict.clear() + if not is_module_cmis_sm_driven(port_mapping, logical_port_name) and \ + is_npu_si_settings_update_required(logical_port_name, xcvr_table_helper, port_mapping, transceiver_dict): + post_npu_si_settings_to_appl_db(logical_port_name, transceiver_dict, xcvr_table_helper, port_mapping) + transceiver_dict.clear() else: retry_eeprom_set.add(logical_port_name) @@ -2141,10 +2366,14 @@ def task_worker(self, stopping_event, sfp_error_event): if rc != SFP_EEPROM_NOT_READY: post_port_dom_threshold_info_to_db(logical_port, self.port_mapping, self.xcvr_table_helper.get_dom_threshold_tbl(asic_index)) - notify_media_setting(logical_port, transceiver_dict, self.xcvr_table_helper.get_app_port_tbl(asic_index), self.port_mapping) + if not is_module_cmis_sm_driven(self.port_mapping, logical_port) and \ + is_npu_si_settings_update_required(logical_port, self.xcvr_table_helper, self.port_mapping, transceiver_dict): + post_npu_si_settings_to_appl_db(logical_port, transceiver_dict, self.xcvr_table_helper, self.port_mapping) transceiver_dict.clear() elif value == sfp_status_helper.SFP_STATUS_REMOVED: helper_logger.log_notice("{}: Got SFP removed event".format(logical_port)) + appl_db = self.xcvr_table_helper.get_non_producer_app_port_tbl(asic_index) + appl_db.set(appl_db.APPL_DB, 'PORT_TABLE:{}'.format(logical_port), "NPU_SI_SETTINGS_SYNC_STATUS", "NPU_SI_SETTINGS_DEFAULT") update_port_transceiver_status_table_sw( logical_port, self.xcvr_table_helper.get_status_tbl(asic_index), sfp_status_helper.SFP_STATUS_REMOVED) helper_logger.log_notice("{}: received plug out and update port sfp status table.".format(logical_port)) @@ -2340,7 +2569,9 @@ def on_add_logical_port(self, port_change_event): self.retry_eeprom_set.add(port_change_event.port_name) else: post_port_dom_threshold_info_to_db(port_change_event.port_name, self.port_mapping, dom_threshold_tbl) - notify_media_setting(port_change_event.port_name, transceiver_dict, self.xcvr_table_helper.get_app_port_tbl(port_change_event.asic_id), self.port_mapping) + if not is_module_cmis_sm_driven(self.port_mapping, port_change_event.port_name) and \ + is_npu_si_settings_update_required(port_change_event.port_name, self.xcvr_table_helper, self.port_mapping, transceiver_dict): + post_npu_si_settings_to_appl_db(port_change_event.port_name, transceiver_dict, self.xcvr_table_helper, self.port_mapping) else: status = sfp_status_helper.SFP_STATUS_REMOVED if not status else status update_port_transceiver_status_table_sw(port_change_event.port_name, status_tbl, status, error_description) @@ -2366,7 +2597,9 @@ def retry_eeprom_reading(self): rc = post_port_sfp_info_to_db(logical_port, self.port_mapping, self.xcvr_table_helper.get_intf_tbl(asic_index), transceiver_dict) if rc != SFP_EEPROM_NOT_READY: post_port_dom_threshold_info_to_db(logical_port, self.port_mapping, self.xcvr_table_helper.get_dom_threshold_tbl(asic_index)) - notify_media_setting(logical_port, transceiver_dict, self.xcvr_table_helper.get_app_port_tbl(asic_index), self.port_mapping) + if not is_module_cmis_sm_driven(self.port_mapping, logical_port) and \ + is_npu_si_settings_update_required(logical_port, self.xcvr_table_helper, self.port_mapping, transceiver_dict): + post_npu_si_settings_to_appl_db(logical_port, transceiver_dict, self.xcvr_table_helper, self.port_mapping) transceiver_dict.clear() retry_success_set.add(logical_port) # Update retry EEPROM set @@ -2421,6 +2654,7 @@ def wait_for_port_config_done(self, namespace): (key, op, fvp) = port_tbl.pop() if key in ["PortConfigDone", "PortInitDone"]: break + sel.removeSelectable(port_tbl) def load_media_settings(self): global g_dict @@ -2434,6 +2668,43 @@ def load_media_settings(self): with open(media_settings_file_path, "r") as media_file: g_dict = json.load(media_file) + """ + Initializes CMIS_REINIT_REQUIRED and NPU_SI_SETTINGS_SYNC_STATUS fields in APPL_DB:PORT_TABLE + if not already present for a port. + """ + def initialize_port_init_fields_in_port_table(self, port_mapping_data): + logical_port_list = port_mapping_data.logical_port_list + for lport in logical_port_list: + asic_index = port_mapping_data.get_asic_id_for_logical_port(lport) + appl_db = self.xcvr_table_helper.get_non_producer_app_port_tbl(asic_index) + if appl_db is None: + helper_logger.log_error("appl_db is None for lport {} during init".format(lport)) + continue + + app_port_table_fvs_dict = appl_db.get_all(appl_db.APPL_DB, 'PORT_TABLE:{}'.format(lport)) + if "CMIS_REINIT_REQUIRED" not in app_port_table_fvs_dict: + appl_db.set(appl_db.APPL_DB, 'PORT_TABLE:{}'.format(lport), "CMIS_REINIT_REQUIRED", "true") + self.log_notice("Added CMIS_REINIT_REQUIRED for lport {}".format(lport)) + if "NPU_SI_SETTINGS_SYNC_STATUS" not in app_port_table_fvs_dict: + appl_db.set(appl_db.APPL_DB, 'PORT_TABLE:{}'.format(lport), "NPU_SI_SETTINGS_SYNC_STATUS", "NPU_SI_SETTINGS_DEFAULT") + self.log_notice("Added NPU_SI_SETTINGS_SYNC_STATUS for lport {}".format(lport)) + + self.log_notice("XCVRD INIT: Port init fields initialized in APP_PORT_TABLE") + + """ + Subscribes APPL_DB PORT_TABLE table events + """ + def subscribe_appl_port_table(self): + sel = swsscommon.Select() + asic_context = {} + for namespace in self.namespaces: + appl_db = daemon_base.db_connect("APPL_DB", namespace=namespace) + appl_port_tbl = swsscommon.SubscriberStateTable(appl_db, swsscommon.APP_PORT_TABLE_NAME) + asic_context[appl_port_tbl] = multi_asic.get_asic_index_from_namespace(namespace) + sel.addSelectable(appl_port_tbl) + + return sel, asic_context + # Initialize daemon def init(self): global platform_sfputil @@ -2463,6 +2734,12 @@ def init(self): except Exception as e: self.log_error("Failed to load sfputil: {}".format(str(e)), True) sys.exit(SFPUTIL_LOAD_ERROR) + # force main thread to cause underlying initialization of SFPs for platforms waiting to dynamically do so, + # since signal handlers can only be installed from within the context of the main thread + try: + platform_chassis.get_num_sfps() + except NotImplementedError: + pass if multi_asic.is_multi_asic(): # Load the namespace details first from the database_global.json file. @@ -2486,7 +2763,11 @@ def init(self): self.wait_for_port_config_done(namespace) self.log_notice("XCVRD INIT: After port config is done") - return port_mapping.get_port_mapping(self.namespaces) + port_mapping_data = port_mapping.get_port_mapping(self.namespaces) + + self.initialize_port_init_fields_in_port_table(port_mapping_data) + + return port_mapping_data # Deinitialize daemon def deinit(self): @@ -2542,9 +2823,34 @@ def run(self): for thread in self.threads: self.log_notice("Started thread {}".format(thread.getName())) - self.stop_event.wait() + sel, asic_context = self.subscribe_appl_port_table() + self.log_notice("Subscribed to appl {} update".format(swsscommon.APP_PORT_TABLE_NAME)) - self.log_info("Stop daemon main loop") + generate_sigabrt = False + + while not self.stop_event.is_set() and not generate_sigabrt: + (state, _) = sel.select(port_mapping.SELECT_TIMEOUT_MSECS) + if state == swsscommon.Select.TIMEOUT: + continue + if state != swsscommon.Select.OBJECT: + self.log_warning("sel.select() did not return swsscommon.Select.OBJECT") + continue + + for port_tbl in asic_context.keys(): + while True: + (key, op, _) = port_tbl.pop() + if not key: + break + self.log_info("APPL_DB subscriber: Received {} op for {} key".format(op, key)) + if op == swsscommon.DEL_COMMAND and "Ethernet" in key: + self.log_notice("DEL_COMMAND received for {}:{} key. Starting graceful restart to xcvrd!".format(swsscommon.APP_PORT_TABLE_NAME, key)) + generate_sigabrt = True + break + + if generate_sigabrt is True: + break + + self.log_notice("Stop daemon main loop") generate_sigkill = False # check all threads are alive @@ -2581,6 +2887,9 @@ def run(self): if self.sfp_error_event.is_set(): sys.exit(SFP_SYSTEM_ERROR) + elif generate_sigabrt is True: + self.log_notice("Sending SIGABRT to xcvrd!") + os.kill(os.getpid(), signal.SIGABRT) class XcvrTableHelper: @@ -2589,6 +2898,7 @@ def __init__(self, namespaces): self.cfg_port_tbl, self.state_port_tbl, self.pm_tbl = {}, {}, {}, {}, {}, {}, {}, {} self.state_db = {} self.cfg_db = {} + self.non_producer_app_port_tbl = {} for namespace in namespaces: asic_id = multi_asic.get_asic_index_from_namespace(namespace) self.state_db[asic_id] = daemon_base.db_connect("STATE_DB", namespace) @@ -2600,6 +2910,8 @@ def __init__(self, namespaces): self.state_port_tbl[asic_id] = swsscommon.Table(self.state_db[asic_id], swsscommon.STATE_PORT_TABLE_NAME) appl_db = daemon_base.db_connect("APPL_DB", namespace) self.app_port_tbl[asic_id] = swsscommon.ProducerStateTable(appl_db, swsscommon.APP_PORT_TABLE_NAME) + self.non_producer_app_port_tbl[asic_id] = swsscommon.SonicV2Connector(use_unix_socket_path=False, namespace=namespace) + self.non_producer_app_port_tbl[asic_id].connect(self.non_producer_app_port_tbl[asic_id].APPL_DB) self.cfg_db[asic_id] = daemon_base.db_connect("CONFIG_DB", namespace) self.cfg_port_tbl[asic_id] = swsscommon.Table(self.cfg_db[asic_id], swsscommon.CFG_PORT_TABLE_NAME) @@ -2621,6 +2933,9 @@ def get_pm_tbl(self, asic_id): def get_app_port_tbl(self, asic_id): return self.app_port_tbl[asic_id] + def get_non_producer_app_port_tbl(self, asic_id): + return self.non_producer_app_port_tbl[asic_id] + def get_state_db(self, asic_id): return self.state_db[asic_id] From c9c4a0d18cfe8234c2976f281164dbed1420bad1 Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Tue, 14 Nov 2023 23:23:44 +0000 Subject: [PATCH 2/7] Addressed review comments --- sonic-xcvrd/xcvrd/xcvrd.py | 64 ++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 701a19f76..93eb33493 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -28,6 +28,9 @@ from .xcvrd_utilities import sfp_status_helper from .xcvrd_utilities import port_mapping from .xcvrd_utilities import optics_si_parser + + from sonic_platform_base.sonic_xcvr.api.public.cmis import CmisApi + except ImportError as e: raise ImportError(str(e) + " - required module not found") @@ -855,10 +858,7 @@ def is_module_cmis_sm_driven(port_mapping, lport): helper_logger.log_notice("{}: Flat memory module found while checking for module CMIS SM driven!".format(lport)) return False - type = api.get_module_type_abbreviation() - if (type is None) or (type not in CMIS_MODULE_TYPES): - helper_logger.log_notice("{}: Module type {} not supported for module CMIS SM driven!".format(lport, type)) - return False + return (isinstance(api, CmisApi)) except AttributeError: # Return False if these essential routines are not available @@ -2633,18 +2633,35 @@ def signal_handler(self, sig, frame): else: self.log_warning("Caught unhandled signal '" + sig + "'") + """ + Subscribes APPL_DB PORT_TABLE table events + """ + def subscribe_appl_port_table(self): + sel = swsscommon.Select() + asic_context = {} + for namespace in self.namespaces: + appl_db = daemon_base.db_connect("APPL_DB", namespace=namespace) + appl_port_tbl = swsscommon.SubscriberStateTable(appl_db, swsscommon.APP_PORT_TABLE_NAME) + asic_context[appl_port_tbl] = multi_asic.get_asic_index_from_namespace(namespace) + sel.addSelectable(appl_port_tbl) + + return sel, asic_context + # Wait for port config is done def wait_for_port_config_done(self, namespace): - # Connect to APPL_DB and subscribe to PORT table notifications - appl_db = daemon_base.db_connect("APPL_DB", namespace=namespace) - - sel = swsscommon.Select() - port_tbl = swsscommon.SubscriberStateTable(appl_db, swsscommon.APP_PORT_TABLE_NAME) - sel.addSelectable(port_tbl) + input_asic_id = multi_asic.get_asic_index_from_namespace(namespace) + port_tbl = None + for current_port_tble, asic_id in self.asic_context.items(): + if asic_id == input_asic_id: + port_tbl = current_port_tble + break + if port_tbl is None: + self.log_error("Failed to find port table for asic {}".format(input_asic_id)) + return # Make sure this daemon started after all port configured while not self.stop_event.is_set(): - (state, c) = sel.select(port_mapping.SELECT_TIMEOUT_MSECS) + (state, c) = self.sel.select(port_mapping.SELECT_TIMEOUT_MSECS) if state == swsscommon.Select.TIMEOUT: continue if state != swsscommon.Select.OBJECT: @@ -2654,7 +2671,6 @@ def wait_for_port_config_done(self, namespace): (key, op, fvp) = port_tbl.pop() if key in ["PortConfigDone", "PortInitDone"]: break - sel.removeSelectable(port_tbl) def load_media_settings(self): global g_dict @@ -2691,20 +2707,6 @@ def initialize_port_init_fields_in_port_table(self, port_mapping_data): self.log_notice("XCVRD INIT: Port init fields initialized in APP_PORT_TABLE") - """ - Subscribes APPL_DB PORT_TABLE table events - """ - def subscribe_appl_port_table(self): - sel = swsscommon.Select() - asic_context = {} - for namespace in self.namespaces: - appl_db = daemon_base.db_connect("APPL_DB", namespace=namespace) - appl_port_tbl = swsscommon.SubscriberStateTable(appl_db, swsscommon.APP_PORT_TABLE_NAME) - asic_context[appl_port_tbl] = multi_asic.get_asic_index_from_namespace(namespace) - sel.addSelectable(appl_port_tbl) - - return sel, asic_context - # Initialize daemon def init(self): global platform_sfputil @@ -2757,6 +2759,9 @@ def init(self): self.load_media_settings() optics_si_parser.load_optics_si_settings() + self.sel, self.asic_context = self.subscribe_appl_port_table() + self.log_notice("Subscribed to appl {} update".format(swsscommon.APP_PORT_TABLE_NAME)) + # Make sure this daemon started after all port configured self.log_notice("XCVRD INIT: Wait for port config is done") for namespace in self.namespaces: @@ -2823,20 +2828,17 @@ def run(self): for thread in self.threads: self.log_notice("Started thread {}".format(thread.getName())) - sel, asic_context = self.subscribe_appl_port_table() - self.log_notice("Subscribed to appl {} update".format(swsscommon.APP_PORT_TABLE_NAME)) - generate_sigabrt = False while not self.stop_event.is_set() and not generate_sigabrt: - (state, _) = sel.select(port_mapping.SELECT_TIMEOUT_MSECS) + (state, _) = self.sel.select(port_mapping.SELECT_TIMEOUT_MSECS) if state == swsscommon.Select.TIMEOUT: continue if state != swsscommon.Select.OBJECT: self.log_warning("sel.select() did not return swsscommon.Select.OBJECT") continue - for port_tbl in asic_context.keys(): + for port_tbl in self.asic_context.keys(): while True: (key, op, _) = port_tbl.pop() if not key: From 7f0fe6f856b75b8c1aa3f73ccb686a14a50c42ce Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Tue, 14 Nov 2023 23:29:00 +0000 Subject: [PATCH 3/7] Removed CMIS_MODULE_TYPES --- sonic-xcvrd/xcvrd/xcvrd.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 93eb33493..239aae7b0 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -51,8 +51,6 @@ TRANSCEIVER_STATUS_TABLE_SW_FIELDS = ["status", "error"] -CMIS_MODULE_TYPES = ['QSFP-DD', 'QSFP_DD', 'OSFP', 'QSFP+C'] - # Mgminit time required as per CMIS spec MGMT_INIT_TIME_DELAY_SECS = 2 From 28aecb03e5117c9984508987baf2c992a7344c92 Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Wed, 15 Nov 2023 06:12:51 +0000 Subject: [PATCH 4/7] Fixed test failures --- sonic-xcvrd/tests/test_xcvrd.py | 22 +++++++++------------- sonic-xcvrd/xcvrd/xcvrd.py | 4 ++-- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index d67398728..3b451f7bf 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -527,6 +527,7 @@ def test_is_module_cmis_sm_driven(self, mock_platform_chassis): mock_sfp.get_presence = MagicMock(return_value=True) mock_xcvr_api = MagicMock() + mock_xcvr_api.__class__ = CmisApi mock_sfp.get_xcvr_api = MagicMock(return_value=None) assert not is_module_cmis_sm_driven(port_mapping, "Ethernet0") @@ -535,13 +536,9 @@ def test_is_module_cmis_sm_driven(self, mock_platform_chassis): assert not is_module_cmis_sm_driven(port_mapping, "Ethernet0") mock_xcvr_api.is_flat_memory = MagicMock(return_value=False) - mock_xcvr_api.get_module_type_abbreviation = MagicMock(return_value='QSFP28') - assert not is_module_cmis_sm_driven(port_mapping, "Ethernet0") - - mock_xcvr_api.get_module_type_abbreviation = MagicMock(return_value='QSFP-DD') assert is_module_cmis_sm_driven(port_mapping, "Ethernet0") - mock_xcvr_api.get_module_type_abbreviation = MagicMock(side_effect=AttributeError) + mock_xcvr_api.is_flat_memory = MagicMock(side_effect=AttributeError) assert not is_module_cmis_sm_driven(port_mapping, "Ethernet0") def test_get_app_port_table_val_by_key(self): @@ -666,19 +663,18 @@ def test_get_port_mapping(self, mock_swsscommon_table): assert port_mapping.get_physical_to_logical(3) == None assert port_mapping.get_logical_to_physical('Ethernet-IB0') == None - @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) - @patch('swsscommon.swsscommon.Select.removeSelectable', MagicMock()) - @patch('swsscommon.swsscommon.SubscriberStateTable') - @patch('swsscommon.swsscommon.Select.select') - def test_DaemonXcvrd_wait_for_port_config_done(self, mock_select, mock_sub_table): + def test_DaemonXcvrd_wait_for_port_config_done(self): mock_selectable = MagicMock() mock_selectable.pop = MagicMock( side_effect=[('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('PortConfigDone', None, None)]) + mock_select = MagicMock() mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) - mock_sub_table.return_value = mock_selectable xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER) + xcvrd.asic_context[mock_selectable] = 0 + xcvrd.sel = MagicMock() + xcvrd.sel.select = mock_select xcvrd.wait_for_port_config_done('') - assert swsscommon.Select.select.call_count == 2 + assert xcvrd.sel.select.call_count == 2 def test_DaemonXcvrd_initialize_port_init_fields_in_port_table(self): port_mapping = PortMapping() @@ -1715,7 +1711,7 @@ def test_get_media_val_str(self): @patch('xcvrd.xcvrd.platform_chassis', MagicMock()) @patch('xcvrd.xcvrd.DaemonXcvrd.load_platform_util', MagicMock()) @patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs', MagicMock(return_value=('/tmp', None))) - @patch('swsscommon.swsscommon.WarmStart', MagicMock()) + @patch('xcvrd.xcvrd.DaemonXcvrd.subscribe_appl_port_table', MagicMock(return_value=(None, None))) @patch('xcvrd.xcvrd.DaemonXcvrd.wait_for_port_config_done', MagicMock()) def test_DaemonXcvrd_init_deinit_fastboot_enabled(self): xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER) diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 239aae7b0..0eea871ae 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -862,8 +862,6 @@ def is_module_cmis_sm_driven(port_mapping, lport): # Return False if these essential routines are not available return False - return True - """ Retrieves the value of a key from APP_DB PORT_TABLE for a given logical port @@ -2617,6 +2615,8 @@ def __init__(self, log_identifier, skip_cmis_mgr=False): self.skip_cmis_mgr = skip_cmis_mgr self.namespaces = [''] self.threads = [] + self.asic_context = {} + self.sel = None # Signal handler def signal_handler(self, sig, frame): From 3102609888d8aa7aa1c95d6faa25e7565ea1c8e9 Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Wed, 29 Nov 2023 22:34:27 +0000 Subject: [PATCH 5/7] Renamed get_cable_type to get_cable_length_type --- sonic-xcvrd/xcvrd/xcvrd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 0eea871ae..feb668e6f 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -1541,7 +1541,7 @@ def get_transceiver_info_dict_for_npu_si_settings(self, pport, lport, api): helper_logger.log_info("Module {} media type not found".format(lport)) return None - cable_type = api.get_cable_type() + cable_type = api.get_cable_length_type() if cable_type is None: helper_logger.log_info("Module {} cable type not found".format(lport)) return None From 695a5c6de2c4e4834f2ba4b2649b04a791cdd619 Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Thu, 30 Nov 2023 02:15:29 +0000 Subject: [PATCH 6/7] Improved code coverage --- sonic-xcvrd/tests/test_xcvrd.py | 116 ++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 3b451f7bf..36e212a75 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -663,6 +663,22 @@ def test_get_port_mapping(self, mock_swsscommon_table): assert port_mapping.get_physical_to_logical(3) == None assert port_mapping.get_logical_to_physical('Ethernet-IB0') == None + @patch('swsscommon.swsscommon.Select.addSelectable') + @patch('swsscommon.swsscommon.SubscriberStateTable') + @patch('swsscommon.swsscommon.Select.select') + def test_DaemonXcvrd_subscribe_appl_port_table(self, mock_select, mock_sub_table, mock_addSelectable): + mock_selectable = MagicMock() + mock_selectable.pop = MagicMock( + side_effect=[('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), (None, None, None)]) + mock_select = MagicMock() + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + mock_sub_table.return_value = mock_selectable + xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER) + xcvrd.sel = MagicMock() + xcvrd.sel.select = mock_select + xcvrd.subscribe_appl_port_table() + assert mock_addSelectable.call_count == 1 + def test_DaemonXcvrd_wait_for_port_config_done(self): mock_selectable = MagicMock() mock_selectable.pop = MagicMock( @@ -713,6 +729,35 @@ def test_DaemonXcvrd_run(self, mock_task_stop1, mock_task_stop2, mock_task_run1, assert mock_deinit.call_count == 1 assert mock_init.call_count == 1 + @patch('xcvrd.xcvrd.DaemonXcvrd.subscribe_appl_port_table', MagicMock(return_value = (None, None))) + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('os.kill') + @patch('xcvrd.xcvrd.DaemonXcvrd.init') + @patch('xcvrd.xcvrd.DaemonXcvrd.deinit') + @patch('xcvrd.xcvrd.DomInfoUpdateTask.start') + @patch('xcvrd.xcvrd.SfpStateUpdateTask.start') + @patch('xcvrd.xcvrd.DomInfoUpdateTask.join') + @patch('xcvrd.xcvrd.SfpStateUpdateTask.join') + def test_DaemonXcvrd_sigabrt(self, mock_task_stop1, mock_task_stop2, mock_task_run1, mock_task_run2, mock_deinit, mock_init, mock_os_kill): + mock_selectable = MagicMock() + mock_selectable.pop = MagicMock( + side_effect=[('Ethernet0', swsscommon.DEL_COMMAND, (('index', '1'), )), ('PortConfigDone', None, None)]) + mock_select = MagicMock() + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER) + xcvrd.asic_context[mock_selectable] = 0 + xcvrd.sel = MagicMock() + xcvrd.sel.select = mock_select + xcvrd.stop_event.is_set = MagicMock(return_value=False) + xcvrd.run() + assert mock_task_stop1.call_count == 1 + assert mock_task_stop2.call_count == 1 + assert mock_task_run1.call_count == 1 + assert mock_task_run2.call_count == 1 + assert mock_deinit.call_count == 1 + assert mock_init.call_count == 1 + assert mock_os_kill.call_count == 1 + @patch('xcvrd.xcvrd._wrapper_get_sfp_type', MagicMock(return_value='QSFP_DD')) def test_CmisManagerTask_handle_port_change_event(self): port_mapping = PortMapping() @@ -966,6 +1011,56 @@ def test_CmisManagerTask_post_port_active_apsel_to_db(self): ret = task.post_port_active_apsel_to_db(mock_xcvr_api, lport, host_lanes_mask) assert int_tbl.getKeys() == [] + def test_CmisManagerTask_get_transceiver_info_dict_for_npu_si_settings(self): + pport = 1 + expected_dict = {pport : {"manufacturer": "ABC", + "model": "M1234", + "cable_type": "Length Cable Assembly(m)", + "cable_length": "1.0", + "specification_compliance": "active_cable_media_interface", + "type_abbrv_name": "QSFP-DD"} + + } + port_mapping = PortMapping() + stop_event = threading.Event() + task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event) + + assert task.get_transceiver_info_dict_for_npu_si_settings(pport, "Ethernet0", None) == None + + mock_xcvr_api = MagicMock() + mock_xcvr_api.get_manufacturer = MagicMock(return_value=None) + assert task.get_transceiver_info_dict_for_npu_si_settings(pport, "Ethernet0", mock_xcvr_api) == None + assert mock_xcvr_api.get_manufacturer.call_count == 1 + + mock_xcvr_api.get_manufacturer = MagicMock(return_value='ABC') + mock_xcvr_api.get_model = MagicMock(return_value=None) + assert task.get_transceiver_info_dict_for_npu_si_settings(pport, "Ethernet0", mock_xcvr_api) == None + assert mock_xcvr_api.get_model.call_count == 1 + + mock_xcvr_api.get_model = MagicMock(return_value='M1234') + mock_xcvr_api.get_module_media_type = MagicMock(return_value=None) + assert task.get_transceiver_info_dict_for_npu_si_settings(pport, "Ethernet0", mock_xcvr_api) == None + assert mock_xcvr_api.get_module_media_type.call_count == 1 + + mock_xcvr_api.get_module_media_type = MagicMock(return_value='active_cable_media_interface') + mock_xcvr_api.get_cable_length_type = MagicMock(return_value=None) + assert task.get_transceiver_info_dict_for_npu_si_settings(pport, "Ethernet0", mock_xcvr_api) == None + assert mock_xcvr_api.get_cable_length_type.call_count == 1 + + mock_xcvr_api.get_cable_length_type = MagicMock(return_value='Length Cable Assembly(m)') + mock_xcvr_api.get_cable_length = MagicMock(return_value=None) + assert task.get_transceiver_info_dict_for_npu_si_settings(pport, "Ethernet0", mock_xcvr_api) == None + assert mock_xcvr_api.get_cable_length.call_count == 1 + + mock_xcvr_api.get_cable_length = MagicMock(return_value='1.0') + mock_xcvr_api.get_module_type_abbreviation = MagicMock(return_value=None) + assert task.get_transceiver_info_dict_for_npu_si_settings(pport, "Ethernet0", mock_xcvr_api) == None + assert mock_xcvr_api.get_module_type_abbreviation.call_count == 1 + + mock_xcvr_api.get_module_type_abbreviation = MagicMock(return_value='QSFP-DD') + assert task.get_transceiver_info_dict_for_npu_si_settings(pport, "Ethernet0", mock_xcvr_api) == expected_dict + + @patch('xcvrd.xcvrd.platform_chassis') @patch('xcvrd.xcvrd_utilities.port_mapping.subscribe_port_update_event', MagicMock(return_value=(None, None))) @patch('xcvrd.xcvrd_utilities.port_mapping.handle_port_update_event', MagicMock()) @@ -1708,6 +1803,27 @@ def test_get_media_val_str(self): media_str = get_media_val_str(num_logical_ports, lane_dict, logical_idx) assert media_str == '3,4' + @patch('xcvrd.xcvrd.g_dict', media_settings_dict) + @patch('xcvrd.xcvrd.get_media_settings_key', MagicMock()) + @patch('xcvrd.xcvrd.get_media_settings_value', MagicMock(return_value={})) + @patch('xcvrd.xcvrd._wrapper_get_presence') + def test_get_npu_si_settings_dict_failure_cases(self, wrapper_get_presence): + transceiver_dict = {0: {}} + port_mapping = PortMapping() + port_mapping.get_logical_to_physical = MagicMock(return_value=[0]) + + # case: When _wrapper_get_presence returns False + wrapper_get_presence.return_value = False + assert get_npu_si_settings_dict('Ethernet0', transceiver_dict, port_mapping) == {} + + wrapper_get_presence.return_value = True + # case: When physical port is not in transceiver_dict + assert get_npu_si_settings_dict('Ethernet0', dict(), port_mapping) == {} + + wrapper_get_presence.return_value = True + # case: When get_media_settings_value returns empty dictionary + assert get_npu_si_settings_dict('Ethernet0', transceiver_dict, port_mapping) == {} + @patch('xcvrd.xcvrd.platform_chassis', MagicMock()) @patch('xcvrd.xcvrd.DaemonXcvrd.load_platform_util', MagicMock()) @patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs', MagicMock(return_value=('/tmp', None))) From 1e945c2b91501aea1715763f82da36cbd6e2c382 Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Fri, 5 Jan 2024 02:00:48 +0000 Subject: [PATCH 7/7] Improved code coverage --- sonic-xcvrd/tests/test_xcvrd.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index fc9d1eacc..db1e7cd0a 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -667,7 +667,7 @@ def test_get_media_settings_value(self, media_settings_dict, port, key, expected result = media_settings_parser.get_media_settings_value(port, key) assert result == expected - @patch('xcvrd.xcvrd.g_dict', media_settings_dict) + @patch('xcvrd.xcvrd_utilities.media_settings_parser.g_dict', media_settings_dict) @patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=True)) @patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock()) @patch('xcvrd.xcvrd.XcvrTableHelper.get_cfg_port_tbl', MagicMock()) @@ -676,7 +676,7 @@ def test_get_media_settings_value(self, media_settings_dict, port, key, expected def test_post_npu_si_settings_to_appl_db(self): self._check_post_npu_si_settings_to_appl_db_setting(1) - @patch('xcvrd.xcvrd.g_dict', media_settings_with_comma_dict) + @patch('xcvrd.xcvrd_utilities.media_settings_parser.g_dict', media_settings_with_comma_dict) @patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=True)) @patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock()) @patch('xcvrd.xcvrd.XcvrTableHelper.get_cfg_port_tbl', MagicMock()) @@ -2036,9 +2036,10 @@ def test_get_media_val_str(self): media_str = get_media_val_str(num_logical_ports, lane_dict, logical_idx) assert media_str == '3,4' - @patch('xcvrd.xcvrd.g_dict', media_settings_dict) + @patch('xcvrd.xcvrd_utilities.media_settings_parser.g_dict', media_settings_dict) @patch('xcvrd.xcvrd_utilities.media_settings_parser.get_media_settings_key', MagicMock()) @patch('xcvrd.xcvrd_utilities.media_settings_parser.get_media_settings_value', MagicMock(return_value={})) + @patch('xcvrd.xcvrd_utilities.media_settings_parser.get_speed_and_lane_count', MagicMock(return_value=(100000, 2))) @patch('xcvrd.xcvrd._wrapper_get_presence') def test_get_npu_si_settings_dict_failure_cases(self, wrapper_get_presence): transceiver_dict = {0: {}}