From c52ba693d9d96b2700dad9acd37f46d9d8ae36d5 Mon Sep 17 00:00:00 2001 From: chiourung_huang Date: Wed, 13 Sep 2023 07:46:00 +0000 Subject: [PATCH 1/7] Fix the exception when configure unsupported frequency 1. Add verify_config_laser_frequency to check frequency eariler 2. Catch the exception for set_laser_freq Signed-off-by: chiourung_huang --- sonic-xcvrd/tests/test_xcvrd.py | 1 + sonic-xcvrd/xcvrd/xcvrd.py | 31 +++++++++++++++++++++---------- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 313a60023..99a5ca398 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -876,6 +876,7 @@ def test_CmisManagerTask_task_worker(self, mock_chassis): task.get_configured_tx_power_from_db = MagicMock(return_value=-13) task.get_configured_laser_freq_from_db = MagicMock(return_value=193100) task.configure_tx_output_power = MagicMock(return_value=1) + task.verify_config_laser_frequency = MagicMock(return_value=1) task.configure_laser_frequency = MagicMock(return_value=1) # Case 1: Module Inserted --> DP_DEINIT diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index d315d918a..79f283eb8 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -1390,18 +1390,28 @@ def configure_tx_output_power(self, api, lport, tx_power): self.log_error("{} configured tx power {} > maximum power {} supported".format(lport, tx_power, max_p)) return api.set_tx_power(tx_power) - def configure_laser_frequency(self, api, lport, freq, grid=75): + def verify_config_laser_frequency(self, api, lport, freq, grid=75): _, _, _, lowf, highf = api.get_supported_freq_config() if freq < lowf: self.log_error("{} configured freq:{} GHz is lower than the supported freq:{} GHz".format(lport, freq, lowf)) + return False if freq > highf: self.log_error("{} configured freq:{} GHz is higher than the supported freq:{} GHz".format(lport, freq, highf)) - chan = int(round((freq - 193100)/25)) - if chan % 3 != 0: - self.log_error("{} configured freq:{} GHz is NOT in 75GHz grid".format(lport, freq)) + return False + if grid == 75: + chan = int(round((freq - 193100)/25)) + if chan % 3 != 0: + self.log_error("{} configured freq:{} GHz is NOT in 75GHz grid".format(lport, freq)) + return False + return True + + def configure_laser_frequency(self, api, lport, freq, grid=75): if api.get_tuning_in_progress(): self.log_error("{} Tuning in progress, subport selection may fail!".format(lport)) - return api.set_laser_freq(freq, grid) + try: + return api.set_laser_freq(freq, grid) + except: + return False def wait_for_port_config_done(self, namespace): # Connect to APPL_DB and subscribe to PORT table notifications @@ -1596,11 +1606,12 @@ def task_worker(self): # For ZR module, Datapath needes to be re-initlialized on new channel selection if api.is_coherent_module(): - freq = self.port_dict[lport]['laser_freq'] - # If user requested frequency is NOT the same as configured on the module - # force datapath re-initialization - if 0 != freq and freq != api.get_laser_config_freq(): - need_update = True + freq = self.port_dict[lport]['laser_freq'] + # If user requested frequency is NOT the same as configured on the module + # force datapath re-initialization + if 0 != freq and freq != api.get_laser_config_freq(): + if self.verify_config_laser_frequency(api, lport, freq) == True: + need_update = True if not need_update: # No application updates From 199338c2e021e7a3f0b6d1883c63b8566cc40d56 Mon Sep 17 00:00:00 2001 From: chiourung_huang Date: Wed, 13 Sep 2023 08:04:43 +0000 Subject: [PATCH 2/7] Add check for grid space --- sonic-xcvrd/xcvrd/xcvrd.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 79f283eb8..89b1322ed 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -1391,7 +1391,7 @@ def configure_tx_output_power(self, api, lport, tx_power): return api.set_tx_power(tx_power) def verify_config_laser_frequency(self, api, lport, freq, grid=75): - _, _, _, lowf, highf = api.get_supported_freq_config() + grid_supported, _, _, lowf, highf = api.get_supported_freq_config() if freq < lowf: self.log_error("{} configured freq:{} GHz is lower than the supported freq:{} GHz".format(lport, freq, lowf)) return False @@ -1399,6 +1399,9 @@ def verify_config_laser_frequency(self, api, lport, freq, grid=75): self.log_error("{} configured freq:{} GHz is higher than the supported freq:{} GHz".format(lport, freq, highf)) return False if grid == 75: + if (grid_supported >> 7) & 0x1 != 1: + self.log_error("{} 75GHz is not supported".format(lport)) + return False chan = int(round((freq - 193100)/25)) if chan % 3 != 0: self.log_error("{} configured freq:{} GHz is NOT in 75GHz grid".format(lport, freq)) From 38477513baf36b3f99e9073c2f5cf866ccd9f0bd Mon Sep 17 00:00:00 2001 From: chiourung_huang Date: Wed, 13 Sep 2023 08:20:49 +0000 Subject: [PATCH 3/7] Add unit test --- sonic-xcvrd/tests/test_xcvrd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 99a5ca398..1281b8b51 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -776,6 +776,7 @@ def test_CmisManagerTask_task_worker(self, mock_chassis): mock_xcvr_api.get_tx_config_power = MagicMock(return_value=0) mock_xcvr_api.get_laser_config_freq = MagicMock(return_value=0) mock_xcvr_api.get_module_type_abbreviation = MagicMock(return_value='QSFP-DD') + mock_xcvr_api.get_supported_freq_config = MagicMock(return_value=(0xA0,0,0,191300,196100)) mock_xcvr_api.get_datapath_init_duration = MagicMock(return_value=60000.0) mock_xcvr_api.get_module_pwr_up_duration = MagicMock(return_value=70000.0) mock_xcvr_api.get_datapath_deinit_duration = MagicMock(return_value=600000.0) @@ -876,7 +877,6 @@ def test_CmisManagerTask_task_worker(self, mock_chassis): task.get_configured_tx_power_from_db = MagicMock(return_value=-13) task.get_configured_laser_freq_from_db = MagicMock(return_value=193100) task.configure_tx_output_power = MagicMock(return_value=1) - task.verify_config_laser_frequency = MagicMock(return_value=1) task.configure_laser_frequency = MagicMock(return_value=1) # Case 1: Module Inserted --> DP_DEINIT From 0a70144cd3c002704b27abea4e40e3007d7c922d Mon Sep 17 00:00:00 2001 From: chiourung_huang Date: Fri, 15 Sep 2023 06:26:06 +0000 Subject: [PATCH 4/7] Add unit test for verify_config_laser_frequency --- sonic-xcvrd/tests/test_xcvrd.py | 15 +++++++++++++++ sonic-xcvrd/xcvrd/xcvrd.py | 7 +++++++ 2 files changed, 22 insertions(+) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 1281b8b51..c41c3b9d6 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -759,6 +759,21 @@ def get_host_lane_assignment_option_side_effect(app): appl = task.get_cmis_application_desired(mock_xcvr_api, host_lane_count, speed) assert task.get_cmis_host_lanes_mask(mock_xcvr_api, appl, host_lane_count, subport) == expected + @pytest.mark.parametrize("lport, freq, grid, expected", [ + (1, 193100, 75, True), + (1, 191000, 100, False) + ]) + def test_CmisManagerTask_verify_config_laser_frequency(self, lport, freq, grid, expected): + mock_xcvr_api = MagicMock() + mock_xcvr_api.get_supported_freq_config = MagicMock() + mock_xcvr_api.get_supported_freq_config.return_value = (0xA0, 0, 0, 191300, 196100) + + port_mapping = PortMapping() + stop_event = threading.Event() + task = CmisManagerTask(port_mapping, stop_event) + result = task.verify_config_laser_frequency(mock_xcvr_api, lport, freq, grid) + assert result == expected + @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()) diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 89b1322ed..f3a3ee766 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -1406,6 +1406,13 @@ def verify_config_laser_frequency(self, api, lport, freq, grid=75): if chan % 3 != 0: self.log_error("{} configured freq:{} GHz is NOT in 75GHz grid".format(lport, freq)) return False + elif grid == 100: + if (grid_supported >> 5) & 0x1 != 1: + self.log_error("{} 100GHz is not supported".format(lport)) + return False + else: + self.log_error("{} {}GHz is not supported".format(lport, grid)) + return False return True def configure_laser_frequency(self, api, lport, freq, grid=75): From 126f845259cef9d6b5b57e2645490e4381797578 Mon Sep 17 00:00:00 2001 From: chiourung_huang Date: Fri, 15 Sep 2023 07:14:37 +0000 Subject: [PATCH 5/7] Fix build error --- sonic-xcvrd/tests/test_xcvrd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 29c7c67f9..28bd9c2f6 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -817,7 +817,7 @@ def test_CmisManagerTask_verify_config_laser_frequency(self, lport, freq, grid, port_mapping = PortMapping() stop_event = threading.Event() - task = CmisManagerTask(port_mapping, stop_event) + task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event) result = task.verify_config_laser_frequency(mock_xcvr_api, lport, freq, grid) assert result == expected From f7df53b5d97b92134bf3e9e7f2636835c9b60eaa Mon Sep 17 00:00:00 2001 From: chiourung_huang Date: Fri, 15 Sep 2023 07:34:39 +0000 Subject: [PATCH 6/7] Add unit test coverage --- sonic-xcvrd/tests/test_xcvrd.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 28bd9c2f6..e81b1b236 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -808,12 +808,14 @@ def get_host_lane_assignment_option_side_effect(app): @pytest.mark.parametrize("lport, freq, grid, expected", [ (1, 193100, 75, True), - (1, 191000, 100, False) + (1, 193100, 100, False), + (1, 193125, 75, False), + (1, 193100, 25, False) ]) def test_CmisManagerTask_verify_config_laser_frequency(self, lport, freq, grid, expected): mock_xcvr_api = MagicMock() mock_xcvr_api.get_supported_freq_config = MagicMock() - mock_xcvr_api.get_supported_freq_config.return_value = (0xA0, 0, 0, 191300, 196100) + mock_xcvr_api.get_supported_freq_config.return_value = (0x80, 0, 0, 191300, 196100) port_mapping = PortMapping() stop_event = threading.Event() From 9a75f5975fa466b2de24bf1453aceed94a178e4c Mon Sep 17 00:00:00 2001 From: chiourung_huang Date: Thu, 19 Oct 2023 02:00:37 +0000 Subject: [PATCH 7/7] skip to set the frequency if it is invalid --- sonic-xcvrd/xcvrd/xcvrd.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 993e7164e..4b99f2cea 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -1323,6 +1323,8 @@ def configure_tx_output_power(self, api, lport, tx_power): self.log_error("{} configured tx power {} > maximum power {} supported".format(lport, tx_power, max_p)) return api.set_tx_power(tx_power) + # Verify if the configured frequency is valid or supported + # Now it only checks for grids 75 and 100. This needs to be implemented when more grids are supported. def verify_config_laser_frequency(self, api, lport, freq, grid=75): grid_supported, _, _, lowf, highf = api.get_supported_freq_config() if freq < lowf: @@ -1585,6 +1587,9 @@ def task_worker(self): if 0 != freq and freq != api.get_laser_config_freq(): if self.verify_config_laser_frequency(api, lport, freq) == True: need_update = True + else: + # clear the invalid freq to prevent setting in AP_CONFIGURED + self.port_dict[lport]['laser_freq'] = 0 if not need_update: # No application updates