From ccbad284526809c2c6f1fd2a52d1e95eecce9549 Mon Sep 17 00:00:00 2001 From: byang Date: Thu, 15 Sep 2022 22:21:42 -0700 Subject: [PATCH 1/9] Fix xcvrd to support 400G ZR/DR optics --- sonic-xcvrd/tests/test_xcvrd.py | 4 +-- sonic-xcvrd/xcvrd/xcvrd.py | 47 ++++++++++++++++++++++++--------- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 4eedfead5..63f2598b3 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -634,14 +634,14 @@ def test_DomInfoUpdateTask_handle_port_change_event(self): task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping) task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE) port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD) - task.on_port_config_change(port_change_event) + task.on_port_config_change(None, port_change_event) assert task.port_mapping.logical_port_list.count('Ethernet0') assert task.port_mapping.get_asic_id_for_logical_port('Ethernet0') == 0 assert task.port_mapping.get_physical_to_logical(1) == ['Ethernet0'] assert task.port_mapping.get_logical_to_physical('Ethernet0') == [1] port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_REMOVE) - task.on_port_config_change(port_change_event) + task.on_port_config_change(None, port_change_event) assert not task.port_mapping.logical_port_list assert not task.port_mapping.logical_to_physical assert not task.port_mapping.physical_to_logical diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index a5c53b0eb..05024dcd1 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -923,6 +923,7 @@ class CmisManagerTask: CMIS_MAX_RETRIES = 3 CMIS_DEF_EXPIRED = 60 # seconds, default expiration time + CMIS_DEF_EXPIRED_ZR = 200 # seconds, expiration time for ZR module CMIS_MODULE_TYPES = ['QSFP-DD', 'QSFP_DD', 'OSFP'] CMIS_NUM_CHANNELS = 8 @@ -986,22 +987,38 @@ def on_port_update_event(self, port_change_event): return if port_change_event.event_type == port_change_event.PORT_SET: + need_update = False if pport >= 0: - self.port_dict[lport]['index'] = pport + if self.port_dict[lport].get('index') != pport: + self.port_dict[lport]['index'] = pport + need_update = True if 'speed' in port_change_event.port_dict and port_change_event.port_dict['speed'] != 'N/A': - self.port_dict[lport]['speed'] = port_change_event.port_dict['speed'] + if self.port_dict[lport].get('speed') != port_change_event.port_dict['speed']: + self.port_dict[lport]['speed'] = port_change_event.port_dict['speed'] + need_update = True if 'lanes' in port_change_event.port_dict: - self.port_dict[lport]['lanes'] = port_change_event.port_dict['lanes'] + if self.port_dict[lport].get('lanes') != port_change_event.port_dict['lanes']: + self.port_dict[lport]['lanes'] = port_change_event.port_dict['lanes'] + need_update = True if 'host_tx_ready' in port_change_event.port_dict: - self.port_dict[lport]['host_tx_ready'] = port_change_event.port_dict['host_tx_ready'] + if self.port_dict[lport].get('host_tx_ready') != port_change_event.port_dict['host_tx_ready']: + self.port_dict[lport]['host_tx_ready'] = port_change_event.port_dict['host_tx_ready'] + need_update = True if 'admin_status' in port_change_event.port_dict: - self.port_dict[lport]['admin_status'] = port_change_event.port_dict['admin_status'] + if self.port_dict[lport].get('admin_status') != port_change_event.port_dict['admin_status']: + self.port_dict[lport]['admin_status'] = port_change_event.port_dict['admin_status'] + need_update = True if 'laser_freq' in port_change_event.port_dict: - self.port_dict[lport]['laser_freq'] = int(port_change_event.port_dict['laser_freq']) + if self.port_dict[lport].get('laser_freq') != int(port_change_event.port_dict['laser_freq']): + self.port_dict[lport]['laser_freq'] = int(port_change_event.port_dict['laser_freq']) + need_update = True if 'tx_power' in port_change_event.port_dict: - self.port_dict[lport]['tx_power'] = float(port_change_event.port_dict['tx_power']) + if self.port_dict[lport].get('tx_power') != float(port_change_event.port_dict['tx_power']): + self.port_dict[lport]['tx_power'] = float(port_change_event.port_dict['tx_power']) + need_update = True - self.force_cmis_reinit(lport, 0) + if need_update: + self.force_cmis_reinit(lport, 0) else: self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_REMOVED @@ -1073,6 +1090,12 @@ def get_cmis_application_desired(self, api, channel, speed): return (appl_code & 0xf) + def get_cmis_expired(self, api): + if api.is_coherent_module(): + return self.CMIS_DEF_EXPIRED_ZR + else: + return self.CMIS_DEF_EXPIRED + def is_cmis_application_update_required(self, api, channel, speed): """ Check if the CMIS application update is required @@ -1458,7 +1481,7 @@ def task_worker(self): # TODO: Make sure this doesn't impact other datapaths api.set_lpmode(False) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_AP_CONF - self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.CMIS_DEF_EXPIRED) + self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_expired(api)) elif state == self.CMIS_STATE_AP_CONF: # TODO: Use fine grained time when the CMIS memory map is available if not self.check_module_state(api, ['ModuleReady']): @@ -1495,7 +1518,7 @@ def task_worker(self): continue # TODO: Use fine grained time when the CMIS memory map is available - self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.CMIS_DEF_EXPIRED) + self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_expired(api)) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_INIT elif state == self.CMIS_STATE_DP_INIT: if not self.check_config_error(api, host_lanes, ['ConfigSuccess']): @@ -1516,7 +1539,7 @@ def task_worker(self): # D.1.3 Software Configuration and Initialization api.set_datapath_init(host_lanes) # TODO: Use fine grained timeout when the CMIS memory map is available - self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.CMIS_DEF_EXPIRED) + self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_expired(api)) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_TXON elif state == self.CMIS_STATE_DP_TXON: if not self.check_datapath_state(api, host_lanes, ['DataPathInitialized']): @@ -1614,7 +1637,7 @@ def task_stop(self): self.task_stopping_event.set() self.task_thread.join() - def on_port_config_change(self, port_change_event): + def on_port_config_change(self, stopping_event, port_change_event): if port_change_event.event_type == port_mapping.PortChangeEvent.PORT_REMOVE: self.on_remove_logical_port(port_change_event) self.port_mapping.handle_port_change_event(port_change_event) From f7f910b260aa4ff96854673eafc2ed4e0b4412e8 Mon Sep 17 00:00:00 2001 From: byang Date: Thu, 15 Sep 2022 22:21:42 -0700 Subject: [PATCH 2/9] Fix xcvrd to support 400G ZR/DR optics --- 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 05024dcd1..a11689d1a 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -1960,7 +1960,7 @@ def task_stop(self): self.task_stopping_event.set() os.kill(self.task_process.pid, signal.SIGKILL) - def on_port_config_change(self , port_change_event): + def on_port_config_change(self, stopping_event, port_change_event): if port_change_event.event_type == port_mapping.PortChangeEvent.PORT_REMOVE: self.on_remove_logical_port(port_change_event) self.port_mapping.handle_port_change_event(port_change_event) From 97a6780c63eb2d81c5b3bfab8d650d6bd58ac9b8 Mon Sep 17 00:00:00 2001 From: byang Date: Mon, 19 Sep 2022 14:55:59 -0700 Subject: [PATCH 3/9] Revert changes in DomInfoUpdateTask::on_port_config_change --- 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 a11689d1a..55c14de77 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -1637,7 +1637,7 @@ def task_stop(self): self.task_stopping_event.set() self.task_thread.join() - def on_port_config_change(self, stopping_event, port_change_event): + def on_port_config_change(self, port_change_event): if port_change_event.event_type == port_mapping.PortChangeEvent.PORT_REMOVE: self.on_remove_logical_port(port_change_event) self.port_mapping.handle_port_change_event(port_change_event) From 96b025d612f410595eec6dbbf53655e3441dd2b1 Mon Sep 17 00:00:00 2001 From: byang Date: Mon, 19 Sep 2022 15:42:04 -0700 Subject: [PATCH 4/9] Fix xcvrd test after modifying on_port_config_change --- sonic-xcvrd/tests/test_xcvrd.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 63f2598b3..f71150128 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -634,14 +634,14 @@ def test_DomInfoUpdateTask_handle_port_change_event(self): task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping) task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE) port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD) - task.on_port_config_change(None, port_change_event) + task.on_port_config_change(port_change_event) assert task.port_mapping.logical_port_list.count('Ethernet0') assert task.port_mapping.get_asic_id_for_logical_port('Ethernet0') == 0 assert task.port_mapping.get_physical_to_logical(1) == ['Ethernet0'] assert task.port_mapping.get_logical_to_physical('Ethernet0') == [1] port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_REMOVE) - task.on_port_config_change(None, port_change_event) + task.on_port_config_change(port_change_event) assert not task.port_mapping.logical_port_list assert not task.port_mapping.logical_to_physical assert not task.port_mapping.physical_to_logical @@ -704,7 +704,7 @@ def test_SfpStateUpdateTask_handle_port_change_event(self, mock_table_helper): port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD) wait_time = 5 while wait_time > 0: - task.on_port_config_change(port_change_event) + task.on_port_config_change(None, port_change_event) if task.port_mapping.logical_port_list: break wait_time -= 1 @@ -717,7 +717,7 @@ def test_SfpStateUpdateTask_handle_port_change_event(self, mock_table_helper): port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_REMOVE) wait_time = 5 while wait_time > 0: - task.on_port_config_change(port_change_event) + task.on_port_config_change(None, port_change_event) if not task.port_mapping.logical_port_list: break wait_time -= 1 From 8fd66f6ce2039f0a86e83fe7edcc2b190836f9ce Mon Sep 17 00:00:00 2001 From: byang Date: Thu, 29 Sep 2022 13:43:50 -0700 Subject: [PATCH 5/9] Call get_datapath_init_duration to get the init expiration time. --- sonic-xcvrd/tests/test_xcvrd.py | 1 + sonic-xcvrd/xcvrd/xcvrd.py | 14 +++++--------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index f71150128..96b479f83 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -514,6 +514,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_datapath_init_duration = MagicMock(return_value=60000.0) mock_xcvr_api.get_application_advertisement = MagicMock(return_value={ 1: { 'host_electrical_interface_id': '400GAUI-8 C2M (Annex 120E)', diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 55c14de77..5eb04a39a 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -923,7 +923,6 @@ class CmisManagerTask: CMIS_MAX_RETRIES = 3 CMIS_DEF_EXPIRED = 60 # seconds, default expiration time - CMIS_DEF_EXPIRED_ZR = 200 # seconds, expiration time for ZR module CMIS_MODULE_TYPES = ['QSFP-DD', 'QSFP_DD', 'OSFP'] CMIS_NUM_CHANNELS = 8 @@ -1090,11 +1089,8 @@ def get_cmis_application_desired(self, api, channel, speed): return (appl_code & 0xf) - def get_cmis_expired(self, api): - if api.is_coherent_module(): - return self.CMIS_DEF_EXPIRED_ZR - else: - return self.CMIS_DEF_EXPIRED + def get_cmis_dp_init_duration(self, api): + return max(api.get_datapath_init_duration()/1000, self.CMIS_DEF_EXPIRED) def is_cmis_application_update_required(self, api, channel, speed): """ @@ -1481,7 +1477,7 @@ def task_worker(self): # TODO: Make sure this doesn't impact other datapaths api.set_lpmode(False) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_AP_CONF - self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_expired(api)) + self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_init_duration(api)) elif state == self.CMIS_STATE_AP_CONF: # TODO: Use fine grained time when the CMIS memory map is available if not self.check_module_state(api, ['ModuleReady']): @@ -1518,7 +1514,7 @@ def task_worker(self): continue # TODO: Use fine grained time when the CMIS memory map is available - self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_expired(api)) + self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_init_duration(api)) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_INIT elif state == self.CMIS_STATE_DP_INIT: if not self.check_config_error(api, host_lanes, ['ConfigSuccess']): @@ -1539,7 +1535,7 @@ def task_worker(self): # D.1.3 Software Configuration and Initialization api.set_datapath_init(host_lanes) # TODO: Use fine grained timeout when the CMIS memory map is available - self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_expired(api)) + self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_init_duration(api)) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_TXON elif state == self.CMIS_STATE_DP_TXON: if not self.check_datapath_state(api, host_lanes, ['DataPathInitialized']): From 3a3b96a1685db7aaadcd306f5d26cc6e1c381be6 Mon Sep 17 00:00:00 2001 From: byang Date: Fri, 30 Sep 2022 18:19:37 -0700 Subject: [PATCH 6/9] Address comments 1. Clean up comments. 2. Revert port breakout fix and corresponding test change. 3. Check deinit duration for deinit case 4. Check datapath init pending --- sonic-xcvrd/tests/test_xcvrd.py | 15 ++++++++++-- sonic-xcvrd/xcvrd/xcvrd.py | 43 ++++++++++++++++++++++++++++----- 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 96b479f83..23f0cc8bf 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -515,6 +515,17 @@ def test_CmisManagerTask_task_worker(self, mock_chassis): 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_datapath_init_duration = MagicMock(return_value=60000.0) + mock_xcvr_api.get_datapath_deinit_duration = MagicMock(return_value=600000.0) + mock_xcvr_api.get_dpinit_pending = MagicMock(return_value={ + 'DPInitPending1': False, + 'DPInitPending2': False, + 'DPInitPending3': False, + 'DPInitPending4': False, + 'DPInitPending5': False, + 'DPInitPending6': False, + 'DPInitPending7': False, + 'DPInitPending8': False + }) mock_xcvr_api.get_application_advertisement = MagicMock(return_value={ 1: { 'host_electrical_interface_id': '400GAUI-8 C2M (Annex 120E)', @@ -705,7 +716,7 @@ def test_SfpStateUpdateTask_handle_port_change_event(self, mock_table_helper): port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD) wait_time = 5 while wait_time > 0: - task.on_port_config_change(None, port_change_event) + task.on_port_config_change(port_change_event) if task.port_mapping.logical_port_list: break wait_time -= 1 @@ -718,7 +729,7 @@ def test_SfpStateUpdateTask_handle_port_change_event(self, mock_table_helper): port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_REMOVE) wait_time = 5 while wait_time > 0: - task.on_port_config_change(None, port_change_event) + task.on_port_config_change(port_change_event) if not task.port_mapping.logical_port_list: break wait_time -= 1 diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 5eb04a39a..206706a37 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -1090,7 +1090,10 @@ def get_cmis_application_desired(self, api, channel, speed): return (appl_code & 0xf) def get_cmis_dp_init_duration(self, api): - return max(api.get_datapath_init_duration()/1000, self.CMIS_DEF_EXPIRED) + return api.get_datapath_init_duration()/1000 + + def get_cmis_dp_deinit_duration(self, api): + return api.get_datapath_deinit_duration()/1000 def is_cmis_application_update_required(self, api, channel, speed): """ @@ -1195,6 +1198,32 @@ def check_config_error(self, api, channel, states): return done + def check_datapath_init_pending(self, api, channel): + """ + Check if the CMIS datapath init is pending + + Args: + api: + XcvrApi object + channel: + Integer, a bitmask of the lanes on the host side + e.g. 0x5 for lane 0 and lane 2. + + Returns: + Boolean, true if any lanes are pending datapath init, otherwise false + """ + pending = False + dpinit_pending_dict = api.get_dpinit_pending() + for lane in range(self.CMIS_NUM_CHANNELS): + if ((1 << lane) & channel) == 0: + continue + key = "DPInitPending{}".format(lane + 1) + if dpinit_pending_dict[key]: + pending = True + break + + return pending + def check_datapath_state(self, api, channel, states): """ Check if the CMIS datapath states are in the specified state @@ -1477,7 +1506,7 @@ def task_worker(self): # TODO: Make sure this doesn't impact other datapaths api.set_lpmode(False) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_AP_CONF - self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_init_duration(api)) + self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_deinit_duration(api)) elif state == self.CMIS_STATE_AP_CONF: # TODO: Use fine grained time when the CMIS memory map is available if not self.check_module_state(api, ['ModuleReady']): @@ -1513,8 +1542,11 @@ def task_worker(self): self.force_cmis_reinit(lport, retries + 1) continue - # TODO: Use fine grained time when the CMIS memory map is available - self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_init_duration(api)) + if self.check_datapath_init_pending(api, host_lanes): + self.log_notice("{}: datapath init pending".format(lport)) + self.force_cmis_reinit(lport, retries + 1) + continue + self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_INIT elif state == self.CMIS_STATE_DP_INIT: if not self.check_config_error(api, host_lanes, ['ConfigSuccess']): @@ -1534,7 +1566,6 @@ def task_worker(self): # D.1.3 Software Configuration and Initialization api.set_datapath_init(host_lanes) - # TODO: Use fine grained timeout when the CMIS memory map is available self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_init_duration(api)) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_TXON elif state == self.CMIS_STATE_DP_TXON: @@ -1956,7 +1987,7 @@ def task_stop(self): self.task_stopping_event.set() os.kill(self.task_process.pid, signal.SIGKILL) - def on_port_config_change(self, stopping_event, port_change_event): + def on_port_config_change(self , port_change_event): if port_change_event.event_type == port_mapping.PortChangeEvent.PORT_REMOVE: self.on_remove_logical_port(port_change_event) self.port_mapping.handle_port_change_event(port_change_event) From 8b52afca8818476bccf6a3412db1624433971b20 Mon Sep 17 00:00:00 2001 From: byang Date: Tue, 4 Oct 2022 14:56:18 -0700 Subject: [PATCH 7/9] 1. Revert changes in on_port_update_event 2. check DpInitPending on module which supports CMIS 5.0 3. Specify sec in return value --- sonic-xcvrd/tests/test_xcvrd.py | 16 ++++----- sonic-xcvrd/xcvrd/xcvrd.py | 59 +++++++++++++-------------------- 2 files changed, 31 insertions(+), 44 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 23f0cc8bf..50678ca67 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -517,14 +517,14 @@ def test_CmisManagerTask_task_worker(self, mock_chassis): mock_xcvr_api.get_datapath_init_duration = MagicMock(return_value=60000.0) mock_xcvr_api.get_datapath_deinit_duration = MagicMock(return_value=600000.0) mock_xcvr_api.get_dpinit_pending = MagicMock(return_value={ - 'DPInitPending1': False, - 'DPInitPending2': False, - 'DPInitPending3': False, - 'DPInitPending4': False, - 'DPInitPending5': False, - 'DPInitPending6': False, - 'DPInitPending7': False, - 'DPInitPending8': False + 'DPInitPending1': True, + 'DPInitPending2': True, + 'DPInitPending3': True, + 'DPInitPending4': True, + 'DPInitPending5': True, + 'DPInitPending6': True, + 'DPInitPending7': True, + 'DPInitPending8': True }) mock_xcvr_api.get_application_advertisement = MagicMock(return_value={ 1: { diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 206706a37..844efa74f 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -986,38 +986,22 @@ def on_port_update_event(self, port_change_event): return if port_change_event.event_type == port_change_event.PORT_SET: - need_update = False if pport >= 0: - if self.port_dict[lport].get('index') != pport: - self.port_dict[lport]['index'] = pport - need_update = True + self.port_dict[lport]['index'] = pport if 'speed' in port_change_event.port_dict and port_change_event.port_dict['speed'] != 'N/A': - if self.port_dict[lport].get('speed') != port_change_event.port_dict['speed']: - self.port_dict[lport]['speed'] = port_change_event.port_dict['speed'] - need_update = True + self.port_dict[lport]['speed'] = port_change_event.port_dict['speed'] if 'lanes' in port_change_event.port_dict: - if self.port_dict[lport].get('lanes') != port_change_event.port_dict['lanes']: - self.port_dict[lport]['lanes'] = port_change_event.port_dict['lanes'] - need_update = True + self.port_dict[lport]['lanes'] = port_change_event.port_dict['lanes'] if 'host_tx_ready' in port_change_event.port_dict: - if self.port_dict[lport].get('host_tx_ready') != port_change_event.port_dict['host_tx_ready']: - self.port_dict[lport]['host_tx_ready'] = port_change_event.port_dict['host_tx_ready'] - need_update = True + self.port_dict[lport]['host_tx_ready'] = port_change_event.port_dict['host_tx_ready'] if 'admin_status' in port_change_event.port_dict: - if self.port_dict[lport].get('admin_status') != port_change_event.port_dict['admin_status']: - self.port_dict[lport]['admin_status'] = port_change_event.port_dict['admin_status'] - need_update = True + self.port_dict[lport]['admin_status'] = port_change_event.port_dict['admin_status'] if 'laser_freq' in port_change_event.port_dict: - if self.port_dict[lport].get('laser_freq') != int(port_change_event.port_dict['laser_freq']): - self.port_dict[lport]['laser_freq'] = int(port_change_event.port_dict['laser_freq']) - need_update = True + self.port_dict[lport]['laser_freq'] = int(port_change_event.port_dict['laser_freq']) if 'tx_power' in port_change_event.port_dict: - if self.port_dict[lport].get('tx_power') != float(port_change_event.port_dict['tx_power']): - self.port_dict[lport]['tx_power'] = float(port_change_event.port_dict['tx_power']) - need_update = True + self.port_dict[lport]['tx_power'] = float(port_change_event.port_dict['tx_power']) - if need_update: - self.force_cmis_reinit(lport, 0) + self.force_cmis_reinit(lport, 0) else: self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_REMOVED @@ -1089,10 +1073,10 @@ def get_cmis_application_desired(self, api, channel, speed): return (appl_code & 0xf) - def get_cmis_dp_init_duration(self, api): + def get_cmis_dp_init_duration_secs(self, api): return api.get_datapath_init_duration()/1000 - def get_cmis_dp_deinit_duration(self, api): + def get_cmis_dp_deinit_duration_secs(self, api): return api.get_datapath_deinit_duration()/1000 def is_cmis_application_update_required(self, api, channel, speed): @@ -1210,16 +1194,16 @@ def check_datapath_init_pending(self, api, channel): e.g. 0x5 for lane 0 and lane 2. Returns: - Boolean, true if any lanes are pending datapath init, otherwise false + Boolean, true if all lanes are pending datapath init, otherwise false """ - pending = False + pending = True dpinit_pending_dict = api.get_dpinit_pending() for lane in range(self.CMIS_NUM_CHANNELS): if ((1 << lane) & channel) == 0: continue key = "DPInitPending{}".format(lane + 1) - if dpinit_pending_dict[key]: - pending = True + if not dpinit_pending_dict[key]: + pending = False break return pending @@ -1506,7 +1490,7 @@ def task_worker(self): # TODO: Make sure this doesn't impact other datapaths api.set_lpmode(False) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_AP_CONF - self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_deinit_duration(api)) + self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_deinit_duration_secs(api)) elif state == self.CMIS_STATE_AP_CONF: # TODO: Use fine grained time when the CMIS memory map is available if not self.check_module_state(api, ['ModuleReady']): @@ -1542,10 +1526,13 @@ def task_worker(self): self.force_cmis_reinit(lport, retries + 1) continue - if self.check_datapath_init_pending(api, host_lanes): - self.log_notice("{}: datapath init pending".format(lport)) - self.force_cmis_reinit(lport, retries + 1) - continue + if getattr(api, 'get_cmis_rev', None): + # Check datapath init pending on module that supports CMIS 5.x + majorRev = int(api.get_cmis_rev().split('.')[0]) + if majorRev >= 5 and not self.check_datapath_init_pending(api, host_lanes): + self.log_notice("{}: datapath init not pending".format(lport)) + self.force_cmis_reinit(lport, retries + 1) + continue self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_INIT elif state == self.CMIS_STATE_DP_INIT: @@ -1566,7 +1553,7 @@ def task_worker(self): # D.1.3 Software Configuration and Initialization api.set_datapath_init(host_lanes) - self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_init_duration(api)) + self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_init_duration_secs(api)) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_TXON elif state == self.CMIS_STATE_DP_TXON: if not self.check_datapath_state(api, host_lanes, ['DataPathInitialized']): From 88641db20c612d78fd62528d101923a8f641e48b Mon Sep 17 00:00:00 2001 From: byang Date: Sun, 9 Oct 2022 23:17:38 -0700 Subject: [PATCH 8/9] Add code coverage --- sonic-xcvrd/tests/test_xcvrd.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 50678ca67..4af09bf23 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -516,6 +516,7 @@ def test_CmisManagerTask_task_worker(self, mock_chassis): mock_xcvr_api.get_module_type_abbreviation = MagicMock(return_value='QSFP-DD') mock_xcvr_api.get_datapath_init_duration = MagicMock(return_value=60000.0) mock_xcvr_api.get_datapath_deinit_duration = MagicMock(return_value=600000.0) + mock_xcvr_api.get_cmis_rev = MagicMock(return_value='5.0') mock_xcvr_api.get_dpinit_pending = MagicMock(return_value={ 'DPInitPending1': True, 'DPInitPending2': True, From ea29589b7b689267498cbd72d99d12ca2d43a68e Mon Sep 17 00:00:00 2001 From: byang Date: Wed, 19 Oct 2022 12:15:49 -0700 Subject: [PATCH 9/9] Add log for DpInit/DpDeinit duration --- sonic-xcvrd/xcvrd/xcvrd.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 844efa74f..8b2328dbf 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -1490,7 +1490,9 @@ def task_worker(self): # TODO: Make sure this doesn't impact other datapaths api.set_lpmode(False) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_AP_CONF - self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_deinit_duration_secs(api)) + dpDeinitDuration = self.get_cmis_dp_deinit_duration_secs(api) + self.log_notice("{} DpDeinit duration {} secs".format(lport, dpDeinitDuration)) + self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=dpDeinitDuration) elif state == self.CMIS_STATE_AP_CONF: # TODO: Use fine grained time when the CMIS memory map is available if not self.check_module_state(api, ['ModuleReady']): @@ -1553,7 +1555,9 @@ def task_worker(self): # D.1.3 Software Configuration and Initialization api.set_datapath_init(host_lanes) - self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_init_duration_secs(api)) + dpInitDuration = self.get_cmis_dp_init_duration_secs(api) + self.log_notice("{} DpInit duration {} secs".format(lport, dpInitDuration)) + self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=dpInitDuration) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_TXON elif state == self.CMIS_STATE_DP_TXON: if not self.check_datapath_state(api, host_lanes, ['DataPathInitialized']):