From 26825cae3313fcb5e415452e8088375b426d22f6 Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Mon, 16 Feb 2026 19:51:36 +0000 Subject: [PATCH 1/8] [xcvrd] Separate VDM basic and statistic observables Signed-off-by: Mihir Patel --- sonic-xcvrd/tests/test_xcvrd.py | 249 ++++++++++++++---- sonic-xcvrd/xcvrd/dom/dom_mgr.py | 65 +++-- .../xcvrd/dom/utilities/vdm/db_utils.py | 29 +- sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py | 25 +- 4 files changed, 277 insertions(+), 91 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index b9da5c520..230b56475 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -1076,14 +1076,7 @@ def mock_get_vdm_threshold_table_func(asic_id, threshold_type): for t in VDM_THRESHOLD_TYPES: assert VDM_THRESHOLD_TABLES[f'vdm_{t}_threshold_tbl'][0].get_size_for_key(logical_port_name) == 9 - def test_post_port_vdm_real_values_to_db(self): - def mock_get_transceiver_diagnostic_values(physical_port): - return { - f'laser_temperature_media{i}': 38 if i <= 4 else 'N/A' for i in range(1, 9) - } | { - f'esnr_media_input{i}': 23.1171875 for i in range(1, 9) - } - + def test_post_port_vdm_real_values_from_dict_to_db(self): logical_port_name = "Ethernet0" port_mapping = PortMapping() port_mapping.get_logical_to_physical = MagicMock(return_value=[0]) @@ -1092,49 +1085,42 @@ def mock_get_transceiver_diagnostic_values(physical_port): mock_sfp_obj_dict = {0 : MagicMock()} vdm_db_utils = VDMDBUtils(mock_sfp_obj_dict, port_mapping, xcvr_table_helper, stop_event, helper_logger) - vdm_db_utils.vdm_utils = MagicMock() # Ensure vdm_utils is a mock object - vdm_db_utils.xcvrd_utils.get_transceiver_presence = MagicMock(return_value=False) - vdm_db_utils.xcvrd_utils.is_transceiver_flat_memory = MagicMock(return_value=False) diagnostic_tbl = Table("STATE_DB", TRANSCEIVER_VDM_REAL_VALUE_TABLE) vdm_db_utils.xcvr_table_helper.get_vdm_real_value_tbl = MagicMock(return_value=diagnostic_tbl) - vdm_db_utils.vdm_utils.get_vdm_real_values = MagicMock(return_value=None) assert diagnostic_tbl.get_size() == 0 - # Ensure table is empty asic_index is None + # Ensure table is empty if asic_index is None port_mapping.get_asic_id_for_logical_port = MagicMock(return_value=None) - vdm_db_utils.post_port_vdm_real_values_to_db(logical_port_name) + vdm_real_values = { + f'laser_temperature_media{i}': 38 if i <= 4 else 'N/A' for i in range(1, 9) + } | { + f'esnr_media_input{i}': 23.1171875 for i in range(1, 9) + } + vdm_db_utils.post_port_vdm_real_values_from_dict_to_db(logical_port_name, vdm_real_values) assert diagnostic_tbl.get_size() == 0 # Set asic_index to 0 port_mapping.get_asic_id_for_logical_port = MagicMock(return_value=0) - # Ensure table is empty if stop_event is set - stop_event.set() - vdm_db_utils.post_port_vdm_real_values_to_db(logical_port_name) - assert diagnostic_tbl.get_size() == 0 - stop_event.clear() - - # Ensure table is empty if transceiver is not present - vdm_db_utils.post_port_vdm_real_values_to_db(logical_port_name) + # Ensure table is empty if vdm_real_values_dict is None + vdm_db_utils.post_port_vdm_real_values_from_dict_to_db(logical_port_name, None) assert diagnostic_tbl.get_size() == 0 - vdm_db_utils.return_value = True - # Ensure table is empty if get_values_func returns None - vdm_db_utils.xcvrd_utils.get_transceiver_presence = MagicMock(return_value=True) - vdm_db_utils.post_port_vdm_real_values_to_db(logical_port_name) + # Ensure table is empty if vdm_real_values_dict is empty + vdm_db_utils.post_port_vdm_real_values_from_dict_to_db(logical_port_name, {}) assert diagnostic_tbl.get_size() == 0 - # Ensure table is populated if get_values_func returns valid values - db_cache = {} - vdm_db_utils.vdm_utils.get_vdm_real_values = MagicMock(side_effect=mock_get_transceiver_diagnostic_values) - vdm_db_utils.post_port_vdm_real_values_to_db(logical_port_name, db_cache=db_cache) + # Ensure table is populated if vdm_real_values_dict has valid values + vdm_db_utils.post_port_vdm_real_values_from_dict_to_db(logical_port_name, vdm_real_values) assert diagnostic_tbl.get_size_for_key(logical_port_name) == 17 - # Ensure db_cache is populated correctly - assert db_cache.get(0) is not None - vdm_db_utils.vdm_utils.get_vdm_real_values = MagicMock(return_value=None) - vdm_db_utils.post_port_vdm_real_values_to_db(logical_port_name, db_cache) - assert diagnostic_tbl.get_size_for_key(logical_port_name) == 17 + # Ensure table is updated with new values + vdm_real_values_updated = { + 'laser_temperature_media1': 40, + 'esnr_media_input1': 25.0 + } + vdm_db_utils.post_port_vdm_real_values_from_dict_to_db(logical_port_name, vdm_real_values_updated) + assert diagnostic_tbl.get_size_for_key(logical_port_name) == 3 def test_post_port_transceiver_hw_status_to_db(self): def mock_get_transceiver_status(physical_port): @@ -4317,7 +4303,6 @@ def test_DomInfoUpdateTask_task_worker(self, mock_post_pm_info, task.vdm_utils.is_transceiver_vdm_supported = MagicMock(return_value=True) task.xcvrd_utils.is_transceiver_lpmode_on = MagicMock(return_value=False) task.vdm_db_utils = MagicMock() - task.vdm_db_utils.post_port_vdm_real_values_to_db = MagicMock() task.task_worker() assert task.port_mapping.logical_port_list.count('Ethernet0') assert task.port_mapping.get_asic_id_for_logical_port('Ethernet0') == 0 @@ -4328,7 +4313,7 @@ def test_DomInfoUpdateTask_task_worker(self, mock_post_pm_info, assert task.dom_db_utils.post_port_dom_flags_to_db.call_count == 0 assert task.status_db_utils.post_port_transceiver_hw_status_to_db.call_count == 0 assert task.status_db_utils.post_port_transceiver_hw_status_flags_to_db.call_count == 0 - assert task.vdm_db_utils.post_port_vdm_real_values_to_db.call_count == 0 + assert task.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.call_count == 0 assert task.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 0 assert mock_post_pm_info.call_count == 0 mock_detect_error.return_value = False @@ -4343,7 +4328,7 @@ def test_DomInfoUpdateTask_task_worker(self, mock_post_pm_info, assert task.dom_db_utils.post_port_dom_flags_to_db.call_count == 1 assert task.status_db_utils.post_port_transceiver_hw_status_to_db.call_count == 1 assert task.status_db_utils.post_port_transceiver_hw_status_flags_to_db.call_count == 1 - assert task.vdm_db_utils.post_port_vdm_real_values_to_db.call_count == 1 + assert task.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.call_count == 1 assert task.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 1 assert mock_post_pm_info.call_count == 1 @@ -4377,21 +4362,27 @@ def test_DomInfoUpdateTask_task_worker_vdm_failure(self, mock_post_pm_info): task.status_db_utils.post_port_transceiver_hw_status_to_db = MagicMock() task.status_db_utils.post_port_transceiver_hw_status_flags_to_db = MagicMock() task.vdm_utils.is_transceiver_vdm_supported = MagicMock(return_value=True) + task.vdm_utils.is_vdm_statistic_supported = MagicMock(return_value=True) + task.vdm_utils.is_coherent_module = MagicMock(return_value=False) + task.vdm_utils.get_vdm_real_values_basic = MagicMock(return_value={'basic_key': 'basic_value'}) + task.vdm_utils.get_vdm_real_values_statistic = MagicMock(return_value={'stat_key': 'stat_value'}) task.vdm_utils._freeze_vdm_stats_and_confirm = MagicMock(return_value=False) task.vdm_utils._unfreeze_vdm_stats_and_confirm = MagicMock(return_value=True) - task.vdm_db_utils.post_port_vdm_real_values_to_db = MagicMock() + task.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db = MagicMock() task.vdm_db_utils.post_port_vdm_flags_to_db = MagicMock() task.xcvrd_utils.is_transceiver_lpmode_on = MagicMock(return_value=False) task.task_worker() + # Freeze failed, so unfreeze is still called (context manager), no statistic values or PM captured. + # But step (b) basic values and step (c) flags still run (no continue on freeze failure). assert task.vdm_utils._unfreeze_vdm_stats_and_confirm.call_count == 1 - assert task.vdm_db_utils.post_port_vdm_real_values_to_db.call_count == 0 - assert task.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 0 + assert task.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.call_count == 1 + assert task.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 1 assert mock_post_pm_info.call_count == 0 # clear the call count task.vdm_utils._freeze_vdm_stats_and_confirm.reset_mock() task.vdm_utils._unfreeze_vdm_stats_and_confirm.reset_mock() - task.vdm_db_utils.post_port_vdm_real_values_to_db.reset_mock() + task.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.reset_mock() task.vdm_db_utils.post_port_vdm_flags_to_db.reset_mock() mock_post_pm_info.reset_mock() @@ -4402,27 +4393,136 @@ def test_DomInfoUpdateTask_task_worker_vdm_failure(self, mock_post_pm_info): task.task_worker() assert task.vdm_utils._freeze_vdm_stats_and_confirm.call_count == 1 assert task.vdm_utils._unfreeze_vdm_stats_and_confirm.call_count == 1 - assert task.vdm_db_utils.post_port_vdm_real_values_to_db.call_count == 1 + assert task.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.call_count == 1 assert task.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 1 assert mock_post_pm_info.call_count == 1 # clear the call count task.vdm_utils._freeze_vdm_stats_and_confirm.reset_mock() task.vdm_utils._unfreeze_vdm_stats_and_confirm.reset_mock() - task.vdm_db_utils.post_port_vdm_real_values_to_db.reset_mock() + task.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.reset_mock() task.vdm_db_utils.post_port_vdm_flags_to_db.reset_mock() mock_post_pm_info.reset_mock() - # mock_post_diagnostic_value raises an exception + # post_port_vdm_real_values_from_dict_to_db raises an exception in step (b). + # Step (c) COR flags still run (no continue), and PM already ran in step (a). task.vdm_utils._unfreeze_vdm_stats_and_confirm.return_value = True - task.vdm_db_utils.post_port_vdm_real_values_to_db.side_effect = TypeError + task.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.side_effect = TypeError task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True]) task.task_worker() assert task.vdm_utils._freeze_vdm_stats_and_confirm.call_count == 1 assert task.vdm_utils._unfreeze_vdm_stats_and_confirm.call_count == 1 - assert task.vdm_db_utils.post_port_vdm_real_values_to_db.call_count == 1 - assert task.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 0 + assert task.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.call_count == 1 + assert task.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 1 + assert mock_post_pm_info.call_count == 1 + + @patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock()) + @patch('xcvrd.xcvrd_utilities.common._wrapper_get_presence', MagicMock(return_value=True)) + @patch('xcvrd.xcvrd_utilities.sfp_status_helper.detect_port_in_error_status', MagicMock(return_value=False)) + @patch('xcvrd.dom.dom_mgr.DomInfoUpdateTask.post_port_sfp_firmware_info_to_db', MagicMock(return_value=True)) + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('xcvrd.xcvrd_utilities.port_event_helper.PortChangeObserver', MagicMock(handle_port_update_event=MagicMock())) + @patch('xcvrd.xcvrd_utilities.port_event_helper.subscribe_port_config_change', MagicMock(return_value=(None, None))) + @patch('xcvrd.xcvrd_utilities.port_event_helper.handle_port_config_change', MagicMock()) + @patch('xcvrd.dom.dom_mgr.DomInfoUpdateTask.post_port_pm_info_to_db') + def test_DomInfoUpdateTask_task_worker_vdm_freeze_conditions(self, mock_post_pm_info): + """Test various need_freeze condition combinations""" + port_mapping = PortMapping() + mock_sfp_obj_dict = MagicMock() + stop_event = threading.Event() + mock_cmis_manager = MagicMock() + + # Test Case 1: Basic-only VDM module (no statistics, not coherent) + # Expected: Skip freeze, only basic + flags, no PM + task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, mock_sfp_obj_dict, stop_event, mock_cmis_manager) + task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE) + task.DOM_INFO_UPDATE_PERIOD_SECS = 0 + task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True]) + task.port_mapping.logical_port_list = ['Ethernet0'] + task.port_mapping.physical_to_logical = {'1': ['Ethernet0']} + task.port_mapping.get_asic_id_for_logical_port = MagicMock(return_value=0) + task.get_dom_polling_from_config_db = MagicMock(return_value='enabled') + task.is_port_in_cmis_terminal_state = MagicMock(return_value=False) + task.dom_db_utils = MagicMock() + task.status_db_utils = MagicMock() + task.vdm_utils.is_transceiver_vdm_supported = MagicMock(return_value=True) + task.vdm_utils.is_vdm_statistic_supported = MagicMock(return_value=False) + task.vdm_utils.is_coherent_module = MagicMock(return_value=False) + task.vdm_utils.get_vdm_real_values_basic = MagicMock(return_value={'basic_key': 'basic_value'}) + task.vdm_db_utils = MagicMock() + task.xcvrd_utils.is_transceiver_lpmode_on = MagicMock(return_value=False) + + task.task_worker() + + # Verify: No freeze, no PM, but basic values + flags posted + assert task.vdm_utils.get_vdm_real_values_basic.call_count == 1 + assert task.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.call_count == 1 + assert task.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 1 + assert mock_post_pm_info.call_count == 0 + + # Test Case 2: Module in LPMODE (statistics supported but lpmode=True) + # Expected: Skip freeze, only basic + flags, no PM + mock_post_pm_info.reset_mock() + task2 = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, mock_sfp_obj_dict, stop_event, mock_cmis_manager) + task2.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE) + task2.DOM_INFO_UPDATE_PERIOD_SECS = 0 + task2.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True]) + task2.port_mapping.logical_port_list = ['Ethernet0'] + task2.port_mapping.physical_to_logical = {'1': ['Ethernet0']} + task2.port_mapping.get_asic_id_for_logical_port = MagicMock(return_value=0) + task2.get_dom_polling_from_config_db = MagicMock(return_value='enabled') + task2.is_port_in_cmis_terminal_state = MagicMock(return_value=False) + task2.dom_db_utils = MagicMock() + task2.status_db_utils = MagicMock() + task2.vdm_utils.is_transceiver_vdm_supported = MagicMock(return_value=True) + task2.vdm_utils.is_vdm_statistic_supported = MagicMock(return_value=True) + task2.vdm_utils.is_coherent_module = MagicMock(return_value=False) + task2.vdm_utils.get_vdm_real_values_basic = MagicMock(return_value={'basic_key': 'basic_value'}) + task2.vdm_db_utils = MagicMock() + task2.xcvrd_utils.is_transceiver_lpmode_on = MagicMock(return_value=True) + + task2.task_worker() + + # Verify: No freeze due to lpmode, only basic values + flags posted + assert task2.vdm_utils.get_vdm_real_values_basic.call_count == 1 + assert task2.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.call_count == 1 + assert task2.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 1 assert mock_post_pm_info.call_count == 0 + + # Test Case 3: Coherent module without statistic support + # Expected: Freeze happens, PM captured, but vdm_statistic_values empty + mock_post_pm_info.reset_mock() + task3 = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, mock_sfp_obj_dict, stop_event, mock_cmis_manager) + task3.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE) + task3.DOM_INFO_UPDATE_PERIOD_SECS = 0 + task3.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True]) + task3.port_mapping.logical_port_list = ['Ethernet0'] + task3.port_mapping.physical_to_logical = {'1': ['Ethernet0']} + task3.port_mapping.get_asic_id_for_logical_port = MagicMock(return_value=0) + task3.get_dom_polling_from_config_db = MagicMock(return_value='enabled') + task3.is_port_in_cmis_terminal_state = MagicMock(return_value=False) + task3.dom_db_utils = MagicMock() + task3.status_db_utils = MagicMock() + task3.vdm_utils.is_transceiver_vdm_supported = MagicMock(return_value=True) + task3.vdm_utils.is_vdm_statistic_supported = MagicMock(return_value=False) + task3.vdm_utils.is_coherent_module = MagicMock(return_value=True) + task3.vdm_utils.get_vdm_real_values_basic = MagicMock(return_value={'basic_key': 'basic_value'}) + task3.vdm_utils.get_vdm_real_values_statistic = MagicMock(return_value={}) + task3.vdm_utils._freeze_vdm_stats_and_confirm = MagicMock(return_value=True) + task3.vdm_utils._unfreeze_vdm_stats_and_confirm = MagicMock(return_value=True) + task3.vdm_db_utils = MagicMock() + task3.xcvrd_utils.is_transceiver_lpmode_on = MagicMock(return_value=False) + + task3.task_worker() + + # Verify: Freeze happened, PM called, but no statistic values captured + assert task3.vdm_utils._freeze_vdm_stats_and_confirm.call_count == 1 + assert task3.vdm_utils._unfreeze_vdm_stats_and_confirm.call_count == 1 + assert task3.vdm_utils.get_vdm_real_values_statistic.call_count == 0 # Not called because is_vdm_statistic_supported=False + assert task3.vdm_utils.get_vdm_real_values_basic.call_count == 1 + assert task3.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.call_count == 1 + assert task3.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 1 + assert mock_post_pm_info.call_count == 1 # PM captured for coherent module @pytest.mark.parametrize( "physical_port, logical_port_list, asic_index, transceiver_presence, port_in_error_status, vdm_supported, expected_logs", @@ -4947,18 +5047,57 @@ def test_get_vdm_thresholds(self): mock_sfp.get_transceiver_vdm_thresholds.side_effect = NotImplementedError assert vdm_utils.get_vdm_thresholds(1) == {} - def test_get_vdm_real_values(self): + def test_is_vdm_statistic_supported(self): + mock_sfp = MagicMock() + vdm_utils = VDMUtils({1 : mock_sfp}, helper_logger) + + mock_sfp.is_vdm_statistic_supported.return_value = True + assert vdm_utils.is_vdm_statistic_supported(1) == True + + mock_sfp.is_vdm_statistic_supported.return_value = False + assert vdm_utils.is_vdm_statistic_supported(1) == False + + mock_sfp.is_vdm_statistic_supported.side_effect = NotImplementedError + assert vdm_utils.is_vdm_statistic_supported(1) == False + + def test_is_coherent_module(self): + mock_sfp = MagicMock() + vdm_utils = VDMUtils({1 : mock_sfp}, helper_logger) + + mock_sfp.is_coherent_module.return_value = True + assert vdm_utils.is_coherent_module(1) == True + + mock_sfp.is_coherent_module.return_value = False + assert vdm_utils.is_coherent_module(1) == False + + mock_sfp.is_coherent_module.side_effect = NotImplementedError + assert vdm_utils.is_coherent_module(1) == False + + def test_get_vdm_real_values_basic(self): + mock_sfp = MagicMock() + vdm_utils = VDMUtils({1 : mock_sfp}, helper_logger) + + mock_sfp.get_transceiver_vdm_real_value_basic.return_value = {'basic_key': 'basic_value'} + assert vdm_utils.get_vdm_real_values_basic(1) == {'basic_key': 'basic_value'} + + mock_sfp.get_transceiver_vdm_real_value_basic.return_value = {} + assert vdm_utils.get_vdm_real_values_basic(1) == {} + + mock_sfp.get_transceiver_vdm_real_value_basic.side_effect = NotImplementedError + assert vdm_utils.get_vdm_real_values_basic(1) == {} + + def test_get_vdm_real_values_statistic(self): mock_sfp = MagicMock() vdm_utils = VDMUtils({1 : mock_sfp}, helper_logger) - mock_sfp.get_transceiver_vdm_real_value.return_value = True - assert vdm_utils.get_vdm_real_values(1) + mock_sfp.get_transceiver_vdm_real_value_statistic.return_value = {'stat_key': 'stat_value'} + assert vdm_utils.get_vdm_real_values_statistic(1) == {'stat_key': 'stat_value'} - mock_sfp.get_transceiver_vdm_real_value.return_value = {} - assert vdm_utils.get_vdm_real_values(1) == {} + mock_sfp.get_transceiver_vdm_real_value_statistic.return_value = {} + assert vdm_utils.get_vdm_real_values_statistic(1) == {} - mock_sfp.get_transceiver_vdm_real_value.side_effect = NotImplementedError - assert vdm_utils.get_vdm_real_values(1) == {} + mock_sfp.get_transceiver_vdm_real_value_statistic.side_effect = NotImplementedError + assert vdm_utils.get_vdm_real_values_statistic(1) == {} def test_get_vdm_flags(self): mock_sfp = MagicMock() diff --git a/sonic-xcvrd/xcvrd/dom/dom_mgr.py b/sonic-xcvrd/xcvrd/dom/dom_mgr.py index 29db78fca..aa34e0a31 100644 --- a/sonic-xcvrd/xcvrd/dom/dom_mgr.py +++ b/sonic-xcvrd/xcvrd/dom/dom_mgr.py @@ -351,32 +351,45 @@ def task_worker(self): self.log_warning("Got exception {} while processing transceiver status hw flags for " "port {}, ignored".format(repr(e), logical_port_name)) continue - if self.vdm_utils.is_transceiver_vdm_supported(physical_port) and (not self.xcvrd_utils.is_transceiver_lpmode_on(physical_port)): - # Freeze VDM stats before reading VDM values - with self.vdm_utils.vdm_freeze_context(physical_port) as vdm_frozen: - if not vdm_frozen: - self.log_error("Failed to freeze VDM stats for port {}".format(physical_port)) - continue - try: - # Read and post VDM real values to DB - self.vdm_db_utils.post_port_vdm_real_values_to_db(logical_port_name) - except (KeyError, TypeError) as e: - #continue to process next port since execption could be raised due to port reset, transceiver removal - self.log_warning("Got exception {} while processing vdm values for port {}, ignored".format(repr(e), logical_port_name)) - continue - try: - # Read and post VDM flags and metadata to DB - self.vdm_db_utils.post_port_vdm_flags_to_db(logical_port_name) - except (KeyError, TypeError) as e: - #continue to process next port since execption could be raised due to port reset, transceiver removal - self.log_warning("Got exception {} while processing vdm flags for port {}, ignored".format(repr(e), logical_port_name)) - continue - try: - self.post_port_pm_info_to_db(logical_port_name, self.port_mapping, self.xcvr_table_helper.get_pm_tbl(asic_index), self.task_stopping_event) - except (KeyError, TypeError) as e: - #continue to process next port since execption could be raised due to port reset, transceiver removal - self.log_warning("Got exception {} while processing pm info for port {}, ignored".format(repr(e), logical_port_name)) - continue + if self.vdm_utils.is_transceiver_vdm_supported(physical_port): + # Step (a): If statistic observables are supported or coherent module, + # and not in LPMODE, freeze VDM, capture statistic + # observables and PM info, then unfreeze VDM. + vdm_statistic_values = {} + need_freeze = (self.vdm_utils.is_vdm_statistic_supported(physical_port) or + self.vdm_utils.is_coherent_module(physical_port)) and \ + not self.xcvrd_utils.is_transceiver_lpmode_on(physical_port) + if need_freeze: + with self.vdm_utils.vdm_freeze_context(physical_port) as vdm_frozen: + if not vdm_frozen: + self.log_error("Failed to freeze VDM stats for port {}".format(physical_port)) + else: + try: + if self.vdm_utils.is_vdm_statistic_supported(physical_port): + vdm_statistic_values = self.vdm_utils.get_vdm_real_values_statistic(physical_port) or {} + except (KeyError, TypeError) as e: + self.log_warning("Got exception {} while processing vdm statistic values for port {}, ignored".format(repr(e), logical_port_name)) + try: + self.post_port_pm_info_to_db(logical_port_name, self.port_mapping, self.xcvr_table_helper.get_pm_tbl(asic_index), self.task_stopping_event) + except (KeyError, TypeError) as e: + self.log_warning("Got exception {} while processing pm info for port {}, ignored".format(repr(e), logical_port_name)) + + # Step (b): Capture basic observables, merge with statistic + # observables, and post to DB + try: + vdm_basic_values = self.vdm_utils.get_vdm_real_values_basic(physical_port) or {} + vdm_merged_values = {**vdm_basic_values, **vdm_statistic_values} + self.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db(logical_port_name, vdm_merged_values) + except (KeyError, TypeError) as e: + self.log_warning("Got exception {} while processing vdm values for port {}, ignored".format(repr(e), logical_port_name)) + + # Step (c): Update VDM flags to DB. + # Flags are COR (Clear On Read), so read them last + # to capture the most recent state. + try: + self.vdm_db_utils.post_port_vdm_flags_to_db(logical_port_name) + except (KeyError, TypeError) as e: + self.log_warning("Got exception {} while processing vdm flags for port {}, ignored".format(repr(e), logical_port_name)) # Set the periodic db update time after all the ports are processed if is_periodic_db_update_needed: diff --git a/sonic-xcvrd/xcvrd/dom/utilities/vdm/db_utils.py b/sonic-xcvrd/xcvrd/dom/utilities/vdm/db_utils.py index b280554ac..de331a14b 100644 --- a/sonic-xcvrd/xcvrd/dom/utilities/vdm/db_utils.py +++ b/sonic-xcvrd/xcvrd/dom/utilities/vdm/db_utils.py @@ -22,18 +22,33 @@ def __init__(self, sfp_obj_dict, port_mapping, xcvr_table_helper, task_stopping_ self.vdm_utils = VDMUtils(self.sfp_obj_dict, logger) self.logger = logger - def post_port_vdm_real_values_to_db(self, logical_port_name, db_cache=None): + def post_port_vdm_real_values_from_dict_to_db(self, logical_port_name, vdm_real_values_dict): + """ + Posts the pre-merged VDM real values dictionary to the DB. + This method is used when the caller has already collected and merged + VDM real values (e.g. basic + statistic) and wants to write them + to the DB in a single operation with one last_update_time. + + Args: + logical_port_name (str): Logical port name. + vdm_real_values_dict (dict): Pre-merged VDM real values dictionary. + """ asic_index = self.port_mapping.get_asic_id_for_logical_port(logical_port_name) if asic_index is None: - self.logger.log_error(f"Post port vdm real values to db failed for {logical_port_name} " + self.logger.log_error(f"Post port vdm real values from dict to db failed for {logical_port_name} " "as no asic index found") return - return self.post_diagnostic_values_to_db(logical_port_name, - self.xcvr_table_helper.get_vdm_real_value_tbl(asic_index), - self.vdm_utils.get_vdm_real_values, - db_cache=db_cache, - enable_flat_memory_check=True) + if not vdm_real_values_dict: + return + + self.beautify_info_dict(vdm_real_values_dict) + fvs = swsscommon.FieldValuePairs( + [(k, v) for k, v in vdm_real_values_dict.items()] + + [("last_update_time", self.get_current_time())] + ) + table = self.xcvr_table_helper.get_vdm_real_value_tbl(asic_index) + table.set(logical_port_name, fvs) def post_port_vdm_flags_to_db(self, logical_port_name, db_cache=None): return self._post_port_vdm_thresholds_or_flags_to_db(logical_port_name, self.xcvr_table_helper.get_vdm_flag_tbl, diff --git a/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py b/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py index 80334a935..606d0c065 100644 --- a/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py +++ b/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py @@ -20,11 +20,30 @@ def is_transceiver_vdm_supported(self, physical_port): except (NotImplementedError): return False - def get_vdm_real_values(self, physical_port): + def is_vdm_statistic_supported(self, physical_port): try: - return self.sfp_obj_dict[physical_port].get_transceiver_vdm_real_value() + return self.sfp_obj_dict[physical_port].is_vdm_statistic_supported() except (NotImplementedError): - self.logger.log_error(f"Failed to get VDM real values for port {physical_port}") + return False + + def is_coherent_module(self, physical_port): + try: + return self.sfp_obj_dict[physical_port].is_coherent_module() + except (NotImplementedError): + return False + + def get_vdm_real_values_basic(self, physical_port): + try: + return self.sfp_obj_dict[physical_port].get_transceiver_vdm_real_value_basic() + except (NotImplementedError): + self.logger.log_error(f"Failed to get VDM basic real values for port {physical_port}") + return {} + + def get_vdm_real_values_statistic(self, physical_port): + try: + return self.sfp_obj_dict[physical_port].get_transceiver_vdm_real_value_statistic() + except (NotImplementedError): + self.logger.log_error(f"Failed to get VDM statistic real values for port {physical_port}") return {} def get_vdm_flags(self, physical_port): From 5c4258ac9de8ef6613ee300d6ca8cfb852516dc1 Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Tue, 17 Feb 2026 00:59:38 +0000 Subject: [PATCH 2/8] Addressed PR comments Signed-off-by: Mihir Patel --- sonic-xcvrd/tests/test_xcvrd.py | 2 ++ sonic-xcvrd/xcvrd/dom/utilities/vdm/db_utils.py | 5 +++++ sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py | 6 +++--- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 230b56475..c4d0ceaa2 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -1083,6 +1083,8 @@ def test_post_port_vdm_real_values_from_dict_to_db(self): xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE) stop_event = threading.Event() mock_sfp_obj_dict = {0 : MagicMock()} + mock_sfp_obj_dict[0].get_presence.return_value = True + mock_sfp_obj_dict[0].get_xcvr_api.return_value.is_flat_memory.return_value = False vdm_db_utils = VDMDBUtils(mock_sfp_obj_dict, port_mapping, xcvr_table_helper, stop_event, helper_logger) diagnostic_tbl = Table("STATE_DB", TRANSCEIVER_VDM_REAL_VALUE_TABLE) diff --git a/sonic-xcvrd/xcvrd/dom/utilities/vdm/db_utils.py b/sonic-xcvrd/xcvrd/dom/utilities/vdm/db_utils.py index de331a14b..5138f9c51 100644 --- a/sonic-xcvrd/xcvrd/dom/utilities/vdm/db_utils.py +++ b/sonic-xcvrd/xcvrd/dom/utilities/vdm/db_utils.py @@ -33,6 +33,11 @@ def post_port_vdm_real_values_from_dict_to_db(self, logical_port_name, vdm_real_ logical_port_name (str): Logical port name. vdm_real_values_dict (dict): Pre-merged VDM real values dictionary. """ + # This function is called mainly to perform basic validation of the port + physical_port = self._validate_and_get_physical_port(logical_port_name, enable_flat_memory_check=True) + if physical_port is None: + return + asic_index = self.port_mapping.get_asic_id_for_logical_port(logical_port_name) if asic_index is None: self.logger.log_error(f"Post port vdm real values from dict to db failed for {logical_port_name} " diff --git a/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py b/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py index 606d0c065..efcded018 100644 --- a/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py +++ b/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py @@ -29,20 +29,20 @@ def is_vdm_statistic_supported(self, physical_port): def is_coherent_module(self, physical_port): try: return self.sfp_obj_dict[physical_port].is_coherent_module() - except (NotImplementedError): + except (NotImplementedError, AttributeError): return False def get_vdm_real_values_basic(self, physical_port): try: return self.sfp_obj_dict[physical_port].get_transceiver_vdm_real_value_basic() - except (NotImplementedError): + except (NotImplementedError, AttributeError): self.logger.log_error(f"Failed to get VDM basic real values for port {physical_port}") return {} def get_vdm_real_values_statistic(self, physical_port): try: return self.sfp_obj_dict[physical_port].get_transceiver_vdm_real_value_statistic() - except (NotImplementedError): + except (NotImplementedError, AttributeError): self.logger.log_error(f"Failed to get VDM statistic real values for port {physical_port}") return {} From 1a50c199eb31fd85b374e985657afac55b8a4bba Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Tue, 17 Feb 2026 01:15:33 +0000 Subject: [PATCH 3/8] Enhanced testcase Signed-off-by: Mihir Patel --- sonic-xcvrd/tests/test_xcvrd.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index c4d0ceaa2..2751c4554 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -5101,6 +5101,9 @@ def test_get_vdm_real_values_statistic(self): mock_sfp.get_transceiver_vdm_real_value_statistic.side_effect = NotImplementedError assert vdm_utils.get_vdm_real_values_statistic(1) == {} + mock_sfp.get_transceiver_vdm_real_value_statistic.side_effect = AttributeError + assert vdm_utils.get_vdm_real_values_statistic(1) == {} + def test_get_vdm_flags(self): mock_sfp = MagicMock() vdm_utils = VDMUtils({1 : mock_sfp}, helper_logger) From 8f4b20bc7eb67d189a9aff826f9900e03a50716b Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Tue, 17 Feb 2026 02:01:46 +0000 Subject: [PATCH 4/8] Enhanced testcases Signed-off-by: Mihir Patel --- sonic-xcvrd/tests/test_xcvrd.py | 12 ++++++++++++ sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 2751c4554..155fcae4a 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -5062,6 +5062,9 @@ def test_is_vdm_statistic_supported(self): mock_sfp.is_vdm_statistic_supported.side_effect = NotImplementedError assert vdm_utils.is_vdm_statistic_supported(1) == False + mock_sfp.is_vdm_statistic_supported.side_effect = AttributeError + assert vdm_utils.is_vdm_statistic_supported(1) == False + def test_is_coherent_module(self): mock_sfp = MagicMock() vdm_utils = VDMUtils({1 : mock_sfp}, helper_logger) @@ -5075,6 +5078,9 @@ def test_is_coherent_module(self): mock_sfp.is_coherent_module.side_effect = NotImplementedError assert vdm_utils.is_coherent_module(1) == False + mock_sfp.is_coherent_module.side_effect = AttributeError + assert vdm_utils.is_coherent_module(1) == False + def test_get_vdm_real_values_basic(self): mock_sfp = MagicMock() vdm_utils = VDMUtils({1 : mock_sfp}, helper_logger) @@ -5088,6 +5094,9 @@ def test_get_vdm_real_values_basic(self): mock_sfp.get_transceiver_vdm_real_value_basic.side_effect = NotImplementedError assert vdm_utils.get_vdm_real_values_basic(1) == {} + mock_sfp.get_transceiver_vdm_real_value_basic.side_effect = AttributeError + assert vdm_utils.get_vdm_real_values_basic(1) == {} + def test_get_vdm_real_values_statistic(self): mock_sfp = MagicMock() vdm_utils = VDMUtils({1 : mock_sfp}, helper_logger) @@ -5104,6 +5113,9 @@ def test_get_vdm_real_values_statistic(self): mock_sfp.get_transceiver_vdm_real_value_statistic.side_effect = AttributeError assert vdm_utils.get_vdm_real_values_statistic(1) == {} + mock_sfp.get_transceiver_vdm_real_value_statistic.side_effect = AttributeError + assert vdm_utils.get_vdm_real_values_statistic(1) == {} + def test_get_vdm_flags(self): mock_sfp = MagicMock() vdm_utils = VDMUtils({1 : mock_sfp}, helper_logger) diff --git a/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py b/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py index efcded018..691178b89 100644 --- a/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py +++ b/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py @@ -23,7 +23,7 @@ def is_transceiver_vdm_supported(self, physical_port): def is_vdm_statistic_supported(self, physical_port): try: return self.sfp_obj_dict[physical_port].is_vdm_statistic_supported() - except (NotImplementedError): + except (NotImplementedError, AttributeError): return False def is_coherent_module(self, physical_port): From 02d8a842ced813a857fb3e70e1af3440fc880e8c Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Tue, 17 Feb 2026 02:10:22 +0000 Subject: [PATCH 5/8] Removed redundant code Signed-off-by: Mihir Patel --- sonic-xcvrd/tests/test_xcvrd.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 155fcae4a..9f8121907 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -5113,9 +5113,6 @@ def test_get_vdm_real_values_statistic(self): mock_sfp.get_transceiver_vdm_real_value_statistic.side_effect = AttributeError assert vdm_utils.get_vdm_real_values_statistic(1) == {} - mock_sfp.get_transceiver_vdm_real_value_statistic.side_effect = AttributeError - assert vdm_utils.get_vdm_real_values_statistic(1) == {} - def test_get_vdm_flags(self): mock_sfp = MagicMock() vdm_utils = VDMUtils({1 : mock_sfp}, helper_logger) From 115b404cd4e97fe5eda15f4d0c77ec78ba5118df Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Tue, 17 Feb 2026 02:22:07 +0000 Subject: [PATCH 6/8] Enhanced tests Signed-off-by: Mihir Patel --- sonic-xcvrd/tests/test_xcvrd.py | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 9f8121907..e38078555 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -4524,7 +4524,32 @@ def test_DomInfoUpdateTask_task_worker_vdm_freeze_conditions(self, mock_post_pm_ assert task3.vdm_utils.get_vdm_real_values_basic.call_count == 1 assert task3.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.call_count == 1 assert task3.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 1 - assert mock_post_pm_info.call_count == 1 # PM captured for coherent module + + # Case 4: Module supports both VDM statistics AND is a coherent module + # Expected: Freeze happens, both basic and statistic values are captured, and PM info is captured + task4 = copy.deepcopy(task3) + # Ensure flags reflect support for statistics and coherent module behavior + setattr(task4, "is_vdm_statistic_supported", True) + setattr(task4, "is_coherent_module", True) + # Reconfigure mocks for the new scenario to have independent call counts and return values + task4.vdm_utils.get_vdm_real_values_basic = MagicMock(return_value={'basic_key': 'basic_value'}) + task4.vdm_utils.get_vdm_real_values_statistic = MagicMock(return_value={'stat_key': 'stat_value'}) + task4.vdm_utils._freeze_vdm_stats_and_confirm = MagicMock(return_value=True) + task4.vdm_utils._unfreeze_vdm_stats_and_confirm = MagicMock(return_value=True) + task4.vdm_db_utils = MagicMock() + task4.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db = MagicMock() + task4.vdm_db_utils.post_port_vdm_flags_to_db = MagicMock() + task4.xcvrd_utils.is_transceiver_lpmode_on = MagicMock(return_value=False) + task4.task_worker() + # Verify: Freeze happened, both basic and statistic values captured, and PM called + assert task4.vdm_utils._freeze_vdm_stats_and_confirm.call_count == 1 + assert task4.vdm_utils._unfreeze_vdm_stats_and_confirm.call_count == 1 + assert task4.vdm_utils.get_vdm_real_values_basic.call_count == 1 + assert task4.vdm_utils.get_vdm_real_values_statistic.call_count == 1 + assert task4.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.call_count == 1 + assert task4.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 1 + # PM should have been captured for both coherent-only and coherent+statistics cases + assert mock_post_pm_info.call_count == 2 @pytest.mark.parametrize( "physical_port, logical_port_list, asic_index, transceiver_presence, port_in_error_status, vdm_supported, expected_logs", @@ -5062,7 +5087,7 @@ def test_is_vdm_statistic_supported(self): mock_sfp.is_vdm_statistic_supported.side_effect = NotImplementedError assert vdm_utils.is_vdm_statistic_supported(1) == False - mock_sfp.is_vdm_statistic_supported.side_effect = AttributeError + mock_sfp.is_vdm_statistic_supported.side_effect = AttributeError assert vdm_utils.is_vdm_statistic_supported(1) == False def test_is_coherent_module(self): From 4633eed2fb9d1a2564464d736cf4b6976b05b066 Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Tue, 17 Feb 2026 02:41:03 +0000 Subject: [PATCH 7/8] Fixed testcase failure Signed-off-by: Mihir Patel --- sonic-xcvrd/tests/test_xcvrd.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index e38078555..5a6403c92 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -4527,18 +4527,25 @@ def test_DomInfoUpdateTask_task_worker_vdm_freeze_conditions(self, mock_post_pm_ # Case 4: Module supports both VDM statistics AND is a coherent module # Expected: Freeze happens, both basic and statistic values are captured, and PM info is captured - task4 = copy.deepcopy(task3) - # Ensure flags reflect support for statistics and coherent module behavior - setattr(task4, "is_vdm_statistic_supported", True) - setattr(task4, "is_coherent_module", True) - # Reconfigure mocks for the new scenario to have independent call counts and return values + task4 = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, mock_sfp_obj_dict, stop_event, mock_cmis_manager) + task4.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE) + task4.DOM_INFO_UPDATE_PERIOD_SECS = 0 + task4.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True]) + task4.port_mapping.logical_port_list = ['Ethernet0'] + task4.port_mapping.physical_to_logical = {'1': ['Ethernet0']} + task4.port_mapping.get_asic_id_for_logical_port = MagicMock(return_value=0) + task4.get_dom_polling_from_config_db = MagicMock(return_value='enabled') + task4.is_port_in_cmis_terminal_state = MagicMock(return_value=False) + task4.dom_db_utils = MagicMock() + task4.status_db_utils = MagicMock() + task4.vdm_utils.is_transceiver_vdm_supported = MagicMock(return_value=True) + task4.vdm_utils.is_vdm_statistic_supported = MagicMock(return_value=True) + task4.vdm_utils.is_coherent_module = MagicMock(return_value=True) task4.vdm_utils.get_vdm_real_values_basic = MagicMock(return_value={'basic_key': 'basic_value'}) task4.vdm_utils.get_vdm_real_values_statistic = MagicMock(return_value={'stat_key': 'stat_value'}) task4.vdm_utils._freeze_vdm_stats_and_confirm = MagicMock(return_value=True) task4.vdm_utils._unfreeze_vdm_stats_and_confirm = MagicMock(return_value=True) task4.vdm_db_utils = MagicMock() - task4.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db = MagicMock() - task4.vdm_db_utils.post_port_vdm_flags_to_db = MagicMock() task4.xcvrd_utils.is_transceiver_lpmode_on = MagicMock(return_value=False) task4.task_worker() # Verify: Freeze happened, both basic and statistic values captured, and PM called From e69284cf33d078d907534d89e79b5b104b61835b Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Thu, 19 Feb 2026 02:19:35 +0000 Subject: [PATCH 8/8] Addresed PR comments Signed-off-by: Mihir Patel --- sonic-xcvrd/tests/test_xcvrd.py | 38 ++++---------------- sonic-xcvrd/xcvrd/dom/dom_mgr.py | 16 ++++----- sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py | 6 ---- 3 files changed, 13 insertions(+), 47 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 5a6403c92..96cf6e9de 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -4365,7 +4365,6 @@ def test_DomInfoUpdateTask_task_worker_vdm_failure(self, mock_post_pm_info): task.status_db_utils.post_port_transceiver_hw_status_flags_to_db = MagicMock() task.vdm_utils.is_transceiver_vdm_supported = MagicMock(return_value=True) task.vdm_utils.is_vdm_statistic_supported = MagicMock(return_value=True) - task.vdm_utils.is_coherent_module = MagicMock(return_value=False) task.vdm_utils.get_vdm_real_values_basic = MagicMock(return_value={'basic_key': 'basic_value'}) task.vdm_utils.get_vdm_real_values_statistic = MagicMock(return_value={'stat_key': 'stat_value'}) task.vdm_utils._freeze_vdm_stats_and_confirm = MagicMock(return_value=False) @@ -4449,7 +4448,6 @@ def test_DomInfoUpdateTask_task_worker_vdm_freeze_conditions(self, mock_post_pm_ task.status_db_utils = MagicMock() task.vdm_utils.is_transceiver_vdm_supported = MagicMock(return_value=True) task.vdm_utils.is_vdm_statistic_supported = MagicMock(return_value=False) - task.vdm_utils.is_coherent_module = MagicMock(return_value=False) task.vdm_utils.get_vdm_real_values_basic = MagicMock(return_value={'basic_key': 'basic_value'}) task.vdm_db_utils = MagicMock() task.xcvrd_utils.is_transceiver_lpmode_on = MagicMock(return_value=False) @@ -4478,7 +4476,6 @@ def test_DomInfoUpdateTask_task_worker_vdm_freeze_conditions(self, mock_post_pm_ task2.status_db_utils = MagicMock() task2.vdm_utils.is_transceiver_vdm_supported = MagicMock(return_value=True) task2.vdm_utils.is_vdm_statistic_supported = MagicMock(return_value=True) - task2.vdm_utils.is_coherent_module = MagicMock(return_value=False) task2.vdm_utils.get_vdm_real_values_basic = MagicMock(return_value={'basic_key': 'basic_value'}) task2.vdm_db_utils = MagicMock() task2.xcvrd_utils.is_transceiver_lpmode_on = MagicMock(return_value=True) @@ -4491,8 +4488,8 @@ def test_DomInfoUpdateTask_task_worker_vdm_freeze_conditions(self, mock_post_pm_ assert task2.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 1 assert mock_post_pm_info.call_count == 0 - # Test Case 3: Coherent module without statistic support - # Expected: Freeze happens, PM captured, but vdm_statistic_values empty + # Test Case 3: VDM supported but no statistic support + # Expected: No freeze, only basic + flags, no PM mock_post_pm_info.reset_mock() task3 = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, mock_sfp_obj_dict, stop_event, mock_cmis_manager) task3.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE) @@ -4507,23 +4504,17 @@ def test_DomInfoUpdateTask_task_worker_vdm_freeze_conditions(self, mock_post_pm_ task3.status_db_utils = MagicMock() task3.vdm_utils.is_transceiver_vdm_supported = MagicMock(return_value=True) task3.vdm_utils.is_vdm_statistic_supported = MagicMock(return_value=False) - task3.vdm_utils.is_coherent_module = MagicMock(return_value=True) task3.vdm_utils.get_vdm_real_values_basic = MagicMock(return_value={'basic_key': 'basic_value'}) - task3.vdm_utils.get_vdm_real_values_statistic = MagicMock(return_value={}) - task3.vdm_utils._freeze_vdm_stats_and_confirm = MagicMock(return_value=True) - task3.vdm_utils._unfreeze_vdm_stats_and_confirm = MagicMock(return_value=True) task3.vdm_db_utils = MagicMock() task3.xcvrd_utils.is_transceiver_lpmode_on = MagicMock(return_value=False) task3.task_worker() - # Verify: Freeze happened, PM called, but no statistic values captured - assert task3.vdm_utils._freeze_vdm_stats_and_confirm.call_count == 1 - assert task3.vdm_utils._unfreeze_vdm_stats_and_confirm.call_count == 1 - assert task3.vdm_utils.get_vdm_real_values_statistic.call_count == 0 # Not called because is_vdm_statistic_supported=False + # Verify: No freeze (no statistics support), only basic + flags assert task3.vdm_utils.get_vdm_real_values_basic.call_count == 1 assert task3.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.call_count == 1 assert task3.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 1 + assert mock_post_pm_info.call_count == 0 # Case 4: Module supports both VDM statistics AND is a coherent module # Expected: Freeze happens, both basic and statistic values are captured, and PM info is captured @@ -4540,7 +4531,6 @@ def test_DomInfoUpdateTask_task_worker_vdm_freeze_conditions(self, mock_post_pm_ task4.status_db_utils = MagicMock() task4.vdm_utils.is_transceiver_vdm_supported = MagicMock(return_value=True) task4.vdm_utils.is_vdm_statistic_supported = MagicMock(return_value=True) - task4.vdm_utils.is_coherent_module = MagicMock(return_value=True) task4.vdm_utils.get_vdm_real_values_basic = MagicMock(return_value={'basic_key': 'basic_value'}) task4.vdm_utils.get_vdm_real_values_statistic = MagicMock(return_value={'stat_key': 'stat_value'}) task4.vdm_utils._freeze_vdm_stats_and_confirm = MagicMock(return_value=True) @@ -4555,8 +4545,8 @@ def test_DomInfoUpdateTask_task_worker_vdm_freeze_conditions(self, mock_post_pm_ assert task4.vdm_utils.get_vdm_real_values_statistic.call_count == 1 assert task4.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db.call_count == 1 assert task4.vdm_db_utils.post_port_vdm_flags_to_db.call_count == 1 - # PM should have been captured for both coherent-only and coherent+statistics cases - assert mock_post_pm_info.call_count == 2 + # PM should have been captured only for Case 4 (statistics supported) + assert mock_post_pm_info.call_count == 1 @pytest.mark.parametrize( "physical_port, logical_port_list, asic_index, transceiver_presence, port_in_error_status, vdm_supported, expected_logs", @@ -5097,22 +5087,6 @@ def test_is_vdm_statistic_supported(self): mock_sfp.is_vdm_statistic_supported.side_effect = AttributeError assert vdm_utils.is_vdm_statistic_supported(1) == False - def test_is_coherent_module(self): - mock_sfp = MagicMock() - vdm_utils = VDMUtils({1 : mock_sfp}, helper_logger) - - mock_sfp.is_coherent_module.return_value = True - assert vdm_utils.is_coherent_module(1) == True - - mock_sfp.is_coherent_module.return_value = False - assert vdm_utils.is_coherent_module(1) == False - - mock_sfp.is_coherent_module.side_effect = NotImplementedError - assert vdm_utils.is_coherent_module(1) == False - - mock_sfp.is_coherent_module.side_effect = AttributeError - assert vdm_utils.is_coherent_module(1) == False - def test_get_vdm_real_values_basic(self): mock_sfp = MagicMock() vdm_utils = VDMUtils({1 : mock_sfp}, helper_logger) diff --git a/sonic-xcvrd/xcvrd/dom/dom_mgr.py b/sonic-xcvrd/xcvrd/dom/dom_mgr.py index aa34e0a31..2412c1090 100644 --- a/sonic-xcvrd/xcvrd/dom/dom_mgr.py +++ b/sonic-xcvrd/xcvrd/dom/dom_mgr.py @@ -352,12 +352,11 @@ def task_worker(self): "port {}, ignored".format(repr(e), logical_port_name)) continue if self.vdm_utils.is_transceiver_vdm_supported(physical_port): - # Step (a): If statistic observables are supported or coherent module, - # and not in LPMODE, freeze VDM, capture statistic - # observables and PM info, then unfreeze VDM. + # Step (a): If statistic observables are supported and not in LPMODE, + # freeze VDM, capture statistic observables and PM info, + # then unfreeze VDM. vdm_statistic_values = {} - need_freeze = (self.vdm_utils.is_vdm_statistic_supported(physical_port) or - self.vdm_utils.is_coherent_module(physical_port)) and \ + need_freeze = self.vdm_utils.is_vdm_statistic_supported(physical_port) and \ not self.xcvrd_utils.is_transceiver_lpmode_on(physical_port) if need_freeze: with self.vdm_utils.vdm_freeze_context(physical_port) as vdm_frozen: @@ -365,14 +364,13 @@ def task_worker(self): self.log_error("Failed to freeze VDM stats for port {}".format(physical_port)) else: try: - if self.vdm_utils.is_vdm_statistic_supported(physical_port): - vdm_statistic_values = self.vdm_utils.get_vdm_real_values_statistic(physical_port) or {} + vdm_statistic_values = self.vdm_utils.get_vdm_real_values_statistic(physical_port) or {} except (KeyError, TypeError) as e: self.log_warning("Got exception {} while processing vdm statistic values for port {}, ignored".format(repr(e), logical_port_name)) try: self.post_port_pm_info_to_db(logical_port_name, self.port_mapping, self.xcvr_table_helper.get_pm_tbl(asic_index), self.task_stopping_event) except (KeyError, TypeError) as e: - self.log_warning("Got exception {} while processing pm info for port {}, ignored".format(repr(e), logical_port_name)) + self.log_warning("Got exception {} while posting pm info to DB for port {}, ignored".format(repr(e), logical_port_name)) # Step (b): Capture basic observables, merge with statistic # observables, and post to DB @@ -381,7 +379,7 @@ def task_worker(self): vdm_merged_values = {**vdm_basic_values, **vdm_statistic_values} self.vdm_db_utils.post_port_vdm_real_values_from_dict_to_db(logical_port_name, vdm_merged_values) except (KeyError, TypeError) as e: - self.log_warning("Got exception {} while processing vdm values for port {}, ignored".format(repr(e), logical_port_name)) + self.log_warning("Got exception {} while posting vdm values to DB for port {}, ignored".format(repr(e), logical_port_name)) # Step (c): Update VDM flags to DB. # Flags are COR (Clear On Read), so read them last diff --git a/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py b/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py index 691178b89..6d9bf4d5b 100644 --- a/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py +++ b/sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py @@ -26,12 +26,6 @@ def is_vdm_statistic_supported(self, physical_port): except (NotImplementedError, AttributeError): return False - def is_coherent_module(self, physical_port): - try: - return self.sfp_obj_dict[physical_port].is_coherent_module() - except (NotImplementedError, AttributeError): - return False - def get_vdm_real_values_basic(self, physical_port): try: return self.sfp_obj_dict[physical_port].get_transceiver_vdm_real_value_basic()