Skip to content

Commit d186e7f

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 ed49ce6 commit d186e7f

File tree

2 files changed

+49
-6
lines changed

2 files changed

+49
-6
lines changed

sonic-xcvrd/tests/test_xcvrd.py

+11
Original file line numberDiff line numberDiff line change
@@ -1519,6 +1519,17 @@ def test_CmisManagerTask_handle_port_change_event(self):
15191519
task.on_port_update_event(port_change_event)
15201520
assert len(task.port_dict) == 1
15211521

1522+
# STATE_DB DEL event doesn't remove port from port_dict
1523+
# this happens when transceiver is plugged-out or DPB is used
1524+
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_DEL, {}, db_name='STATE_DB')
1525+
task.on_port_update_event(port_change_event)
1526+
assert len(task.port_dict) == 1
1527+
1528+
# CONFIG_DB DEL event removes port from port_dict
1529+
# this happens when DPB is used
1530+
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_DEL, {}, db_name='CONFIG_DB', table_name='PORT')
1531+
task.on_port_update_event(port_change_event)
1532+
assert len(task.port_dict) == 0
15221533

15231534
@patch('xcvrd.xcvrd.XcvrTableHelper')
15241535
def test_CmisManagerTask_get_configured_freq(self, mock_table_helper):

sonic-xcvrd/xcvrd/xcvrd.py

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

897897
self.force_cmis_reinit(lport, 0)
898898

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

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

0 commit comments

Comments
 (0)