On 2022-11-09 23:53, Stephen Hemminger wrote:
On Wed, 9 Nov 2022 22:46:55 +0100
Mattias Rönnblom <hof...@lysator.liu.se> wrote:
On 2022-11-09 05:10, Stephen Hemminger wrote:
Do a clean shutdown of testpmd when a signal is received;
instead of having testpmd kill itself.
This fixes problem where a signal could be received
in the middle of a PMD and then the signal handler would call
PMD's close routine which could cause a deadlock.
Added benefit is it gets rid of Windows specific code.
Fixes: d9a191a00e81 ("app/testpmd: fix quitting in container")
Signed-off-by: Stephen Hemminger <step...@networkplumber.org>
---
v4 - use select() because that is available on Windows; and other
functions poll() and sigaction() are not.
app/test-pmd/testpmd.c | 63 +++++++++++++++++++++++-------------------
1 file changed, 34 insertions(+), 29 deletions(-)
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index cf5942d0c422..274e96cac2d4 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -12,6 +12,7 @@
#ifndef RTE_EXEC_ENV_WINDOWS
#include <sys/mman.h>
#endif
+#include <sys/select.h>
#include <sys/types.h>
#include <errno.h>
#include <stdbool.h>
@@ -4251,26 +4252,11 @@ print_stats(void)
static void
signal_handler(int signum)
{
- 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
- }
+ fprintf(stderr, "\nSignal %d %s received, preparing to exit...\n",
+ signum, strsignal(signum));
fprintf() is not async signal safe, and neither is strsignal().
This is not a regression introduced by this patch, but I thought it
might be worth fixing.
+
+ /* Set flag to indicate the force termination. */
+ f_quit = 1;
}
int
@@ -4449,9 +4435,6 @@ main(int argc, char** argv)
} else
#endif
{
- char c;
- int rc;
-
f_quit = 0;
printf("No commandline core given, start packet forwarding\n");
@@ -4476,15 +4459,37 @@ 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");
- rc = read(0, &c, 1);
- pmd_test_exit();
- if (rc < 0)
- return 1;
+ printf("Press enter to exit\n");
+
+ FD_ZERO(&fds);
+ FD_SET(0, &fds);
+
+ if (select(1, &fds, NULL, NULL, NULL) <= 0) {
+ fprintf(stderr, "Select failed: %s\n",
+ strerror(errno));
Why is select() needed? Wouldn't a blocking read suffice? Or getchar().
On Linux, signal set SA_RESTART so a simple read is not interrupted.
One option was to use sigaction() which allows controlling flags, but that
won't work on Windows. Using select() works on both.
OK, so select() is used because a signal might interrupt read() on Windows?
while (read(0, &c, 1) == -1 && errno == EINTR)
;
Would that work?
(select() won't return 0 since you don't have a timeout.)
