DPDK patches and discussions
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: dev@dpdk.org
Cc: Stephen Hemminger <stephen@networkplumber.org>,
	Aman Singh <aman.deep.singh@intel.com>,
	Yuying Zhang <yuying.zhang@intel.com>,
	Phil Yang <phil.yang@arm.com>, Jianbo Liu <jianbo.liu@arm.com>
Subject: [PATCH v9] testpmd: cleanup cleanly from signal
Date: Wed, 25 Jan 2023 10:32:05 -0800	[thread overview]
Message-ID: <20230125183205.96554-1-stephen@networkplumber.org> (raw)
In-Reply-To: <20221112172839.70087-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.

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>
---
v9 - also handle the interactive case.
     use cmdine_poll() rather than signals and atexit

 app/test-pmd/cmdline.c | 28 +++++----------
 app/test-pmd/testpmd.c | 77 ++++++++++++++++++++----------------------
 app/test-pmd/testpmd.h |  1 +
 3 files changed, 46 insertions(+), 60 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b32dc8bfd445..fbe9ad694dee 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -65,7 +65,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);
@@ -12921,28 +12920,19 @@ 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);
-}
-
-void
-prompt_exit(void)
-{
-	if (testpmd_cl != NULL) {
-		cmdline_quit(testpmd_cl);
-		cmdline_stdin_exit(testpmd_cl);
+	while (f_quit == 0 && cl_quit == 0) {
+		if (cmdline_poll(cl) < 0)
+			break;
 	}
+
+	cmdline_quit(cl);
+	cmdline_stdin_exit(cl);
 }
 
 void
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 134d79a55547..d01b6105b7de 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
@@ -4536,15 +4511,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) {
@@ -4567,15 +4536,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-01-25 18:32 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     ` Stephen Hemminger [this message]
2023-01-30 18:48       ` [PATCH v9] " 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     ` [PATCH v11 3/3] testpmd: cleanup cleanly from signal Stephen Hemminger
2023-02-07 14:49       ` 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=20230125183205.96554-1-stephen@networkplumber.org \
    --to=stephen@networkplumber.org \
    --cc=aman.deep.singh@intel.com \
    --cc=dev@dpdk.org \
    --cc=jianbo.liu@arm.com \
    --cc=phil.yang@arm.com \
    --cc=yuying.zhang@intel.com \
    /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).