Skip to content

Commit 78db08e

Browse files
mihirpat1yxieca
mihirpat1
authored and
yxieca
committed
Separate periodic v/s fixed EEPROM reads between threads and optimize 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>
1 parent 1a672b0 commit 78db08e

File tree

3 files changed

+137
-170
lines changed

3 files changed

+137
-170
lines changed

‎sonic-xcvrd/tests/test_xcvrd.py‎

Lines changed: 22 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,13 @@ def test_DomInfoUpdateTask_task_run_with_exception(self):
8484
assert("sonic-xcvrd/xcvrd/xcvrd.py" in str(trace))
8585
assert("subscribe_port_config_change" in str(trace))
8686

87+
@patch('xcvrd.xcvrd.SfpStateUpdateTask.init', MagicMock())
8788
@patch('xcvrd.xcvrd_utilities.port_mapping.subscribe_port_config_change', MagicMock(side_effect = NotImplementedError))
8889
def test_SfpStateUpdateTask_task_run_with_exception(self):
8990
port_mapping = PortMapping()
90-
retry_eeprom_set = set()
9191
stop_event = threading.Event()
9292
sfp_error_event = threading.Event()
93-
sfp_state_update = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
93+
sfp_state_update = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
9494
exception_received = None
9595
trace = None
9696
try:
@@ -393,13 +393,16 @@ def test_post_port_sfp_info_to_db(self):
393393
'tx6power': '0.7',
394394
'tx7power': '0.7',
395395
'tx8power': '0.7', }))
396-
def test_post_port_sfp_dom_info_to_db(self):
396+
@patch('swsscommon.swsscommon.WarmStart', MagicMock())
397+
def test_post_port_sfp_info_and_dom_thr_to_db_once(self):
397398
port_mapping = PortMapping()
398399
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD)
399400
port_mapping.handle_port_change_event(port_change_event)
400401
stop_event = threading.Event()
401402
xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
402-
post_port_sfp_dom_info_to_db(True, port_mapping, xcvr_table_helper, stop_event)
403+
sfp_error_event = threading.Event()
404+
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
405+
task._post_port_sfp_info_and_dom_thr_to_db_once(port_mapping, xcvr_table_helper, stop_event)
403406

404407
@patch('xcvrd.xcvrd_utilities.port_mapping.PortMapping.logical_port_name_to_physical_port_list', MagicMock(return_value=[0]))
405408
@patch('xcvrd.xcvrd.platform_sfputil', MagicMock(return_value=[0]))
@@ -412,7 +415,9 @@ def test_init_port_sfp_status_tbl(self):
412415
port_mapping.handle_port_change_event(port_change_event)
413416
stop_event = threading.Event()
414417
xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
415-
init_port_sfp_status_tbl(port_mapping, xcvr_table_helper, stop_event)
418+
sfp_error_event = threading.Event()
419+
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
420+
task._init_port_sfp_status_tbl(port_mapping, xcvr_table_helper, stop_event)
416421

417422
def test_get_media_settings_key(self):
418423
xcvr_info_dict = {
@@ -944,14 +949,13 @@ def test_DomInfoUpdateTask_task_run_stop(self):
944949
@patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock())
945950
@patch('xcvrd.xcvrd_utilities.sfp_status_helper.detect_port_in_error_status')
946951
@patch('xcvrd.xcvrd.post_port_dom_info_to_db')
947-
@patch('xcvrd.xcvrd.post_port_dom_threshold_info_to_db')
948952
@patch('swsscommon.swsscommon.Select.addSelectable', MagicMock())
949953
@patch('swsscommon.swsscommon.SubscriberStateTable')
950954
@patch('swsscommon.swsscommon.Select.select')
951955
@patch('xcvrd.xcvrd.update_port_transceiver_status_table_hw')
952956
@patch('xcvrd.xcvrd.post_port_pm_info_to_db')
953957
def test_DomInfoUpdateTask_task_worker(self, mock_post_pm_info, mock_update_status_hw,
954-
mock_select, mock_sub_table, mock_post_dom_th,
958+
mock_select, mock_sub_table,
955959
mock_post_dom_info, mock_detect_error):
956960
mock_selectable = MagicMock()
957961
mock_selectable.pop = MagicMock(
@@ -970,14 +974,12 @@ def test_DomInfoUpdateTask_task_worker(self, mock_post_pm_info, mock_update_stat
970974
assert task.port_mapping.get_asic_id_for_logical_port('Ethernet0') == 0
971975
assert task.port_mapping.get_physical_to_logical(1) == ['Ethernet0']
972976
assert task.port_mapping.get_logical_to_physical('Ethernet0') == [1]
973-
assert mock_post_dom_th.call_count == 0
974977
assert mock_post_dom_info.call_count == 0
975978
assert mock_update_status_hw.call_count == 0
976979
assert mock_post_pm_info.call_count == 0
977980
mock_detect_error.return_value = False
978981
task.task_stopping_event.wait = MagicMock(side_effect=[False, True])
979982
task.task_worker()
980-
assert mock_post_dom_th.call_count == 1
981983
assert mock_post_dom_info.call_count == 1
982984
assert mock_update_status_hw.call_count == 1
983985
assert mock_post_pm_info.call_count == 1
@@ -995,8 +997,7 @@ def test_SfpStateUpdateTask_handle_port_change_event(self, mock_update_status_hw
995997
stop_event = threading.Event()
996998
sfp_error_event = threading.Event()
997999
port_mapping = PortMapping()
998-
retry_eeprom_set = set()
999-
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
1000+
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
10001001
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
10011002
task.xcvr_table_helper.get_status_tbl = mock_table_helper.get_status_tbl
10021003
task.xcvr_table_helper.get_intf_tbl = mock_table_helper.get_intf_tbl
@@ -1032,10 +1033,9 @@ def test_SfpStateUpdateTask_handle_port_change_event(self, mock_update_status_hw
10321033
@patch('xcvrd.xcvrd_utilities.port_mapping.subscribe_port_config_change', MagicMock(return_value=(None, None)))
10331034
def test_SfpStateUpdateTask_task_run_stop(self):
10341035
port_mapping = PortMapping()
1035-
retry_eeprom_set = set()
10361036
stop_event = threading.Event()
10371037
sfp_error_event = threading.Event()
1038-
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
1038+
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
10391039
task.start()
10401040
assert wait_until(5, 1, task.is_alive)
10411041
task.raise_exception()
@@ -1044,50 +1044,42 @@ def test_SfpStateUpdateTask_task_run_stop(self):
10441044

10451045
@patch('xcvrd.xcvrd.XcvrTableHelper', MagicMock())
10461046
@patch('xcvrd.xcvrd.post_port_sfp_info_to_db')
1047-
@patch('xcvrd.xcvrd.update_port_transceiver_status_table_hw')
1048-
def test_SfpStateUpdateTask_retry_eeprom_reading(self, mock_update_status_hw, mock_post_sfp_info):
1047+
def test_SfpStateUpdateTask_retry_eeprom_reading(self, mock_post_sfp_info):
10491048
mock_table = MagicMock()
10501049
mock_table.get = MagicMock(return_value=(False, None))
10511050

10521051
port_mapping = PortMapping()
1053-
retry_eeprom_set = set()
10541052
stop_event = threading.Event()
10551053
sfp_error_event = threading.Event()
1056-
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
1054+
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
10571055
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
10581056
task.xcvr_table_helper.get_intf_tbl = MagicMock(return_value=mock_table)
1059-
task.xcvr_table_helper.get_dom_tbl = MagicMock(return_value=mock_table)
10601057
task.xcvr_table_helper.get_dom_threshold_tbl = MagicMock(return_value=mock_table)
10611058
task.xcvr_table_helper.get_app_port_tbl = MagicMock(return_value=mock_table)
10621059
task.xcvr_table_helper.get_status_tbl = MagicMock(return_value=mock_table)
1063-
task.xcvr_table_helper.get_pm_tbl = MagicMock(return_value=mock_table)
10641060
task.retry_eeprom_reading()
10651061
assert mock_post_sfp_info.call_count == 0
10661062

10671063
task.retry_eeprom_set.add('Ethernet0')
10681064
task.last_retry_eeprom_time = time.time()
10691065
task.retry_eeprom_reading()
10701066
assert mock_post_sfp_info.call_count == 0
1071-
assert mock_update_status_hw.call_count == 0
10721067

10731068
task.last_retry_eeprom_time = 0
10741069
mock_post_sfp_info.return_value = SFP_EEPROM_NOT_READY
10751070
task.retry_eeprom_reading()
10761071
assert 'Ethernet0' in task.retry_eeprom_set
1077-
assert mock_update_status_hw.call_count == 0
10781072

10791073
task.last_retry_eeprom_time = 0
10801074
mock_post_sfp_info.return_value = None
10811075
task.retry_eeprom_reading()
10821076
assert 'Ethernet0' not in task.retry_eeprom_set
1083-
assert mock_update_status_hw.call_count == 1
10841077

10851078
def test_SfpStateUpdateTask_mapping_event_from_change_event(self):
10861079
port_mapping = PortMapping()
1087-
retry_eeprom_set = set()
10881080
stop_event = threading.Event()
10891081
sfp_error_event = threading.Event()
1090-
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
1082+
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
10911083
port_dict = {}
10921084
assert task._mapping_event_from_change_event(False, port_dict) == SYSTEM_FAIL
10931085
assert port_dict[EVENT_ON_ALL_SFP] == SYSTEM_FAIL
@@ -1107,26 +1099,23 @@ def test_SfpStateUpdateTask_mapping_event_from_change_event(self):
11071099
@patch('xcvrd.xcvrd._wrapper_soak_sfp_insert_event', MagicMock())
11081100
@patch('xcvrd.xcvrd_utilities.port_mapping.subscribe_port_config_change', MagicMock(return_value=(None, None)))
11091101
@patch('xcvrd.xcvrd_utilities.port_mapping.handle_port_config_change', MagicMock())
1102+
@patch('xcvrd.xcvrd.SfpStateUpdateTask.init', MagicMock())
11101103
@patch('os.kill')
11111104
@patch('xcvrd.xcvrd.SfpStateUpdateTask._mapping_event_from_change_event')
11121105
@patch('xcvrd.xcvrd._wrapper_get_transceiver_change_event')
11131106
@patch('xcvrd.xcvrd.del_port_sfp_dom_info_from_db')
11141107
@patch('xcvrd.xcvrd.notify_media_setting')
11151108
@patch('xcvrd.xcvrd.post_port_dom_threshold_info_to_db')
1116-
@patch('xcvrd.xcvrd.post_port_dom_info_to_db')
11171109
@patch('xcvrd.xcvrd.post_port_sfp_info_to_db')
11181110
@patch('xcvrd.xcvrd.update_port_transceiver_status_table_sw')
1119-
@patch('xcvrd.xcvrd.update_port_transceiver_status_table_hw')
11201111
@patch('xcvrd.xcvrd.delete_port_from_status_table_hw')
1121-
@patch('xcvrd.xcvrd.post_port_pm_info_to_db')
1122-
def test_SfpStateUpdateTask_task_worker(self, mock_post_pm_info, mock_del_status_hw, mock_update_status_hw,
1123-
mock_update_status, mock_post_sfp_info, mock_post_dom_info, mock_post_dom_th, mock_update_media_setting,
1112+
def test_SfpStateUpdateTask_task_worker(self, mock_del_status_hw,
1113+
mock_update_status, mock_post_sfp_info, mock_post_dom_th, mock_update_media_setting,
11241114
mock_del_dom, mock_change_event, mock_mapping_event, mock_os_kill):
11251115
port_mapping = PortMapping()
1126-
retry_eeprom_set = set()
11271116
stop_event = threading.Event()
11281117
sfp_error_event = threading.Event()
1129-
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
1118+
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
11301119
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
11311120
mock_change_event.return_value = (True, {0: 0}, {})
11321121
mock_mapping_event.return_value = SYSTEM_NOT_READY
@@ -1172,10 +1161,7 @@ def test_SfpStateUpdateTask_task_worker(self, mock_post_pm_info, mock_del_status
11721161
task.task_worker(stop_event, sfp_error_event)
11731162
assert mock_update_status.call_count == 1
11741163
assert mock_post_sfp_info.call_count == 2 # first call and retry call
1175-
assert mock_post_dom_info.call_count == 0
11761164
assert mock_post_dom_th.call_count == 0
1177-
assert mock_update_status_hw.call_count == 0
1178-
assert mock_post_pm_info.call_count == 0
11791165
assert mock_update_media_setting.call_count == 0
11801166
assert 'Ethernet0' in task.retry_eeprom_set
11811167
task.retry_eeprom_set.clear()
@@ -1188,45 +1174,37 @@ def test_SfpStateUpdateTask_task_worker(self, mock_post_pm_info, mock_del_status
11881174
task.task_worker(stop_event, sfp_error_event)
11891175
assert mock_update_status.call_count == 1
11901176
assert mock_post_sfp_info.call_count == 1
1191-
assert mock_post_dom_info.call_count == 1
11921177
assert mock_post_dom_th.call_count == 1
1193-
assert mock_update_status_hw.call_count == 1
1194-
assert mock_post_pm_info.call_count == 1
11951178
assert mock_update_media_setting.call_count == 1
11961179

11971180
stop_event.is_set = MagicMock(side_effect=[False, True])
11981181
mock_change_event.return_value = (True, {1: SFP_STATUS_REMOVED}, {})
11991182
mock_update_status.reset_mock()
1200-
mock_update_status_hw.reset_mock()
12011183
# Test state machine: handle SFP remove event
12021184
task.task_worker(stop_event, sfp_error_event)
12031185
assert mock_update_status.call_count == 1
1204-
assert mock_update_status_hw.call_count == 0 # only SW fields are updated
12051186
assert mock_del_dom.call_count == 1
12061187
assert mock_del_status_hw.call_count == 1
12071188

12081189
stop_event.is_set = MagicMock(side_effect=[False, True])
12091190
error = int(SFP_STATUS_INSERTED) | SfpBase.SFP_ERROR_BIT_BLOCKING | SfpBase.SFP_ERROR_BIT_POWER_BUDGET_EXCEEDED
12101191
mock_change_event.return_value = (True, {1: error}, {})
12111192
mock_update_status.reset_mock()
1212-
mock_update_status_hw.reset_mock()
12131193
mock_del_dom.reset_mock()
12141194
mock_del_status_hw.reset_mock()
12151195
# Test state machine: handle SFP error event
12161196
task.task_worker(stop_event, sfp_error_event)
12171197
assert mock_update_status.call_count == 1
1218-
assert mock_update_status_hw.call_count == 0 # only SW fields are updated
12191198
assert mock_del_dom.call_count == 1
12201199
assert mock_del_status_hw.call_count == 1
12211200

12221201
@patch('xcvrd.xcvrd.XcvrTableHelper')
12231202
@patch('xcvrd.xcvrd._wrapper_get_presence')
12241203
@patch('xcvrd.xcvrd.notify_media_setting')
12251204
@patch('xcvrd.xcvrd.post_port_dom_threshold_info_to_db')
1226-
@patch('xcvrd.xcvrd.post_port_dom_info_to_db')
12271205
@patch('xcvrd.xcvrd.post_port_sfp_info_to_db')
12281206
@patch('xcvrd.xcvrd.update_port_transceiver_status_table_sw')
1229-
def test_SfpStateUpdateTask_on_add_logical_port(self, mock_update_status, mock_post_sfp_info, mock_post_dom_info,
1207+
def test_SfpStateUpdateTask_on_add_logical_port(self, mock_update_status, mock_post_sfp_info,
12301208
mock_post_dom_th, mock_update_media_setting, mock_get_presence, mock_table_helper):
12311209
class MockTable:
12321210
pass
@@ -1237,26 +1215,20 @@ class MockTable:
12371215
int_tbl = MockTable()
12381216
int_tbl.get = MagicMock(return_value=(True, (('key2', 'value2'),)))
12391217
int_tbl.set = MagicMock()
1240-
dom_tbl = MockTable()
1241-
dom_tbl.get = MagicMock(return_value=(True, (('key3', 'value3'),)))
1242-
dom_tbl.set = MagicMock()
12431218
dom_threshold_tbl = MockTable()
12441219
dom_threshold_tbl.get = MagicMock(return_value=(True, (('key4', 'value4'),)))
12451220
dom_threshold_tbl.set = MagicMock()
12461221
mock_table_helper.get_status_tbl = MagicMock(return_value=status_tbl)
12471222
mock_table_helper.get_intf_tbl = MagicMock(return_value=int_tbl)
1248-
mock_table_helper.get_dom_tbl = MagicMock(return_value=dom_tbl)
12491223
mock_table_helper.get_dom_threshold_tbl = MagicMock(return_value=dom_threshold_tbl)
12501224

12511225
port_mapping = PortMapping()
1252-
retry_eeprom_set = set()
12531226
stop_event = threading.Event()
12541227
sfp_error_event = threading.Event()
1255-
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, retry_eeprom_set, stop_event, sfp_error_event)
1228+
task = SfpStateUpdateTask(DEFAULT_NAMESPACE, port_mapping, stop_event, sfp_error_event)
12561229
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
12571230
task.xcvr_table_helper.get_status_tbl = mock_table_helper.get_status_tbl
12581231
task.xcvr_table_helper.get_intf_tbl = mock_table_helper.get_intf_tbl
1259-
task.xcvr_table_helper.get_dom_tbl = mock_table_helper.get_dom_tbl
12601232
task.xcvr_table_helper.get_dom_threshold_tbl = mock_table_helper.get_dom_threshold_tbl
12611233
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD)
12621234
task.port_mapping.handle_port_change_event(port_change_event)
@@ -1270,7 +1242,6 @@ class MockTable:
12701242
mock_update_status.assert_called_with('Ethernet0', status_tbl, SFP_STATUS_INSERTED, 'N/A')
12711243
assert mock_post_sfp_info.call_count == 1
12721244
mock_post_sfp_info.assert_called_with('Ethernet0', task.port_mapping, int_tbl, {})
1273-
assert mock_post_dom_info.call_count == 0
12741245
assert mock_post_dom_th.call_count == 0
12751246
assert mock_update_media_setting.call_count == 0
12761247
assert 'Ethernet0' in task.retry_eeprom_set
@@ -1285,8 +1256,6 @@ class MockTable:
12851256
mock_update_status.assert_called_with('Ethernet0', status_tbl, SFP_STATUS_INSERTED, 'N/A')
12861257
assert mock_post_sfp_info.call_count == 1
12871258
mock_post_sfp_info.assert_called_with('Ethernet0', task.port_mapping, int_tbl, {})
1288-
assert mock_post_dom_info.call_count == 1
1289-
mock_post_dom_info.assert_called_with('Ethernet0', task.port_mapping, dom_tbl)
12901259
assert mock_post_dom_th.call_count == 1
12911260
mock_post_dom_th.assert_called_with('Ethernet0', task.port_mapping, dom_threshold_tbl)
12921261
assert mock_update_media_setting.call_count == 1

0 commit comments

Comments
 (0)