From fd5b907bb3d3ab31ef2ffb12e30b6602218ce467 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Thu, 9 Nov 2023 14:59:34 +0200 Subject: [PATCH] Fix issue: QSFP module with id 0x0d can be parsed using 8636 1. It should first read the revision compliance to decide which standard to use 8636 should be used if it is greater than 3 2. For 8636 "Specification compliance", the bit 7 should be ignored and be parsed in "Extended Specification Compliance" Signed-off-by: Stephen Sun --- .../sonic_xcvr/mem_maps/public/sff8636.py | 4 +++- .../sonic_xcvr/xcvr_api_factory.py | 21 +++++++++++++++---- tests/sonic_xcvr/test_xcvr_api_factory.py | 16 ++++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/sonic_platform_base/sonic_xcvr/mem_maps/public/sff8636.py b/sonic_platform_base/sonic_xcvr/mem_maps/public/sff8636.py index 2a9032877..d91d65a83 100644 --- a/sonic_platform_base/sonic_xcvr/mem_maps/public/sff8636.py +++ b/sonic_platform_base/sonic_xcvr/mem_maps/public/sff8636.py @@ -50,7 +50,9 @@ def __init__(self, codes): ), CodeRegField(consts.CONNECTOR_FIELD, self.get_addr(0, 130), self.codes.CONNECTORS), RegGroupField(consts.SPEC_COMPLIANCE_FIELD, - CodeRegField(consts.ETHERNET_10_40G_COMPLIANCE_FIELD, self.get_addr(0, 131), self.codes.ETHERNET_10_40G_COMPLIANCE), + CodeRegField(consts.ETHERNET_10_40G_COMPLIANCE_FIELD, self.get_addr(0, 131), self.codes.ETHERNET_10_40G_COMPLIANCE, + *(RegBitField("%s_%d" % (consts.ETHERNET_10_40G_COMPLIANCE_FIELD, bit), bit) for bit in range(0, 7)) + ), CodeRegField(consts.SONET_COMPLIANCE_FIELD, self.get_addr(0, 132), self.codes.SONET_COMPLIANCE), CodeRegField(consts.SAS_SATA_COMPLIANCE_FIELD, self.get_addr(0, 133), self.codes.SAS_SATA_COMPLIANCE), CodeRegField(consts.GIGABIT_ETHERNET_COMPLIANCE_FIELD, self.get_addr(0, 134), self.codes.GIGABIT_ETHERNET_COMPLIANCE), diff --git a/sonic_platform_base/sonic_xcvr/xcvr_api_factory.py b/sonic_platform_base/sonic_xcvr/xcvr_api_factory.py index 08e695030..56592121e 100644 --- a/sonic_platform_base/sonic_xcvr/xcvr_api_factory.py +++ b/sonic_platform_base/sonic_xcvr/xcvr_api_factory.py @@ -45,6 +45,12 @@ def _get_id(self): return None return id_byte_raw[0] + def _get_revision_compliance(self): + id_byte_raw = self.reader(1, 1) + if id_byte_raw is None: + return None + return id_byte_raw[0] + def _get_vendor_name(self): name_data = self.reader(VENDOR_NAME_OFFSET, VENDOR_NAME_LENGTH) if name_data is None: @@ -90,10 +96,17 @@ def create_xcvr_api(self): api = Sff8636Api(xcvr_eeprom) # QSFP+ elif id == 0x0D: - codes = Sff8436Codes - mem_map = Sff8436MemMap(codes) - xcvr_eeprom = XcvrEeprom(self.reader, self.writer, mem_map) - api = Sff8436Api(xcvr_eeprom) + revision_compliance = self._get_revision_compliance() + if revision_compliance >= 3: + codes = Sff8636Codes + mem_map = Sff8636MemMap(codes) + xcvr_eeprom = XcvrEeprom(self.reader, self.writer, mem_map) + api = Sff8636Api(xcvr_eeprom) + else: + codes = Sff8436Codes + mem_map = Sff8436MemMap(codes) + xcvr_eeprom = XcvrEeprom(self.reader, self.writer, mem_map) + api = Sff8436Api(xcvr_eeprom) elif id == 0x03: codes = Sff8472Codes mem_map = Sff8472MemMap(codes) diff --git a/tests/sonic_xcvr/test_xcvr_api_factory.py b/tests/sonic_xcvr/test_xcvr_api_factory.py index d87be8862..87175b7c0 100644 --- a/tests/sonic_xcvr/test_xcvr_api_factory.py +++ b/tests/sonic_xcvr/test_xcvr_api_factory.py @@ -8,6 +8,14 @@ from sonic_platform_base.sonic_xcvr.codes.credo.aec_800g import CmisAec800gCodes from sonic_platform_base.sonic_xcvr.fields import consts from sonic_platform_base.sonic_xcvr.xcvr_api_factory import XcvrApiFactory +from sonic_platform_base.sonic_xcvr.api.public.sff8636 import Sff8636Api +from sonic_platform_base.sonic_xcvr.api.public.sff8436 import Sff8436Api + +def mock_reader_sff8636(start, length): + return bytes([0x0d]) if start == 0 else bytes ([0x06]) + +def mock_reader_sff8436(start, length): + return bytes([0x0d]) if start == 0 else bytes ([0x00]) class BytesMock(bytes): def decode(self, encoding='utf-8', errors='strict'): @@ -45,3 +53,11 @@ def test_create_xcvr_api(self): CmisAec800gApi = MagicMock() self.api.create_xcvr_api() + @pytest.mark.parametrize("reader, expected_api", [ + (mock_reader_sff8636, Sff8636Api), + (mock_reader_sff8436, Sff8436Api), + ]) + def test_create_xcvr_api_8436_8636(self, reader, expected_api): + self.api.reader = reader + api = self.api.create_xcvr_api() + assert isinstance(api, expected_api)