From 11a7267c687666f75e7931c991f2e734ebb5056f Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox Date: Wed, 15 Mar 2023 18:21:42 +0800 Subject: [PATCH 1/4] Fix issue: user must set admin down before toggling LPM --- .../mlnx-platform-api/sonic_platform/sfp.py | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py index e982f0eda909..bff7968e5d4b 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py +++ b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py @@ -540,19 +540,24 @@ def get_logical_ports(cls, sdk_handle, sdk_index, slot_id): assert rc == SX_STATUS_SUCCESS, "sx_api_port_device_get failed, rc = %d" % rc port_cnt = uint32_t_p_value(port_cnt_p) - log_port_list = [] + up_port_list = [] + down_port_list = [] for i in range(0, port_cnt): port_attributes = sx_port_attributes_t_arr_getitem(port_attributes_list, i) if not cls.is_nve(int(port_attributes.log_port)) \ and not cls.is_cpu(int(port_attributes.log_port)) \ and port_attributes.port_mapping.module_port == sdk_index \ - and port_attributes.port_mapping.slot == slot_id \ - and cls.is_port_admin_status_up(sdk_handle, port_attributes.log_port): - log_port_list.append(port_attributes.log_port) + and port_attributes.port_mapping.slot == slot_id: + + if cls.is_port_admin_status_up(sdk_handle, port_attributes.log_port): + up_port_list.append(port_attributes.log_port) + else: + down_port_list.append(port_attributes.log_port) delete_sx_port_attributes_t_arr(port_attributes_list) delete_uint32_t_p(port_cnt_p) - return log_port_list + + return up_port_list, down_port_list @classmethod @@ -582,25 +587,14 @@ def mgmt_phy_mod_pwr_attr_set(cls, sdk_handle, sdk_index, slot_id, power_attr_ty @classmethod - def _set_lpmode_raw(cls, sdk_handle, sdk_index, slot_id, ports, attr_type, power_mode): + def _set_lpmode_raw(cls, sdk_handle, sdk_index, slot_id, attr_type, power_mode): result = False # Check if the module already works in the same mode admin_pwr_mode, oper_pwr_mode = cls.mgmt_phy_mod_pwr_attr_get(attr_type, sdk_handle, sdk_index, slot_id) if (power_mode == SX_MGMT_PHY_MOD_PWR_MODE_LOW_E and oper_pwr_mode == SX_MGMT_PHY_MOD_PWR_MODE_LOW_E) \ or (power_mode == SX_MGMT_PHY_MOD_PWR_MODE_AUTO_E and admin_pwr_mode == SX_MGMT_PHY_MOD_PWR_MODE_AUTO_E): return True - try: - # Bring the port down - for port in ports: - cls.set_port_admin_status_by_log_port(sdk_handle, port, SX_PORT_ADMIN_STATUS_DOWN) - # Set the desired power mode - result = cls.mgmt_phy_mod_pwr_attr_set(sdk_handle, sdk_index, slot_id, attr_type, power_mode) - finally: - # Bring the port up - for port in ports: - cls.set_port_admin_status_by_log_port(sdk_handle, port, SX_PORT_ADMIN_STATUS_UP) - - return result + return cls.mgmt_phy_mod_pwr_attr_set(sdk_handle, sdk_index, slot_id, attr_type, power_mode) def set_lpmode(self, lpmode): @@ -626,6 +620,9 @@ def set_lpmode(self, lpmode): # Set LPM try: output = subprocess.check_output(lpm_cmd, shell=True, universal_newlines=True) + for line in output.splitlines(): + if line.startswith('Error') or line.startswith('Notice'): + print('\n' + line) return 'True' in output except subprocess.CalledProcessError as e: print("Error! Unable to set LPM for {}, rc = {}, err msg: {}".format(self.sdk_index, e.returncode, e.output)) @@ -636,12 +633,19 @@ def set_lpmode(self, lpmode): @classmethod def _set_lpmode(cls, lpmode, sdk_handle, sdk_index, slot_id): - log_port_list = cls.get_logical_ports(sdk_handle, sdk_index, slot_id) + up_port_list, down_port_list = cls.get_logical_ports(sdk_handle, sdk_index, slot_id) + if up_port_list and lpmode: + print('Error: Please put port in admin down state before entering low power mode') + logger.log_error('Please put port in admin down state before entering low power mode') + return False + + if not lpmode and down_port_list: + print('Notice: Be aware you exit LPM mode but still port is in admin down state') + sdk_lpmode = SX_MGMT_PHY_MOD_PWR_MODE_LOW_E if lpmode else SX_MGMT_PHY_MOD_PWR_MODE_AUTO_E cls._set_lpmode_raw(sdk_handle, sdk_index, slot_id, - log_port_list, SX_MGMT_PHY_MOD_PWR_ATTR_PWR_MODE_E, sdk_lpmode) logger.log_info("{} low power mode for module {}, slot {}".format("Enabled" if lpmode else "Disabled", sdk_index, slot_id)) From 9376d88abd6960c89ed27880a6ea8cfa759c1e83 Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox Date: Wed, 15 Mar 2023 18:24:01 +0800 Subject: [PATCH 2/4] fix typo --- 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 bff7968e5d4b..a9b535a10eb4 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py +++ b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py @@ -640,7 +640,7 @@ def _set_lpmode(cls, lpmode, sdk_handle, sdk_index, slot_id): return False if not lpmode and down_port_list: - print('Notice: Be aware you exit LPM mode but still port is in admin down state') + print('Notice: Be aware you exit low power mode but still port is in admin down state') sdk_lpmode = SX_MGMT_PHY_MOD_PWR_MODE_LOW_E if lpmode else SX_MGMT_PHY_MOD_PWR_MODE_AUTO_E cls._set_lpmode_raw(sdk_handle, From af9bd44571a674336e0818cacfaabdd50f721e2c Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox Date: Tue, 21 Mar 2023 10:03:21 +0800 Subject: [PATCH 3/4] Fix message --- .../mlnx-platform-api/sonic_platform/sfp.py | 93 +------------------ .../mlnx-platform-api/tests/test_sfp.py | 8 -- 2 files changed, 2 insertions(+), 99 deletions(-) diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py index a9b535a10eb4..5ebc8594d759 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py +++ b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py @@ -481,85 +481,6 @@ def _reset(cls, sdk_handle, sdk_index, slot_id): return rc == SX_STATUS_SUCCESS - - @classmethod - def is_nve(cls, port): - return (port & NVE_MASK) != 0 - - - @classmethod - def is_cpu(cls, port): - return (port & CPU_MASK) != 0 - - - @classmethod - def _fetch_port_status(cls, sdk_handle, log_port): - oper_state_p = new_sx_port_oper_state_t_p() - admin_state_p = new_sx_port_admin_state_t_p() - module_state_p = new_sx_port_module_state_t_p() - rc = sx_api_port_state_get(sdk_handle, log_port, oper_state_p, admin_state_p, module_state_p) - assert rc == SXD_STATUS_SUCCESS, "sx_api_port_state_get failed, rc = %d" % rc - - admin_state = sx_port_admin_state_t_p_value(admin_state_p) - oper_state = sx_port_oper_state_t_p_value(oper_state_p) - - delete_sx_port_oper_state_t_p(oper_state_p) - delete_sx_port_admin_state_t_p(admin_state_p) - delete_sx_port_module_state_t_p(module_state_p) - - return oper_state, admin_state - - - @classmethod - def is_port_admin_status_up(cls, sdk_handle, log_port): - _, admin_state = cls._fetch_port_status(sdk_handle, log_port); - return admin_state == SX_PORT_ADMIN_STATUS_UP - - - @classmethod - def set_port_admin_status_by_log_port(cls, sdk_handle, log_port, admin_status): - rc = sx_api_port_state_set(sdk_handle, log_port, admin_status) - if SX_STATUS_SUCCESS != rc: - logger.log_error("sx_api_port_state_set failed, rc = %d" % rc) - - return SX_STATUS_SUCCESS == rc - - - @classmethod - def get_logical_ports(cls, sdk_handle, sdk_index, slot_id): - # Get all the ports related to the sfp, if port admin status is up, put it to list - port_cnt_p = new_uint32_t_p() - uint32_t_p_assign(port_cnt_p, 0) - rc = sx_api_port_device_get(sdk_handle, DEVICE_ID, SWITCH_ID, None, port_cnt_p) - - assert rc == SX_STATUS_SUCCESS, "sx_api_port_device_get failed, rc = %d" % rc - port_cnt = uint32_t_p_value(port_cnt_p) - port_attributes_list = new_sx_port_attributes_t_arr(port_cnt) - - rc = sx_api_port_device_get(sdk_handle, DEVICE_ID , SWITCH_ID, port_attributes_list, port_cnt_p) - assert rc == SX_STATUS_SUCCESS, "sx_api_port_device_get failed, rc = %d" % rc - - port_cnt = uint32_t_p_value(port_cnt_p) - up_port_list = [] - down_port_list = [] - for i in range(0, port_cnt): - port_attributes = sx_port_attributes_t_arr_getitem(port_attributes_list, i) - if not cls.is_nve(int(port_attributes.log_port)) \ - and not cls.is_cpu(int(port_attributes.log_port)) \ - and port_attributes.port_mapping.module_port == sdk_index \ - and port_attributes.port_mapping.slot == slot_id: - - if cls.is_port_admin_status_up(sdk_handle, port_attributes.log_port): - up_port_list.append(port_attributes.log_port) - else: - down_port_list.append(port_attributes.log_port) - - delete_sx_port_attributes_t_arr(port_attributes_list) - delete_uint32_t_p(port_cnt_p) - - return up_port_list, down_port_list - - @classmethod def mgmt_phy_mod_pwr_attr_set(cls, sdk_handle, sdk_index, slot_id, power_attr_type, admin_pwr_mode): result = False @@ -585,10 +506,8 @@ def mgmt_phy_mod_pwr_attr_set(cls, sdk_handle, sdk_index, slot_id, power_attr_ty return result - @classmethod def _set_lpmode_raw(cls, sdk_handle, sdk_index, slot_id, attr_type, power_mode): - result = False # Check if the module already works in the same mode admin_pwr_mode, oper_pwr_mode = cls.mgmt_phy_mod_pwr_attr_get(attr_type, sdk_handle, sdk_index, slot_id) if (power_mode == SX_MGMT_PHY_MOD_PWR_MODE_LOW_E and oper_pwr_mode == SX_MGMT_PHY_MOD_PWR_MODE_LOW_E) \ @@ -630,18 +549,10 @@ def set_lpmode(self, lpmode): else: return self._set_lpmode(lpmode, self.sdk_handle, self.sdk_index, self.slot_id) - @classmethod def _set_lpmode(cls, lpmode, sdk_handle, sdk_index, slot_id): - up_port_list, down_port_list = cls.get_logical_ports(sdk_handle, sdk_index, slot_id) - if up_port_list and lpmode: - print('Error: Please put port in admin down state before entering low power mode') - logger.log_error('Please put port in admin down state before entering low power mode') - return False - - if not lpmode and down_port_list: - print('Notice: Be aware you exit low power mode but still port is in admin down state') - + print('\nNotice: please set port admin status to down before setting power mode, \ + ignore this message if already set') sdk_lpmode = SX_MGMT_PHY_MOD_PWR_MODE_LOW_E if lpmode else SX_MGMT_PHY_MOD_PWR_MODE_AUTO_E cls._set_lpmode_raw(sdk_handle, sdk_index, diff --git a/platform/mellanox/mlnx-platform-api/tests/test_sfp.py b/platform/mellanox/mlnx-platform-api/tests/test_sfp.py index 2a79b39308b5..bdf1ac880795 100644 --- a/platform/mellanox/mlnx-platform-api/tests/test_sfp.py +++ b/platform/mellanox/mlnx-platform-api/tests/test_sfp.py @@ -143,14 +143,6 @@ def test_sfp_read_eeprom(self, mock_get_page): handle.read.side_effect = OSError('') assert sfp.read_eeprom(0, 1) is None - @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) - assert SFP.is_port_admin_status_up(None, None) - - mock_port_status.return_value = (0, False) - assert not SFP.is_port_admin_status_up(None, None) - @mock.patch('sonic_platform.sfp.SFP._get_eeprom_path', mock.MagicMock(return_value = None)) @mock.patch('sonic_platform.sfp.SFP._get_sfp_type_str') def test_is_write_protected(self, mock_get_type_str): From 6124da5fa435774086fc23f528addef0d074d860 Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox Date: Tue, 21 Mar 2023 11:07:57 +0800 Subject: [PATCH 4/4] Fix message --- platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py | 3 +-- 1 file changed, 1 insertion(+), 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 5ebc8594d759..94ecbeee7389 100644 --- a/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py +++ b/platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py @@ -551,8 +551,7 @@ def set_lpmode(self, lpmode): @classmethod def _set_lpmode(cls, lpmode, sdk_handle, sdk_index, slot_id): - print('\nNotice: please set port admin status to down before setting power mode, \ - ignore this message if already set') + print('\nNotice: please set port admin status to down before setting power mode, ignore this message if already set') sdk_lpmode = SX_MGMT_PHY_MOD_PWR_MODE_LOW_E if lpmode else SX_MGMT_PHY_MOD_PWR_MODE_AUTO_E cls._set_lpmode_raw(sdk_handle, sdk_index,