[2/2] examples/l2fwd-crypto: Free resources on exit
diff mbox series

Message ID 20200722125026.2595-2-ibtisam.tariq@emumba.com
State New
Delegated to: Thomas Monjalon
Headers show
Series
  • [1/2] examples/l2fwd-jobstats: Free resources on exit
Related show

Checks

Context Check Description
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Ibtisam Tariq July 22, 2020, 12:50 p.m. UTC
When exiting the application, resources should be cleared.

Signed-off-by: Ibtisam Tariq <ibtisam.tariq@emumba.com>
---
 examples/l2fwd-crypto/main.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

Comments

Thomas Monjalon July 30, 2020, 9:15 p.m. UTC | #1
Declan is not reviewing patches anymore.
Adding more maintainers in Cc.

PS: Maybe we should update the file MAINTAINERS
to avoid patches being stuck.


22/07/2020 14:50, Ibtisam Tariq:
> When exiting the application, resources should be cleared.
> 
> Signed-off-by: Ibtisam Tariq <ibtisam.tariq@emumba.com>
> ---
>  examples/l2fwd-crypto/main.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
> index 827da9b3e..5b00e2ade 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_atomic.h>
> @@ -47,6 +48,8 @@
>  #include <rte_cryptodev_scheduler.h>
>  #endif
>  
> +static volatile bool force_quit;
> +
>  enum cdev_type {
>  	CDEV_TYPE_ANY,
>  	CDEV_TYPE_HW,
> @@ -838,7 +841,7 @@ l2fwd_main_loop(struct l2fwd_crypto_options *options)
>  	 * so user can see the crypto information.
>  	 */
>  	prev_tsc = rte_rdtsc();
> -	while (1) {
> +	while (!force_quit) {
>  
>  		cur_tsc = rte_rdtsc();
>  
> @@ -1738,8 +1741,12 @@ check_all_ports_link_status(uint32_t port_mask)
>  	printf("\nChecking link status");
>  	fflush(stdout);
>  	for (count = 0; count <= MAX_CHECK_TIME; count++) {
> +		if (force_quit)
> +			return;
>  		all_ports_up = 1;
>  		RTE_ETH_FOREACH_DEV(portid) {
> +			if (force_quit)
> +				return;
>  			if ((port_mask & (1 << portid)) == 0)
>  				continue;
>  			memset(&link, 0, sizeof(link));
> @@ -2662,6 +2669,16 @@ reserve_key_memory(struct l2fwd_crypto_options *options)
>  	options->aad.phys_addr = rte_malloc_virt2iova(options->aad.data);
>  }
>  
> +static void
> +signal_handler(int signum)
> +{
> +	if (signum == SIGINT || signum == SIGTERM) {
> +		printf("\n\nSignal %d received, preparing to exit...\n",
> +				signum);
> +		force_quit = true;
> +	}
> +}
> +
>  int
>  main(int argc, char **argv)
>  {
> @@ -2681,6 +2698,10 @@ main(int argc, char **argv)
>  	argc -= ret;
>  	argv += ret;
>  
> +	force_quit = false;
> +	signal(SIGINT, signal_handler);
> +	signal(SIGTERM, signal_handler);
> +
>  	/* reserve memory for Cipher/Auth key and IV */
>  	reserve_key_memory(&options);
>  
> @@ -2807,6 +2828,14 @@ main(int argc, char **argv)
>  		if (rte_eal_wait_lcore(lcore_id) < 0)
>  			return -1;
>  	}
> +	RTE_ETH_FOREACH_DEV(portid) {
> +		printf("Closing port %d...", portid);
> +		rte_eth_dev_stop(portid);
> +		rte_eth_dev_close(portid);
> +		printf(" Done\n");
> +	}
> +	rte_eal_cleanup();
> +	printf("Bye...\n");
>  
>  	return 0;
>  }
>
Thomas Monjalon Nov. 22, 2020, 6:05 p.m. UTC | #2
30/07/2020 23:15, Thomas Monjalon:
> Declan is not reviewing patches anymore.
> Adding more maintainers in Cc.
> 
> PS: Maybe we should update the file MAINTAINERS
> to avoid patches being stuck.

No progress since July.

I will apply this series and all others not reviewed patches
on the same topic after 20.11, so the risk will be low.
Zhang, Roy Fan Nov. 23, 2020, 3:44 p.m. UTC | #3
Hi Ibtisam,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ibtisam Tariq
> Sent: Wednesday, July 22, 2020 1:50 PM
> To: Doherty, Declan <declan.doherty@intel.com>
> Cc: dev@dpdk.org; Ibtisam Tariq <ibtisam.tariq@emumba.com>
> Subject: [dpdk-dev] [PATCH 2/2] examples/l2fwd-crypto: Free resources on
> exit
> 
> When exiting the application, resources should be cleared.
> 
> Signed-off-by: Ibtisam Tariq <ibtisam.tariq@emumba.com>
> ---
>  examples/l2fwd-crypto/main.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
> index 827da9b3e..5b00e2ade 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_atomic.h>
> @@ -47,6 +48,8 @@
>  #include <rte_cryptodev_scheduler.h>
>  #endif
> 
> +static volatile bool force_quit;
> +
>  enum cdev_type {
>  	CDEV_TYPE_ANY,
>  	CDEV_TYPE_HW,
> @@ -838,7 +841,7 @@ l2fwd_main_loop(struct l2fwd_crypto_options
> *options)
>  	 * so user can see the crypto information.
>  	 */
>  	prev_tsc = rte_rdtsc();
> -	while (1) {
> +	while (!force_quit) {
> 
>  		cur_tsc = rte_rdtsc();
> 
> @@ -1738,8 +1741,12 @@ check_all_ports_link_status(uint32_t port_mask)
>  	printf("\nChecking link status");
>  	fflush(stdout);
>  	for (count = 0; count <= MAX_CHECK_TIME; count++) {
> +		if (force_quit)
> +			return;
>  		all_ports_up = 1;
>  		RTE_ETH_FOREACH_DEV(portid) {
> +			if (force_quit)
> +				return;
>  			if ((port_mask & (1 << portid)) == 0)
>  				continue;
>  			memset(&link, 0, sizeof(link));
> @@ -2662,6 +2669,16 @@ reserve_key_memory(struct
> l2fwd_crypto_options *options)
>  	options->aad.phys_addr = rte_malloc_virt2iova(options->aad.data);
>  }
> 
> +static void
> +signal_handler(int signum)
> +{
> +	if (signum == SIGINT || signum == SIGTERM) {
> +		printf("\n\nSignal %d received, preparing to exit...\n",
> +				signum);
> +		force_quit = true;
> +	}
> +}
> +
>  int
>  main(int argc, char **argv)
>  {
> @@ -2681,6 +2698,10 @@ main(int argc, char **argv)
>  	argc -= ret;
>  	argv += ret;
> 
> +	force_quit = false;
> +	signal(SIGINT, signal_handler);
> +	signal(SIGTERM, signal_handler);
> +
>  	/* reserve memory for Cipher/Auth key and IV */
>  	reserve_key_memory(&options);
> 
> @@ -2807,6 +2828,14 @@ main(int argc, char **argv)
>  		if (rte_eal_wait_lcore(lcore_id) < 0)
>  			return -1;
>  	}
> +	RTE_ETH_FOREACH_DEV(portid) {
> +		printf("Closing port %d...", portid);
> +		rte_eth_dev_stop(portid);
> +		rte_eth_dev_close(portid);
> +		printf(" Done\n");
> +	}
> +	rte_eal_cleanup();
> +	printf("Bye...\n");
> 
>  	return 0;
>  }
> --
> 2.17.1

Thanks for the patch and sorry for not reviewing for so long.
In addition to Ethdev resources, some Cryptodev resource may need to be cleaned too, e.g. draining all pending crypto operations from Cryptodev queue and stop/close the device before closing the application. For SW Crypto PMDs and QAT not doing the above steps are fine. But I am not sure about if it is a necessary step for other vendor's PMDs.

Other than that the patch looks good to me.

Akhil what do you think?

Regards,
Fan

Patch
diff mbox series

diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
index 827da9b3e..5b00e2ade 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_atomic.h>
@@ -47,6 +48,8 @@ 
 #include <rte_cryptodev_scheduler.h>
 #endif
 
+static volatile bool force_quit;
+
 enum cdev_type {
 	CDEV_TYPE_ANY,
 	CDEV_TYPE_HW,
@@ -838,7 +841,7 @@  l2fwd_main_loop(struct l2fwd_crypto_options *options)
 	 * so user can see the crypto information.
 	 */
 	prev_tsc = rte_rdtsc();
-	while (1) {
+	while (!force_quit) {
 
 		cur_tsc = rte_rdtsc();
 
@@ -1738,8 +1741,12 @@  check_all_ports_link_status(uint32_t port_mask)
 	printf("\nChecking link status");
 	fflush(stdout);
 	for (count = 0; count <= MAX_CHECK_TIME; count++) {
+		if (force_quit)
+			return;
 		all_ports_up = 1;
 		RTE_ETH_FOREACH_DEV(portid) {
+			if (force_quit)
+				return;
 			if ((port_mask & (1 << portid)) == 0)
 				continue;
 			memset(&link, 0, sizeof(link));
@@ -2662,6 +2669,16 @@  reserve_key_memory(struct l2fwd_crypto_options *options)
 	options->aad.phys_addr = rte_malloc_virt2iova(options->aad.data);
 }
 
+static void
+signal_handler(int signum)
+{
+	if (signum == SIGINT || signum == SIGTERM) {
+		printf("\n\nSignal %d received, preparing to exit...\n",
+				signum);
+		force_quit = true;
+	}
+}
+
 int
 main(int argc, char **argv)
 {
@@ -2681,6 +2698,10 @@  main(int argc, char **argv)
 	argc -= ret;
 	argv += ret;
 
+	force_quit = false;
+	signal(SIGINT, signal_handler);
+	signal(SIGTERM, signal_handler);
+
 	/* reserve memory for Cipher/Auth key and IV */
 	reserve_key_memory(&options);
 
@@ -2807,6 +2828,14 @@  main(int argc, char **argv)
 		if (rte_eal_wait_lcore(lcore_id) < 0)
 			return -1;
 	}
+	RTE_ETH_FOREACH_DEV(portid) {
+		printf("Closing port %d...", portid);
+		rte_eth_dev_stop(portid);
+		rte_eth_dev_close(portid);
+		printf(" Done\n");
+	}
+	rte_eal_cleanup();
+	printf("Bye...\n");
 
 	return 0;
 }