patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Kevin Traynor <ktraynor@redhat.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>,
	stable@dpdk.org, Ferruh Yigit <ferruh.yigit@amd.com>
Subject: [PATCH 3/5] app/testpmd: cleanup cleanly from signal
Date: Sat, 31 Aug 2024 10:09:27 -0700	[thread overview]
Message-ID: <20240831171035.27505-4-stephen@networkplumber.org> (raw)
In-Reply-To: <20240831171035.27505-1-stephen@networkplumber.org>

Do a clean shutdown of testpmd when a signal is received; instead of
having testpmd kill itself.  This fixes the problem where a signal could
be received in the middle of a PMD and then the signal handler would
call PMD's close routine leading to locking problems.

The cmdline structure no longer needs to be global it can
just be local to the prompt() function.

An added benefit is it gets rid of some Windows specific code.

(cherry picked from commit 0fd1386c30c3ad9365d7fdd2829bf7cb2e1b9dff)

Fixes: d9a191a00e81 ("app/testpmd: fix quitting in container")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
---
 app/test-pmd/cmdline.c | 31 +++++++----------
 app/test-pmd/testpmd.c | 78 ++++++++++++++++++++----------------------
 app/test-pmd/testpmd.h |  3 ++
 3 files changed, 52 insertions(+), 60 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 6e10afeedd..6da5198ac7 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -68,8 +68,6 @@
 #include "cmdline_tm.h"
 #include "bpf_cmd.h"
 
-static struct cmdline *testpmd_cl;
-
 static void cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue);
 
 /* *** Help command with introduction. *** */
@@ -17985,31 +17983,26 @@ cmdline_read_from_file(const char *filename)
 void
 prompt(void)
 {
-	int ret;
+	struct cmdline *cl;
+
 	/* initialize non-constant commands */
 	cmd_set_fwd_mode_init();
 	cmd_set_fwd_retry_mode_init();
 
-	testpmd_cl = cmdline_stdin_new(main_ctx, "testpmd> ");
-	if (testpmd_cl == NULL)
+	cl = cmdline_stdin_new(main_ctx, "testpmd> ");
+	if (cl == NULL)
 		return;
 
-	ret = atexit(prompt_exit);
-	if (ret != 0)
-		fprintf(stderr, "Cannot set exit function for cmdline\n");
+	/* loop until signal or quit command */
+	while (f_quit == 0 && cl_quit == 0) {
+		int status = cmdline_poll(cl);
 
-	cmdline_interact(testpmd_cl);
-	if (ret != 0)
-		cmdline_stdin_exit(testpmd_cl);
-}
-
-void
-prompt_exit(void)
-{
-	if (testpmd_cl != NULL) {
-		cmdline_quit(testpmd_cl);
-		cmdline_stdin_exit(testpmd_cl);
+		if (status < 0 || status == RDLINE_EXITED)
+			break;
 	}
+
+	cmdline_quit(cl);
+	cmdline_stdin_exit(cl);
 }
 
 static void
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 55eb293cc0..40dff96e5c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -11,6 +11,7 @@
 #include <fcntl.h>
 #ifndef RTE_EXEC_ENV_WINDOWS
 #include <sys/mman.h>
+#include <sys/select.h>
 #endif
 #include <sys/types.h>
 #include <errno.h>
@@ -219,7 +220,8 @@ unsigned int xstats_display_num; /**< Size of extended statistics to show */
  * In container, it cannot terminate the process which running with 'stats-period'
  * option. Set flag to exit stats period loop after received SIGINT/SIGTERM.
  */
-uint8_t f_quit;
+volatile uint8_t f_quit;
+uint8_t cl_quit; /* Quit testpmd from cmdline. */
 
 /*
  * Max Rx frame size, set by '--max-pkt-len' parameter.
@@ -4044,13 +4046,6 @@ init_port(void)
 	memset(txring_numa, NUMA_NO_CONFIG, RTE_MAX_ETHPORTS);
 }
 
-static void
-force_quit(void)
-{
-	pmd_test_exit();
-	prompt_exit();
-}
-
 static void
 print_stats(void)
 {
@@ -4069,28 +4064,9 @@ print_stats(void)
 }
 
 static void
-signal_handler(int signum)
+signal_handler(int signum __rte_unused)
 {
-	if (signum == SIGINT || signum == SIGTERM) {
-		fprintf(stderr, "\nSignal %d received, preparing to exit...\n",
-			signum);
-#ifdef RTE_LIB_PDUMP
-		/* uninitialize packet capture framework */
-		rte_pdump_uninit();
-#endif
-#ifdef RTE_LIB_LATENCYSTATS
-		if (latencystats_enabled != 0)
-			rte_latencystats_uninit();
-#endif
-		force_quit();
-		/* Set flag to indicate the force termination. */
-		f_quit = 1;
-		/* exit with the expected status */
-#ifndef RTE_EXEC_ENV_WINDOWS
-		signal(signum, SIG_DFL);
-		kill(getpid(), signum);
-#endif
-	}
+	f_quit = 1;
 }
 
 int
@@ -4261,15 +4237,9 @@ main(int argc, char** argv)
 			start_packet_forwarding(0);
 		}
 		prompt();
-		pmd_test_exit();
 	} else
 #endif
 	{
-		char c;
-		int rc;
-
-		f_quit = 0;
-
 		printf("No commandline core given, start packet forwarding\n");
 		start_packet_forwarding(tx_first);
 		if (stats_period != 0) {
@@ -4292,15 +4262,41 @@ main(int argc, char** argv)
 				prev_time = cur_time;
 				rte_delay_us_sleep(US_PER_S);
 			}
-		}
+		} else {
+			char c;
+			fd_set fds;
+
+			printf("Press enter to exit\n");
 
-		printf("Press enter to exit\n");
-		rc = read(0, &c, 1);
-		pmd_test_exit();
-		if (rc < 0)
-			return 1;
+			FD_ZERO(&fds);
+			FD_SET(0, &fds);
+
+			/* wait for signal or enter */
+			ret = select(1, &fds, NULL, NULL, NULL);
+			if (ret < 0 && errno != EINTR)
+				rte_exit(EXIT_FAILURE,
+					 "Select failed: %s\n",
+					 strerror(errno));
+
+			/* if got enter then consume it */
+			if (ret == 1 && read(0, &c, 1) < 0)
+				rte_exit(EXIT_FAILURE,
+					 "Read failed: %s\n",
+					 strerror(errno));
+		}
 	}
 
+	pmd_test_exit();
+
+#ifdef RTE_LIB_PDUMP
+	/* uninitialize packet capture framework */
+	rte_pdump_uninit();
+#endif
+#ifdef RTE_LIB_LATENCYSTATS
+	if (latencystats_enabled != 0)
+		rte_latencystats_uninit();
+#endif
+
 	ret = rte_eal_cleanup();
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 2149ecd93a..33b4f5504f 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -32,6 +32,9 @@
 #define RTE_PORT_CLOSED         (uint16_t)2
 #define RTE_PORT_HANDLING       (uint16_t)3
 
+extern uint8_t cl_quit;
+extern volatile uint8_t f_quit;
+
 /*
  * It is used to allocate the memory for hash key.
  * The hash key size is NIC dependent.
-- 
2.45.2


  parent reply	other threads:[~2024-08-31 17:10 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20240831171035.27505-1-stephen@networkplumber.org>
2024-08-31 17:09 ` [PATCH 1/5] cmdline: make rdline status not private Stephen Hemminger
2024-08-31 17:09 ` [PATCH 2/5] cmdline: handle EOF as quit Stephen Hemminger
2024-08-31 17:09 ` Stephen Hemminger [this message]
2024-08-31 17:09 ` [PATCH 4/5] app/testpmd: fix early exit from signal Stephen Hemminger
2024-08-31 17:09 ` [PATCH 5/5] app/testpmd: fix interactive mode on Windows Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240831171035.27505-4-stephen@networkplumber.org \
    --to=stephen@networkplumber.org \
    --cc=ferruh.yigit@amd.com \
    --cc=ktraynor@redhat.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).