[PATCH] app/testpmd: support retrying device stop

[PATCH] app/testpmd: support retrying device stop

[Maayan Kashani]

From: Dariusz Sosnowski <dsosnowski@nvidia.com>

In some cases rte_eth_dev_stop() can fail with EBUSY error code meaning
that port cannot be stopped, because of other resources referencing this
port and port must be stopped again after these resources are freed.

This patch adds handling of EBUSY error code on port stop to testpmd.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/testpmd.c | 47 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 8 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 70ea257fda..2d4d583cdf 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3361,6 +3361,7 @@ stop_port(portid_t pid)
 	int need_check_link_status = 0;
 	portid_t peer_pl[RTE_MAX_ETHPORTS];
 	int peer_pi;
+	bool left_ebusy = false;
 	int ret;
 
 	if (port_id_is_invalid(pid, ENABLED_WARN))
@@ -3412,12 +3413,25 @@ stop_port(portid_t pid)
 			port_flow_flush(pi);
 
 		ret = eth_dev_stop_mp(pi);
-		if (ret != 0) {
-			RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
-				pi);
+		if (ret == -EBUSY) {
+			if (pid == (portid_t)RTE_PORT_ALL) {
+				left_ebusy = true;
+				printf("Stopping port %u will be retried.\n", pi);
+			} else {
+				printf("rte_eth_dev_stop failed for port %u. "
+				       "It can be stopped again after related ports "
+				       "are stopped\n", pi);
+			}
+			/* Allow to retry stopping the port. */
+			port->port_status = RTE_PORT_STARTED;
+			continue;
+		} else if (ret != 0) {
+			printf("rte_eth_dev_stop failed for port %u\n", pi);
 			/* Allow to retry stopping the port. */
 			port->port_status = RTE_PORT_STARTED;
 			continue;
+		} else {
+			printf("Port %u is stopped\n", pi);
 		}
 
 		if (port->port_status == RTE_PORT_HANDLING)
@@ -3427,6 +3441,27 @@ stop_port(portid_t pid)
 				pi);
 		need_check_link_status = 1;
 	}
+	if (left_ebusy) {
+		RTE_ETH_FOREACH_DEV(pi) {
+			port = &ports[pi];
+
+			if (port->port_status == RTE_PORT_STARTED)
+				port->port_status = RTE_PORT_HANDLING;
+			else
+				continue;
+
+			if (eth_dev_stop_mp(pi) != 0)
+				printf("rte_eth_dev_stop failed for port %u\n", pi);
+			else
+				printf("Port %u is stopped\n", pi);
+
+			if (port->port_status == RTE_PORT_HANDLING)
+				port->port_status = RTE_PORT_STOPPED;
+			else
+				fprintf(stderr, "Port %d can not be set into stopped\n",
+					pi);
+		}
+	}
 	if (need_check_link_status && !no_link_check)
 		check_all_ports_link_status(RTE_PORT_ALL);
 
@@ -3797,11 +3832,7 @@ pmd_test_exit(void)
 #endif
 	if (ports != NULL) {
 		no_link_check = 1;
-		RTE_ETH_FOREACH_DEV(pt_id) {
-			printf("\nStopping port %d...\n", pt_id);
-			fflush(stdout);
-			stop_port(pt_id);
-		}
+		stop_port(RTE_PORT_ALL);
 		RTE_ETH_FOREACH_DEV(pt_id) {
 			printf("\nShutting down port %d...\n", pt_id);
 			fflush(stdout);
-- 
2.25.1

Re: [PATCH] app/testpmd: support retrying device stop

[Stephen Hemminger]

On Sun, 2 Jun 2024 13:30:35 +0300
Maayan Kashani <mkashani@nvidia.com> wrote:

> From: Dariusz Sosnowski <dsosnowski@nvidia.com>
> 
> In some cases rte_eth_dev_stop() can fail with EBUSY error code meaning
> that port cannot be stopped, because of other resources referencing this
> port and port must be stopped again after these resources are freed.
> 
> This patch adds handling of EBUSY error code on port stop to testpmd.
> 
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>

What causes EBUSY. Seems like a driver bug or a test scenario problem.
You can't expect every DPDK application to retry.

Re: [PATCH] app/testpmd: support retrying device stop

[Ferruh Yigit]

On 6/3/2024 6:09 PM, Stephen Hemminger wrote:
> On Sun, 2 Jun 2024 13:30:35 +0300
> Maayan Kashani <mkashani@nvidia.com> wrote:
> 
>> From: Dariusz Sosnowski <dsosnowski@nvidia.com>
>>
>> In some cases rte_eth_dev_stop() can fail with EBUSY error code meaning
>> that port cannot be stopped, because of other resources referencing this
>> port and port must be stopped again after these resources are freed.
>>
>> This patch adds handling of EBUSY error code on port stop to testpmd.
>>
>> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> 
> What causes EBUSY. Seems like a driver bug or a test scenario problem.
> You can't expect every DPDK application to retry.
>

'rte_eth_dev_stop()' returning EBUSY anticipates this kind of usage already.

But I also agree we should avoid propagating driver issues to the
application, that is why it is better to clarify motivation of this
patch, why specific driver returns EBUSY on stop.

RE: [PATCH] app/testpmd: support retrying device stop

[Dariusz Sosnowski]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Friday, July 5, 2024 13:29
> To: Stephen Hemminger <stephen@networkplumber.org>; Maayan Kashani
> <mkashani@nvidia.com>
> Cc: dev@dpdk.org; Dariusz Sosnowski <dsosnowski@nvidia.com>; Raslan
> Darawsheh <rasland@nvidia.com>; Aman Singh <aman.deep.singh@intel.com>;
> Yuying Zhang <yuying.zhang@intel.com>
> Subject: Re: [PATCH] app/testpmd: support retrying device stop
> 
> On 6/3/2024 6:09 PM, Stephen Hemminger wrote:
> > On Sun, 2 Jun 2024 13:30:35 +0300
> > Maayan Kashani <mkashani@nvidia.com> wrote:
> >
> >> From: Dariusz Sosnowski <dsosnowski@nvidia.com>
> >>
> >> In some cases rte_eth_dev_stop() can fail with EBUSY error code
> >> meaning that port cannot be stopped, because of other resources
> >> referencing this port and port must be stopped again after these resources are
> freed.
> >>
> >> This patch adds handling of EBUSY error code on port stop to testpmd.
> >>
> >> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> >
> > What causes EBUSY. Seems like a driver bug or a test scenario problem.
> > You can't expect every DPDK application to retry.
> >
> 
> 'rte_eth_dev_stop()' returning EBUSY anticipates this kind of usage already.
> 
> But I also agree we should avoid propagating driver issues to the application, that
> is why it is better to clarify motivation of this patch, why specific driver returns
> EBUSY on stop.

Agreed regarding not propagating it to application.

Currently mlx5 PMD can return EBUSY because of a limitation in the PMD,
regarding ordering of start/stop operations of representor ports
(http://git.dpdk.org/dpdk/tree/drivers/net/mlx5/mlx5_trigger.c#n1417).
We should eventually resolve that limitation in PMD.

I think we can drop this patch.

Best regards,
Dariusz Sosnowski