[RFC,2/2] testpmd: cleanup cleanly from signal
Checks
Commit Message
The original behavior of testpmd was to kill itself when
it received a SIGINT or SIGTERM. This makes it hard to use
testpmd in test automation where forwarding loop is started
and then stopped via SIGTERM.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
app/test-pmd/testpmd.c | 76 +++++++++++++++++++++++++-----------------
1 file changed, 46 insertions(+), 30 deletions(-)
Comments
On 10/14/22 20:23, Stephen Hemminger wrote:
> The original behavior of testpmd was to kill itself when
> it received a SIGINT or SIGTERM. This makes it hard to use
> testpmd in test automation where forwarding loop is started
> and then stopped via SIGTERM.
Can automatic stop it using SIGINT?
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
[snip]
On Sun, 6 Nov 2022 13:50:36 +0300
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote:
> On 10/14/22 20:23, Stephen Hemminger wrote:
> > The original behavior of testpmd was to kill itself when
> > it received a SIGINT or SIGTERM. This makes it hard to use
> > testpmd in test automation where forwarding loop is started
> > and then stopped via SIGTERM.
>
> Can automatic stop it using SIGINT?
Doesn't matter the testpmd code has same bug in SIGINT and SIGTERM.
The fundamental problem is that regular signals in Unix model are delivered
to the process and any thread may receive them.
In the case of testpmd, the signal may arrive inside some driver which
is doing some operations with hardware and even holding a lock.
Then signal_handler() is called. This code does operations that
may interact with driver and other parts of the system.
It is a broken way to handle this.
There are two ways to handle this problem. One way would be to block
the signals and use signalfd() and epoll() to handle them. This would
be more difficult and invasive in the testpmd logic.
The simpler way is to just set a flag and do the cleanup in the
main loop when the flag is detected.
@@ -15,6 +15,7 @@
#include <sys/types.h>
#include <errno.h>
#include <stdbool.h>
+#include <poll.h>
#include <sys/queue.h>
#include <sys/stat.h>
@@ -4243,25 +4244,37 @@ 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));
+
+ /* Set flag to indicate the force termination. */
+ f_quit = 1;
+}
+
+static int
+wait_for_input(void)
+{
+ struct pollfd pfd = {
+ .fd = STDIN_FILENO,
+ .events = POLLIN,
+ };
+ char c;
+
+ printf("Press enter to exit\n");
+ for (;;) {
+ if (f_quit)
+ return 0;
+
+ if (poll(&pfd, 1, -1) < 0) {
+ if (errno == EINTR)
+ continue;
+ return -1;
+ }
+
+ if (read(STDIN_FILENO, &c, 1) < 0)
+ return -1;
+
+ return 0;
}
}
@@ -4441,11 +4454,6 @@ main(int argc, char** argv)
} 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) {
@@ -4468,15 +4476,23 @@ main(int argc, char** argv)
prev_time = cur_time;
rte_delay_us_sleep(US_PER_S);
}
+ } else {
+ if (wait_for_input() < 0)
+ return 1;
}
-
- printf("Press enter to exit\n");
- rc = read(0, &c, 1);
- pmd_test_exit();
- if (rc < 0)
- return 1;
+ stop_packet_forwarding();
+ force_quit();
}
+#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,