From ee44641feab303a34f5e4b63d2e358b1dffba6ff Mon Sep 17 00:00:00 2001 From: tshalvi Date: Tue, 26 Aug 2025 18:23:39 +0300 Subject: [PATCH 1/6] Add retry logic to read_eeprom() and write_eeprom() Signed-off-by: tshalvi --- .../mlnx-platform-api/sonic_platform/sfp.py | 102 +++++++++++++----- 1 file changed, 75 insertions(+), 27 deletions(-) diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py index ab2506ca483..7d9c1a167d3 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py +++ b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py @@ -224,6 +224,9 @@ CMIS_MCI_EEPROM_OFFSET = 2 CMIS_MCI_MASK = 0b00001100 +MAX_ATTEMPTS = 50 +RETRY_SLEEP_SEC = 0.1 + STATE_DOWN = 'Down' # Initial state STATE_INIT = 'Initializing' # Module starts initializing, check module present, also power on the module if need STATE_RESETTING = 'Resetting' # Module is resetting the firmware @@ -476,23 +479,15 @@ def get_presence(self): Returns: bool: True if device is present, False if not """ - - try: - presence_file = 'hw_present' if self.is_sw_control() else 'present' - if utils.read_int_from_file(f'/sys/module/sx_core/asic0/module{self.sdk_index}/{presence_file}', log_func=None) != 1: - return False - eeprom_raw = self._read_eeprom(0, 1, log_on_error=False) - return eeprom_raw is not None - except Exception as e: - logger.log_warning(f'Failed to check presence of SFP {self.sdk_index}: {e}') - return False - + presence_sysfs = f'/sys/module/sx_core/asic0/module{self.sdk_index}/hw_present' if self.is_sw_control() else f'/sys/module/sx_core/asic0/module{self.sdk_index}/present' + return utils.read_int_from_file(presence_sysfs) == 1 + @classmethod def wait_sfp_eeprom_ready(cls, sfp_list, wait_time): not_ready_list = sfp_list while wait_time > 0: - not_ready_list = [s for s in not_ready_list if s.state == STATE_FW_CONTROL and s._read_eeprom(0, 2,False) is None] + not_ready_list = [s for s in not_ready_list if s.state == STATE_FW_CONTROL and s._read_eeprom(0, 1,False) is None] if not_ready_list: time.sleep(0.1) wait_time -= 0.1 @@ -515,17 +510,36 @@ def check_eeprom_ready_if_present(self): return self._read_eeprom(0, 1, log_on_error=False) is not None # read eeprom specfic bytes beginning from offset with size as num_bytes - def read_eeprom(self, offset, num_bytes): + def read_eeprom(self, offset, num_bytes, log_on_error=True): """ - Read eeprom specfic bytes beginning from a random offset with size as num_bytes + Read eeprom specfic bytes beginning from a random offset with size as num_bytes. Tries up to 50 times total on every 0.1s. Returns: bytearray, if raw sequence of bytes are read correctly from the offset of size num_bytes None, if the read_eeprom fails """ - return self._read_eeprom(offset, num_bytes) + for attempt in range(MAX_ATTEMPTS): + result = self._read_eeprom(offset, num_bytes, log_on_error) + if result is not None: + logger.log_notice( + f"EEPROM read success after attempt {attempt + 1}/{MAX_ATTEMPTS} " + f"(sfp={self.sdk_index}, offset={offset}, size={num_bytes})") + return result + if attempt < MAX_ATTEMPTS - 1: # only sleep if another retry will happen + logger.log_notice( + f"EEPROM read attempt {attempt + 1}/{MAX_ATTEMPTS} failed " + f"(sfp={self.sdk_index}, offset={offset}, size={num_bytes}). " + f"Retrying in {RETRY_SLEEP_SEC:.1f}s...") + time.sleep(RETRY_SLEEP_SEC) + + if log_on_error: + logger.log_error( + f"EEPROM read failed after {MAX_ATTEMPTS} attempts " + f"(sfp={self.sdk_index}, offset={offset}, size={num_bytes})") + return None def _read_eeprom(self, offset, num_bytes, log_on_error=True): - """Read eeprom specfic bytes beginning from a random offset with size as num_bytes + """ + Single-attempt read: Read eeprom specfic bytes beginning from a random offset with size as num_bytes Args: offset (int): read offset @@ -563,12 +577,18 @@ def _read_eeprom(self, offset, num_bytes, log_on_error=True): num_bytes = 0 if ctypes.get_errno() != 0: raise IOError(f'errno = {os.strerror(ctypes.get_errno())}') - logger.log_debug(f'read EEPROM sfp={self.sdk_index}, page={page}, page_offset={page_offset}, '\ - f'size={read_length}, data={content}') + + logger.log_debug( + f"read EEPROM sfp={self.sdk_index}, page={page}, page_offset={page_offset}, " + f"size={read_length}, data={content}" + ) + except (OSError, IOError) as e: if log_on_error: - logger.log_warning(f'Failed to read sfp={self.sdk_index} EEPROM page={page}, page_offset={page_offset}, '\ - f'size={num_bytes}, offset={offset}, error = {e}') + logger.log_warning( + f"Failed to read sfp={self.sdk_index} EEPROM page={page}, page_offset={page_offset}, " + f"size={num_bytes}, offset={offset}, error = {e}" + ) return None return bytearray(result) @@ -577,16 +597,43 @@ def _read_eeprom(self, offset, num_bytes, log_on_error=True): def write_eeprom(self, offset, num_bytes, write_buffer): """ write eeprom specfic bytes beginning from a random offset with size as num_bytes - and write_buffer as the required bytes + and write_buffer as the required bytes. Tries up to 50 times total on every 0.1s. Returns: Boolean, true if the write succeeded and false if it did not succeed. - Example: - mlxreg -d /dev/mst/mt52100_pciconf0 --reg_name MCIA --indexes slot_index=0,module=1,device_address=154,page_number=5,i2c_device_address=0x50,size=1,bank_number=0 --set dword[0]=0x01000000 -y """ if num_bytes != len(write_buffer): logger.log_error("Error mismatch between buffer length and number of bytes to be written") return False + for attempt in range(MAX_ATTEMPTS): + ret = self._write_eeprom(offset, num_bytes, write_buffer) + if ret: + logger.log_notice( + f"EEPROM write success after attempt {attempt + 1}/{MAX_ATTEMPTS} " + f"for sfp={self.sdk_index}, offset={offset}, size={num_bytes}") + return True + logger.log_notice( + f"EEPROM write attempt {attempt + 1}/{MAX_ATTEMPTS} failed " + f"for sfp={self.sdk_index}, offset={offset}, size={num_bytes}. " + f"Retrying in {RETRY_SLEEP_SEC:.1f}s..." + ) + time.sleep(RETRY_SLEEP_SEC) + + logger.log_error( + f"EEPROM write failed after {MAX_ATTEMPTS} attempts " + f"for sfp={getattr(self, 'sdk_index', 'N/A')}, offset={offset}, size={num_bytes}" + ) + return False + + def _write_eeprom(self, offset, num_bytes, write_buffer): + """ + Single-attempt write: write eeprom specfic bytes beginning from a random offset with size as num_bytes + and write_buffer as the required bytes + Returns: + Boolean, true if the write succeeded and false if it did not succeed. + Example: + mlxreg -d /dev/mst/mt52100_pciconf0 --reg_name MCIA --indexes slot_index=0,module=1,device_address=154,page_number=5,i2c_device_address=0x50,size=1,bank_number=0 --set dword[0]=0x01000000 -y + """ while num_bytes > 0: page_num, page, page_offset = self._get_page_and_page_offset(offset) if not page: @@ -611,12 +658,13 @@ def write_eeprom(self, offset, num_bytes, write_buffer): num_bytes -= ret if ctypes.get_errno() != 0: raise IOError(f'errno = {os.strerror(ctypes.get_errno())}') - logger.log_debug(f'write EEPROM sfp={self.sdk_index}, page={page}, page_offset={page_offset}, '\ - f'size={ret}, left={num_bytes}, data={written_buffer}') + logger.log_debug('write EEPROM sfp={}, page={}, page_offset={}, size={}, left={}, data={}'.format(self.sdk_index, page, page_offset, ret, num_bytes, written_buffer)) except (OSError, IOError) as e: data = ''.join('{:02x}'.format(x) for x in write_buffer) - logger.log_error(f'Failed to write EEPROM data sfp={self.sdk_index} EEPROM page={page}, page_offset={page_offset}, size={num_bytes}, '\ - f'offset={offset}, data = {data}, error = {e}') + logger.log_error( + f"Failed to write EEPROM: sfp={self.sdk_index}, page={page}, page_offset={page_offset}, " + f"size={num_bytes}, offset={offset}, data={data}, error={e}" + ) return False return True From 37bfd6b3905c432f54f5ffbea4a2cffa3c206239 Mon Sep 17 00:00:00 2001 From: tshalvi Date: Mon, 1 Sep 2025 22:11:47 +0300 Subject: [PATCH 2/6] Correct typo in comments Signed-off-by: tshalvi --- .../mlnx-platform-api/sonic_platform/sfp.py | 24 ++++--------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py index 7d9c1a167d3..f9c2ae75f49 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py +++ b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py @@ -497,22 +497,9 @@ def wait_sfp_eeprom_ready(cls, sfp_list, wait_time): for s in not_ready_list: logger.log_error(f'SFP {s.sdk_index} eeprom is not ready') - def check_eeprom_ready_if_present(self): - """ - Check if the eeprom is ready for a present SFP - - Returns: - bool: False if the SFP is present and the eeprom is not ready, True otherwise - """ - presence_file = 'hw_present' if self.is_sw_control() else 'present' - if utils.read_int_from_file(f'/sys/module/sx_core/asic0/module{self.sdk_index}/{presence_file}', log_func=None) != 1: - return True - return self._read_eeprom(0, 1, log_on_error=False) is not None - - # read eeprom specfic bytes beginning from offset with size as num_bytes def read_eeprom(self, offset, num_bytes, log_on_error=True): """ - Read eeprom specfic bytes beginning from a random offset with size as num_bytes. Tries up to 50 times total on every 0.1s. + Read eeprom specific bytes beginning from a random offset with size as num_bytes. Tries up to 50 times total on every 0.1s. Returns: bytearray, if raw sequence of bytes are read correctly from the offset of size num_bytes None, if the read_eeprom fails @@ -539,7 +526,7 @@ def read_eeprom(self, offset, num_bytes, log_on_error=True): def _read_eeprom(self, offset, num_bytes, log_on_error=True): """ - Single-attempt read: Read eeprom specfic bytes beginning from a random offset with size as num_bytes + Single-attempt read: Read eeprom specific bytes beginning from a random offset with size as num_bytes Args: offset (int): read offset @@ -593,10 +580,9 @@ def _read_eeprom(self, offset, num_bytes, log_on_error=True): return bytearray(result) - # write eeprom specfic bytes beginning from offset with size as num_bytes def write_eeprom(self, offset, num_bytes, write_buffer): """ - write eeprom specfic bytes beginning from a random offset with size as num_bytes + write eeprom specific bytes beginning from a random offset with size as num_bytes and write_buffer as the required bytes. Tries up to 50 times total on every 0.1s. Returns: Boolean, true if the write succeeded and false if it did not succeed. @@ -621,13 +607,13 @@ def write_eeprom(self, offset, num_bytes, write_buffer): logger.log_error( f"EEPROM write failed after {MAX_ATTEMPTS} attempts " - f"for sfp={getattr(self, 'sdk_index', 'N/A')}, offset={offset}, size={num_bytes}" + f"for sfp={self.sdk_index}, offset={offset}, size={num_bytes}" ) return False def _write_eeprom(self, offset, num_bytes, write_buffer): """ - Single-attempt write: write eeprom specfic bytes beginning from a random offset with size as num_bytes + Single-attempt write: write eeprom specific bytes beginning from a random offset with size as num_bytes and write_buffer as the required bytes Returns: Boolean, true if the write succeeded and false if it did not succeed. From 27b75fa7f1345ae1ebdebe54a3a540c42a53c41c Mon Sep 17 00:00:00 2001 From: tshalvi Date: Wed, 17 Sep 2025 15:58:01 +0300 Subject: [PATCH 3/6] Update EEPROM access retry mechanism to retry only on I2C errors reported by the kernel Signed-off-by: tshalvi --- .../mlnx-platform-api/sonic_platform/sfp.py | 151 +++++++++++++----- .../mlnx-platform-api/tests/test_sfp.py | 2 +- 2 files changed, 110 insertions(+), 43 deletions(-) diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py index f9c2ae75f49..a98528a586d 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py +++ b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py @@ -29,6 +29,7 @@ import os import threading import time + import errno from sonic_py_common.logger import Logger from sonic_py_common import multi_asic from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector @@ -226,6 +227,7 @@ MAX_ATTEMPTS = 50 RETRY_SLEEP_SEC = 0.1 +EEPROM_RETRY_ERR_THRESHOLD = 10 STATE_DOWN = 'Down' # Initial state STATE_INIT = 'Initializing' # Module starts initializing, check module present, also power on the module if need @@ -499,24 +501,44 @@ def wait_sfp_eeprom_ready(cls, sfp_list, wait_time): def read_eeprom(self, offset, num_bytes, log_on_error=True): """ - Read eeprom specific bytes beginning from a random offset with size as num_bytes. Tries up to 50 times total on every 0.1s. + Read eeprom specific bytes beginning from a random offset with size as num_bytes. + Retries up to 50 times in total (every 0.1s), but only if previous attempts failed due to I2C errors + (errno.EIO, typically reported as -5 from the kernel). + Returns: - bytearray, if raw sequence of bytes are read correctly from the offset of size num_bytes - None, if the read_eeprom fails + bytearray: If the data was successfully read. + None: If all attempts failed (whether due to I2C errors after max retries, or due to other errors without retry). """ for attempt in range(MAX_ATTEMPTS): - result = self._read_eeprom(offset, num_bytes, log_on_error) + result, err = self._read_eeprom(offset, num_bytes, log_on_error) if result is not None: - logger.log_notice( + logger.log_debug( f"EEPROM read success after attempt {attempt + 1}/{MAX_ATTEMPTS} " f"(sfp={self.sdk_index}, offset={offset}, size={num_bytes})") return result - if attempt < MAX_ATTEMPTS - 1: # only sleep if another retry will happen - logger.log_notice( - f"EEPROM read attempt {attempt + 1}/{MAX_ATTEMPTS} failed " + + log_func = (logger.log_error if attempt + 1 > EEPROM_RETRY_ERR_THRESHOLD else logger.log_debug) + + # Retry only on EIO (-5) + if err == errno.EIO and attempt < MAX_ATTEMPTS - 1: + log_func( + f"EEPROM read attempt {attempt + 1}/{MAX_ATTEMPTS} failed with I2C error " f"(sfp={self.sdk_index}, offset={offset}, size={num_bytes}). " f"Retrying in {RETRY_SLEEP_SEC:.1f}s...") time.sleep(RETRY_SLEEP_SEC) + continue + + # Non-I2C-error or last attempt → stop retrying + if err is not None: + log_func( + f"EEPROM read failed (errno={err}, {os.strerror(err)}) " + f"after attempt {attempt + 1}/{MAX_ATTEMPTS} " + f"(sfp={self.sdk_index}, offset={offset}, size={num_bytes})") + else: + log_func( + f"EEPROM read failed after attempt {attempt + 1}/{MAX_ATTEMPTS} " + f"(sfp={self.sdk_index}, offset={offset}, size={num_bytes})") + return None if log_on_error: logger.log_error( @@ -534,26 +556,29 @@ def _read_eeprom(self, offset, num_bytes, log_on_error=True): log_on_error (bool, optional): whether log error when exception occurs. Defaults to True. Returns: - bytearray: the content of EEPROM + (bytearray, None): On success, returns the data read and None for errno. + (None, errno): On failure due to a specific OS error, returns None and the errno value. + (None, None): On failure without an associated errno (e.g., empty page or invalid page). """ result = bytearray(0) while num_bytes > 0: _, page, page_offset = self._get_page_and_page_offset(offset) if not page: - return None + return None, None try: with open(page, mode='rb', buffering=0) as f: f.seek(page_offset) content = f.read(num_bytes) + read_length = len(content) + if read_length == 0: + logger.log_error(f'SFP {self.sdk_index}: EEPROM page {page} is empty, no data retrieved') + return None, None + if not result: result = content else: result += content - read_length = len(content) - if read_length == 0: - logger.log_error(f'SFP {self.sdk_index}: EEPROM page {page} is empty, no data retrieved') - return None num_bytes -= read_length if num_bytes > 0: page_size = f.seek(0, os.SEEK_END) @@ -562,48 +587,76 @@ def _read_eeprom(self, offset, num_bytes, log_on_error=True): else: # Indicate read finished num_bytes = 0 + if ctypes.get_errno() != 0: - raise IOError(f'errno = {os.strerror(ctypes.get_errno())}') + return None, ctypes.get_errno() logger.log_debug( f"read EEPROM sfp={self.sdk_index}, page={page}, page_offset={page_offset}, " f"size={read_length}, data={content}" ) - except (OSError, IOError) as e: + except OSError as e: if log_on_error: - logger.log_warning( - f"Failed to read sfp={self.sdk_index} EEPROM page={page}, page_offset={page_offset}, " - f"size={num_bytes}, offset={offset}, error = {e}" - ) - return None + if e.errno is not None: + logger.log_warning( + f"Failed to read sfp={self.sdk_index} EEPROM page={page}, page_offset={page_offset}, " + f"size={num_bytes}, offset={offset}, error={e} " + f"(errno={e.errno}, {os.strerror(e.errno)})" + ) + else: + logger.log_warning( + f"Failed to read sfp={self.sdk_index} EEPROM page={page}, page_offset={page_offset}, " + f"size={num_bytes}, offset={offset}, error={e}" + ) + return None, e.errno + + return bytearray(result), None - return bytearray(result) def write_eeprom(self, offset, num_bytes, write_buffer): """ - write eeprom specific bytes beginning from a random offset with size as num_bytes - and write_buffer as the required bytes. Tries up to 50 times total on every 0.1s. + Write EEPROM specific bytes beginning from a random offset with size as num_bytes + and write_buffer as the required bytes. Retries up to 50 times (every 0.1s) only if + previous attempts failed due to I2C errors (errno.EIO). + Returns: - Boolean, true if the write succeeded and false if it did not succeed. + Boolean, True if the write succeeded, False if it did not succeed. """ if num_bytes != len(write_buffer): logger.log_error("Error mismatch between buffer length and number of bytes to be written") return False for attempt in range(MAX_ATTEMPTS): - ret = self._write_eeprom(offset, num_bytes, write_buffer) + ret, err = self._write_eeprom(offset, num_bytes, write_buffer) if ret: - logger.log_notice( + logger.log_debug( f"EEPROM write success after attempt {attempt + 1}/{MAX_ATTEMPTS} " f"for sfp={self.sdk_index}, offset={offset}, size={num_bytes}") return True - logger.log_notice( - f"EEPROM write attempt {attempt + 1}/{MAX_ATTEMPTS} failed " - f"for sfp={self.sdk_index}, offset={offset}, size={num_bytes}. " - f"Retrying in {RETRY_SLEEP_SEC:.1f}s..." - ) - time.sleep(RETRY_SLEEP_SEC) + + log_func = (logger.log_error if attempt + 1 > EEPROM_RETRY_ERR_THRESHOLD else logger.log_debug) + + # Retry only on EIO (-5) + if err == errno.EIO and attempt < MAX_ATTEMPTS - 1: + log_func( + f"EEPROM write attempt {attempt + 1}/{MAX_ATTEMPTS} failed with I2C error " + f"for sfp={self.sdk_index}, offset={offset}, size={num_bytes}. " + f"Retrying in {RETRY_SLEEP_SEC:.1f}s...") + time.sleep(RETRY_SLEEP_SEC) + continue + + # Non-I2C-error or last attempt → stop retrying + if err is not None: + log_func( + f"EEPROM write failed (errno={err}, {os.strerror(err)}) " + f"for sfp={self.sdk_index}, offset={offset}, size={num_bytes} " + f"after attempt {attempt + 1}/{MAX_ATTEMPTS}") + else: + log_func( + f"EEPROM write failed for sfp={self.sdk_index}, offset={offset}, size={num_bytes} " + f"after attempt {attempt + 1}/{MAX_ATTEMPTS}") + return False logger.log_error( f"EEPROM write failed after {MAX_ATTEMPTS} attempts " @@ -616,19 +669,23 @@ def _write_eeprom(self, offset, num_bytes, write_buffer): Single-attempt write: write eeprom specific bytes beginning from a random offset with size as num_bytes and write_buffer as the required bytes Returns: - Boolean, true if the write succeeded and false if it did not succeed. + (True, None): On success. + (False, errno): On failure with a specific OS error. + (False, None): On failure without an associated errno (e.g., unexpected short write). + Example: mlxreg -d /dev/mst/mt52100_pciconf0 --reg_name MCIA --indexes slot_index=0,module=1,device_address=154,page_number=5,i2c_device_address=0x50,size=1,bank_number=0 --set dword[0]=0x01000000 -y """ while num_bytes > 0: page_num, page, page_offset = self._get_page_and_page_offset(offset) if not page: - return False + return False, None try: if self._is_write_protected(page_num, page_offset, num_bytes): # write limited eeprom is not supported - raise IOError('write limited bytes') + raise OSError(errno.EPERM, 'write limited bytes') + with open(page, mode='r+b', buffering=0) as f: f.seek(page_offset) ret = f.write(write_buffer[0:num_bytes]) @@ -640,19 +697,29 @@ def _write_eeprom(self, offset, num_bytes, write_buffer): write_buffer = write_buffer[ret:num_bytes] offset += ret else: - raise IOError(f'write return code = {ret}') + logger.log_error( + f"Unexpected short write (ret={ret} < {num_bytes}) " + f"for sfp={self.sdk_index}, page={page}, page_offset={page_offset}, offset={offset}" + ) + return False, None num_bytes -= ret if ctypes.get_errno() != 0: - raise IOError(f'errno = {os.strerror(ctypes.get_errno())}') - logger.log_debug('write EEPROM sfp={}, page={}, page_offset={}, size={}, left={}, data={}'.format(self.sdk_index, page, page_offset, ret, num_bytes, written_buffer)) - except (OSError, IOError) as e: + return False, ctypes.get_errno() + + logger.log_debug( + 'write EEPROM sfp={}, page={}, page_offset={}, size={}, left={}, data={}' + .format(self.sdk_index, page, page_offset, ret, num_bytes, written_buffer) + ) + + except OSError as e: data = ''.join('{:02x}'.format(x) for x in write_buffer) logger.log_error( f"Failed to write EEPROM: sfp={self.sdk_index}, page={page}, page_offset={page_offset}, " f"size={num_bytes}, offset={offset}, data={data}, error={e}" ) - return False - return True + return False, e.errno + + return True, None def get_lpmode(self): """ diff --git a/platform/mellanox/mlnx-platform-api/tests/test_sfp.py b/platform/mellanox/mlnx-platform-api/tests/test_sfp.py index 59f37aaffdc..28d8c259b98 100644 --- a/platform/mellanox/mlnx-platform-api/tests/test_sfp.py +++ b/platform/mellanox/mlnx-platform-api/tests/test_sfp.py @@ -245,7 +245,7 @@ def test_get_page_and_page_offset(self, mock_get_type_str, mock_eeprom_path, moc assert page_offset is 0 @mock.patch('sonic_platform.utils.read_int_from_file') - @mock.patch('sonic_platform.sfp.SFP._read_eeprom') + @mock.patch('sonic_platform.sfp.SFP.read_eeprom') def test_sfp_get_presence(self, mock_read, mock_read_int): sfp = SFP(0) From a0ce32ef91ecbe16a81b59ebd8b9cf3e8d927c73 Mon Sep 17 00:00:00 2001 From: tshalvi Date: Sun, 21 Sep 2025 18:32:58 +0300 Subject: [PATCH 4/6] Update success log to NOTICE level starting from the second attempt Signed-off-by: tshalvi --- .../mlnx-platform-api/sonic_platform/sfp.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py index a98528a586d..cd6906cf65a 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py +++ b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py @@ -512,9 +512,10 @@ def read_eeprom(self, offset, num_bytes, log_on_error=True): for attempt in range(MAX_ATTEMPTS): result, err = self._read_eeprom(offset, num_bytes, log_on_error) if result is not None: - logger.log_debug( - f"EEPROM read success after attempt {attempt + 1}/{MAX_ATTEMPTS} " - f"(sfp={self.sdk_index}, offset={offset}, size={num_bytes})") + if attempt > 0: + logger.log_notice( + f"EEPROM read success after attempt {attempt + 1}/{MAX_ATTEMPTS} " + f"(sfp={self.sdk_index}, offset={offset}, size={num_bytes})") return result log_func = (logger.log_error if attempt + 1 > EEPROM_RETRY_ERR_THRESHOLD else logger.log_debug) @@ -630,9 +631,10 @@ def write_eeprom(self, offset, num_bytes, write_buffer): for attempt in range(MAX_ATTEMPTS): ret, err = self._write_eeprom(offset, num_bytes, write_buffer) if ret: - logger.log_debug( - f"EEPROM write success after attempt {attempt + 1}/{MAX_ATTEMPTS} " - f"for sfp={self.sdk_index}, offset={offset}, size={num_bytes}") + if attempt > 0: + logger.log_notice( + f"EEPROM write success after attempt {attempt + 1}/{MAX_ATTEMPTS} " + f"for sfp={self.sdk_index}, offset={offset}, size={num_bytes}") return True log_func = (logger.log_error if attempt + 1 > EEPROM_RETRY_ERR_THRESHOLD else logger.log_debug) From ccd3b349913615234d4aeecd9943fbe6a4c648cf Mon Sep 17 00:00:00 2001 From: tshalvi Date: Mon, 29 Sep 2025 19:04:45 +0300 Subject: [PATCH 5/6] Update max number of retry attempt Signed-off-by: tshalvi --- platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py index cd6906cf65a..52f2545cb59 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py +++ b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py @@ -225,9 +225,9 @@ CMIS_MCI_EEPROM_OFFSET = 2 CMIS_MCI_MASK = 0b00001100 -MAX_ATTEMPTS = 50 +MAX_ATTEMPTS = 5 RETRY_SLEEP_SEC = 0.1 -EEPROM_RETRY_ERR_THRESHOLD = 10 +EEPROM_RETRY_ERR_THRESHOLD = 2 STATE_DOWN = 'Down' # Initial state STATE_INIT = 'Initializing' # Module starts initializing, check module present, also power on the module if need From 0f39f617ca017f1b910d441e8a6ed7718f244685 Mon Sep 17 00:00:00 2001 From: tshalvi Date: Mon, 25 May 2026 20:51:05 +0300 Subject: [PATCH 6/6] Align test_sfp_get_presence with updated get_presence() implementation Signed-off-by: tshalvi --- platform/mellanox/mlnx-platform-api/tests/test_sfp.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/platform/mellanox/mlnx-platform-api/tests/test_sfp.py b/platform/mellanox/mlnx-platform-api/tests/test_sfp.py index 28d8c259b98..0a0c4b5a324 100644 --- a/platform/mellanox/mlnx-platform-api/tests/test_sfp.py +++ b/platform/mellanox/mlnx-platform-api/tests/test_sfp.py @@ -245,20 +245,13 @@ def test_get_page_and_page_offset(self, mock_get_type_str, mock_eeprom_path, moc assert page_offset is 0 @mock.patch('sonic_platform.utils.read_int_from_file') - @mock.patch('sonic_platform.sfp.SFP.read_eeprom') - def test_sfp_get_presence(self, mock_read, mock_read_int): + def test_sfp_get_presence(self, mock_read_int): sfp = SFP(0) mock_read_int.return_value = 1 - mock_read.return_value = None - assert not sfp.get_presence() - mock_read.return_value = 0 assert sfp.get_presence() mock_read_int.return_value = 0 - mock_read.return_value = None - assert not sfp.get_presence() - mock_read.return_value = 0 assert not sfp.get_presence() @mock.patch('sonic_platform.utils.read_int_from_file')