From de82dbeb046f18f1a6cba59a16742beac1ad6b3a Mon Sep 17 00:00:00 2001 From: Tamer Ahmed Date: Tue, 28 Jul 2026 09:27:56 -0700 Subject: [PATCH] [xcvrd] Verify that the module activated the requested CMIS application is_cmis_application_update_required() compares the desired application against get_application(), which reads the staged control set. The staged value reflects what xcvrd requested rather than what the module is running, so a module that accepts an application, reports ConfigSuccess and activates its datapath while leaving the Active Control Set unchanged is treated as correctly configured. When that happens the port is declared READY while the module keeps running the previous application on the media side. The link never comes up and neither the logs nor TRANSCEIVER_STATUS indicate why, since every value xcvrd inspects looks healthy. Compare the active application select code against the desired application before skipping an update and before transitioning to READY. A mismatch now forces a datapath reinitialization and, once CMIS_MAX_RETRIES is exhausted, the port transitions to FAILED instead of silently running the wrong application. The check is skipped when the active application code is not readable, so modules that do not report it keep the previous behaviour. Signed-off-by: Tamer Ahmed --- sonic-xcvrd/tests/test_xcvrd.py | 51 ++++++++++++++++++ sonic-xcvrd/xcvrd/cmis/cmis_manager_task.py | 58 +++++++++++++++++++++ 2 files changed, 109 insertions(+) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index a25d742ea..d575b50e4 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -3800,6 +3800,57 @@ def get_application(lane): assert task.is_cmis_application_update_required(mock_xcvr_api, app_new, host_lanes_mask) == expected + @pytest.mark.parametrize("appl, host_lanes_mask, active_apsel, expected", [ + # The module activated the desired application + (1, 0x0F, {'ActiveAppSelLane1': 1, 'ActiveAppSelLane2': 1, + 'ActiveAppSelLane3': 1, 'ActiveAppSelLane4': 1}, True), + # The module is running a different application than the desired one + (12, 0x0F, {'ActiveAppSelLane1': 4, 'ActiveAppSelLane2': 4, + 'ActiveAppSelLane3': 4, 'ActiveAppSelLane4': 4}, False), + # Only a subset of the host lanes failed to activate the desired application + (12, 0x0F, {'ActiveAppSelLane1': 12, 'ActiveAppSelLane2': 12, + 'ActiveAppSelLane3': 12, 'ActiveAppSelLane4': 4}, False), + # Host lanes outside of the mask are ignored + (12, 0x03, {'ActiveAppSelLane1': 12, 'ActiveAppSelLane2': 12, + 'ActiveAppSelLane3': 4, 'ActiveAppSelLane4': 4}, True), + # The active application code is not readable + (12, 0x0F, {'ActiveAppSelLane1': 'N/A', 'ActiveAppSelLane2': 'N/A', + 'ActiveAppSelLane3': 'N/A', 'ActiveAppSelLane4': 'N/A'}, True), + (12, 0x0F, {}, True), + ]) + def test_CmisManagerTask_is_active_apsel_matching_desired(self, appl, host_lanes_mask, active_apsel, expected): + mock_xcvr_api = MagicMock() + mock_xcvr_api.get_active_apsel_hostlane = MagicMock(return_value=active_apsel) + + port_mapping = PortMapping() + stop_event = threading.Event() + task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event, platform_chassis=MagicMock()) + + assert task.is_active_apsel_matching_desired(mock_xcvr_api, appl, host_lanes_mask) == expected + + def test_CmisManagerTask_is_cmis_application_update_required_active_apsel_mismatch(self): + """ + A module can accept the staged application and report ConfigSuccess without + ever updating the Active Control Set. The staged application then matches the + desired one and the datapath looks healthy, while the module keeps running the + previous application, so an update still has to be forced. + """ + mock_xcvr_api = MagicMock() + mock_xcvr_api.is_flat_memory = MagicMock(return_value=False) + mock_xcvr_api.get_application = MagicMock(return_value=12) + mock_xcvr_api.get_datapath_state = MagicMock(return_value=self.DEFAULT_DP_STATE) + mock_xcvr_api.get_config_datapath_hostlane_status = MagicMock(return_value=self.DEFAULT_CONFIG_STATUS) + mock_xcvr_api.get_active_apsel_hostlane = MagicMock(return_value={ + 'ActiveAppSelLane1': 4, 'ActiveAppSelLane2': 4, + 'ActiveAppSelLane3': 4, 'ActiveAppSelLane4': 4 + }) + + port_mapping = PortMapping() + stop_event = threading.Event() + task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event, platform_chassis=MagicMock()) + + assert task.is_cmis_application_update_required(mock_xcvr_api, 12, 0x0F) == True + @pytest.mark.parametrize("ifname, expected", [ ('1.6TBASE-CR8 (Clause179)', 1600000), ('1.6TAUI-8 (Annex176E)', 1600000), diff --git a/sonic-xcvrd/xcvrd/cmis/cmis_manager_task.py b/sonic-xcvrd/xcvrd/cmis/cmis_manager_task.py index b39992fa7..54293fd5b 100644 --- a/sonic-xcvrd/xcvrd/cmis/cmis_manager_task.py +++ b/sonic-xcvrd/xcvrd/cmis/cmis_manager_task.py @@ -572,9 +572,58 @@ def is_cmis_application_update_required(self, api, app_new, host_lanes_mask): if conf_state[name] != 'ConfigSuccess': skip = False break + if skip and not self.is_active_apsel_matching_desired(api, app_new, host_lanes_mask): + self.log_notice("Forcing application update since the module has not " + "activated the desired application {}...".format(app_new)) + skip = False return (not skip) return True + def is_active_apsel_matching_desired(self, api, appl, host_lanes_mask): + """ + Check whether the module has actually activated the desired application. + + get_application() reads the staged control set, which reflects what was + requested rather than what the module is running. A module can accept a + staged application and report ConfigSuccess while leaving the Active + Control Set unchanged, in which case the media side keeps running the + previous application. Comparing against the active application select + code detects that case. + + Args: + api: + XcvrApi object + appl: + Integer, the desired transceiver-specific application code + host_lanes_mask: + Integer, a bitmask of the lanes on the host side + e.g. 0x5 for lane 0 and lane 2. + + Returns: + Boolean, true if the active application matches the desired one on + every host lane of the mask, or if the active application code + cannot be read. + """ + active_apsel = api.get_active_apsel_hostlane() + if not active_apsel: + return True + + for lane in range(self.CMIS_MAX_HOST_LANES): + if ((1 << lane) & host_lanes_mask) == 0: + continue + active = active_apsel.get("ActiveAppSelLane{}".format(lane + 1)) + # The active application code is not always readable, e.g. on flat + # memory modules it is reported as 'N/A'. Skip the check instead of + # forcing a needless datapath reinitialization. + if not isinstance(active, int): + return True + if active != appl: + self.log_error("Active application {} on host lane {} does not match " + "the desired application {}".format(active, lane + 1, appl)) + return False + + return True + def force_cmis_reinit(self, lport, retries=0): """ Try to force the restart of CMIS state machine @@ -1240,6 +1289,15 @@ def process_cmis_state_machine(self, lport): self.force_cmis_reinit(lport, retries + 1) return + # A module can report ConfigSuccess and activate its datapath while + # still running a different application than the one requested. Verify + # the active application before declaring the port ready, otherwise the + # module silently stays on the wrong media interface. + if not self.is_active_apsel_matching_desired(api, appl, host_lanes_mask): + self.log_error("{}: module did not activate application {}".format(lport, appl)) + self.force_cmis_reinit(lport, retries + 1) + return + self.log_notice("{}: READY".format(lport)) self.update_port_transceiver_status_table_sw_cmis_state(lport, CMIS_STATE_READY) self.post_port_active_apsel_to_db(api, lport, host_lanes_mask)