[v11,3/3] testpmd: cleanup cleanly from signal
Checks
Commit Message
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(-)
Comments
On 2/3/2023 7:14 PM, Stephen Hemminger wrote:
> 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>
Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
@@ -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
@@ -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,
@@ -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.