app/testpmd: fix interactive mode with no ports

PATCH Superseded Gregory Etelson 4 participants 9 messages Nov 9, 2022 – Jan 27, 2023

lore

Share & tools

lore

Gregory Etelson

author

2022-11-09 16:54 UTC #0 lore raw

PATCH 1/1 app/testpmd: fix interactive mode with no ports

Testpmd terminated unconditionally if it failed to start all ports.

The patch allows testpmd to get into the command line, if the interactive mode was requested.

Fixes 6937d2103e22 ("app/testpmd: add option to not start device")

Signed-off-by Gregory Etelson <[email protected]>

---
 app/test-pmd/testpmd.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

app/test-pmd/testpmd.c +6 -2

index 7381dfd9e5..8517f376db 100644
@@ -4433,8 +4433,11 @@ main(int argc, char** argv)
 		}
 	}
 
-	if (!no_device_start && start_port(RTE_PORT_ALL) != 0)
-		rte_exit(EXIT_FAILURE, "Start ports failed\n");
+	if (!no_device_start && start_port(RTE_PORT_ALL) != 0) {
+		fprintf(stderr, "Start ports failed\n");
+		if (!interactive)
+			goto eal_out;
+	}
 
 	/* set all ports to promiscuous mode by default */
 	RTE_ETH_FOREACH_DEV(port_id) {

@@ -4526,6 +4529,7 @@ main(int argc, char** argv)
 			return 1;
 	}
 
+eal_out:
 	ret = rte_eal_cleanup();
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE,

Signature · 1 line

2.34.1

Gregory Etelson

author

2022-11-09 17:09 UTC #1 lore raw

PATCH v2 1/1 app/testpmd: fix interactive mode with no ports

Testpmd terminated unconditionally if it failed to start all ports.

The patch allows testpmd to get into the command line, if the interactive mode was requested.

Fixes 6937d2103e22 ("app/testpmd: add option to not start device")

Cc [email protected]

Signed-off-by Gregory Etelson <[email protected]>

---
v2: add Cc

---
 app/test-pmd/testpmd.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

app/test-pmd/testpmd.c +6 -2

index 7381dfd9e5..8517f376db 100644
@@ -4433,8 +4433,11 @@ main(int argc, char** argv)
 		}
 	}
 
-	if (!no_device_start && start_port(RTE_PORT_ALL) != 0)
-		rte_exit(EXIT_FAILURE, "Start ports failed\n");
+	if (!no_device_start && start_port(RTE_PORT_ALL) != 0) {
+		fprintf(stderr, "Start ports failed\n");
+		if (!interactive)
+			goto eal_out;
+	}
 
 	/* set all ports to promiscuous mode by default */
 	RTE_ETH_FOREACH_DEV(port_id) {

@@ -4526,6 +4529,7 @@ main(int argc, char** argv)
 			return 1;
 	}
 
+eal_out:
 	ret = rte_eal_cleanup();
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE,

Signature · 1 line

2.34.1

Singh, Aman Deep

2022-11-16 04:23 UTC #2 lore raw

Hi Gregory,

Thanks for the patch.

Quoted 4 lines · Gregory Etelson

Testpmd terminated unconditionally if it failed to start all ports.
The patch allows testpmd to get into the command line, if the interactive mode was requested.

I tested testpmd in interactive mode with no ports. It did get into command line-

$./dpdk-testpmd -n 4 -c 0x1f -- -i
testpmd: No probed ethernet devices
Interactive-mode selected
testpmd: create a new mbuf pool <mb_pool_0>: n=179456, size=2176, socket=0
testpmd: preferred mempool ops selected: ring_mp_mc
Done
testpmd>

Quoted 18 lines

Fixes 6937d2103e22 ("app/testpmd: add option to not start device")
Cc [email protected]
Cc [email protected]
Signed-off-by Gregory Etelson <[email protected]>
---
v2: add Cc
---
  app/test-pmd/testpmd.c | 8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)
app/test-pmd/testpmd.c +0 -0
index 7381dfd9e5..8517f376db 100644

app/test-pmd/testpmd.c main(int argc, char** argv) · line 4436

Quoted 4 lines

app/test-pmd/testpmd.c +0 -1

@@ -4433,8 +4433,11 @@ main(int argc, char** argv)
  		}
  	}
 
-	if (!no_device_start && start_port(RTE_PORT_ALL) != 0)

With no ports, start_port returns 0 so it doesn't exits.

Quoted 17 lines

app/test-pmd/testpmd.c +6 -1

-		rte_exit(EXIT_FAILURE, "Start ports failed\n");
+	if (!no_device_start && start_port(RTE_PORT_ALL) != 0) {
+		fprintf(stderr, "Start ports failed\n");
+		if (!interactive)
+			goto eal_out;
+	}
 
  	/* set all ports to promiscuous mode by default */
  	RTE_ETH_FOREACH_DEV(port_id) {

@@ -4526,6 +4529,7 @@ main(int argc, char** argv)
  			return 1;
  	}
 
+eal_out:
  	ret = rte_eal_cleanup();
  	if (ret != 0)
  		rte_exit(EXIT_FAILURE,

Ferruh Yigit

2023-01-19 15:09 UTC #3 lore raw

Quoted 34 lines · Aman Deep

Hi Gregory,
Thanks for the patch.
Quoted 4 lines · Gregory Etelson
Testpmd terminated unconditionally if it failed to start all ports.
The patch allows testpmd to get into the command line, if the interactive mode was requested.
I tested testpmd in interactive mode with no ports. It did get into command line-
$./dpdk-testpmd -n 4 -c 0x1f -- -i
testpmd: No probed ethernet devices
Interactive-mode selected
testpmd: create a new mbuf pool <mb_pool_0>: n=179456, size=2176, socket=0
testpmd: preferred mempool ops selected: ring_mp_mc
Done
testpmd>
Quoted 13 lines
Fixes 6937d2103e22 ("app/testpmd: add option to not start device")
Cc [email protected]
Cc [email protected]
Signed-off-by Gregory Etelson <[email protected]>
---
v2: add Cc
---
app/test-pmd/testpmd.c | 8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)

Quoted 11 lines

app/test-pmd/testpmd.c main(int argc, char** argv) · line 4435
Quoted 8 lines
app/test-pmd/testpmd.c +0 -0
index 7381dfd9e5..8517f376db 100644
@@ -4433,8 +4433,11 @@ main(int argc, char** argv)
           }
       }
   -    if (!no_device_start && start_port(RTE_PORT_ALL) != 0)
With no ports, start_port returns 0 so it doesn't exits.

Hi Aman,

I think patch is for the case port exists and 'start_port()' failed, in that case testpmd exits.

With patch, testpmd only continues after 'start_port()' failure only if interactive mode is selected, that also looks good to me.

Gregory,

Only for non interactive case if 'start_port()' fails, application exit value changed to 'EXIT_SUCCESS', which is not quite right, can you please fix it?

Quoted 16 lines

Quoted 15 lines

app/test-pmd/testpmd.c +5 -1

-        rte_exit(EXIT_FAILURE, "Start ports failed\n");
+    if (!no_device_start && start_port(RTE_PORT_ALL) != 0) {
+        fprintf(stderr, "Start ports failed\n");
+        if (!interactive)
+            goto eal_out;
+    }
         /* set all ports to promiscuous mode by default */
       RTE_ETH_FOREACH_DEV(port_id) {

@@ -4526,6 +4529,7 @@ main(int argc, char** argv)
               return 1;
       }

+eal_out:
      ret = rte_eal_cleanup();
      if (ret != 0)
          rte_exit(EXIT_FAILURE,

Gregory Etelson

author

2023-01-19 17:16 UTC #4 lore raw

PATCH v3 1/1 app/testpmd: fix interactive mode with no ports

In reply to Gregory Etelson

Testpmd terminated unconditionally if it failed to start all ports.

The patch allows testpmd to get into the command line, if the interactive mode was requested.

Fixes 6937d2103e22 ("app/testpmd: add option to not start device")

Cc [email protected]

Signed-off-by Gregory Etelson <[email protected]>

---
v2: Add Cc.
v3: Update patch.

---
 app/test-pmd/testpmd.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

app/test-pmd/testpmd.c +7 -2

index 134d79a555..3d336a4d3e 100644
@@ -4483,8 +4483,13 @@ main(int argc, char** argv)
 		}
 	}
 
-	if (!no_device_start && start_port(RTE_PORT_ALL) != 0)
-		rte_exit(EXIT_FAILURE, "Start ports failed\n");
+	if (!no_device_start && start_port(RTE_PORT_ALL) != 0) {
+		fprintf(stderr, "Start ports failed\n");
+		if (!interactive) {
+			rte_eal_cleanup();
+			rte_exit(EXIT_FAILURE, "Start ports failed\n");
+		}
+	}
 
 	/* set all ports to promiscuous mode by default */
 	RTE_ETH_FOREACH_DEV(port_id) {

Signature · 1 line

2.34.1

Gregory Etelson

author

2023-01-19 17:23 UTC #5 lore raw

PATCH v3 1/1 app/testpmd: fix interactive mode with no ports

In reply to Gregory Etelson

Testpmd terminated unconditionally if it failed to start all ports.

The patch allows testpmd to get into the command line, if the interactive mode was requested.

Fixes 6937d2103e22 ("app/testpmd: add option to not start device")

Cc [email protected]

Signed-off-by Gregory Etelson <[email protected]>

---
v2: Add Cc.
v3: Update patch.

---
 app/test-pmd/testpmd.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

app/test-pmd/testpmd.c +7 -2

index 134d79a555..3d336a4d3e 100644
@@ -4483,8 +4483,13 @@ main(int argc, char** argv)
 		}
 	}
 
-	if (!no_device_start && start_port(RTE_PORT_ALL) != 0)
-		rte_exit(EXIT_FAILURE, "Start ports failed\n");
+	if (!no_device_start && start_port(RTE_PORT_ALL) != 0) {
+		fprintf(stderr, "Start ports failed\n");
+		if (!interactive) {
+			rte_eal_cleanup();
+			rte_exit(EXIT_FAILURE, "Start ports failed\n");
+		}
+	}
 
 	/* set all ports to promiscuous mode by default */
 	RTE_ETH_FOREACH_DEV(port_id) {

Signature · 1 line

2.34.1

Stephen Hemminger

2023-01-19 17:28 UTC #6 lore raw

app/test-pmd/testpmd.c main(int argc, char** argv) · line 4492

Quoted 7 lines · On Thu, 19 Jan 2023 19:23:12 +0200 Gregory Etelson <[email protected]> wrote:

app/test-pmd/testpmd.c +7 -0

+	if (!no_device_start && start_port(RTE_PORT_ALL) != 0) {
+		fprintf(stderr, "Start ports failed\n");
+		if (!interactive) {
+			rte_eal_cleanup();
+			rte_exit(EXIT_FAILURE, "Start ports failed\n");
+		}
+	}

Ok, but you are printing message twice now.

Gregory Etelson

author

2023-01-19 17:46 UTC #7 lore raw

PATCH v4 1/1 app/testpmd: fix interactive mode with no ports

In reply to Gregory Etelson

Testpmd terminated unconditionally if it failed to start all ports.

The patch allows testpmd to get into the command line, if the interactive mode was requested.

Fixes 6937d2103e22 ("app/testpmd: add option to not start device")

Cc [email protected]

Signed-off-by Gregory Etelson <[email protected]>

---
v2: Add Cc.
v3: Update patch.
v4: Update patch.

---
 app/test-pmd/testpmd.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

app/test-pmd/testpmd.c +7 -2

index 134d79a555..e366f81a0f 100644
@@ -4483,8 +4483,13 @@ main(int argc, char** argv)
 		}
 	}
 
-	if (!no_device_start && start_port(RTE_PORT_ALL) != 0)
-		rte_exit(EXIT_FAILURE, "Start ports failed\n");
+	if (!no_device_start && start_port(RTE_PORT_ALL) != 0) {
+		if (!interactive) {
+			rte_eal_cleanup();
+			rte_exit(EXIT_FAILURE, "Start ports failed\n");
+		}
+		fprintf(stderr, "Start ports failed\n");
+	}
 
 	/* set all ports to promiscuous mode by default */
 	RTE_ETH_FOREACH_DEV(port_id) {

Signature · 1 line

2.34.1

Ferruh Yigit

Acked-by on v4 1/1 Applied

2023-01-27 23:04 UTC #8 lore raw

Quoted 11 lines · Gregory Etelson

Testpmd terminated unconditionally if it failed to start all ports.
The patch allows testpmd to get into the command line, if the interactive mode was requested.
Fixes 6937d2103e22 ("app/testpmd: add option to not start device")
Cc [email protected]
Cc [email protected]
Signed-off-by Gregory Etelson <[email protected]>

Acked-by Ferruh Yigit <[email protected]>

Applied to dpdk-next-net/main, thanks.