diff mbox series

[v2,3/7] examples/l2fwd-crypto: add signal handler for exit

Message ID 20220517033858.40394-4-g.singh@nxp.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series Crypto related changes in sample/test apps |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Gagandeep Singh May 17, 2022, 3:38 a.m. UTC 
  Handle SIGINT and SIGTERM signals.

Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
---
 examples/l2fwd-crypto/main.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Stephen Hemminger May 17, 2022, 4:32 p.m. UTC | #1 
On Tue, 17 May 2022 09:08:54 +0530
Gagandeep Singh <g.singh@nxp.com> wrote:

> Handle SIGINT and SIGTERM signals.
> 
> Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> ---
>  examples/l2fwd-crypto/main.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
> index b1e2613ccf..0a1fc790fc 100644
> --- a/examples/l2fwd-crypto/main.c
> +++ b/examples/l2fwd-crypto/main.c
> @@ -18,6 +18,7 @@
>  #include <getopt.h>
>  #include <fcntl.h>
>  #include <unistd.h>
> +#include <signal.h>
>  
>  #include <rte_string_fns.h>
>  #include <rte_branch_prediction.h>
> @@ -256,6 +257,9 @@ struct l2fwd_crypto_statistics crypto_statistics[RTE_CRYPTO_MAX_DEVS];
>  #define MAX_TIMER_PERIOD 86400UL /* 1 day max */
>  #define DEFAULT_TIMER_PERIOD 10UL
>  
> +/* Global signal */
> +unsigned int signal_received;

This won't work as expected.

This kind of flag needs to either be volatile or use explicit atomic builtins
because the compiler and CPU are free to believe that it never changes.

Traditional way to address this would be:

static volatile bool signal_received;

More advanced way would be to use __atomic_load/store builtin.

Also printf() is not technically safe to call from a signal handler.
Gagandeep Singh May 18, 2022, 4:23 a.m. UTC | #2 
Hi,

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, May 17, 2022 10:02 PM
> To: Gagandeep Singh <G.Singh@nxp.com>
> Cc: gakhil@marvell.com; dev@dpdk.org
> Subject: Re: [PATCH v2 3/7] examples/l2fwd-crypto: add signal handler for exit
> 
> On Tue, 17 May 2022 09:08:54 +0530
> Gagandeep Singh <g.singh@nxp.com> wrote:
> 
> > Handle SIGINT and SIGTERM signals.
> >
> > Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> > ---
> >  examples/l2fwd-crypto/main.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/examples/l2fwd-crypto/main.c
> > b/examples/l2fwd-crypto/main.c index b1e2613ccf..0a1fc790fc 100644
> > --- a/examples/l2fwd-crypto/main.c
> > +++ b/examples/l2fwd-crypto/main.c
> > @@ -18,6 +18,7 @@
> >  #include <getopt.h>
> >  #include <fcntl.h>
> >  #include <unistd.h>
> > +#include <signal.h>
> >
> >  #include <rte_string_fns.h>
> >  #include <rte_branch_prediction.h>
> > @@ -256,6 +257,9 @@ struct l2fwd_crypto_statistics
> > crypto_statistics[RTE_CRYPTO_MAX_DEVS];
> >  #define MAX_TIMER_PERIOD 86400UL /* 1 day max */  #define
> > DEFAULT_TIMER_PERIOD 10UL
> >
> > +/* Global signal */
> > +unsigned int signal_received;
> 
> This won't work as expected.
> 
> This kind of flag needs to either be volatile or use explicit atomic builtins
> because the compiler and CPU are free to believe that it never changes.
> 
> Traditional way to address this would be:
> 
> static volatile bool signal_received;
> 
> More advanced way would be to use __atomic_load/store builtin.
> 
> Also printf() is not technically safe to call from a signal handler.

Ok, I will update it to "static volatile bool signal_received;" .
diff mbox series

Patch

diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
index b1e2613ccf..0a1fc790fc 100644
--- a/examples/l2fwd-crypto/main.c
+++ b/examples/l2fwd-crypto/main.c
@@ -18,6 +18,7 @@ 
 #include <getopt.h>
 #include <fcntl.h>
 #include <unistd.h>
+#include <signal.h>
 
 #include <rte_string_fns.h>
 #include <rte_branch_prediction.h>
@@ -256,6 +257,9 @@  struct l2fwd_crypto_statistics crypto_statistics[RTE_CRYPTO_MAX_DEVS];
 #define MAX_TIMER_PERIOD 86400UL /* 1 day max */
 #define DEFAULT_TIMER_PERIOD 10UL
 
+/* Global signal */
+unsigned int signal_received;
+
 /* Print out statistics on packets dropped */
 static void
 print_stats(void)
@@ -922,6 +926,8 @@  l2fwd_main_loop(struct l2fwd_crypto_options *options)
 
 			nb_rx = rte_eth_rx_burst(portid, 0,
 						 pkts_burst, MAX_PKT_BURST);
+			if (unlikely(signal_received))
+				return;
 
 			port_statistics[portid].rx += nb_rx;
 
@@ -2760,6 +2766,13 @@  reserve_key_memory(struct l2fwd_crypto_options *options)
 	options->aad.phys_addr = rte_malloc_virt2iova(options->aad.data);
 }
 
+static void
+raise_signal(int signum)
+{
+	signal_received = 1;
+	printf("Exiting on signal (%d)\n", signum);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -2772,6 +2785,9 @@  main(int argc, char **argv)
 	int ret, enabled_cdevcount, enabled_portcount;
 	uint8_t enabled_cdevs[RTE_CRYPTO_MAX_DEVS] = {0};
 
+	signal(SIGINT, raise_signal);
+	signal(SIGTERM, raise_signal);
+
 	/* init EAL */
 	ret = rte_eal_init(argc, argv);
 	if (ret < 0)