Skip to content

Commit

Permalink
Fix checkReplyType failed issue via recreating xcvr_table_helper on f…
Browse files Browse the repository at this point in the history 
…orking subprocess (sonic-net#255)

* Fix message interleaving issue via recreating xcvr_table_helper on forking subprocess

Signed-off-by: Stephen Sun <stephens@nvidia.com>

* Address comments: change xcvr_table_helper to class member

Signed-off-by: Stephen Sun <stephens@nvidia.com>

* Fix a typo

Signed-off-by: Stephen Sun <stephens@nvidia.com>
  • Loading branch information
@stephenxs
stephenxs authored Apr 21, 2022
1 parent 9ac12bf commit e0f8a35
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 46 deletions.
37 changes: 27 additions & 10 deletions sonic-xcvrd/tests/test_xcvrd.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def test_post_port_sfp_info_to_db(self):
@patch('xcvrd.xcvrd.platform_sfputil', MagicMock(return_value=[0]))
@patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=True))
@patch('xcvrd.xcvrd._wrapper_is_replaceable', MagicMock(return_value=True))
@patch('xcvrd.xcvrd.xcvr_table_helper', MagicMock())
@patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock())
@patch('xcvrd.xcvrd._wrapper_get_transceiver_info', MagicMock(return_value={'type': '22.75',
'vendor_rev': '0.5',
'serial': '0.7',
Expand Down Expand Up @@ -219,19 +219,21 @@ def test_post_port_sfp_dom_info_to_db(self):
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD)
port_mapping.handle_port_change_event(port_change_event)
stop_event = threading.Event()
post_port_sfp_dom_info_to_db(True, port_mapping, stop_event)
xcvr_table_helper = XcvrTableHelper()
post_port_sfp_dom_info_to_db(True, port_mapping, xcvr_table_helper, stop_event)

@patch('xcvrd.xcvrd_utilities.port_mapping.PortMapping.logical_port_name_to_physical_port_list', MagicMock(return_value=[0]))
@patch('xcvrd.xcvrd.platform_sfputil', MagicMock(return_value=[0]))
@patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=True))
@patch('xcvrd.xcvrd._wrapper_is_replaceable', MagicMock(return_value=True))
@patch('xcvrd.xcvrd.xcvr_table_helper', MagicMock())
@patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock())
def test_init_port_sfp_status_tbl(self):
port_mapping = PortMapping()
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD)
port_mapping.handle_port_change_event(port_change_event)
stop_event = threading.Event()
init_port_sfp_status_tbl(port_mapping, stop_event)
xcvr_table_helper = XcvrTableHelper()
init_port_sfp_status_tbl(port_mapping, xcvr_table_helper, stop_event)

def test_get_media_settings_key(self):
xcvr_info_dict = {
Expand Down Expand Up @@ -544,10 +546,11 @@ def test_CmisManagerTask_task_worker(self, mock_chassis):
task.task_worker()
assert mock_xcvr_api.tx_disable_channel.call_count == 2

@patch('xcvrd.xcvrd.xcvr_table_helper', MagicMock())
@patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock())
def test_DomInfoUpdateTask_handle_port_change_event(self):
port_mapping = PortMapping()
task = DomInfoUpdateTask(port_mapping)
task.xcvr_table_helper = XcvrTableHelper()
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD)
task.on_port_config_change(port_change_event)
assert task.port_mapping.logical_port_list.count('Ethernet0')
Expand All @@ -571,7 +574,7 @@ def test_DomInfoUpdateTask_task_run_stop(self):
task.task_stop()
assert not task.task_thread.is_alive()

@patch('xcvrd.xcvrd.xcvr_table_helper', MagicMock())
@patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock())
@patch('xcvrd.xcvrd_utilities.sfp_status_helper.detect_port_in_error_status')
@patch('xcvrd.xcvrd.post_port_dom_info_to_db')
@patch('xcvrd.xcvrd.post_port_dom_threshold_info_to_db')
Expand All @@ -587,6 +590,7 @@ def test_DomInfoUpdateTask_task_worker(self, mock_select, mock_sub_table, mock_p

port_mapping = PortMapping()
task = DomInfoUpdateTask(port_mapping)
task.xcvr_table_helper = XcvrTableHelper()
task.task_stopping_event.wait = MagicMock(side_effect=[False, True])
mock_detect_error.return_value = True
task.task_worker()
Expand All @@ -603,7 +607,7 @@ def test_DomInfoUpdateTask_task_worker(self, mock_select, mock_sub_table, mock_p
assert mock_post_dom_info.call_count == 1

@patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=False))
@patch('xcvrd.xcvrd.xcvr_table_helper')
@patch('xcvrd.xcvrd.XcvrTableHelper')
def test_SfpStateUpdateTask_handle_port_change_event(self, mock_table_helper):
mock_table = MagicMock()
mock_table.get = MagicMock(return_value=(False, None))
Expand All @@ -614,6 +618,7 @@ def test_SfpStateUpdateTask_handle_port_change_event(self, mock_table_helper):
port_mapping = PortMapping()
retry_eeprom_set = set()
task = SfpStateUpdateTask(port_mapping, retry_eeprom_set)
task.xcvr_table_helper = XcvrTableHelper()
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD)
wait_time = 5
while wait_time > 0:
Expand Down Expand Up @@ -650,12 +655,19 @@ def test_SfpStateUpdateTask_task_run_stop(self):
task.task_stop()
assert wait_until(5, 1, lambda: task.task_process.is_alive() is False)

@patch('xcvrd.xcvrd.xcvr_table_helper', MagicMock())
@patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock())
@patch('xcvrd.xcvrd.post_port_sfp_info_to_db')
def test_SfpStateUpdateTask_retry_eeprom_reading(self, mock_post_sfp_info):
mock_table = MagicMock()
mock_table.get = MagicMock(return_value=(False, None))

port_mapping = PortMapping()
retry_eeprom_set = set()
task = SfpStateUpdateTask(port_mapping, retry_eeprom_set)
task.xcvr_table_helper = XcvrTableHelper()
task.xcvr_table_helper.get_intf_tbl = MagicMock(return_value=mock_table)
task.xcvr_table_helper.get_dom_tbl = MagicMock(return_value=mock_table)
task.xcvr_table_helper.get_app_port_tbl = MagicMock(return_value=mock_table)
task.retry_eeprom_reading()
assert mock_post_sfp_info.call_count == 0

Expand Down Expand Up @@ -693,7 +705,7 @@ def test_SfpStateUpdateTask_mapping_event_from_change_event(self):
assert task._mapping_event_from_change_event(True, port_dict) == NORMAL_EVENT

@patch('time.sleep', MagicMock())
@patch('xcvrd.xcvrd.xcvr_table_helper', MagicMock())
@patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock())
@patch('xcvrd.xcvrd._wrapper_soak_sfp_insert_event', MagicMock())
@patch('xcvrd.xcvrd_utilities.port_mapping.subscribe_port_config_change', MagicMock(return_value=(None, None)))
@patch('xcvrd.xcvrd_utilities.port_mapping.handle_port_config_change', MagicMock())
Expand All @@ -710,6 +722,7 @@ def test_SfpStateUpdateTask_task_worker(self, mock_updata_status, mock_post_sfp_
port_mapping = PortMapping()
retry_eeprom_set = set()
task = SfpStateUpdateTask(port_mapping, retry_eeprom_set)
task.xcvr_table_helper = XcvrTableHelper()
stop_event = multiprocessing.Event()
sfp_error_event = multiprocessing.Event()
mock_change_event.return_value = (True, {0: 0}, {})
Expand Down Expand Up @@ -792,7 +805,7 @@ def test_SfpStateUpdateTask_task_worker(self, mock_updata_status, mock_post_sfp_
assert mock_updata_status.call_count == 1
assert mock_del_dom.call_count == 1

@patch('xcvrd.xcvrd.xcvr_table_helper')
@patch('xcvrd.xcvrd.XcvrTableHelper')
@patch('xcvrd.xcvrd._wrapper_get_presence')
@patch('xcvrd.xcvrd.notify_media_setting')
@patch('xcvrd.xcvrd.post_port_dom_threshold_info_to_db')
Expand All @@ -819,6 +832,10 @@ class MockTable:
port_mapping = PortMapping()
retry_eeprom_set = set()
task = SfpStateUpdateTask(port_mapping, retry_eeprom_set)
task.xcvr_table_helper = XcvrTableHelper()
task.xcvr_table_helper.get_status_tbl = mock_table_helper.get_status_tbl
task.xcvr_table_helper.get_intf_tbl = mock_table_helper.get_intf_tbl
task.xcvr_table_helper.get_dom_tbl = mock_table_helper.get_dom_tbl
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD)
task.port_mapping.handle_port_change_event(port_change_event)
# SFP information is in the DB, copy the SFP information for the newly added logical port
Expand Down
Loading

0 comments on commit e0f8a35

Please sign in to comment.