From 42122ed91fcab887fa23c5d9f16ba4dac9450c4d Mon Sep 17 00:00:00 2001 From: Yael Tzur Date: Tue, 3 Jun 2025 12:42:03 +0300 Subject: [PATCH 1/3] improve running time of test_sfp::test_reset --- tests/platform_tests/api/test_sfp.py | 80 +++++++++++++++++----------- 1 file changed, 48 insertions(+), 32 deletions(-) diff --git a/tests/platform_tests/api/test_sfp.py b/tests/platform_tests/api/test_sfp.py index 285e0f85831..58668064bc0 100644 --- a/tests/platform_tests/api/test_sfp.py +++ b/tests/platform_tests/api/test_sfp.py @@ -347,6 +347,33 @@ def is_xcvr_support_power_override(self, xcvr_info_dict): is_valid_xcvr_type = "QSFP" in xcvr_type and xcvr_type != "QSFP-DD" return self.is_xcvr_optical(xcvr_info_dict) and is_valid_xcvr_type + def get_interfaces_to_flap_after_sfp_reset(self, port_index_to_info_dict, duthost): + intf_list = [] + admin_up_port_list = set(duthost.get_admin_up_ports()) + for intf in self.sfp_setup['conn_interfaces']: + logger.info("Processing interface {} for flap after SFP reset".format(intf)) + if intf not in admin_up_port_list: + # skip interfaces which are not in admin up state. + logger.info("Skipping interface {} as it is not in admin up state".format(intf)) + continue + + # skip if info_dict is not retrieved during reset, which also means reset was not performed. + sfp_port_idx = self.sfp_setup['physical_port_index_map'][intf] + if sfp_port_idx not in port_index_to_info_dict: + logger.info("Skipping interface {} \ + as SFP reset was not performed on port index {}".format(intf, sfp_port_idx)) + continue + + info_dict = port_index_to_info_dict[sfp_port_idx] + # only flap interfaces where are CMIS optics, + # non-CMIS optics should stay up after sfp_reset(), no need to flap. + if "cmis_rev" in info_dict: + logger.info("Flapping interface {} as it has CMIS optics".format(intf)) + intf_list.append(intf) + else: + logger.info("Skipping interface {} as it does not have CMIS optics".format(intf)) + return intf_list + # # Functions to test methods inherited from DeviceBase class # @@ -714,37 +741,26 @@ def test_reset(self, request, duthosts, enum_rand_one_per_hwsku_hostname, localh # allow the I2C interface to recover post sfp reset time.sleep(I2C_WAIT_TIME_AFTER_SFP_RESET) - - # shutdown and bring up in batch so that we don't have to add delay for each interface. - intfs_changed = [] - admin_up_port_list = duthost.get_admin_up_ports() - for intf in self.sfp_setup['conn_interfaces']: - if intf not in admin_up_port_list: - # skip interfaces which are not in admin up state. - continue - - sfp_port_idx = self.sfp_setup['physical_port_index_map'][intf] - # skip if info_dict is not retrieved during reset, which also means reset was not performed. - if sfp_port_idx not in port_index_to_info_dict: - continue - info_dict = port_index_to_info_dict[sfp_port_idx] - - # only flap interfaces where are CMIS optics, - # non-CMIS optics should stay up after sfp_reset(), no need to flap. - if "cmis_rev" in info_dict: - duthost.shutdown_interface(intf) - intfs_changed.append(intf) - - time.sleep(WAIT_TIME_AFTER_INTF_SHUTDOWN) - - for intf in intfs_changed: - duthost.no_shutdown_interface(intf) - - _, port_up_wait_time = default_port_toggle_wait_time(duthost, len(intfs_changed)) - if not wait_until(port_up_wait_time, 10, 0, - check_interface_status_of_up_ports, duthost): - self.expect(False, "Not all interfaces are up after reset") - + intf_list = self.get_interfaces_to_flap_after_sfp_reset(port_index_to_info_dict, duthost) + if intf_list: + intf_to_flap_joined = ",".join(intf_list) + logger.info("Flapping interfaces: {}".format(intf_to_flap_joined)) + try: + duthost.shutdown_interface(intf_to_flap_joined) + except Exception as e: + logger.error("Failed to shutdown interfaces: {}".format(e)) + shutdown_wait_scale_factor = max(1, len(intf_list)*0.01) + time.sleep(WAIT_TIME_AFTER_INTF_SHUTDOWN*shutdown_wait_scale_factor) + try: + duthost.no_shutdown_interface(intf_to_flap_joined) + except Exception as e: + logger.error("Failed to startup interfaces: {}".format(e)) + _, port_up_wait_time = default_port_toggle_wait_time(duthost, len(intf_list)) + if not wait_until(port_up_wait_time, 10, 0, + check_interface_status_of_up_ports, duthost): + self.expect(False, "Not all interfaces are up after reset") + else: + logger.info("No interfaces to flap after SFP reset") self.assert_expectations() def test_tx_disable(self, duthosts, enum_rand_one_per_hwsku_hostname, localhost, platform_api_conn): # noqa F811 @@ -943,7 +959,7 @@ def test_get_error_description(self, duthosts, enum_rand_one_per_hwsku_hostname, if self.expect(isinstance(error_description, str) or isinstance(error_description, str), "Transceiver {} error description appears incorrect".format(i)): self.expect(error_description == expected_state, - f"Transceiver {i} is not {expected_state}, actual state is:{error_description}.") + f"Transceiver {i} is not {expected_state}, actual state is: {error_description}.") self.assert_expectations() def test_thermals(self, platform_api_conn): # noqa F811 From dadb829cd349fd1766334540a36debbf91a3b6b5 Mon Sep 17 00:00:00 2001 From: Yael Tzur Date: Sun, 27 Jul 2025 10:17:43 +0300 Subject: [PATCH 2/3] update test_sfp.py with latest commit --- tests/platform_tests/api/test_sfp.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/tests/platform_tests/api/test_sfp.py b/tests/platform_tests/api/test_sfp.py index 58668064bc0..8f703fbfc4a 100644 --- a/tests/platform_tests/api/test_sfp.py +++ b/tests/platform_tests/api/test_sfp.py @@ -348,7 +348,7 @@ def is_xcvr_support_power_override(self, xcvr_info_dict): return self.is_xcvr_optical(xcvr_info_dict) and is_valid_xcvr_type def get_interfaces_to_flap_after_sfp_reset(self, port_index_to_info_dict, duthost): - intf_list = [] + interfaces_to_flap = [] admin_up_port_list = set(duthost.get_admin_up_ports()) for intf in self.sfp_setup['conn_interfaces']: logger.info("Processing interface {} for flap after SFP reset".format(intf)) @@ -360,19 +360,16 @@ def get_interfaces_to_flap_after_sfp_reset(self, port_index_to_info_dict, duthos # skip if info_dict is not retrieved during reset, which also means reset was not performed. sfp_port_idx = self.sfp_setup['physical_port_index_map'][intf] if sfp_port_idx not in port_index_to_info_dict: - logger.info("Skipping interface {} \ - as SFP reset was not performed on port index {}".format(intf, sfp_port_idx)) + logger.info( + "Skipping interface {} as SFP reset was not performed on port index {}".format(intf, sfp_port_idx) + ) continue info_dict = port_index_to_info_dict[sfp_port_idx] - # only flap interfaces where are CMIS optics, - # non-CMIS optics should stay up after sfp_reset(), no need to flap. - if "cmis_rev" in info_dict: - logger.info("Flapping interface {} as it has CMIS optics".format(intf)) - intf_list.append(intf) - else: - logger.info("Skipping interface {} as it does not have CMIS optics".format(intf)) - return intf_list + if self.is_xcvr_support_lpmode(info_dict): + logger.info("Flapping interface {} - xcvr supports lpmode and needs to be flapped".format(intf)) + interfaces_to_flap.append(intf) + return interfaces_to_flap # # Functions to test methods inherited from DeviceBase class From 33da283e3aa12712d1cb01b72dccd231f0d1b6be Mon Sep 17 00:00:00 2001 From: Yael Tzur Date: Sun, 10 Aug 2025 10:14:34 +0300 Subject: [PATCH 3/3] Fix shutdown/startup flow in test_reset to support multi-asic (#20120) Fixed shutdown/startup flow in test_reset to support multi-asic for multi-asic devices, shutdown and startup commands with be preformed for each port and not in bulk Fix #18772 --- tests/platform_tests/api/test_sfp.py | 35 ++++++++++++++++++---------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/tests/platform_tests/api/test_sfp.py b/tests/platform_tests/api/test_sfp.py index 8f703fbfc4a..10a2af8df5f 100644 --- a/tests/platform_tests/api/test_sfp.py +++ b/tests/platform_tests/api/test_sfp.py @@ -371,6 +371,27 @@ def get_interfaces_to_flap_after_sfp_reset(self, port_index_to_info_dict, duthos interfaces_to_flap.append(intf) return interfaces_to_flap + def _shutdown_and_no_shutdown_ports(self, duthost, intf_list): + intf_to_flap_joined = ",".join(intf_list) + try: + if duthost.is_multi_asic: + for intf in intf_list: + duthost.shutdown_interface(intf) + else: + duthost.shutdown_interface(intf_to_flap_joined) + except Exception as e: + logger.error("Failed to shutdown interfaces: {}".format(e)) + + shutdown_wait_scale_factor = max(1, len(intf_list)*0.01) + time.sleep(WAIT_TIME_AFTER_INTF_SHUTDOWN*shutdown_wait_scale_factor) + try: + if duthost.is_multi_asic: + for intf in intf_list: + duthost.no_shutdown_interface(intf) + else: + duthost.no_shutdown_interface(intf_to_flap_joined) + except Exception as e: + logger.error("Failed to startup interfaces: {}".format(e)) # # Functions to test methods inherited from DeviceBase class # @@ -740,18 +761,8 @@ def test_reset(self, request, duthosts, enum_rand_one_per_hwsku_hostname, localh time.sleep(I2C_WAIT_TIME_AFTER_SFP_RESET) intf_list = self.get_interfaces_to_flap_after_sfp_reset(port_index_to_info_dict, duthost) if intf_list: - intf_to_flap_joined = ",".join(intf_list) - logger.info("Flapping interfaces: {}".format(intf_to_flap_joined)) - try: - duthost.shutdown_interface(intf_to_flap_joined) - except Exception as e: - logger.error("Failed to shutdown interfaces: {}".format(e)) - shutdown_wait_scale_factor = max(1, len(intf_list)*0.01) - time.sleep(WAIT_TIME_AFTER_INTF_SHUTDOWN*shutdown_wait_scale_factor) - try: - duthost.no_shutdown_interface(intf_to_flap_joined) - except Exception as e: - logger.error("Failed to startup interfaces: {}".format(e)) + logger.info("Flapping interfaces: {}".format(intf_list)) + self._shutdown_and_no_shutdown_ports(duthost, intf_list) _, port_up_wait_time = default_port_toggle_wait_time(duthost, len(intf_list)) if not wait_until(port_up_wait_time, 10, 0, check_interface_status_of_up_ports, duthost):