diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/chassis.py b/platform/mellanox/mlnx-platform-api/sonic_platform/chassis.py index 2ae93e14d4c..b18d780faba 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/chassis.py +++ b/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 diff --git a/platform/mellanox/mlnx-platform-api/tests/test_sfp.py b/platform/mellanox/mlnx-platform-api/tests/test_sfp.py new file mode 100644 index 00000000000..0c24eb83354 --- /dev/null +++ b/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)