Skip to content

Commit

Permalink
Separate periodic v/s fixed EEPROM reads between threads and optimize…
Browse files Browse the repository at this point in the history 
… xcvrd boot-up time (#360)

* Separate periodic v/s fixed EEPROM reads between threads and optimize xcvrd boot-up time

Signed-off-by: Mihir Patel <patelmi@microsoft.com>
  • Loading branch information
@mihirpat1
mihirpat1 authored Jun 13, 2023
1 parent b56e534 commit 55996c7
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 170 deletions.
75 changes: 22 additions & 53 deletions sonic-xcvrd/tests/test_xcvrd.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ def test_DomInfoUpdateTask_task_run_with_exception(self):
assert("sonic-xcvrd/xcvrd/xcvrd.py" in str(trace))
assert("subscribe_port_config_change" in str(trace))

@patch('xcvrd.xcvrd.SfpStateUpdateTask.init', MagicMock())
@patch('xcvrd.xcvrd_utilities.port_mapping.subscribe_port_config_change', MagicMock(side_effect = NotImplementedError))
def test_SfpStateUpdateTask_task_run_with_exception(self):
port_mapping = PortMapping()
retry_eeprom_set = set()
stop_event = threading.Event()
sfp_error_event = threading.Event()
sfp_state_update = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
sfp_state_update = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
exception_received = None
trace = None
try:
Expand Down Expand Up @@ -393,13 +393,16 @@ def test_post_port_sfp_info_to_db(self):
'tx6power': '0.7',
'tx7power': '0.7',
'tx8power': '0.7', }))
def test_post_port_sfp_dom_info_to_db(self):
@patch('swsscommon.swsscommon.WarmStart', MagicMock())
def test_post_port_sfp_info_and_dom_thr_to_db_once(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()
xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
post_port_sfp_dom_info_to_db(True, port_mapping, xcvr_table_helper, stop_event)
sfp_error_event = threading.Event()
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
task._post_port_sfp_info_and_dom_thr_to_db_once(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]))
Expand All @@ -412,7 +415,9 @@ def test_init_port_sfp_status_tbl(self):
port_mapping.handle_port_change_event(port_change_event)
stop_event = threading.Event()
xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
init_port_sfp_status_tbl(port_mapping, xcvr_table_helper, stop_event)
sfp_error_event = threading.Event()
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
task._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 @@ -943,14 +948,13 @@ def test_DomInfoUpdateTask_task_run_stop(self):
@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')
@patch('swsscommon.swsscommon.Select.addSelectable', MagicMock())
@patch('swsscommon.swsscommon.SubscriberStateTable')
@patch('swsscommon.swsscommon.Select.select')
@patch('xcvrd.xcvrd.update_port_transceiver_status_table_hw')
@patch('xcvrd.xcvrd.post_port_pm_info_to_db')
def test_DomInfoUpdateTask_task_worker(self, mock_post_pm_info, mock_update_status_hw,
mock_select, mock_sub_table, mock_post_dom_th,
mock_select, mock_sub_table,
mock_post_dom_info, mock_detect_error):
mock_selectable = MagicMock()
mock_selectable.pop = MagicMock(
Expand All @@ -969,14 +973,12 @@ def test_DomInfoUpdateTask_task_worker(self, mock_post_pm_info, mock_update_stat
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]
assert mock_post_dom_th.call_count == 0
assert mock_post_dom_info.call_count == 0
assert mock_update_status_hw.call_count == 0
assert mock_post_pm_info.call_count == 0
mock_detect_error.return_value = False
task.task_stopping_event.wait = MagicMock(side_effect=[False, True])
task.task_worker()
assert mock_post_dom_th.call_count == 1
assert mock_post_dom_info.call_count == 1
assert mock_update_status_hw.call_count == 1
assert mock_post_pm_info.call_count == 1
Expand All @@ -994,8 +996,7 @@ def test_SfpStateUpdateTask_handle_port_change_event(self, mock_update_status_hw
stop_event = threading.Event()
sfp_error_event = threading.Event()
port_mapping = PortMapping()
retry_eeprom_set = set()
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
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
Expand Down Expand Up @@ -1031,10 +1032,9 @@ def test_SfpStateUpdateTask_handle_port_change_event(self, mock_update_status_hw
@patch('xcvrd.xcvrd_utilities.port_mapping.subscribe_port_config_change', MagicMock(return_value=(None, None)))
def test_SfpStateUpdateTask_task_run_stop(self):
port_mapping = PortMapping()
retry_eeprom_set = set()
stop_event = threading.Event()
sfp_error_event = threading.Event()
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
task.start()
assert wait_until(5, 1, task.is_alive)
task.raise_exception()
Expand All @@ -1043,50 +1043,42 @@ def test_SfpStateUpdateTask_task_run_stop(self):

@patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock())
@patch('xcvrd.xcvrd.post_port_sfp_info_to_db')
@patch('xcvrd.xcvrd.update_port_transceiver_status_table_hw')
def test_SfpStateUpdateTask_retry_eeprom_reading(self, mock_update_status_hw, mock_post_sfp_info):
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()
stop_event = threading.Event()
sfp_error_event = threading.Event()
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
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_dom_threshold_tbl = MagicMock(return_value=mock_table)
task.xcvr_table_helper.get_app_port_tbl = MagicMock(return_value=mock_table)
task.xcvr_table_helper.get_status_tbl = MagicMock(return_value=mock_table)
task.xcvr_table_helper.get_pm_tbl = MagicMock(return_value=mock_table)
task.retry_eeprom_reading()
assert mock_post_sfp_info.call_count == 0

task.retry_eeprom_set.add('Ethernet0')
task.last_retry_eeprom_time = time.time()
task.retry_eeprom_reading()
assert mock_post_sfp_info.call_count == 0
assert mock_update_status_hw.call_count == 0

task.last_retry_eeprom_time = 0
mock_post_sfp_info.return_value = SFP_EEPROM_NOT_READY
task.retry_eeprom_reading()
assert 'Ethernet0' in task.retry_eeprom_set
assert mock_update_status_hw.call_count == 0

task.last_retry_eeprom_time = 0
mock_post_sfp_info.return_value = None
task.retry_eeprom_reading()
assert 'Ethernet0' not in task.retry_eeprom_set
assert mock_update_status_hw.call_count == 1

def test_SfpStateUpdateTask_mapping_event_from_change_event(self):
port_mapping = PortMapping()
retry_eeprom_set = set()
stop_event = threading.Event()
sfp_error_event = threading.Event()
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
port_dict = {}
assert task._mapping_event_from_change_event(False, port_dict) == SYSTEM_FAIL
assert port_dict[EVENT_ON_ALL_SFP] == SYSTEM_FAIL
Expand All @@ -1106,26 +1098,23 @@ def test_SfpStateUpdateTask_mapping_event_from_change_event(self):
@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())
@patch('xcvrd.xcvrd.SfpStateUpdateTask.init', MagicMock())
@patch('os.kill')
@patch('xcvrd.xcvrd.SfpStateUpdateTask._mapping_event_from_change_event')
@patch('xcvrd.xcvrd._wrapper_get_transceiver_change_event')
@patch('xcvrd.xcvrd.del_port_sfp_dom_info_from_db')
@patch('xcvrd.xcvrd.notify_media_setting')
@patch('xcvrd.xcvrd.post_port_dom_threshold_info_to_db')
@patch('xcvrd.xcvrd.post_port_dom_info_to_db')
@patch('xcvrd.xcvrd.post_port_sfp_info_to_db')
@patch('xcvrd.xcvrd.update_port_transceiver_status_table_sw')
@patch('xcvrd.xcvrd.update_port_transceiver_status_table_hw')
@patch('xcvrd.xcvrd.delete_port_from_status_table_hw')
@patch('xcvrd.xcvrd.post_port_pm_info_to_db')
def test_SfpStateUpdateTask_task_worker(self, mock_post_pm_info, mock_del_status_hw, mock_update_status_hw,
mock_update_status, mock_post_sfp_info, mock_post_dom_info, mock_post_dom_th, mock_update_media_setting,
def test_SfpStateUpdateTask_task_worker(self, mock_del_status_hw,
mock_update_status, mock_post_sfp_info, mock_post_dom_th, mock_update_media_setting,
mock_del_dom, mock_change_event, mock_mapping_event, mock_os_kill):
port_mapping = PortMapping()
retry_eeprom_set = set()
stop_event = threading.Event()
sfp_error_event = threading.Event()
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
mock_change_event.return_value = (True, {0: 0}, {})
mock_mapping_event.return_value = SYSTEM_NOT_READY
Expand Down Expand Up @@ -1171,10 +1160,7 @@ def test_SfpStateUpdateTask_task_worker(self, mock_post_pm_info, mock_del_status
task.task_worker(stop_event, sfp_error_event)
assert mock_update_status.call_count == 1
assert mock_post_sfp_info.call_count == 2 # first call and retry call
assert mock_post_dom_info.call_count == 0
assert mock_post_dom_th.call_count == 0
assert mock_update_status_hw.call_count == 0
assert mock_post_pm_info.call_count == 0
assert mock_update_media_setting.call_count == 0
assert 'Ethernet0' in task.retry_eeprom_set
task.retry_eeprom_set.clear()
Expand All @@ -1187,45 +1173,37 @@ def test_SfpStateUpdateTask_task_worker(self, mock_post_pm_info, mock_del_status
task.task_worker(stop_event, sfp_error_event)
assert mock_update_status.call_count == 1
assert mock_post_sfp_info.call_count == 1
assert mock_post_dom_info.call_count == 1
assert mock_post_dom_th.call_count == 1
assert mock_update_status_hw.call_count == 1
assert mock_post_pm_info.call_count == 1
assert mock_update_media_setting.call_count == 1

stop_event.is_set = MagicMock(side_effect=[False, True])
mock_change_event.return_value = (True, {1: SFP_STATUS_REMOVED}, {})
mock_update_status.reset_mock()
mock_update_status_hw.reset_mock()
# Test state machine: handle SFP remove event
task.task_worker(stop_event, sfp_error_event)
assert mock_update_status.call_count == 1
assert mock_update_status_hw.call_count == 0 # only SW fields are updated
assert mock_del_dom.call_count == 1
assert mock_del_status_hw.call_count == 1

stop_event.is_set = MagicMock(side_effect=[False, True])
error = int(SFP_STATUS_INSERTED) | SfpBase.SFP_ERROR_BIT_BLOCKING | SfpBase.SFP_ERROR_BIT_POWER_BUDGET_EXCEEDED
mock_change_event.return_value = (True, {1: error}, {})
mock_update_status.reset_mock()
mock_update_status_hw.reset_mock()
mock_del_dom.reset_mock()
mock_del_status_hw.reset_mock()
# Test state machine: handle SFP error event
task.task_worker(stop_event, sfp_error_event)
assert mock_update_status.call_count == 1
assert mock_update_status_hw.call_count == 0 # only SW fields are updated
assert mock_del_dom.call_count == 1
assert mock_del_status_hw.call_count == 1

@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')
@patch('xcvrd.xcvrd.post_port_dom_info_to_db')
@patch('xcvrd.xcvrd.post_port_sfp_info_to_db')
@patch('xcvrd.xcvrd.update_port_transceiver_status_table_sw')
def test_SfpStateUpdateTask_on_add_logical_port(self, mock_update_status, mock_post_sfp_info, mock_post_dom_info,
def test_SfpStateUpdateTask_on_add_logical_port(self, mock_update_status, mock_post_sfp_info,
mock_post_dom_th, mock_update_media_setting, mock_get_presence, mock_table_helper):
class MockTable:
pass
Expand All @@ -1236,26 +1214,20 @@ class MockTable:
int_tbl = MockTable()
int_tbl.get = MagicMock(return_value=(True, (('key2', 'value2'),)))
int_tbl.set = MagicMock()
dom_tbl = MockTable()
dom_tbl.get = MagicMock(return_value=(True, (('key3', 'value3'),)))
dom_tbl.set = MagicMock()
dom_threshold_tbl = MockTable()
dom_threshold_tbl.get = MagicMock(return_value=(True, (('key4', 'value4'),)))
dom_threshold_tbl.set = MagicMock()
mock_table_helper.get_status_tbl = MagicMock(return_value=status_tbl)
mock_table_helper.get_intf_tbl = MagicMock(return_value=int_tbl)
mock_table_helper.get_dom_tbl = MagicMock(return_value=dom_tbl)
mock_table_helper.get_dom_threshold_tbl = MagicMock(return_value=dom_threshold_tbl)

port_mapping = PortMapping()
retry_eeprom_set = set()
stop_event = threading.Event()
sfp_error_event = threading.Event()
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
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
task.xcvr_table_helper.get_dom_threshold_tbl = mock_table_helper.get_dom_threshold_tbl
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD)
task.port_mapping.handle_port_change_event(port_change_event)
Expand All @@ -1269,7 +1241,6 @@ class MockTable:
mock_update_status.assert_called_with('Ethernet0', status_tbl, SFP_STATUS_INSERTED, 'N/A')
assert mock_post_sfp_info.call_count == 1
mock_post_sfp_info.assert_called_with('Ethernet0', task.port_mapping, int_tbl, {})
assert mock_post_dom_info.call_count == 0
assert mock_post_dom_th.call_count == 0
assert mock_update_media_setting.call_count == 0
assert 'Ethernet0' in task.retry_eeprom_set
Expand All @@ -1284,8 +1255,6 @@ class MockTable:
mock_update_status.assert_called_with('Ethernet0', status_tbl, SFP_STATUS_INSERTED, 'N/A')
assert mock_post_sfp_info.call_count == 1
mock_post_sfp_info.assert_called_with('Ethernet0', task.port_mapping, int_tbl, {})
assert mock_post_dom_info.call_count == 1
mock_post_dom_info.assert_called_with('Ethernet0', task.port_mapping, dom_tbl)
assert mock_post_dom_th.call_count == 1
mock_post_dom_th.assert_called_with('Ethernet0', task.port_mapping, dom_threshold_tbl)
assert mock_update_media_setting.call_count == 1
Expand Down
Loading

0 comments on commit 55996c7

Please sign in to comment.