Skip to content

Commit 7c1b475

Browse files
ishidawataruishidawataru
committed
fix(xcvrd): fix DPB support with CMIS transceivers
CmisManagerTask's `port_dict` and `port_mapping` must be updated according to the port add/remove events. Before this commit, `port_mapping` is only intialized when CmisManagerTask is initialized and not updated after that, which was causing KeyError exception when DBP is used. (sonic-net/sonic-buildimage#18893) This commit removes the `port_mapping` field from CmisManagerTask as `port_mapping` was used just for storing `asic_id` information and that can be simply done by `port_dict` instead. Also, this commit updates `port_dict` accoding to the port add/remove events to support DPB. Signed-off-by: Wataru Ishida <wataru.ishid@gmail.com>
1 parent f581c06 commit 7c1b475

File tree

2 files changed

+57
-63
lines changed

2 files changed

+57
-63
lines changed

‎sonic-xcvrd/tests/test_xcvrd.py

+29-32
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ def test_CmisManagerTask_get_xcvr_api_exception(self, mock_platform_chassis, moc
214214
mock_sfp.get_xcvr_api = MagicMock(side_effect=NotImplementedError)
215215
task.task_worker()
216216
assert mock_log_exception_traceback.call_count == 1
217-
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_FAILED
217+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_FAILED
218218

219219
# Case 2: is_flat_memory() raises AttributeError. In this case, CMIS SM should transition to READY state
220220
mock_xcvr_api = MagicMock()
@@ -223,7 +223,7 @@ def test_CmisManagerTask_get_xcvr_api_exception(self, mock_platform_chassis, moc
223223
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
224224
task.on_port_update_event(port_change_event)
225225
task.task_worker()
226-
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
226+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_READY
227227

228228
# Case 3: get_cmis_application_desired() raises an exception
229229
mock_xcvr_api.is_flat_memory = MagicMock(return_value=False)
@@ -234,7 +234,7 @@ def test_CmisManagerTask_get_xcvr_api_exception(self, mock_platform_chassis, moc
234234
task.get_cmis_host_lanes_mask = MagicMock()
235235
task.task_worker()
236236
assert mock_log_exception_traceback.call_count == 2
237-
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_FAILED
237+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_FAILED
238238
assert task.get_cmis_host_lanes_mask.call_count == 0
239239

240240
@patch('xcvrd.xcvrd_utilities.port_event_helper.subscribe_port_config_change', MagicMock(side_effect = NotImplementedError))
@@ -297,7 +297,7 @@ def test_SfpStateUpdateTask_task_run_with_exception(self):
297297
@patch('xcvrd.xcvrd.SffManagerTask.join')
298298
def test_DaemonXcvrd_run_with_exception(self, mock_task_join_sff, mock_task_join_sfp,
299299
mock_task_join_dom, mock_init, mock_os_kill):
300-
mock_init.return_value = (PortMapping(), set())
300+
mock_init.return_value = PortMapping()
301301
xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER)
302302
xcvrd.enable_sff_mgr = True
303303
xcvrd.load_feature_flags = MagicMock()
@@ -1199,7 +1199,7 @@ def test_DaemonXcvrd_wait_for_port_config_done(self, mock_select, mock_sub_table
11991199
@patch('xcvrd.xcvrd.DomInfoUpdateTask.join')
12001200
@patch('xcvrd.xcvrd.SfpStateUpdateTask.join')
12011201
def test_DaemonXcvrd_run(self, mock_task_stop1, mock_task_stop2, mock_task_run1, mock_task_run2, mock_deinit, mock_init):
1202-
mock_init.return_value = (PortMapping(), set())
1202+
mock_init.return_value = PortMapping()
12031203
xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER)
12041204
xcvrd.load_feature_flags = MagicMock()
12051205
xcvrd.stop_event.wait = MagicMock()
@@ -1492,9 +1492,10 @@ def test_CmisManagerTask_handle_port_change_event(self):
14921492

14931493
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_DEL)
14941494
task.on_port_update_event(port_change_event)
1495-
assert len(task.port_dict) == 1
1495+
assert len(task.port_dict) == 0
14961496

1497-
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_SET)
1497+
port_dict = {'speed':'400000', 'lanes':'1,2,3,4,5,6,7,8'}
1498+
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_SET, port_dict)
14981499
task.on_port_update_event(port_change_event)
14991500
assert len(task.port_dict) == 1
15001501

@@ -1896,16 +1897,15 @@ def test_CmisManagerTask_task_worker(self, mock_chassis, mock_get_status_tbl):
18961897
mock_chassis.get_sfp = MagicMock(return_value=mock_sfp)
18971898

18981899
port_mapping = PortMapping()
1900+
port_mapping.handle_port_change_event(PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD))
18991901
stop_event = threading.Event()
19001902
task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
1901-
task.port_mapping.logical_port_list = ['Ethernet0']
19021903
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
19031904
task.xcvr_table_helper.get_status_tbl.return_value = mock_get_status_tbl
19041905
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
19051906
task.task_worker()
1906-
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
1907+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_UNKNOWN
19071908

1908-
task.port_mapping.logical_port_list = MagicMock()
19091909
port_change_event = PortChangeEvent('PortConfigDone', -1, 0, PortChangeEvent.PORT_SET)
19101910
task.on_port_update_event(port_change_event)
19111911
assert task.isPortConfigDone
@@ -1914,7 +1914,7 @@ def test_CmisManagerTask_task_worker(self, mock_chassis, mock_get_status_tbl):
19141914
{'speed':'400000', 'lanes':'1,2,3,4,5,6,7,8'})
19151915
task.on_port_update_event(port_change_event)
19161916
assert len(task.port_dict) == 1
1917-
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
1917+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_INSERTED
19181918

19191919
task.get_host_tx_status = MagicMock(return_value='true')
19201920
task.get_port_admin_status = MagicMock(return_value='up')
@@ -1928,51 +1928,50 @@ def test_CmisManagerTask_task_worker(self, mock_chassis, mock_get_status_tbl):
19281928
mock_xcvr_api.decommission_all_datapaths = MagicMock(return_value=True)
19291929
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
19301930
task.task_worker()
1931-
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
1931+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_DP_DEINIT
19321932
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
19331933
task.task_worker()
19341934
assert mock_xcvr_api.set_datapath_deinit.call_count == 1
19351935
assert mock_xcvr_api.tx_disable_channel.call_count == 1
19361936
assert mock_xcvr_api.set_lpmode.call_count == 1
1937-
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
1937+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_AP_CONF
19381938

19391939
# Case 2: DP_DEINIT --> AP Configured
19401940
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
19411941
task.task_worker()
19421942
assert mock_xcvr_api.set_application.call_count == 1
1943-
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
1943+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_DP_INIT
19441944

19451945
# Case 3: AP Configured --> DP_INIT
19461946
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
19471947
task.task_worker()
19481948
assert mock_xcvr_api.set_datapath_init.call_count == 1
1949-
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
1949+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_DP_TXON
19501950

19511951
# Case 4: DP_INIT --> DP_TXON
19521952
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
19531953
task.task_worker()
19541954
assert mock_xcvr_api.tx_disable_channel.call_count == 2
1955-
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
1955+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_DP_ACTIVATE
19561956

19571957
# Case 5: DP_TXON --> DP_ACTIVATION
19581958
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
19591959
task.post_port_active_apsel_to_db = MagicMock()
19601960
task.task_worker()
19611961
assert task.post_port_active_apsel_to_db.call_count == 1
1962-
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
1962+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_READY
19631963

19641964
# Fail test coverage - Module Inserted state failing to reach DP_DEINIT
19651965
port_mapping = PortMapping()
1966+
port_mapping.handle_port_change_event(PortChangeEvent('Ethernet1', 1, 0, PortChangeEvent.PORT_ADD))
19661967
stop_event = threading.Event()
19671968
task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
1968-
task.port_mapping.logical_port_list = ['Ethernet1']
19691969
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
19701970
task.xcvr_table_helper.get_status_tbl.return_value = mock_get_status_tbl
19711971
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
19721972
task.task_worker()
1973-
assert get_cmis_state_from_state_db('Ethernet1', task.xcvr_table_helper.get_status_tbl(task.port_mapping.get_asic_id_for_logical_port('Ethernet1'))) == CMIS_STATE_UNKNOWN
1973+
assert get_cmis_state_from_state_db('Ethernet1', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet1'))) == CMIS_STATE_UNKNOWN
19741974

1975-
task.port_mapping.logical_port_list = MagicMock()
19761975
port_change_event = PortChangeEvent('PortConfigDone', -1, 0, PortChangeEvent.PORT_SET)
19771976
task.on_port_update_event(port_change_event)
19781977
assert task.isPortConfigDone
@@ -1981,7 +1980,7 @@ def test_CmisManagerTask_task_worker(self, mock_chassis, mock_get_status_tbl):
19811980
{'speed':'400000', 'lanes':'1,2,3,4,5,6,7,8'})
19821981
task.on_port_update_event(port_change_event)
19831982
assert len(task.port_dict) == 1
1984-
assert get_cmis_state_from_state_db('Ethernet1', task.xcvr_table_helper.get_status_tbl(task.port_mapping.get_asic_id_for_logical_port('Ethernet1'))) == CMIS_STATE_INSERTED
1983+
assert get_cmis_state_from_state_db('Ethernet1', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet1'))) == CMIS_STATE_INSERTED
19851984

19861985
task.get_host_tx_status = MagicMock(return_value='true')
19871986
task.get_port_admin_status = MagicMock(return_value='up')
@@ -1995,7 +1994,7 @@ def test_CmisManagerTask_task_worker(self, mock_chassis, mock_get_status_tbl):
19951994
mock_xcvr_api.decommission_all_datapaths = MagicMock(return_value=False)
19961995
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
19971996
task.task_worker()
1998-
assert not get_cmis_state_from_state_db('Ethernet1', task.xcvr_table_helper.get_status_tbl(task.port_mapping.get_asic_id_for_logical_port('Ethernet1'))) == CMIS_STATE_DP_DEINIT
1997+
assert not get_cmis_state_from_state_db('Ethernet1', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet1'))) == CMIS_STATE_DP_DEINIT
19991998

20001999
@patch('xcvrd.xcvrd.XcvrTableHelper.get_status_tbl')
20012000
@patch('xcvrd.xcvrd.platform_chassis')
@@ -2100,16 +2099,15 @@ def test_CmisManagerTask_task_worker_fastboot(self, mock_chassis, mock_get_statu
21002099
mock_chassis.get_sfp = MagicMock(return_value=mock_sfp)
21012100

21022101
port_mapping = PortMapping()
2102+
port_mapping.handle_port_change_event(PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD))
21032103
stop_event = threading.Event()
21042104
task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
2105-
task.port_mapping.logical_port_list = ['Ethernet0']
21062105
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
21072106
task.xcvr_table_helper.get_status_tbl.return_value = mock_get_status_tbl
21082107
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
21092108
task.task_worker()
2110-
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
2109+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_UNKNOWN
21112110

2112-
task.port_mapping.logical_port_list = MagicMock()
21132111
port_change_event = PortChangeEvent('PortConfigDone', -1, 0, PortChangeEvent.PORT_SET)
21142112
task.on_port_update_event(port_change_event)
21152113
assert task.isPortConfigDone
@@ -2118,7 +2116,7 @@ def test_CmisManagerTask_task_worker_fastboot(self, mock_chassis, mock_get_statu
21182116
{'speed':'400000', 'lanes':'1,2,3,4,5,6,7,8'})
21192117
task.on_port_update_event(port_change_event)
21202118
assert len(task.port_dict) == 1
2121-
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
2119+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_INSERTED
21222120

21232121
task.get_host_tx_status = MagicMock(return_value='false')
21242122
task.get_port_admin_status = MagicMock(return_value='up')
@@ -2130,7 +2128,7 @@ def test_CmisManagerTask_task_worker_fastboot(self, mock_chassis, mock_get_statu
21302128
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
21312129
task.task_worker()
21322130

2133-
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
2131+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_READY
21342132

21352133

21362134
@patch('xcvrd.xcvrd.XcvrTableHelper.get_status_tbl')
@@ -2236,16 +2234,15 @@ def test_CmisManagerTask_task_worker_host_tx_ready_false(self, mock_chassis, moc
22362234
mock_chassis.get_sfp = MagicMock(return_value=mock_sfp)
22372235

22382236
port_mapping = PortMapping()
2237+
port_mapping.handle_port_change_event(PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD))
22392238
stop_event = threading.Event()
22402239
task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event)
2241-
task.port_mapping.logical_port_list = ['Ethernet0']
22422240
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
22432241
task.xcvr_table_helper.get_status_tbl.return_value = mock_get_status_tbl
22442242
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
22452243
task.task_worker()
2246-
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
2244+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_UNKNOWN
22472245

2248-
task.port_mapping.logical_port_list = MagicMock()
22492246
port_change_event = PortChangeEvent('PortConfigDone', -1, 0, PortChangeEvent.PORT_SET)
22502247
task.on_port_update_event(port_change_event)
22512248
assert task.isPortConfigDone
@@ -2254,7 +2251,7 @@ def test_CmisManagerTask_task_worker_host_tx_ready_false(self, mock_chassis, moc
22542251
{'speed':'400000', 'lanes':'1,2,3,4,5,6,7,8'})
22552252
task.on_port_update_event(port_change_event)
22562253
assert len(task.port_dict) == 1
2257-
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
2254+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_INSERTED
22582255

22592256
task.get_host_tx_status = MagicMock(return_value='false')
22602257
task.get_port_admin_status = MagicMock(return_value='up')
@@ -2267,7 +2264,7 @@ def test_CmisManagerTask_task_worker_host_tx_ready_false(self, mock_chassis, moc
22672264
task.task_worker()
22682265

22692266
assert mock_xcvr_api.tx_disable_channel.call_count == 1
2270-
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
2267+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_READY
22712268

22722269
@pytest.mark.parametrize("lport, expected_dom_polling", [
22732270
('Ethernet0', 'disabled'),

0 commit comments

Comments
 (0)