From 82666ec875f2cf85502f17ec6775cc813cb84317 Mon Sep 17 00:00:00 2001 From: Kebo Liu Date: Fri, 24 Jun 2022 13:49:22 +0800 Subject: [PATCH 1/5] add retry when dom capability is not been detected due to read failure Signed-off-by: Kebo Liu --- .../mlnx-platform-api/sonic_platform/sfp.py | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py index 0650cf20741c..3fc9a6ab2884 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py +++ b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py @@ -369,7 +369,8 @@ def __init__(self): self.calibration = 0 self.qsfp_page3_available = False self.second_application_list = False - + self.dom_detect_finished = False + class SFP(SfpBase): """Platform-specific SFP class""" @@ -437,7 +438,7 @@ def sfp_type(self): return self._sfp_type def _dom_capability_detect(self): - if self._sfp_capability: + if self._sfp_capability and self._sfp_capability.dom_detect_finished: return self._sfp_capability = SfpCapability() @@ -456,8 +457,9 @@ def _dom_capability_detect(self): # need to add more code for determining the capability and version compliance # in SFF-8636 dom capability definitions evolving with the versions. qsfp_dom_capability_raw = self._read_eeprom_specific_bytes((offset + XCVR_DOM_CAPABILITY_OFFSET), XCVR_DOM_CAPABILITY_WIDTH) - if qsfp_dom_capability_raw is not None: - qsfp_version_compliance_raw = self._read_eeprom_specific_bytes(QSFP_VERSION_COMPLIANCE_OFFSET, QSFP_VERSION_COMPLIANCE_WIDTH) + qsfp_version_compliance_raw = self._read_eeprom_specific_bytes(QSFP_VERSION_COMPLIANCE_OFFSET, QSFP_VERSION_COMPLIANCE_WIDTH) + qsfp_option_value_raw = self._read_eeprom_specific_bytes(QSFP_OPTION_VALUE_OFFSET, QSFP_OPTION_VALUE_WIDTH) + if None not in (qsfp_dom_capability_raw, qsfp_version_compliance_raw, qsfp_option_value_raw): qsfp_version_compliance = int(qsfp_version_compliance_raw[0], 16) dom_capability = sfpi_obj.parse_dom_capability(qsfp_dom_capability_raw, 0) if qsfp_version_compliance >= 0x08: @@ -475,12 +477,12 @@ def _dom_capability_detect(self): sfpd_obj = sff8436Dom() if sfpd_obj is None: return None - qsfp_option_value_raw = self._read_eeprom_specific_bytes(QSFP_OPTION_VALUE_OFFSET, QSFP_OPTION_VALUE_WIDTH) - if qsfp_option_value_raw is not None: - optional_capability = sfpd_obj.parse_option_params(qsfp_option_value_raw, 0) - self._sfp_capability.dom_tx_disable_supported = optional_capability['data']['TxDisable']['value'] == 'On' + + optional_capability = sfpd_obj.parse_option_params(qsfp_option_value_raw, 0) + self._sfp_capability.dom_tx_disable_supported = optional_capability['data']['TxDisable']['value'] == 'On' dom_status_indicator = sfpd_obj.parse_dom_status_indicator(qsfp_version_compliance_raw, 1) self._sfp_capability.qsfp_page3_available = dom_status_indicator['data']['FlatMem']['value'] == 'Off' + self._sfp_capability.dom_detect_finished = True else: self._sfp_capability.dom_supported = False self._sfp_capability.dom_temp_supported = False @@ -489,6 +491,7 @@ def _dom_capability_detect(self): self._sfp_capability.dom_tx_power_supported = False self._sfp_capability.calibration = 0 self._sfp_capability.qsfp_page3_available = False + self._sfp_capability.dom_detect_finished = False elif self.sfp_type == QSFP_DD_TYPE: sfpi_obj = qsfp_dd_InterfaceId() @@ -518,6 +521,7 @@ def _dom_capability_detect(self): self._sfp_capability.dom_tx_bias_power_supported = False self._sfp_capability.dom_thresholds_supported = False self._sfp_capability.dom_rx_tx_power_bias_supported = False + self._sfp_capability.dom_detect_finished = True else: self._sfp_capability.dom_supported = False self._sfp_capability.dom_temp_supported = False @@ -527,6 +531,7 @@ def _dom_capability_detect(self): self._sfp_capability.dom_tx_bias_power_supported = False self._sfp_capability.dom_thresholds_supported = False self._sfp_capability.dom_rx_tx_power_bias_supported = False + self._sfp_capability.dom_detect_finished = False elif self.sfp_type == SFP_TYPE: sfpi_obj = sff8472InterfaceId() @@ -554,12 +559,14 @@ def _dom_capability_detect(self): self._sfp_capability.dom_tx_power_supported = False self._sfp_capability.calibration = 0 self._sfp_capability.dom_tx_disable_supported = (int(sfp_dom_capability_raw[1], 16) & 0x40 != 0) + self._sfp_capability.dom_detect_finished = True else: self._sfp_capability.dom_supported = False self._sfp_capability.dom_temp_supported = False self._sfp_capability.dom_volt_supported = False self._sfp_capability.dom_rx_power_supported = False self._sfp_capability.dom_tx_power_supported = False + self._sfp_capability.dom_detect_finished = True @property @utils.pre_initialize(_dom_capability_detect) From 595385f562d467d7b90b4b59b681f11842dfa1d7 Mon Sep 17 00:00:00 2001 From: Kebo Liu Date: Fri, 24 Jun 2022 19:46:15 +0800 Subject: [PATCH 2/5] add retry mechanisim for SFP type detect during init Signed-off-by: Kebo Liu --- .../mlnx-platform-api/sonic_platform/sfp.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py index 3fc9a6ab2884..d9fe602b326c 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py +++ b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py @@ -418,11 +418,11 @@ def sdk_handle(self): logger.log_error('Failed to open SDK handle') return SFP.shared_sdk_handle - @property - def sfp_type(self): + def _detect_sfp_type(self): if not self._sfp_type: eeprom_raw = [] eeprom_raw = self._read_eeprom_specific_bytes(XCVR_TYPE_OFFSET, XCVR_TYPE_WIDTH) + if eeprom_raw: if eeprom_raw[0] in SFP_TYPE_CODE_LIST: self._sfp_type = SFP_TYPE @@ -431,11 +431,10 @@ def sfp_type(self): elif eeprom_raw[0] in QSFP_DD_TYPE_CODE_LIST: self._sfp_type = QSFP_DD_TYPE - # we don't regonize this identifier value, treat the xSFP module as the default type - if not self._sfp_type: - raise RuntimeError("Failed to detect SFP type for SFP {}".format(self.index)) - else: - return self._sfp_type + @property + @utils.pre_initialize(_detect_sfp_type) + def sfp_type(self): + return self._sfp_type def _dom_capability_detect(self): if self._sfp_capability and self._sfp_capability.dom_detect_finished: @@ -914,7 +913,7 @@ def get_transceiver_info(self): transceiver_info_dict['nominal_bit_rate'] = "Not supported for CMIS cables" transceiver_info_dict['application_advertisement'] = host_media_list - else: + elif self.sfp_type == SFP_TYPE: offset = 0 vendor_rev_width = XCVR_HW_REV_WIDTH_SFP interface_info_bulk_width = XCVR_INTFACE_BULK_WIDTH_SFP @@ -923,6 +922,9 @@ def get_transceiver_info(self): if sfpi_obj is None: print("Error: sfp_object open failed") return None + else: + # None of any supported SFP type, could be SFP object not correctly initialized. + return None if self.sfp_type != QSFP_DD_TYPE: sfp_interface_bulk_raw = self._read_eeprom_specific_bytes(offset + XCVR_INTERFACE_DATA_START, XCVR_INTERFACE_DATA_SIZE) From d7e6901c3d6994bb276c2e9332f6a64e6407c19c Mon Sep 17 00:00:00 2001 From: Kebo Liu Date: Sun, 26 Jun 2022 15:53:52 +0800 Subject: [PATCH 3/5] add unit test Signed-off-by: Kebo Liu --- .../mlnx-platform-api/tests/test_sfp.py | 58 ++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/platform/mellanox/mlnx-platform-api/tests/test_sfp.py b/platform/mellanox/mlnx-platform-api/tests/test_sfp.py index 0ad9537430b9..2d467f6f75c4 100644 --- a/platform/mellanox/mlnx-platform-api/tests/test_sfp.py +++ b/platform/mellanox/mlnx-platform-api/tests/test_sfp.py @@ -25,9 +25,10 @@ modules_path = os.path.dirname(test_path) sys.path.insert(0, modules_path) -from sonic_platform.sfp import SFP, SX_PORT_MODULE_STATUS_INITIALIZING, SX_PORT_MODULE_STATUS_PLUGGED, SX_PORT_MODULE_STATUS_UNPLUGGED, SX_PORT_MODULE_STATUS_PLUGGED_WITH_ERROR, SX_PORT_MODULE_STATUS_PLUGGED_DISABLED +from sonic_platform.sfp import QSFP_TYPE, QSFP_DD_TYPE, SFP_TYPE, SFP, SX_PORT_MODULE_STATUS_INITIALIZING, SX_PORT_MODULE_STATUS_PLUGGED, SX_PORT_MODULE_STATUS_UNPLUGGED, SX_PORT_MODULE_STATUS_PLUGGED_WITH_ERROR, SX_PORT_MODULE_STATUS_PLUGGED_DISABLED from sonic_platform.chassis import Chassis + class TestSfp: @mock.patch('sonic_platform.device_data.DeviceDataManager.get_linecard_count', mock.MagicMock(return_value=8)) @mock.patch('sonic_platform.device_data.DeviceDataManager.get_linecard_max_port_count') @@ -80,3 +81,58 @@ def test_sfp_get_error_status(self, mock_get_error_code): description = sfp.get_error_description() assert description == expected_description + + def test_detect_sfp_type(self): + sfp = SFP(0) + assert sfp._sfp_type == None + sfp._read_eeprom_specific_bytes = mock.MagicMock(return_value = ['03']) + assert sfp.sfp_type == SFP_TYPE + + sfp._sfp_type = None + sfp._read_eeprom_specific_bytes = mock.MagicMock(return_value = ['0d']) + assert sfp.sfp_type == QSFP_TYPE + + sfp._sfp_type = None + sfp._read_eeprom_specific_bytes = mock.MagicMock(return_value = ['18']) + assert sfp.sfp_type == QSFP_DD_TYPE + + def test_detect_dom_capability(self): + sfp = SFP(0) + sfp.get_presence = mock.MagicMock(return_value = True) + + # QSFP postive flow + sfp._sfp_type = QSFP_TYPE + sfp._read_eeprom_specific_bytes = mock.MagicMock(return_value = ['19', '08', 'ff', '04']) + assert sfp.dom_supported == True + assert sfp._sfp_capability.dom_detect_finished == True + + # QSFP negative flow + sfp._sfp_capability.dom_detect_finished = False + sfp._read_eeprom_specific_bytes = mock.MagicMock(return_value = None) + assert sfp.dom_supported == False + assert sfp._sfp_capability.dom_detect_finished == False + + # SFP postive flow + sfp._sfp_type = SFP_TYPE + sfp._read_eeprom_specific_bytes = mock.MagicMock(return_value = ['ff', '00']) + assert sfp.dom_supported == True + assert sfp._sfp_capability.dom_detect_finished == True + + # SFP negative flow + sfp._sfp_capability.dom_detect_finished = False + sfp._read_eeprom_specific_bytes = mock.MagicMock(return_value = None) + assert sfp.dom_supported == False + assert sfp._sfp_capability.dom_detect_finished == False + + # QSFPDD postive flow + sfp._sfp_type = QSFP_DD_TYPE + sfp._read_eeprom_specific_bytes = mock.MagicMock(return_value = ['00']) + assert sfp.dom_supported == True + assert sfp._sfp_capability.dom_detect_finished == True + + # QSFPDD negative flow + sfp._sfp_capability.dom_detect_finished = False + sfp._read_eeprom_specific_bytes = mock.MagicMock(return_value = None) + assert sfp.dom_supported == False + assert sfp._sfp_capability.dom_detect_finished == False + From 1b792db1882f5162d3c9a54a7d4abae9de156df7 Mon Sep 17 00:00:00 2001 From: Kebo Liu Date: Mon, 27 Jun 2022 11:12:59 +0800 Subject: [PATCH 4/5] fix review comments Signed-off-by: Kebo Liu --- platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py index d9fe602b326c..ff69b80f14d1 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py +++ b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py @@ -565,7 +565,7 @@ def _dom_capability_detect(self): self._sfp_capability.dom_volt_supported = False self._sfp_capability.dom_rx_power_supported = False self._sfp_capability.dom_tx_power_supported = False - self._sfp_capability.dom_detect_finished = True + self._sfp_capability.dom_detect_finished = False @property @utils.pre_initialize(_dom_capability_detect) From 454cc8b42810b97d8f328512a909e74f8ec6c015 Mon Sep 17 00:00:00 2001 From: Kebo Liu Date: Mon, 27 Jun 2022 13:17:13 +0800 Subject: [PATCH 5/5] add log in different scenarios Signed-off-by: Kebo Liu --- platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py index ff69b80f14d1..60b12f794022 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py +++ b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py @@ -483,6 +483,7 @@ def _dom_capability_detect(self): self._sfp_capability.qsfp_page3_available = dom_status_indicator['data']['FlatMem']['value'] == 'Off' self._sfp_capability.dom_detect_finished = True else: + logger.log_warning("SFP {}: Dom capabilty parsing is failed due to eeprom read fail, will re-try next time.".format(self.index)) self._sfp_capability.dom_supported = False self._sfp_capability.dom_temp_supported = False self._sfp_capability.dom_volt_supported = False @@ -531,6 +532,7 @@ def _dom_capability_detect(self): self._sfp_capability.dom_thresholds_supported = False self._sfp_capability.dom_rx_tx_power_bias_supported = False self._sfp_capability.dom_detect_finished = False + logger.log_warning("SFP {}: Dom capabilty parsing is failed due to eeprom read fail, will re-try next time.".format(self.index)) elif self.sfp_type == SFP_TYPE: sfpi_obj = sff8472InterfaceId() @@ -566,6 +568,7 @@ def _dom_capability_detect(self): self._sfp_capability.dom_rx_power_supported = False self._sfp_capability.dom_tx_power_supported = False self._sfp_capability.dom_detect_finished = False + logger.log_warning("SFP {}: Dom capabilty parsing is failed due to sfp type is not one of the supported ones, will re-try next time.".format(self.index)) @property @utils.pre_initialize(_dom_capability_detect) @@ -924,6 +927,7 @@ def get_transceiver_info(self): return None else: # None of any supported SFP type, could be SFP object not correctly initialized. + logger.log_warning("SFP {}: type is not one the supported type, or SFP object initialization is not finished yet.".format(self.index)) return None if self.sfp_type != QSFP_DD_TYPE: