diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index a25d742ea..19608e66d 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -452,6 +452,40 @@ def test_CmisManagerTask_task_run_with_exception(self): assert("sonic-xcvrd/xcvrd/cmis/cmis_manager_task.py" in str(trace)) assert("update_port_transceiver_status_table_sw_cmis_state" in str(trace)) + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('swsscommon.swsscommon.SubscriberStateTable') + @patch('swsscommon.swsscommon.Select.select') + def test_CmisManagerTask_wait_for_port_config_done(self, mock_select, mock_sub_table): + mock_selectable = MagicMock() + mock_selectable.pop = MagicMock( + side_effect=[('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('PortConfigDone', None, None)]) + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + mock_sub_table.return_value = mock_selectable + port_mapping = PortMapping() + stop_event = threading.Event() + cmis_manager = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event, platform_chassis=MagicMock()) + # Neither PortConfigDone nor PortInitDone exist yet in APPL_DB: must + # fall through to the subscribe-and-wait path below. + with patch('sonic_py_common.daemon_base.db_connect') as mock_db_connect: + mock_db_connect.return_value.exists = MagicMock(return_value=False) + cmis_manager.wait_for_port_config_done('') + assert swsscommon.Select.select.call_count == 2 + + def test_CmisManagerTask_wait_for_port_config_done_fast_path(self): + # If PortConfigDone/PortInitDone already exist in APPL_DB (daemon + # (re)started independently of swss, after the one-time pub/sub + # notification already fired), wait_for_port_config_done() must + # return immediately instead of blocking on a notification that + # will never repeat. + port_mapping = PortMapping() + stop_event = threading.Event() + cmis_manager = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event, platform_chassis=MagicMock()) + with patch('sonic_py_common.daemon_base.db_connect') as mock_db_connect, \ + patch('swsscommon.swsscommon.Select.select') as mock_select: + mock_db_connect.return_value.exists = MagicMock(return_value=True) + cmis_manager.wait_for_port_config_done('') + mock_select.assert_not_called() + @patch('xcvrd.cmis.cmis_manager_task.PortChangeObserver', MagicMock(handle_port_update_event=MagicMock())) @patch('xcvrd.cmis.CmisManagerTask.wait_for_port_config_done', MagicMock()) @patch('xcvrd.xcvrd_utilities.common.is_fast_reboot_enabled', MagicMock(return_value=False)) @@ -2656,9 +2690,26 @@ def test_DaemonXcvrd_wait_for_port_config_done(self, mock_select, mock_sub_table mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) mock_sub_table.return_value = mock_selectable xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER) - xcvrd.wait_for_port_config_done('') + # Neither PortConfigDone nor PortInitDone exist yet in APPL_DB: must + # fall through to the subscribe-and-wait path below. + with patch('sonic_py_common.daemon_base.db_connect') as mock_db_connect: + mock_db_connect.return_value.exists = MagicMock(return_value=False) + xcvrd.wait_for_port_config_done('') assert swsscommon.Select.select.call_count == 2 + def test_DaemonXcvrd_wait_for_port_config_done_fast_path(self): + # If PortConfigDone/PortInitDone already exist in APPL_DB (daemon + # (re)started independently of swss, after the one-time pub/sub + # notification already fired), wait_for_port_config_done() must + # return immediately instead of blocking on a notification that + # will never repeat. + xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER) + with patch('sonic_py_common.daemon_base.db_connect') as mock_db_connect, \ + patch('swsscommon.swsscommon.Select.select') as mock_select: + mock_db_connect.return_value.exists = MagicMock(return_value=True) + xcvrd.wait_for_port_config_done('') + mock_select.assert_not_called() + def test_DaemonXcvrd_initialize_port_init_control_fields_in_port_table(self): port_mapping = PortMapping() xcvrd = DaemonXcvrd(SYSLOG_IDENTIFIER) diff --git a/sonic-xcvrd/xcvrd/cmis/cmis_manager_task.py b/sonic-xcvrd/xcvrd/cmis/cmis_manager_task.py index b39992fa7..f8514686f 100644 --- a/sonic-xcvrd/xcvrd/cmis/cmis_manager_task.py +++ b/sonic-xcvrd/xcvrd/cmis/cmis_manager_task.py @@ -797,6 +797,16 @@ def wait_for_port_config_done(self, namespace): # Connect to APPL_DB and subscribe to PORT table notifications appl_db = daemon_base.db_connect("APPL_DB", namespace=namespace) + # PortConfigDone/PortInitDone is a one-time pub/sub notification from + # portsyncd, emitted when swss finishes port bring-up. If this daemon + # (re)starts after that notification already fired (e.g. a crash + # restart, or a manual restart independent of swss), the + # SubscriberStateTable below will never see it and this loop blocks + # forever, even though the key itself is a normal, still-present + # entry in APPL_DB. Check for it directly before subscribing. + if appl_db.exists("PORT_TABLE:PortConfigDone") or appl_db.exists("PORT_TABLE:PortInitDone"): + return + sel = swsscommon.Select() port_tbl = swsscommon.SubscriberStateTable(appl_db, swsscommon.APP_PORT_TABLE_NAME) sel.addSelectable(port_tbl) diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index ee1545334..d1cb0f385 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -917,6 +917,16 @@ def wait_for_port_config_done(self, namespace): # Connect to APPL_DB and subscribe to PORT table notifications appl_db = daemon_base.db_connect("APPL_DB", namespace=namespace) + # PortConfigDone/PortInitDone is a one-time pub/sub notification from + # portsyncd, emitted when swss finishes port bring-up. If this daemon + # (re)starts after that notification already fired (e.g. a crash + # restart, or a manual restart independent of swss), the + # SubscriberStateTable below will never see it and this loop blocks + # forever, even though the key itself is a normal, still-present + # entry in APPL_DB. Check for it directly before subscribing. + if appl_db.exists("PORT_TABLE:PortConfigDone") or appl_db.exists("PORT_TABLE:PortInitDone"): + return + sel = swsscommon.Select() port_tbl = swsscommon.SubscriberStateTable(appl_db, swsscommon.APP_PORT_TABLE_NAME) sel.addSelectable(port_tbl)