Skip to content

Commit 2770fd2

Browse files
authored
Disable periodic polling of port in DomInfoUpdateTask thread during CMIS init (#449)
* Disable periodic polling of port in DomInfoUpdateTask thread during CMIS init Signed-off-by: Mihir Patel <[email protected]> * Replaced sharing of cmis_state from instance to a field in TRANSCEIVER_STATUS table * Improved code coverage * Moved update_port_transceiver_status_table_sw_cmis_state to CmisManagerTask class * Initializing cmis_state to CMIS_STATE_UNKNOWN while spawning CmisManagerTask --------- Signed-off-by: Mihir Patel <[email protected]>
1 parent d513eca commit 2770fd2

File tree

2 files changed

+185
-62
lines changed

2 files changed

+185
-62
lines changed

sonic-xcvrd/tests/test_xcvrd.py

+80-11
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,8 @@ def test_CmisManagerTask_task_run_with_exception(self):
182182
def test_DomInfoUpdateTask_task_run_with_exception(self):
183183
port_mapping = PortMapping()
184184
stop_event = threading.Event()
185-
dom_info_update = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
185+
mock_cmis_manager = MagicMock()
186+
dom_info_update = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, mock_cmis_manager)
186187
exception_received = None
187188
trace = None
188189
try:
@@ -356,6 +357,16 @@ def test_del_port_sfp_dom_info_from_db(self):
356357
firmware_info_tbl = Table("STATE_DB", TRANSCEIVER_FIRMWARE_INFO_TABLE)
357358
del_port_sfp_dom_info_from_db(logical_port_name, port_mapping, init_tbl, dom_tbl, dom_threshold_tbl, pm_tbl, firmware_info_tbl)
358359

360+
@pytest.mark.parametrize("mock_found, mock_status_dict, expected_cmis_state", [
361+
(True, {'cmis_state': CMIS_STATE_INSERTED}, CMIS_STATE_INSERTED),
362+
(False, {}, CMIS_STATE_UNKNOWN),
363+
(True, {'other_key': 'some_value'}, CMIS_STATE_UNKNOWN)
364+
])
365+
def test_get_cmis_state_from_state_db(self, mock_found, mock_status_dict, expected_cmis_state):
366+
status_tbl = MagicMock()
367+
status_tbl.get.return_value = (mock_found, mock_status_dict)
368+
assert get_cmis_state_from_state_db("Ethernet0", status_tbl) == expected_cmis_state
369+
359370
@patch('xcvrd.xcvrd.get_physical_port_name_dict', MagicMock(return_value={0: 'Ethernet0'}))
360371
@patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=True))
361372
@patch('xcvrd.xcvrd._wrapper_get_transceiver_status', MagicMock(return_value={'module_state': 'ModuleReady',
@@ -1308,6 +1319,22 @@ def test_SffManagerTask_task_worker(self, mock_chassis):
13081319
assert mock_xcvr_api.tx_disable_channel.call_count == 2
13091320
mock_sfp.get_presence = MagicMock(return_value=True)
13101321

1322+
def test_CmisManagerTask_update_port_transceiver_status_table_sw_cmis_state(self):
1323+
port_mapping = PortMapping()
1324+
stop_event = threading.Event()
1325+
task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
1326+
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_SET)
1327+
task.on_port_update_event(port_change_event)
1328+
1329+
task.xcvr_table_helper.get_status_tbl = MagicMock(return_value=None)
1330+
task.update_port_transceiver_status_table_sw_cmis_state("Ethernet0", CMIS_STATE_INSERTED)
1331+
1332+
mock_get_status_tbl = MagicMock()
1333+
mock_get_status_tbl.set = MagicMock()
1334+
task.xcvr_table_helper.get_status_tbl.return_value = mock_get_status_tbl
1335+
task.update_port_transceiver_status_table_sw_cmis_state("Ethernet0", CMIS_STATE_INSERTED)
1336+
assert mock_get_status_tbl.set.call_count == 1
1337+
13111338
@patch('xcvrd.xcvrd._wrapper_get_sfp_type', MagicMock(return_value='QSFP_DD'))
13121339
def test_CmisManagerTask_handle_port_change_event(self):
13131340
port_mapping = PortMapping()
@@ -1585,12 +1612,14 @@ def test_CmisManagerTask_post_port_active_apsel_to_db(self):
15851612
assert int_tbl.getKeys() == []
15861613

15871614

1615+
@patch('xcvrd.xcvrd.XcvrTableHelper.get_status_tbl')
15881616
@patch('xcvrd.xcvrd.platform_chassis')
15891617
@patch('xcvrd.xcvrd.PortChangeObserver', MagicMock(handle_port_update_event=MagicMock()))
15901618
@patch('xcvrd.xcvrd._wrapper_get_sfp_type', MagicMock(return_value='QSFP_DD'))
15911619
@patch('xcvrd.xcvrd.CmisManagerTask.wait_for_port_config_done', MagicMock())
15921620
@patch('xcvrd.xcvrd.is_cmis_api', MagicMock(return_value=True))
1593-
def test_CmisManagerTask_task_worker(self, mock_chassis):
1621+
def test_CmisManagerTask_task_worker(self, mock_chassis, mock_get_status_tbl):
1622+
mock_get_status_tbl = Table("STATE_DB", TRANSCEIVER_STATUS_TABLE)
15941623
mock_xcvr_api = MagicMock()
15951624
mock_xcvr_api.set_datapath_deinit = MagicMock(return_value=True)
15961625
mock_xcvr_api.set_datapath_init = MagicMock(return_value=True)
@@ -1687,7 +1716,13 @@ def test_CmisManagerTask_task_worker(self, mock_chassis):
16871716
port_mapping = PortMapping()
16881717
stop_event = threading.Event()
16891718
task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
1719+
task.port_mapping.logical_port_list = ['Ethernet0']
1720+
task.xcvr_table_helper.get_status_tbl.return_value = mock_get_status_tbl
1721+
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
1722+
task.task_worker()
1723+
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
16901724

1725+
task.port_mapping.logical_port_list = MagicMock()
16911726
port_change_event = PortChangeEvent('PortConfigDone', -1, 0, PortChangeEvent.PORT_SET)
16921727
task.on_port_update_event(port_change_event)
16931728
assert task.isPortConfigDone
@@ -1696,6 +1731,7 @@ def test_CmisManagerTask_task_worker(self, mock_chassis):
16961731
{'speed':'400000', 'lanes':'1,2,3,4,5,6,7,8'})
16971732
task.on_port_update_event(port_change_event)
16981733
assert len(task.port_dict) == 1
1734+
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
16991735

17001736
task.get_host_tx_status = MagicMock(return_value='true')
17011737
task.get_port_admin_status = MagicMock(return_value='up')
@@ -1707,31 +1743,38 @@ def test_CmisManagerTask_task_worker(self, mock_chassis):
17071743
# Case 1: Module Inserted --> DP_DEINIT
17081744
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
17091745
task.task_worker()
1710-
assert task.port_dict['Ethernet0']['cmis_state'] == 'DP_DEINIT'
1746+
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
17111747
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
17121748
task.task_worker()
17131749
assert mock_xcvr_api.set_datapath_deinit.call_count == 1
17141750
assert mock_xcvr_api.tx_disable_channel.call_count == 1
17151751
assert mock_xcvr_api.set_lpmode.call_count == 1
1716-
assert task.port_dict['Ethernet0']['cmis_state'] == 'AP_CONFIGURED'
1752+
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
17171753

17181754
# Case 2: DP_DEINIT --> AP Configured
17191755
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
17201756
task.task_worker()
17211757
assert mock_xcvr_api.set_application.call_count == 1
1722-
assert task.port_dict['Ethernet0']['cmis_state'] == 'DP_INIT'
1758+
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
17231759

17241760
# Case 3: AP Configured --> DP_INIT
17251761
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
17261762
task.task_worker()
17271763
assert mock_xcvr_api.set_datapath_init.call_count == 1
1728-
assert task.port_dict['Ethernet0']['cmis_state'] == 'DP_TXON'
1764+
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
17291765

17301766
# Case 4: DP_INIT --> DP_TXON
17311767
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
17321768
task.task_worker()
17331769
assert mock_xcvr_api.tx_disable_channel.call_count == 2
1734-
assert task.port_dict['Ethernet0']['cmis_state'] == 'DP_ACTIVATION'
1770+
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
1771+
1772+
# Case 5: DP_TXON --> DP_ACTIVATION
1773+
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
1774+
task.post_port_active_apsel_to_db = MagicMock()
1775+
task.task_worker()
1776+
assert task.post_port_active_apsel_to_db.call_count == 1
1777+
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
17351778

17361779
@pytest.mark.parametrize("lport, expected_dom_polling", [
17371780
('Ethernet0', 'disabled'),
@@ -1753,7 +1796,8 @@ def mock_get(key):
17531796

17541797
port_mapping = PortMapping()
17551798
stop_event = threading.Event()
1756-
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
1799+
mock_cmis_manager = MagicMock()
1800+
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, mock_cmis_manager)
17571801
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
17581802
task.port_mapping.handle_port_change_event(PortChangeEvent('Ethernet4', 1, 0, PortChangeEvent.PORT_ADD))
17591803
task.port_mapping.handle_port_change_event(PortChangeEvent('Ethernet12', 1, 0, PortChangeEvent.PORT_ADD))
@@ -1766,12 +1810,34 @@ def mock_get(key):
17661810

17671811
assert task.get_dom_polling_from_config_db(lport) == expected_dom_polling
17681812

1813+
@pytest.mark.parametrize("skip_cmis_manager, is_asic_index_none, mock_cmis_state, expected_result", [
1814+
(True, False, None, False),
1815+
(False, False, CMIS_STATE_INSERTED, True),
1816+
(False, False, CMIS_STATE_READY, False),
1817+
(False, False, CMIS_STATE_UNKNOWN, True),
1818+
(False, True, None, False),
1819+
])
1820+
@patch('xcvrd.xcvrd.get_cmis_state_from_state_db')
1821+
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):
1822+
port_mapping = PortMapping()
1823+
lport = 'Ethernet0'
1824+
port_change_event = PortChangeEvent(lport, 1, 0, PortChangeEvent.PORT_ADD)
1825+
stop_event = threading.Event()
1826+
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, skip_cmis_manager)
1827+
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
1828+
task.on_port_config_change(port_change_event)
1829+
mock_get_cmis_state_from_state_db.return_value = mock_cmis_state
1830+
if is_asic_index_none:
1831+
lport='INVALID_PORT'
1832+
assert task.is_port_in_cmis_initialization_process(lport) == expected_result
1833+
17691834
@patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock())
17701835
@patch('xcvrd.xcvrd.delete_port_from_status_table_hw')
17711836
def test_DomInfoUpdateTask_handle_port_change_event(self, mock_del_status_tbl_hw):
17721837
port_mapping = PortMapping()
17731838
stop_event = threading.Event()
1774-
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
1839+
mock_cmis_manager = MagicMock()
1840+
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, mock_cmis_manager)
17751841
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
17761842
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD)
17771843
task.on_port_config_change(port_change_event)
@@ -1794,7 +1860,8 @@ def test_DomInfoUpdateTask_handle_port_change_event(self, mock_del_status_tbl_hw
17941860
def test_DomInfoUpdateTask_task_run_stop(self):
17951861
port_mapping = PortMapping()
17961862
stop_event = threading.Event()
1797-
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
1863+
mock_cmis_manager = MagicMock()
1864+
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, mock_cmis_manager)
17981865
task.start()
17991866
task.join()
18001867
assert not task.is_alive()
@@ -1819,10 +1886,12 @@ def test_DomInfoUpdateTask_task_worker(self, mock_post_pm_info, mock_update_stat
18191886

18201887
port_mapping = PortMapping()
18211888
stop_event = threading.Event()
1822-
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
1889+
mock_cmis_manager = MagicMock()
1890+
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, mock_cmis_manager)
18231891
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
18241892
task.task_stopping_event.wait = MagicMock(side_effect=[False, True])
18251893
task.get_dom_polling_from_config_db = MagicMock(return_value='enabled')
1894+
task.is_port_in_cmis_terminal_state = MagicMock(return_value=False)
18261895
mock_detect_error.return_value = True
18271896
task.task_worker()
18281897
assert task.port_mapping.logical_port_list.count('Ethernet0')

0 commit comments

Comments
 (0)