[Mellanox] Optimize SFP modules initialization

platform/mellanox/mlnx-platform-api/sonic_platform/chassis.py

@@ -66,10 +66,38 @@ def __init__(self):
 
         # Initialize Platform name
         self.platform_name = device_info.get_platform()
-        
+
         # move the initialization of each components to their dedicated initializer
         # which will be called from platform
-        self.sfp_module_initialized = False
+        #
+        # Multiple scenarios need to be taken into consideration regarding the SFP modules initialization.
+        # - Platform daemons
+        #   - Can access multiple or all SFP modules
+        # - sfputil
+        #   - Sometimes can access only one SFP module
+        #   - Call get_sfp to get one SFP module object
+        #
+        # We should initialize all SFP modules only if it is necessary because initializing SFP module is time-consuming.
+        # This means,
+        # - If get_sfp is called,
+        #    - If the _sfp_list isn't initialized, initialize it first.
+        #    - Only the SFP module being required should be initialized.
+        # - If get_all_sfps is called,
+        #    - If the _sfp_list isn't initialized, initialize it first.
+        #    - All SFP modules need to be initialized.
+        #      But the SFP modules that have already been initialized should not be initialized for the second time.
+        #      This can caused by get_sfp being called before.
+        #
+        # Due to the complexity of SFP modules initialization, we have to introduce two initialized flags for SFP modules
+        # - sfp_module_partial_initialized:
+        #    - False: The _sfp_list is [] (SFP stuff has never been accessed)
+        #    - True: The _sfp_list is a list whose length is number of SFP modules supported by the platform
+        # - sfp_module_full_initialized:
+        #    - False: All SFP modules have not been created
+        #    - True: All SFP modules have been created
+        #
+        self.sfp_module_partial_initialized = False
+        self.sfp_module_full_initialized = False
         self.sfp_event_initialized = False
         self.reboot_cause_initialized = False
         self.sdk_handle = None
@@ -115,7 +143,17 @@ def initialize_fan(self):
                 self._fan_list.append(fan)
 
 
-    def initialize_sfp(self):
+    def initialize_single_sfp(self, index):
+        if not self._sfp_list[index]:
+            if index >= self.QSFP_PORT_START and index < self.PORTS_IN_BLOCK:
+                sfp_module = self.sfp_module(index, 'QSFP', self.get_sdk_handle, self.platform_name)
+            else:
+                sfp_module = self.sfp_module(index, 'SFP', self.get_sdk_handle, self.platform_name)
+
+            self._sfp_list[index] = sfp_module
+
+
+    def initialize_sfp(self, index=None):
         from sonic_platform.sfp import SFP
 
         self.sfp_module = SFP
@@ -127,15 +165,22 @@ def initialize_sfp(self):
         self.PORT_END = port_position_tuple[2]
         self.PORTS_IN_BLOCK = port_position_tuple[3]
 
-        for index in range(self.PORT_START, self.PORT_END + 1):
-            if index in range(self.QSFP_PORT_START, self.PORTS_IN_BLOCK + 1):
-                sfp_module = SFP(index, 'QSFP', self.get_sdk_handle, self.platform_name)
-            else:
-                sfp_module = SFP(index, 'SFP', self.get_sdk_handle, self.platform_name)
-
-            self._sfp_list.append(sfp_module)
+        if index is not None:
+            if not self.sfp_module_partial_initialized:
+                if index >= self.PORT_START and index < self.PORT_END:
+                    self._sfp_list = list([None]*(self.PORT_END + 1))
+                else:
+                    raise IndexError("{} is not a valid index of SPF modules. Valid index range:[{}, {}]".format(
+                        index, self.PORT_START + 1, self.PORT_END + 1))
+                self.sfp_module_partial_initialized = True
+        else:
+            if not self.sfp_module_partial_initialized:
+                self._sfp_list = list([None]*(self.PORT_END + 1))
+                self.sfp_module_partial_initialized = True
+            for index in range(self.PORT_START, self.PORT_END + 1):
+                self.initialize_single_sfp(index)
 
-        self.sfp_module_initialized = True
+            self.sfp_module_full_initialized = True
 
 
     def get_sdk_handle(self):
@@ -206,7 +251,7 @@ def get_num_sfps(self):
         Returns:
             An integer, the number of sfps available on this chassis
         """
-        if not self.sfp_module_initialized:
+        if not self.sfp_module_full_initialized:
             self.initialize_sfp()
         return len(self._sfp_list)
 
@@ -219,7 +264,7 @@ def get_all_sfps(self):
             A list of objects derived from SfpBase representing all sfps 
             available on this chassis
         """
-        if not self.sfp_module_initialized:
+        if not self.sfp_module_full_initialized:
             self.initialize_sfp()
         return self._sfp_list
 
@@ -237,13 +282,17 @@ def get_sfp(self, index):
         Returns:
             An object dervied from SfpBase representing the specified sfp
         """
-        if not self.sfp_module_initialized:
-            self.initialize_sfp()
-
         sfp = None
         index -= 1
+
         try:
+            if not self.sfp_module_partial_initialized:
+                self.initialize_sfp(index)
+
             sfp = self._sfp_list[index]
+            if not sfp:
+                self.initialize_single_sfp(index)
+                sfp = self._sfp_list[index]
         except IndexError:
             sys.stderr.write("SFP index {} out of range (0-{})\n".format(
                              index, len(self._sfp_list)-1))
@@ -488,7 +537,7 @@ def reinit_sfps(self, port_dict):
         :return:
         """
         # SFP not initialize yet, do nothing
-        if not self.sfp_module_initialized:
+        if not self.sfp_module_full_initialized:
             return
 
         from . import sfp

platform/mellanox/mlnx-platform-api/tests/test_sfp.py

@@ -0,0 +1,84 @@
+import os
+import sys
+import pytest
+from mock import MagicMock
+from .mock_platform import MockFan
+
+test_path = os.path.dirname(os.path.abspath(__file__))
+modules_path = os.path.dirname(test_path)
+sys.path.insert(0, modules_path)
+
+from sonic_py_common import device_info
+from sonic_platform.sfp import SFP
+from sonic_platform.chassis import Chassis
+
+
+def mock_get_platform():
+    return 'x86_64-mlnx_msn2410-r0'
+
+
+def mock_read_eeprom_specific_bytes(self, offset, num_bytes):
+    return None
+
+
+def mock_get_sdk_handle(self):
+    if not self.sdk_handle:
+        self.sdk_handle = 1
+    return self.sdk_handle
+
+device_info.get_platform = mock_get_platform
+SFP._read_eeprom_specific_bytes = mock_read_eeprom_specific_bytes
+Chassis.get_sdk_handle = mock_get_sdk_handle
+
+
+def test_sfp_partial_and_then_full_initialize():
+    """
+        Verify SFP initialization flow (partial and then full):
+        1. get_sfp to tirgger a partial initialization
+        2. get_sfp for another SPF module and verify the partial initialization isn't executed again
+        3. get_all_sfps to trigger a full initialization
+    """
+    chassis = Chassis()
+
+    # Fetch a sfp
+    # This should trigger SFP modules be partial initialized
+    sfp1 = chassis.get_sfp(1)
+    # Verify the SFP list has been created
+    assert len(chassis._sfp_list) == chassis.PORT_END + 1
+    assert chassis.sfp_module_partial_initialized == True
+    assert chassis.sfp_module_full_initialized == False
+
+    # Fetch another SFP module
+    sfp2 = chassis.get_sfp(2)
+    # Verify the previous SFP module isn't changed
+    assert sfp1 == chassis.get_sfp(1)
+
+    # Fetch all SFP modules
+    allsfp = chassis.get_all_sfps()
+    # Verify sfp1 and sfp2 aren't changed
+    assert sfp1 == chassis.get_sfp(1)
+    assert sfp2 == chassis.get_sfp(2)
+    # Verify the SFP has been fully initialized
+    assert chassis.sfp_module_partial_initialized == True
+    assert chassis.sfp_module_full_initialized == True
+
+
+def test_sfp_full_initialize_without_partial():
+    """
+        Verify SFP initialization flow (full):
+        1. get_all_sfps to trigger a full initialization
+        2. get_sfp for a certain SFP module and verify the partial initialization isn't executed again
+    """
+    chassis = Chassis()
+
+    # Fetch all SFP modules
+    allsfp = chassis.get_all_sfps()
+    # Verify the SFP has been fully initialized
+    assert chassis.sfp_module_partial_initialized == True
+    assert chassis.sfp_module_full_initialized == True
+    for sfp in allsfp:
+        assert sfp is not None
+
+    # Verify when get_sfp is called, the SFP modules won't be initialized again
+    sfp1 = allsfp[0]
+    assert sfp1 == chassis.get_sfp(1)