Skip to content

Commit

Permalink
Disable periodic polling of port in DomInfoUpdateTask thread during C…
Browse files Browse the repository at this point in the history
…MIS init (#449) (#450)

Cherry-pick for #449

Description
We currently need to disable periodic DOM polling of a port through DomInfoUpdateTask thread during CMIS initialization.

Motivation and Context
Disabling of DOM polling during CMIS initialization is primarily needed to disable sending CDB commands to read FW version from DomInfoUpdateTask thread during CMIS initialization.
For transceivers which do not support CDB background mode, any EEPROM access to the module can fail if a CDB command is executed at the same time.

In order to disable DOM polling during CMIS initialization, the cmis_state from CmisManagerTask thread is now being updated in the TRANSCEIVER_STATUS table of STATE_DB.
If the current cmis_state does not belong to CMIS_TERMINAL_STATES, DomInfoUpdateTask will disable DOM polling for the port to allow CmisManagerTask thread to complete CMIS initialization if required.
For platforms with CmisManagerTask disabled, the function is_port_in_cmis_initialization_process will always return False to allow DOM polling.
In case of device boot-up or transceiver insertion, the DomInfoUpdateTask thread will wait for the port to be in either of the CMIS_TERMINAL_STATES before proceeding with DOM polling. For non-CMIS transceivers, the expected cmis_state is CMIS_STATE_READY. Hence, once the corresponding port reaches CMIS_STATE_READY state, DOM polling will be enabled for such port.

Also, the cmis_state is not planned to be modified by DomInfoUpdateTask thread at any time to prevent race condition with CmisManagerTask.

How Has This Been Tested?
Following is the summary of tests performed

1 Ensure DOM thread polls for non-CMIS transceivers
2 Ensure DOM thread polls for platform with CMIS manager disabled
3 Ensure DOM thread waits during CMIS initialization
4 Ensure DOM polling is resumed after transceiver insertion
4.1 After removal
4.2 After insertion
5. Ensured `show interface transceiver status` CLI works with the current changes

Redis-db dump snippet to show `cmis_state` field in TRANSCEIVER_STATUS table
root@sonic:/home/admin# redis-cli -n 6 hgetall "TRANSCEIVER_STATUS|Ethernet0"
  1) "cmis_state"
  2) "READY"
Additional Information (Optional)
MSFT ADO - 26993372
  • Loading branch information
mihirpat1 authored Mar 21, 2024
1 parent 72e9a28 commit e6e34c1
Show file tree
Hide file tree
Showing 2 changed files with 185 additions and 61 deletions.
91 changes: 80 additions & 11 deletions sonic-xcvrd/tests/test_xcvrd.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ def test_CmisManagerTask_task_run_with_exception(self):
def test_DomInfoUpdateTask_task_run_with_exception(self):
port_mapping = PortMapping()
stop_event = threading.Event()
dom_info_update = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
mock_cmis_manager = MagicMock()
dom_info_update = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, mock_cmis_manager)
exception_received = None
trace = None
try:
Expand Down Expand Up @@ -230,6 +231,16 @@ def test_del_port_sfp_dom_info_from_db(self):
firmware_info_tbl = Table("STATE_DB", TRANSCEIVER_FIRMWARE_INFO_TABLE)
del_port_sfp_dom_info_from_db(logical_port_name, port_mapping, init_tbl, dom_tbl, dom_threshold_tbl, pm_tbl, firmware_info_tbl)

@pytest.mark.parametrize("mock_found, mock_status_dict, expected_cmis_state", [
(True, {'cmis_state': CMIS_STATE_INSERTED}, CMIS_STATE_INSERTED),
(False, {}, CMIS_STATE_UNKNOWN),
(True, {'other_key': 'some_value'}, CMIS_STATE_UNKNOWN)
])
def test_get_cmis_state_from_state_db(self, mock_found, mock_status_dict, expected_cmis_state):
status_tbl = MagicMock()
status_tbl.get.return_value = (mock_found, mock_status_dict)
assert get_cmis_state_from_state_db("Ethernet0", status_tbl) == expected_cmis_state

@patch('xcvrd.xcvrd.get_physical_port_name_dict', MagicMock(return_value={0: 'Ethernet0'}))
@patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=True))
@patch('xcvrd.xcvrd._wrapper_get_transceiver_status', MagicMock(return_value={'module_state': 'ModuleReady',
Expand Down Expand Up @@ -649,6 +660,22 @@ def test_DaemonXcvrd_run(self, mock_task_stop1, mock_task_stop2, mock_task_run1,
assert mock_deinit.call_count == 1
assert mock_init.call_count == 1

def test_CmisManagerTask_update_port_transceiver_status_table_sw_cmis_state(self):
port_mapping = PortMapping()
stop_event = threading.Event()
task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_SET)
task.on_port_update_event(port_change_event)

task.xcvr_table_helper.get_status_tbl = MagicMock(return_value=None)
task.update_port_transceiver_status_table_sw_cmis_state("Ethernet0", CMIS_STATE_INSERTED)

mock_get_status_tbl = MagicMock()
mock_get_status_tbl.set = MagicMock()
task.xcvr_table_helper.get_status_tbl.return_value = mock_get_status_tbl
task.update_port_transceiver_status_table_sw_cmis_state("Ethernet0", CMIS_STATE_INSERTED)
assert mock_get_status_tbl.set.call_count == 1

@patch('xcvrd.xcvrd._wrapper_get_sfp_type', MagicMock(return_value='QSFP_DD'))
def test_CmisManagerTask_handle_port_change_event(self):
port_mapping = PortMapping()
Expand Down Expand Up @@ -903,12 +930,14 @@ def test_CmisManagerTask_post_port_active_apsel_to_db(self):
ret = task.post_port_active_apsel_to_db(mock_xcvr_api, lport, host_lanes_mask)
assert int_tbl.getKeys() == []

@patch('xcvrd.xcvrd.XcvrTableHelper.get_status_tbl')
@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())
@patch('xcvrd.xcvrd._wrapper_get_sfp_type', MagicMock(return_value='QSFP_DD'))
@patch('xcvrd.xcvrd.CmisManagerTask.wait_for_port_config_done', MagicMock())
def test_CmisManagerTask_task_worker(self, mock_chassis):
def test_CmisManagerTask_task_worker(self, mock_chassis, mock_get_status_tbl):
mock_get_status_tbl = Table("STATE_DB", TRANSCEIVER_STATUS_TABLE)
mock_xcvr_api = MagicMock()
mock_xcvr_api.set_datapath_deinit = MagicMock(return_value=True)
mock_xcvr_api.set_datapath_init = MagicMock(return_value=True)
Expand Down Expand Up @@ -1005,7 +1034,13 @@ def test_CmisManagerTask_task_worker(self, mock_chassis):
port_mapping = PortMapping()
stop_event = threading.Event()
task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
task.port_mapping.logical_port_list = ['Ethernet0']
task.xcvr_table_helper.get_status_tbl.return_value = mock_get_status_tbl
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
task.task_worker()
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.port_mapping.get_asic_id_for_logical_port('Ethernet0'))) == CMIS_STATE_UNKNOWN

task.port_mapping.logical_port_list = MagicMock()
port_change_event = PortChangeEvent('PortConfigDone', -1, 0, PortChangeEvent.PORT_SET)
task.on_port_update_event(port_change_event)
assert task.isPortConfigDone
Expand All @@ -1014,6 +1049,7 @@ def test_CmisManagerTask_task_worker(self, mock_chassis):
{'speed':'400000', 'lanes':'1,2,3,4,5,6,7,8'})
task.on_port_update_event(port_change_event)
assert len(task.port_dict) == 1
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.port_mapping.get_asic_id_for_logical_port('Ethernet0'))) == CMIS_STATE_INSERTED

task.get_host_tx_status = MagicMock(return_value='true')
task.get_port_admin_status = MagicMock(return_value='up')
Expand All @@ -1025,31 +1061,38 @@ def test_CmisManagerTask_task_worker(self, mock_chassis):
# Case 1: Module Inserted --> DP_DEINIT
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
task.task_worker()
assert task.port_dict['Ethernet0']['cmis_state'] == 'DP_DEINIT'
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.port_mapping.get_asic_id_for_logical_port('Ethernet0'))) == CMIS_STATE_DP_DEINIT
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
task.task_worker()
assert mock_xcvr_api.set_datapath_deinit.call_count == 1
assert mock_xcvr_api.tx_disable_channel.call_count == 1
assert mock_xcvr_api.set_lpmode.call_count == 1
assert task.port_dict['Ethernet0']['cmis_state'] == 'AP_CONFIGURED'
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.port_mapping.get_asic_id_for_logical_port('Ethernet0'))) == CMIS_STATE_AP_CONF

# Case 2: DP_DEINIT --> AP Configured
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
task.task_worker()
assert mock_xcvr_api.set_application.call_count == 1
assert task.port_dict['Ethernet0']['cmis_state'] == 'DP_INIT'
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.port_mapping.get_asic_id_for_logical_port('Ethernet0'))) == CMIS_STATE_DP_INIT

# Case 3: AP Configured --> DP_INIT
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
task.task_worker()
assert mock_xcvr_api.set_datapath_init.call_count == 1
assert task.port_dict['Ethernet0']['cmis_state'] == 'DP_TXON'
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.port_mapping.get_asic_id_for_logical_port('Ethernet0'))) == CMIS_STATE_DP_TXON

# Case 4: DP_INIT --> DP_TXON
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
task.task_worker()
assert mock_xcvr_api.tx_disable_channel.call_count == 2
assert task.port_dict['Ethernet0']['cmis_state'] == 'DP_ACTIVATION'
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.port_mapping.get_asic_id_for_logical_port('Ethernet0'))) == CMIS_STATE_DP_ACTIVATE

# Case 5: DP_TXON --> DP_ACTIVATION
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
task.post_port_active_apsel_to_db = MagicMock()
task.task_worker()
assert task.post_port_active_apsel_to_db.call_count == 1
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.port_mapping.get_asic_id_for_logical_port('Ethernet0'))) == CMIS_STATE_READY

@pytest.mark.parametrize("lport, expected_dom_polling", [
('Ethernet0', 'disabled'),
Expand All @@ -1071,7 +1114,8 @@ def mock_get(key):

port_mapping = PortMapping()
stop_event = threading.Event()
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
mock_cmis_manager = MagicMock()
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, mock_cmis_manager)
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
task.port_mapping.handle_port_change_event(PortChangeEvent('Ethernet4', 1, 0, PortChangeEvent.PORT_ADD))
task.port_mapping.handle_port_change_event(PortChangeEvent('Ethernet12', 1, 0, PortChangeEvent.PORT_ADD))
Expand All @@ -1084,12 +1128,34 @@ def mock_get(key):

assert task.get_dom_polling_from_config_db(lport) == expected_dom_polling

@pytest.mark.parametrize("skip_cmis_manager, is_asic_index_none, mock_cmis_state, expected_result", [
(True, False, None, False),
(False, False, CMIS_STATE_INSERTED, True),
(False, False, CMIS_STATE_READY, False),
(False, False, CMIS_STATE_UNKNOWN, True),
(False, True, None, False),
])
@patch('xcvrd.xcvrd.get_cmis_state_from_state_db')
def test_DomInfoUpdateTask_is_port_in_cmis_initialization_process(self, mock_get_cmis_state_from_state_db, skip_cmis_manager, is_asic_index_none, mock_cmis_state, expected_result):
port_mapping = PortMapping()
lport = 'Ethernet0'
port_change_event = PortChangeEvent(lport, 1, 0, PortChangeEvent.PORT_ADD)
stop_event = threading.Event()
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, skip_cmis_manager)
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
task.on_port_config_change(port_change_event)
mock_get_cmis_state_from_state_db.return_value = mock_cmis_state
if is_asic_index_none:
lport='INVALID_PORT'
assert task.is_port_in_cmis_initialization_process(lport) == expected_result

@patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock())
@patch('xcvrd.xcvrd.delete_port_from_status_table_hw')
def test_DomInfoUpdateTask_handle_port_change_event(self, mock_del_status_tbl_hw):
port_mapping = PortMapping()
stop_event = threading.Event()
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
mock_cmis_manager = MagicMock()
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, mock_cmis_manager)
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)
Expand All @@ -1112,7 +1178,8 @@ def test_DomInfoUpdateTask_handle_port_change_event(self, mock_del_status_tbl_hw
def test_DomInfoUpdateTask_task_run_stop(self):
port_mapping = PortMapping()
stop_event = threading.Event()
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
mock_cmis_manager = MagicMock()
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, mock_cmis_manager)
task.start()
task.join()
assert not task.is_alive()
Expand All @@ -1137,10 +1204,12 @@ def test_DomInfoUpdateTask_task_worker(self, mock_post_pm_info, mock_update_stat

port_mapping = PortMapping()
stop_event = threading.Event()
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
mock_cmis_manager = MagicMock()
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, mock_cmis_manager)
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
task.task_stopping_event.wait = MagicMock(side_effect=[False, True])
task.get_dom_polling_from_config_db = MagicMock(return_value='enabled')
task.is_port_in_cmis_terminal_state = MagicMock(return_value=False)
mock_detect_error.return_value = True
task.task_worker()
assert task.port_mapping.logical_port_list.count('Ethernet0')
Expand Down
Loading

0 comments on commit e6e34c1

Please sign in to comment.