DPDK patches and discussions
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: dev@dpdk.org
Cc: stable@dpdk.org, Stephen Hemminger <stephen@networkplumber.org>
Subject: [PATCH v11 3/3] testpmd: cleanup cleanly from signal
Date: Fri,  3 Feb 2023 11:14:09 -0800	[thread overview]
Message-ID: <20230203191409.97567-4-stephen@networkplumber.org> (raw)
In-Reply-To: <20230203191409.97567-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.

Fixes: d9a191a00e81 ("app/testpmd: fix quitting in container")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test-pmd/cmdline.c | 29 ++++++----------
 app/test-pmd/testpmd.c | 77 ++++++++++++++++++++----------------------
 app/test-pmd/testpmd.h |  1 +
 3 files changed, 48 insertions(+), 59 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index cb8c174020b0..135c5c439bfc 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -66,7 +66,6 @@
 #include "cmdline_tm.h"
 #include "bpf_cmd.h"
 
-static struct cmdline *testpmd_cl;
 static cmdline_parse_ctx_t *main_ctx;
 static TAILQ_HEAD(, testpmd_driver_commands) driver_commands_head =
 	TAILQ_HEAD_INITIALIZER(driver_commands_head);
@@ -12936,28 +12935,22 @@ cmdline_read_from_file(const char *filename)
 void
 prompt(void)
 {
-	int ret;
+	struct cmdline *cl;
 
-	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");
-
-	cmdline_interact(testpmd_cl);
-	if (ret != 0)
-		cmdline_stdin_exit(testpmd_cl);
-}
+	/* loop until signal or quit command */
+	while (f_quit == 0 && cl_quit == 0) {
+		int status = cmdline_poll(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);
 }
 
 void
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e366f81a0f46..60eb9579ded1 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>
@@ -231,7 +232,7 @@ 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.
  */
-static volatile uint8_t f_quit;
+volatile uint8_t f_quit;
 uint8_t cl_quit; /* Quit testpmd from cmdline. */
 
 /*
@@ -4315,13 +4316,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)
 {
@@ -4340,28 +4334,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
@@ -4541,15 +4516,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) {
@@ -4572,15 +4541,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 7d24d25970d2..022210a7a964 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -34,6 +34,7 @@
 #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.
-- 
2.39.0


  parent reply	other threads:[~2023-02-03 19:14 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-14 17:23 [RFC 1/2] testpmd: make f_quit flag volatile Stephen Hemminger
2022-10-14 17:23 ` [RFC 2/2] testpmd: cleanup cleanly from signal Stephen Hemminger
2022-11-06 10:50   ` Andrew Rybchenko
2022-11-08 18:16     ` Stephen Hemminger
2022-11-08 18:53   ` [PATCH v2] " Stephen Hemminger
2022-11-08 20:24   ` [PATCH v3] " Stephen Hemminger
2022-11-09  4:10   ` [PATCH v4] " Stephen Hemminger
2022-11-09 21:46     ` Mattias Rönnblom
2022-11-09 22:53       ` Stephen Hemminger
2022-11-10  7:50         ` Mattias Rönnblom
2022-11-10 16:14           ` Stephen Hemminger
2022-11-10 22:06             ` Mattias Rönnblom
2022-11-09 17:29   ` [PATCH v5] " Stephen Hemminger
2022-11-10  7:14     ` Andrew Rybchenko
2022-11-10 16:13       ` Stephen Hemminger
2022-11-10 16:53   ` [PATCH v6] " Stephen Hemminger
2022-11-11  8:05     ` Andrew Rybchenko
2022-11-11 16:49       ` Stephen Hemminger
2022-11-11 16:51   ` [PATCH v7] " Stephen Hemminger
2022-11-12 17:28   ` [PATCH v8] " Stephen Hemminger
2023-01-19 15:53     ` Ferruh Yigit
2023-01-25 18:32     ` [PATCH v9] " Stephen Hemminger
2023-01-30 18:48       ` Ferruh Yigit
2023-01-30 20:11         ` Stephen Hemminger
2022-11-06 10:48 ` [RFC 1/2] testpmd: make f_quit flag volatile Andrew Rybchenko
2022-11-08 18:07 ` [PATCH v2] " Stephen Hemminger
2022-11-09 10:11   ` Ruifeng Wang
2022-11-09 10:37     ` Andrew Rybchenko
2023-01-30 20:09 ` [PATCH v10 0/2] testpmd: handle signals safely Stephen Hemminger
2023-01-30 20:09   ` [PATCH v10 1/2] cmdline: handle EOF in cmdline_poll Stephen Hemminger
2023-01-30 22:12     ` Ferruh Yigit
2023-01-31  2:54       ` Stephen Hemminger
2023-01-30 20:09   ` [PATCH v10 2/2] testpmd: cleanup cleanly from signal Stephen Hemminger
2023-01-31  9:30     ` Ferruh Yigit
2023-01-30 22:13   ` [PATCH v10 0/2] testpmd: handle signals safely Ferruh Yigit
2023-02-03 19:14   ` [PATCH v11 0/3] Fix cmdline_poll and testpmd signal handling Stephen Hemminger
2023-02-03 19:14     ` [PATCH v11 1/3] cmdline: make rdline status not private Stephen Hemminger
2023-02-06  2:31       ` fengchengwen
2023-02-03 19:14     ` [PATCH v11 2/3] cmdline: handle EOF in cmdline_poll Stephen Hemminger
2023-02-03 19:14     ` Stephen Hemminger [this message]
2023-02-07 14:49       ` [PATCH v11 3/3] testpmd: cleanup cleanly from signal Ferruh Yigit
2023-02-07 14:48     ` [PATCH v11 0/3] Fix cmdline_poll and testpmd signal handling Ferruh Yigit
2023-02-19 17:53     ` Stephen Hemminger
2023-03-11 10:17       ` Thomas Monjalon
2023-03-12 17:18         ` Tal Shnaiderman
2023-03-13 10:34           ` Ling, WeiX
2023-03-13 15:53             ` Stephen Hemminger
2023-03-14  7:05               ` Ling, WeiX

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=20230203191409.97567-4-stephen@networkplumber.org \
    --to=stephen@networkplumber.org \
    --cc=dev@dpdk.org \
    --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).