[Mellanox] Support read/write more than 1 page in a single platform A…

…PI call (sonic-net#18881) - Why I did it Support read/write more than 1 page in a single platform API call. - How to verify it Manual and mock test Signed-off-by: Stephen Sun <[email protected]>
DavidZagury · May 7, 2024 · 4f5cb87 · 4f5cb87
1 parent 7822c97
commit 4f5cb87
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 39 deletions.
diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py
@@ -355,25 +355,40 @@ def _read_eeprom(self, offset, num_bytes, log_on_error=True):
         Returns:
             bytearray: the content of EEPROM
         """
-        _, page, page_offset = self._get_page_and_page_offset(offset)
-        if not page:
-            return None
+        result = None
+        while num_bytes > 0:
+            _, page, page_offset = self._get_page_and_page_offset(offset)
+            if not page:
+                return None
 
-        try:
-            with open(page, mode='rb', buffering=0) as f:
-                f.seek(page_offset)
-                content = f.read(num_bytes)
-                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}, \
-                    size={num_bytes}, 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}, \
-                    size={num_bytes}, offset={offset}, error = {e}')
-            return None
+            try:
+                with open(page, mode='rb', buffering=0) as f:
+                    f.seek(page_offset)
+                    content = f.read(num_bytes)
+                    if not result:
+                        result = content
+                    else:
+                        result += content
+                    read_length = len(content)
+                    num_bytes -= read_length
+                    if num_bytes > 0:
+                        page_size = f.seek(0, os.SEEK_END)
+                        if page_offset + read_length == page_size:
+                            offset += read_length
+                        else:
+                            # Indicate read finished
+                            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}')
+            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}')
+                return None
 
-        return bytearray(content)
+        return bytearray(result)
 
     # write eeprom specfic bytes beginning from offset with size as num_bytes
     def write_eeprom(self, offset, num_bytes, write_buffer):
@@ -389,29 +404,37 @@ def write_eeprom(self, offset, num_bytes, write_buffer):
             logger.log_error("Error mismatch between buffer length and number of bytes to be written")
             return False
 
-        page_num, page, page_offset = self._get_page_and_page_offset(offset)
-        if not page:
-            return False
+        while num_bytes > 0:
+            page_num, page, page_offset = self._get_page_and_page_offset(offset)
+            if not page:
+                return False
 
-        try:
-            if self._is_write_protected(page_num, page_offset, num_bytes):
-                # write limited eeprom is not supported
-                raise IOError('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])
-                if ret != num_bytes:
-                    raise IOError(f'write return code = {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}, \
-                    size={num_bytes}, data={write_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}, \
-                offset={offset}, data = {data}, error = {e}')
-            return False
+            try:
+                if self._is_write_protected(page_num, page_offset, num_bytes):
+                    # write limited eeprom is not supported
+                    raise IOError('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])
+                    written_buffer = write_buffer[0:ret]
+                    if ret != num_bytes:
+                        page_size = f.seek(0, os.SEEK_END)
+                        if page_offset + ret == page_size:
+                            # Move to next page
+                            write_buffer = write_buffer[ret:num_bytes]
+                            offset += ret
+                        else:
+                            raise IOError(f'write return code = {ret}')
+                    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}')
+            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}')
+                return False
         return True
 
     @classmethod

diff --git a/platform/mellanox/mlnx-platform-api/tests/test_sfp.py b/platform/mellanox/mlnx-platform-api/tests/test_sfp.py
@@ -131,6 +131,17 @@ def test_sfp_write_eeprom(self, mock_limited_eeprom, mock_get_page):
             handle.write.side_effect = OSError('')
             assert not sfp.write_eeprom(0, 1, bytearray([1]))
 
+        mo = mock.mock_open()
+        print('after mock open')
+        with mock.patch('sonic_platform.sfp.open', mo):
+            handle = mo()
+            handle.write.side_effect = [128, 128, 64]
+            handle.seek.side_effect = [0, 128, 0, 128, 0]
+            bytes_to_write = bytearray([0]*128 + [1]*128 + [2]*64)
+            assert sfp.write_eeprom(0, 320, bytes_to_write)
+            expected_calls = [mock.call(bytes_to_write), mock.call(bytes_to_write[128:]), mock.call(bytes_to_write[256:])]
+            handle.write.assert_has_calls(expected_calls)
+
     @mock.patch('sonic_platform.sfp.SFP._get_page_and_page_offset')
     def test_sfp_read_eeprom(self, mock_get_page):
         sfp = SFP(0)
@@ -152,6 +163,13 @@ def test_sfp_read_eeprom(self, mock_get_page):
             handle.read.side_effect = OSError('')
             assert sfp.read_eeprom(0, 1) is None
 
+        mo = mock.mock_open()
+        with mock.patch('sonic_platform.sfp.open', mo):
+            handle = mo()
+            handle.read.side_effect = [b'\x00'*128, b'\x01'*128, b'\x02'*64]
+            handle.seek.side_effect = [0, 128, 0, 128, 0]
+            assert sfp.read_eeprom(0, 320) == bytearray([0]*128 + [1]*128 + [2]*64)
+
     @mock.patch('sonic_platform.sfp.SFP._fetch_port_status')
     def test_is_port_admin_status_up(self, mock_port_status):
         mock_port_status.return_value = (0, True)