diff --git a/sonic_platform_base/sonic_xcvr/api/public/cmis.py b/sonic_platform_base/sonic_xcvr/api/public/cmis.py index 416d5b581..2d2e9bdfa 100644 --- a/sonic_platform_base/sonic_xcvr/api/public/cmis.py +++ b/sonic_platform_base/sonic_xcvr/api/public/cmis.py @@ -1050,10 +1050,27 @@ def get_module_media_interface(self): @read_only_cached_api_return def is_coherent_module(self): ''' - Returns True if the module follow C-CMIS spec, False otherwise + Returns True if the module follows the C-CMIS spec, False otherwise. + + Detection is the union of two independent signals, so the bit can + only add coherent modules and never drops one that used to be + detected: + * the media interface name contains 'ZR' or 'FOIC' - works on + every CMIS revision and matches the behavior from before this + bit existed, and + * CoherentPagesSupported (Page 01h byte 142 bit 4) is set. That + bit is only defined from OIF-CMIS 5.3 on (Reserved before, where + a real module may report it as a stray 1), so it is only honored + when the module reports CMIS 5.3 or later. ''' mintf = self.get_module_media_interface() - return False if 'ZR' not in mintf else True + if any(kw in mintf for kw in ('ZR', 'FOIC')): + return True + cmis_major = self.xcvr_eeprom.read(consts.CMIS_MAJOR_REVISION) + cmis_minor = self.xcvr_eeprom.read(consts.CMIS_MINOR_REVISION) + if cmis_major is not None and cmis_minor is not None and (cmis_major, cmis_minor) >= (5, 3): + return bool(self.xcvr_eeprom.read(consts.COHERENT_PAGES_SUPPORTED)) + return False @read_only_cached_api_return def get_datapath_init_duration(self): diff --git a/sonic_platform_base/sonic_xcvr/fields/consts.py b/sonic_platform_base/sonic_xcvr/fields/consts.py index c4881da34..2ac47ca7c 100644 --- a/sonic_platform_base/sonic_xcvr/fields/consts.py +++ b/sonic_platform_base/sonic_xcvr/fields/consts.py @@ -292,6 +292,7 @@ FLAGS_ADVT_FIELD = "Supported Flags Advertisement" PAGE_SUPPORT_ADVT_FIELD = "Supported Pages Advertisement" DIAG_PAGE_SUPPORT_ADVT_FIELD = "Supported Diagnostic Pages Advertisement" +COHERENT_PAGES_SUPPORTED = "CoherentPagesSupported" TX_FLAGS_ADVT_FIELD = "Supported TX Flags Advertisement" RX_FLAGS_ADVT_FIELD = "Supported RX Flags Advertisement" LANE_MON_ADVT_FIELD = "Supported Lane Monitor Advertisement" diff --git a/sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page01.py b/sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page01.py index 76fa40725..36dcedbd9 100644 --- a/sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page01.py +++ b/sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page01.py @@ -85,6 +85,7 @@ def __init__(self, codes, page=ADVERTISING_PAGE): NumberRegField(consts.PAGE_SUPPORT_ADVT_FIELD, self.getaddr(142), RegBitField(consts.VDM_SUPPORTED, 6), RegBitField(consts.DIAG_PAGE_SUPPORT_ADVT_FIELD, 5), + RegBitField(consts.COHERENT_PAGES_SUPPORTED, 4), ), CodeRegField(consts.BANKS_SUPPORTED_FIELD, self.getaddr(142), codes.MAX_BANKS_SUPPORTED, *(RegBitField("Bit%d" % bit, bit) for bit in range(0, 2)) diff --git a/tests/sonic_xcvr/test_cmis.py b/tests/sonic_xcvr/test_cmis.py index 4b3b7314e..4f0d1400c 100755 --- a/tests/sonic_xcvr/test_cmis.py +++ b/tests/sonic_xcvr/test_cmis.py @@ -965,13 +965,103 @@ def test_get_module_media_interface(self, mock_response1, mock_response2, expect @pytest.mark.parametrize("mock_response, expected", [ ('Copper cable', False), ('400ZR', True), + # FOIC-named media interfaces (e.g. 800G-ZR+ FOIC, no 'ZR' substring) + # are coherent too; only reachable when CoherentPagesSupported is + # unavailable (mocked read() below defaults to None). + ('FOIC1.4-DO (G.709.3/Y.1331.3)', True), ]) def test_is_coherent_module(self, mock_response, expected): + self.clear_cache('is_coherent_module') + # CoherentPagesSupported unavailable: force the string-matching + # fallback path regardless of what earlier tests left behind on the + # shared self.api.xcvr_eeprom mock. + self.api.xcvr_eeprom.read = MagicMock(return_value=None) self.api.get_module_media_interface = MagicMock() self.api.get_module_media_interface.return_value = mock_response result = self.api.is_coherent_module() assert result == expected + @pytest.mark.parametrize("mock_response, expected", [ + (1, True), + (0, False), + ]) + def test_is_coherent_module_coherent_pages_bit(self, mock_response, expected): + # On CMIS 5.3+, CoherentPagesSupported is advertised and takes + # precedence over the media interface name (which is deliberately + # left un-mocked / not matching 'ZR' or 'FOIC', to prove the bit + # alone decides this). + self.clear_cache('is_coherent_module') + self.api.get_module_media_interface = MagicMock(return_value='Copper cable') + def mock_read(field): + if field == consts.CMIS_MAJOR_REVISION: + return 5 + if field == consts.CMIS_MINOR_REVISION: + return 3 + if field == consts.COHERENT_PAGES_SUPPORTED: + return mock_response + return None + self.api.xcvr_eeprom.read = MagicMock(side_effect=mock_read) + result = self.api.is_coherent_module() + assert result == expected + + @pytest.mark.parametrize("cmis_minor, mintf, expected", [ + (2, '400ZR', True), + (2, 'Copper cable', False), + ]) + def test_is_coherent_module_ignores_reserved_bit_pre_5_3(self, cmis_minor, mintf, expected): + # Byte 142 bit 4 is Reserved prior to CMIS 5.3 (OIF-CMIS-05.2 Table + # 8-41). A pre-5.3 module may report this bit as 0 (the spec's + # convention for reserved bits) or 1 (not spec-guaranteed, but not + # excluded either) while still being a real coherent module - the + # bit must never override the media interface name check for + # modules that predate the bit's definition. + self.clear_cache('is_coherent_module') + self.api.get_module_media_interface = MagicMock(return_value=mintf) + def mock_read(field): + if field == consts.CMIS_MAJOR_REVISION: + return 5 + if field == consts.CMIS_MINOR_REVISION: + return cmis_minor + if field == consts.COHERENT_PAGES_SUPPORTED: + return 0 + return None + self.api.xcvr_eeprom.read = MagicMock(side_effect=mock_read) + result = self.api.is_coherent_module() + assert result == expected + + def test_is_coherent_module_unknown_cmis_revision_falls_back(self): + # CmisMajorRevision/CmisMinorRevision reads failing (None) must not + # crash the (major, minor) >= (5, 3) comparison; behave as if the + # bit is untrustworthy and use the string-matching fallback. + self.clear_cache('is_coherent_module') + self.api.get_module_media_interface = MagicMock(return_value='400ZR') + self.api.xcvr_eeprom.read = MagicMock(return_value=None) + result = self.api.is_coherent_module() + assert result is True + + @pytest.mark.parametrize("mintf", [ + '400ZR', + 'FOIC1.4-DO (G.709.3/Y.1331.3)', + ]) + def test_is_coherent_module_name_match_overrides_cleared_bit(self, mintf): + # A CMIS 5.3+ coherent module that mis-advertises + # CoherentPagesSupported as 0 while still naming a coherent media + # interface must stay coherent: the bit can only add detection, it + # must never drop a module the name match already covers. This + # guarantees no regression vs. the pre-bit ('ZR'/'FOIC') behavior. + self.clear_cache('is_coherent_module') + self.api.get_module_media_interface = MagicMock(return_value=mintf) + def mock_read(field): + if field == consts.CMIS_MAJOR_REVISION: + return 5 + if field == consts.CMIS_MINOR_REVISION: + return 3 + if field == consts.COHERENT_PAGES_SUPPORTED: + return 0 + return None + self.api.xcvr_eeprom.read = MagicMock(side_effect=mock_read) + assert self.api.is_coherent_module() is True + @pytest.mark.parametrize("mock_response1, mock_response2, expected", [ (True, '1', 0 ), (False, None, 0),