Skip to content

Commit 954e9cb

Browse files
committed
fix(xcvrd): fix PORT_DEL handling due to xcvr plug-out
Signed-off-by: Wataru Ishida <wataru.ishid@gmail.com>
1 parent 6ef3822 commit 954e9cb

File tree

2 files changed

+51
-7
lines changed

2 files changed

+51
-7
lines changed

sonic-xcvrd/tests/test_xcvrd.py

+13-1
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ def test_CmisManagerTask_get_xcvr_api_exception(self, mock_platform_chassis, moc
303303
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
304304
task.on_port_update_event(port_change_event)
305305
task.task_worker()
306-
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
306+
assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.get_asic_id('Ethernet0'))) == CMIS_STATE_READY
307307

308308
# Case 3: get_cmis_application_desired() raises an exception
309309
mock_xcvr_api.is_flat_memory = MagicMock(return_value=False)
@@ -1662,6 +1662,18 @@ def test_CmisManagerTask_handle_port_change_event(self):
16621662
task.on_port_update_event(port_change_event)
16631663
assert len(task.port_dict) == 1
16641664

1665+
# STATE_DB DEL event doesn't remove port from port_dict
1666+
# this happens when transceiver is plugged-out or DPB is used
1667+
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_DEL, {}, db_name='STATE_DB')
1668+
task.on_port_update_event(port_change_event)
1669+
assert len(task.port_dict) == 1
1670+
1671+
# CONFIG_DB DEL event removes port from port_dict
1672+
# this happens when DPB is used
1673+
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_DEL, {}, db_name='CONFIG_DB', table_name='PORT')
1674+
task.on_port_update_event(port_change_event)
1675+
assert len(task.port_dict) == 0
1676+
16651677
@patch('xcvrd.xcvrd.XcvrTableHelper')
16661678
def test_CmisManagerTask_get_configured_freq(self, mock_table_helper):
16671679
port_mapping = PortMapping()

sonic-xcvrd/xcvrd/xcvrd.py

+38-6
Original file line numberDiff line numberDiff line change
@@ -897,12 +897,44 @@ def on_port_update_event(self, port_change_event):
897897

898898
self.force_cmis_reinit(lport, 0)
899899

900-
# PORT_DEL event for the same lport happens 3 times because
901-
# we are subscribing to CONFIG_DB, STATE_DB|TRANSCEIVER_INFO, and STATE_DB|PORT_TABLE.
902-
# To only handle the first one, check if the lport exists in the port_dict.
903-
elif lport in self.port_dict:
904-
self.update_port_transceiver_status_table_sw_cmis_state(lport, CMIS_STATE_REMOVED)
905-
self.port_dict.pop(lport)
900+
elif port_change_event.event_type == port_change_event.PORT_DEL:
901+
# In handling the DEL event, the following two scenarios must be considered:
902+
# 1. PORT_DEL event due to transceiver plug-out
903+
# 2. PORT_DEL event due to Dynamic Port Breakout (DPB)
904+
#
905+
# Scenario 1 is simple, as only a STATE_DB|TRANSCEIVER_INFO PORT_DEL event occurs,
906+
# so we just need to set SW_CMIS_STATE to CMIS_STATE_REMOVED.
907+
#
908+
# Scenario 2 is a bit more complex. First, for the port(s) before DPB, a CONFIG_DB|PORT PORT_DEL
909+
# and a STATE_DB|PORT_TABLE PORT_DEL event occur. Next, for the port(s) after DPB,
910+
# a CONFIG_DB|PORT PORT_SET and a STATE_DB|PORT_TABLE PORT_SET event occur.
911+
# After that (after a short delay), a STATE_DB|TRANSCEIVER_INFO PORT_DEL event
912+
# occurs for the port(s) before DPB, and finally, a STATE_DB|TRANSCEIVER_INFO
913+
# PORT_SET event occurs for the port(s) after DPB.
914+
#
915+
# Below is the event sequence when configuring Ethernet0 from "2x200G" to "1x400G"
916+
# (based on actual logs).
917+
#
918+
# 1. SET Ethernet0 CONFIG_DB|PORT
919+
# 2. DEL Ethernet2 CONFIG_DB|PORT
920+
# 3. DEL Ethernet0 CONFIG_DB|PORT
921+
# 4. DEL Ethernet0 STATE_DB|PORT_TABLE
922+
# 5. DEL Ethernet2 STATE_DB|PORT_TABLE
923+
# 6. SET Ethernet0 CONFIG_DB|PORT
924+
# 7. SET Ethernet0 STATE_DB|PORT_TABLE
925+
# 8. SET Ethernet0 STATE_DB|PORT_TABLE
926+
# 9. DEL Ethernet2 STATE_DB|TRANSCEIVER_INFO
927+
# 10. DEL Ethernet0 STATE_DB|TRANSCEIVER_INFO
928+
# 11. SET Ethernet0 STATE_DB|TRANSCEIVER_INFO
929+
#
930+
# To handle both scenarios, if the lport exists in port_dict for any DEL EVENT,
931+
# set SW_CMIS_STATE to REMOVED. Additionally, for DEL EVENTS from CONFIG_DB due to DPB,
932+
# remove the lport from port_dict.
933+
if lport in self.port_dict:
934+
self.update_port_transceiver_status_table_sw_cmis_state(lport, CMIS_STATE_REMOVED)
935+
936+
if port_change_event.db_name == 'CONFIG_DB' and port_change_event.table_name == 'PORT':
937+
self.port_dict.pop(lport)
906938

907939
def get_cmis_dp_init_duration_secs(self, api):
908940
return api.get_datapath_init_duration()/1000

0 commit comments

Comments
 (0)