[dpdk-dev] [PATCH 00/14] cleanup resources on shutdown

DPDK patches and discussions
 help / color / mirror / Atom feed

* [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown
@ 2020-01-04  1:33 Stephen Hemminger
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 01/14] eal: log: close on cleanup Stephen Hemminger
                   ` (18 more replies)
  0 siblings, 19 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Recently started using valgrind with DPDK, and the results
are not clean.

The DPDK has a function that applications can use to tell it
to cleanup resources on shutdown (rte_eal_cleanup). But the
current coverage of that API is spotty. Many internal parts of
DPDK leave files and allocated memory behind.

This patch set is a start at getting the sub-parts of
DPDK to cleanup after themselves. These are the easier ones,
the harder and more critical ones are in the drivers
and the memory subsystem.

There are no visible API or ABI changes here.

Stephen Hemminger (14):
  eal: log: close on cleanup
  eal: log: free dynamic state on cleanup
  eal: alarm: close timerfd on eal cleanup
  eal: cleanup threads
  eal: intr: cleanup resources
  eal: mp: end the multiprocess thread during cleanup
  eal: interrupts close epoll fd on shutdown
  eal: vfio: cleanup the mp sync handle
  eal: close mem config on cleanup
  tap: close netlink socket on device close
  eal: cleanup plugins data
  ethdev: raise priority of old driver warning
  eal: hotplug: cleanup multiprocess resources
  eal: malloc: cleanup mp resources

 drivers/net/tap/rte_eth_tap.c               |  7 ++++-
 lib/librte_eal/common/eal_common_log.c      | 30 +++++++++++++++++-
 lib/librte_eal/common/eal_common_options.c  | 12 +++++++
 lib/librte_eal/common/eal_common_proc.c     | 17 +++++++---
 lib/librte_eal/common/eal_options.h         |  1 +
 lib/librte_eal/common/eal_private.h         | 30 ++++++++++++++++++
 lib/librte_eal/common/hotplug_mp.c          |  5 +++
 lib/librte_eal/common/hotplug_mp.h          |  6 ++++
 lib/librte_eal/common/malloc_heap.c         |  6 ++++
 lib/librte_eal/common/malloc_heap.h         |  3 ++
 lib/librte_eal/common/malloc_mp.c           | 12 +++++++
 lib/librte_eal/common/malloc_mp.h           |  3 ++
 lib/librte_eal/linux/eal/eal.c              | 28 +++++++++++++++++
 lib/librte_eal/linux/eal/eal_alarm.c        | 11 +++++++
 lib/librte_eal/linux/eal/eal_interrupts.c   | 35 ++++++++++++++++++---
 lib/librte_eal/linux/eal/eal_log.c          | 14 +++++++++
 lib/librte_eal/linux/eal/eal_vfio.h         |  1 +
 lib/librte_eal/linux/eal/eal_vfio_mp_sync.c |  8 +++++
 lib/librte_ethdev/rte_ethdev.c              |  2 +-
 19 files changed, 218 insertions(+), 13 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 01/14] eal: log: close on cleanup
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 02/14] eal: log: free dynamic state " Stephen Hemminger
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When application calls rte_eal_cleanup on shutdown,
the DPDK log should be closed and cleaned up.

Fixes: af75078fece3 ("first public release")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_common_log.c | 12 ++++++++++++
 lib/librte_eal/common/eal_private.h    | 13 +++++++++++++
 lib/librte_eal/linux/eal/eal.c         |  1 +
 lib/librte_eal/linux/eal/eal_log.c     | 14 ++++++++++++++
 4 files changed, 40 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index c0efd5214fa9..64d6e20947ed 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -463,3 +463,15 @@ eal_log_set_default(FILE *default_log)
 		"Debug dataplane logs available - lower performance\n");
 #endif
 }
+
+/*
+ * Called by envrionment-specific cleanup function.
+ */
+void
+eal_log_cleanup(void)
+{
+	if (default_log_stream) {
+		fclose(default_log_stream);
+		default_log_stream = NULL;
+	}
+}
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 8a9d493f0c54..fdd942d71a36 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -90,6 +90,12 @@ int rte_eal_memzone_init(void);
  */
 void eal_log_set_default(FILE *default_log);
 
+/**
+ * Common log cleanup function (private to eal).
+ * Closes the default log stream. Called from rte_eal_cleanup().
+ */
+void eal_log_cleanup(void);
+
 /**
  * Fill configuration with number of physical and logical processors
  *
@@ -151,6 +157,13 @@ int rte_eal_timer_init(void);
  */
 int rte_eal_log_init(const char *id, int facility);
 
+/**
+ * Close the default log stream
+ *
+ * This function is private to EAL.
+ */
+void rte_eal_log_cleanup(void);
+
 /**
  * Save the log regexp for later
  */
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index c4233ec3c8ac..a3381fd01a23 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -1327,6 +1327,7 @@ rte_eal_cleanup(void)
 	rte_service_finalize();
 	rte_mp_channel_cleanup();
 	eal_cleanup_config(&internal_config);
+	rte_eal_log_cleanup();
 	return 0;
 }
 
diff --git a/lib/librte_eal/linux/eal/eal_log.c b/lib/librte_eal/linux/eal/eal_log.c
index 9d02dddbed33..e1adbc91637a 100644
--- a/lib/librte_eal/linux/eal/eal_log.c
+++ b/lib/librte_eal/linux/eal/eal_log.c
@@ -37,8 +37,16 @@ console_log_write(__attribute__((unused)) void *c, const char *buf, size_t size)
 	return ret;
 }
 
+static int
+console_log_close(__attribute__((unused)) void *c)
+{
+	closelog();
+	return 0;
+}
+
 static cookie_io_functions_t console_log_func = {
 	.write = console_log_write,
+	.close = console_log_close,
 };
 
 /*
@@ -60,3 +68,9 @@ rte_eal_log_init(const char *id, int facility)
 
 	return 0;
 }
+
+void
+rte_eal_log_cleanup(void)
+{
+	eal_log_cleanup();
+}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 02/14] eal: log: free dynamic state on cleanup
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 01/14] eal: log: close on cleanup Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-04-25 16:26   ` David Marchand
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 03/14] eal: alarm: close timerfd on eal cleanup Stephen Hemminger
                   ` (16 subsequent siblings)
  18 siblings, 1 reply; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, olivier.matz

When rte_eal_cleanup is called, free all the memory
associated with dynamic log levels and types.

Fixes: c1b5fa94a46f ("eal: support dynamic log types")
Cc: olivier.matz@6wind.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_common_log.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 64d6e20947ed..7583bdc57619 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -14,6 +14,7 @@
 #include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_per_lcore.h>
+#include <rte_tailq.h>
 
 #include "eal_private.h"
 
@@ -54,7 +55,7 @@ struct log_cur_msg {
 };
 
 struct rte_log_dynamic_type {
-	const char *name;
+	char *name;
 	uint32_t loglevel;
 };
 
@@ -470,8 +471,23 @@ eal_log_set_default(FILE *default_log)
 void
 eal_log_cleanup(void)
 {
+	struct rte_eal_opt_loglevel *opt_ll, *tmp;
+	size_t i;
+
 	if (default_log_stream) {
 		fclose(default_log_stream);
 		default_log_stream = NULL;
 	}
+
+	TAILQ_FOREACH_SAFE(opt_ll, &opt_loglevel_list, next, tmp) {
+		free(opt_ll->pattern);
+		free(opt_ll);
+	}
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++)
+		free(rte_logs.dynamic_types[i].name);
+
+	rte_logs.dynamic_types_len = 0;
+	free(rte_logs.dynamic_types);
+	rte_logs.dynamic_types = NULL;
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 02/14] eal: log: free dynamic state on cleanup
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 02/14] eal: log: free dynamic state " Stephen Hemminger
@ 2020-04-25 16:26   ` David Marchand
  0 siblings, 0 replies; 74+ messages in thread
From: David Marchand @ 2020-04-25 16:26 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Olivier Matz

On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> When rte_eal_cleanup is called, free all the memory
> associated with dynamic log levels and types.
>
> Fixes: c1b5fa94a46f ("eal: support dynamic log types")
> Cc: olivier.matz@6wind.com
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_eal/common/eal_common_log.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> index 64d6e20947ed..7583bdc57619 100644
> --- a/lib/librte_eal/common/eal_common_log.c
> +++ b/lib/librte_eal/common/eal_common_log.c
> @@ -470,8 +471,23 @@ eal_log_set_default(FILE *default_log)
>  void
>  eal_log_cleanup(void)
>  {
> +       struct rte_eal_opt_loglevel *opt_ll, *tmp;
> +       size_t i;
> +
>         if (default_log_stream) {
>                 fclose(default_log_stream);
>                 default_log_stream = NULL;
>         }
> +
> +       TAILQ_FOREACH_SAFE(opt_ll, &opt_loglevel_list, next, tmp) {
> +               free(opt_ll->pattern);

In regexp case, we have a leak on the regexp buffer.
Fixed with:

+        if (opt_ll->pattern != NULL)
+            free(opt_ll->pattern);
+        else
+            regfree(&opt_ll->re_match);


> +               free(opt_ll);
> +       }
> +
> +       for (i = 0; i < rte_logs.dynamic_types_len; i++)
> +               free(rte_logs.dynamic_types[i].name);
> +
> +       rte_logs.dynamic_types_len = 0;
> +       free(rte_logs.dynamic_types);
> +       rte_logs.dynamic_types = NULL;
>  }
> --
> 2.20.1
>


-- 
David Marchand


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 03/14] eal: alarm: close timerfd on eal cleanup
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 01/14] eal: log: close on cleanup Stephen Hemminger
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 02/14] eal: log: free dynamic state " Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-04-25 16:32   ` David Marchand
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 04/14] eal: cleanup threads Stephen Hemminger
                   ` (15 subsequent siblings)
  18 siblings, 1 reply; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Calling rte_eal_cleanup() should cause DPDK to cleanup all
outstanding resources including file descriptors.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_private.h  |  7 +++++++
 lib/librte_eal/linux/eal/eal.c       |  1 +
 lib/librte_eal/linux/eal/eal_alarm.c | 11 +++++++++++
 3 files changed, 19 insertions(+)

diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index fdd942d71a36..38682e79827c 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -202,6 +202,13 @@ int rte_eal_intr_init(void);
  */
 int rte_eal_alarm_init(void);
 
+/**
+ * Cleanup alarm resources.
+ *
+ * This function is private to EAL.
+ */
+void rte_eal_alarm_cleanup(void);
+
 /**
  * Function is to check if the kernel module(like, vfio, vfio_iommu_type1,
  * etc.) loaded.
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index a3381fd01a23..a1b928820b11 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -1325,6 +1325,7 @@ rte_eal_cleanup(void)
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_memseg_walk(mark_freeable, NULL);
 	rte_service_finalize();
+	rte_eal_alarm_cleanup();
 	rte_mp_channel_cleanup();
 	eal_cleanup_config(&internal_config);
 	rte_eal_log_cleanup();
diff --git a/lib/librte_eal/linux/eal/eal_alarm.c b/lib/librte_eal/linux/eal/eal_alarm.c
index 0924c9205c84..e6aeee933ece 100644
--- a/lib/librte_eal/linux/eal/eal_alarm.c
+++ b/lib/librte_eal/linux/eal/eal_alarm.c
@@ -6,6 +6,7 @@
 #include <signal.h>
 #include <errno.h>
 #include <string.h>
+#include <unistd.h>
 #include <sys/queue.h>
 #include <sys/time.h>
 #include <sys/timerfd.h>
@@ -72,6 +73,16 @@ rte_eal_alarm_init(void)
 	return -1;
 }
 
+void
+rte_eal_alarm_cleanup(void)
+{
+	if (intr_handle.fd == -1)
+		return;
+
+	close(intr_handle.fd);
+	intr_handle.fd = -1;
+}
+
 static void
 eal_alarm_callback(void *arg __rte_unused)
 {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 03/14] eal: alarm: close timerfd on eal cleanup
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 03/14] eal: alarm: close timerfd on eal cleanup Stephen Hemminger
@ 2020-04-25 16:32   ` David Marchand
  0 siblings, 0 replies; 74+ messages in thread
From: David Marchand @ 2020-04-25 16:32 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Bruce Richardson

On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> Calling rte_eal_cleanup() should cause DPDK to cleanup all
> outstanding resources including file descriptors.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_eal/common/eal_private.h  |  7 +++++++
>  lib/librte_eal/linux/eal/eal.c       |  1 +
>  lib/librte_eal/linux/eal/eal_alarm.c | 11 +++++++++++
>  3 files changed, 19 insertions(+)

I won't merge this as my FreeBSD vm is broken but I suppose the bits
for FreeBSD would be:

diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index 540b7d38c5..582ff0920a 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -973,6 +973,7 @@ int
 rte_eal_cleanup(void)
 {
        rte_service_finalize();
+       rte_eal_alarm_cleanup();
        rte_mp_channel_cleanup();
        rte_trace_save();
        eal_trace_fini();
diff --git a/lib/librte_eal/freebsd/eal_alarm.c
b/lib/librte_eal/freebsd/eal_alarm.c
index c38b2e04f8..b2089d0b53 100644
--- a/lib/librte_eal/freebsd/eal_alarm.c
+++ b/lib/librte_eal/freebsd/eal_alarm.c
@@ -61,6 +61,16 @@ rte_eal_alarm_init(void)
        return 0;
 }

+void
+rte_eal_alarm_cleanup(void)
+{
+       if (intr_handle.fd == -1)
+               return;
+
+       close(intr_handle.fd);
+       intr_handle.fd = -1;
+}
+
 static inline int
 timespec_cmp(const struct timespec *now, const struct timespec *at)
 {


-- 
David Marchand


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 04/14] eal: cleanup threads
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (2 preceding siblings ...)
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 03/14] eal: alarm: close timerfd on eal cleanup Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-02-04 14:48   ` Aaron Conole
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 05/14] eal: intr: cleanup resources Stephen Hemminger
                   ` (14 subsequent siblings)
  18 siblings, 1 reply; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When rte_eal_cleanup is called it should stop all the child threads
and close the pipes between threads.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/linux/eal/eal.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index a1b928820b11..d98a2afe85da 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -1319,11 +1319,24 @@ mark_freeable(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
 int
 rte_eal_cleanup(void)
 {
+	int i;
+
 	/* if we're in a primary process, we need to mark hugepages as freeable
 	 * so that finalization can release them back to the system.
 	 */
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_memseg_walk(mark_freeable, NULL);
+
+	RTE_LCORE_FOREACH_SLAVE(i) {
+		pthread_cancel(lcore_config[i].thread_id);
+		pthread_join(lcore_config[i].thread_id, NULL);
+
+		close(lcore_config[i].pipe_master2slave[0]);
+		close(lcore_config[i].pipe_master2slave[1]);
+		close(lcore_config[i].pipe_slave2master[0]);
+		close(lcore_config[i].pipe_slave2master[1]);
+	}
+
 	rte_service_finalize();
 	rte_eal_alarm_cleanup();
 	rte_mp_channel_cleanup();
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 04/14] eal: cleanup threads
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 04/14] eal: cleanup threads Stephen Hemminger
@ 2020-02-04 14:48   ` Aaron Conole
  0 siblings, 0 replies; 74+ messages in thread
From: Aaron Conole @ 2020-02-04 14:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

Stephen Hemminger <stephen@networkplumber.org> writes:

> When rte_eal_cleanup is called it should stop all the child threads
> and close the pipes between threads.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 05/14] eal: intr: cleanup resources
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (3 preceding siblings ...)
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 04/14] eal: cleanup threads Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-04-25 16:49   ` David Marchand
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 06/14] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
                   ` (13 subsequent siblings)
  18 siblings, 1 reply; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When rte_eal_cleanup is called the interrupt thread and
associated resources should be cleaned up.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_private.h       | 10 ++++++++++
 lib/librte_eal/linux/eal/eal.c            |  1 +
 lib/librte_eal/linux/eal/eal_interrupts.c |  9 +++++++++
 3 files changed, 20 insertions(+)

diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 38682e79827c..c62f35d3ac0f 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -191,6 +191,16 @@ int rte_eal_tailqs_init(void);
  */
 int rte_eal_intr_init(void);
 
+/**
+ * Cleanup interrupt handling.
+ *
+ * This function is private to EAL.
+ *
+ * @return
+ *  0 on success, negative on error
+ */
+void rte_eal_intr_cleanup(void);
+
 /**
  * Init alarm mechanism. This is to allow a callback be called after
  * specific time.
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index d98a2afe85da..eb95f4f0c317 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -1338,6 +1338,7 @@ rte_eal_cleanup(void)
 	}
 
 	rte_service_finalize();
+	rte_eal_intr_cleanup();
 	rte_eal_alarm_cleanup();
 	rte_mp_channel_cleanup();
 	eal_cleanup_config(&internal_config);
diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
index 14ebb108cee9..fa08ac4171bd 100644
--- a/lib/librte_eal/linux/eal/eal_interrupts.c
+++ b/lib/librte_eal/linux/eal/eal_interrupts.c
@@ -1137,6 +1137,15 @@ rte_eal_intr_init(void)
 	return ret;
 }
 
+void
+rte_eal_intr_cleanup(void)
+{
+	pthread_cancel(intr_thread);
+	pthread_join(intr_thread, NULL);
+	close(intr_pipe.readfd);
+	close(intr_pipe.writefd);
+}
+
 static void
 eal_intr_proc_rxtx_intr(int fd, const struct rte_intr_handle *intr_handle)
 {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 05/14] eal: intr: cleanup resources
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 05/14] eal: intr: cleanup resources Stephen Hemminger
@ 2020-04-25 16:49   ` David Marchand
  2020-04-26 16:24     ` Stephen Hemminger
  0 siblings, 1 reply; 74+ messages in thread
From: David Marchand @ 2020-04-25 16:49 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> When rte_eal_cleanup is called the interrupt thread and
> associated resources should be cleaned up.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_eal/common/eal_private.h       | 10 ++++++++++
>  lib/librte_eal/linux/eal/eal.c            |  1 +
>  lib/librte_eal/linux/eal/eal_interrupts.c |  9 +++++++++
>  3 files changed, 20 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> index 38682e79827c..c62f35d3ac0f 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -191,6 +191,16 @@ int rte_eal_tailqs_init(void);
>   */
>  int rte_eal_intr_init(void);
>
> +/**
> + * Cleanup interrupt handling.
> + *
> + * This function is private to EAL.
> + *
> + * @return
> + *  0 on success, negative on error
> + */
> +void rte_eal_intr_cleanup(void);
> +
>  /**
>   * Init alarm mechanism. This is to allow a callback be called after
>   * specific time.
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index d98a2afe85da..eb95f4f0c317 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -1338,6 +1338,7 @@ rte_eal_cleanup(void)
>         }
>
>         rte_service_finalize();
> +       rte_eal_intr_cleanup();
>         rte_eal_alarm_cleanup();
>         rte_mp_channel_cleanup();
>         eal_cleanup_config(&internal_config);
> diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
> index 14ebb108cee9..fa08ac4171bd 100644
> --- a/lib/librte_eal/linux/eal/eal_interrupts.c
> +++ b/lib/librte_eal/linux/eal/eal_interrupts.c
> @@ -1137,6 +1137,15 @@ rte_eal_intr_init(void)
>         return ret;
>  }
>
> +void
> +rte_eal_intr_cleanup(void)
> +{
> +       pthread_cancel(intr_thread);
> +       pthread_join(intr_thread, NULL);
> +       close(intr_pipe.readfd);
> +       close(intr_pipe.writefd);

What happens to the intr_sources callbacks?
I am unsure we can expect the application to clean this before the eal cleanup.

It would be worth a followup patch.

> +}
> +
>  static void
>  eal_intr_proc_rxtx_intr(int fd, const struct rte_intr_handle *intr_handle)
>  {
> --
> 2.20.1
>


-- 
David Marchand


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 05/14] eal: intr: cleanup resources
  2020-04-25 16:49   ` David Marchand
@ 2020-04-26 16:24     ` Stephen Hemminger
  0 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-26 16:24 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

On Sat, 25 Apr 2020 18:49:23 +0200
David Marchand <david.marchand@redhat.com> wrote:

> >
> > +void
> > +rte_eal_intr_cleanup(void)
> > +{
> > +       pthread_cancel(intr_thread);
> > +       pthread_join(intr_thread, NULL);
> > +       close(intr_pipe.readfd);
> > +       close(intr_pipe.writefd);  
> 
> What happens to the intr_sources callbacks?
> I am unsure we can expect the application to clean this before the eal cleanup.
> 
> It would be worth a followup patch.

The callbacks should be not run after cleanup.
The goal was to cleanup outstanding system resources (as reported by valgrind)
on eal_cleanup

^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 06/14] eal: mp: end the multiprocess thread during cleanup
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (4 preceding siblings ...)
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 05/14] eal: intr: cleanup resources Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-04-27 15:37   ` Burakov, Anatoly
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 07/14] eal: interrupts close epoll fd on shutdown Stephen Hemminger
                   ` (12 subsequent siblings)
  18 siblings, 1 reply; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, qi.z.zhang, stable

When rte_eal_cleanup is called, all control threads should exit.
For the mp thread, this best handled by closing the mp_socket
and letting the thread see that.

This also fixes potential problems where the mp_socket gets
another hard error, and the thread runs away repeating itself
by reading the same error.

Fixes: 85d6815fa6d0 ("eal: close multi-process socket during cleanup")
Cc: qi.z.zhang@intel.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_common_proc.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
index 935e8fefeba8..f369d8bf6dd8 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -276,8 +276,17 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 	msgh.msg_control = control;
 	msgh.msg_controllen = sizeof(control);
 
+retry:
 	msglen = recvmsg(mp_fd, &msgh, 0);
+
+	/* zero length message means socket was closed */
+	if (msglen == 0)
+		return 0;
+
 	if (msglen < 0) {
+		if (errno == EINTR)
+			goto retry;
+
 		RTE_LOG(ERR, EAL, "recvmsg failed, %s\n", strerror(errno));
 		return -1;
 	}
@@ -305,7 +314,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 		RTE_LOG(ERR, EAL, "invalid received data length\n");
 		return -1;
 	}
-	return 0;
+	return msglen;
 }
 
 static void
@@ -376,10 +385,8 @@ mp_handle(void *arg __rte_unused)
 	struct mp_msg_internal msg;
 	struct sockaddr_un sa;
 
-	while (1) {
-		if (read_msg(&msg, &sa) == 0)
-			process_msg(&msg, &sa);
-	}
+	while (read_msg(&msg, &sa) > 0)
+		process_msg(&msg, &sa);
 
 	return NULL;
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 06/14] eal: mp: end the multiprocess thread during cleanup
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 06/14] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
@ 2020-04-27 15:37   ` Burakov, Anatoly
  0 siblings, 0 replies; 74+ messages in thread
From: Burakov, Anatoly @ 2020-04-27 15:37 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: qi.z.zhang, stable

On 04-Jan-20 1:33 AM, Stephen Hemminger wrote:
> When rte_eal_cleanup is called, all control threads should exit.
> For the mp thread, this best handled by closing the mp_socket
> and letting the thread see that.
> 
> This also fixes potential problems where the mp_socket gets
> another hard error, and the thread runs away repeating itself
> by reading the same error.
> 
> Fixes: 85d6815fa6d0 ("eal: close multi-process socket during cleanup")
> Cc: qi.z.zhang@intel.com
> Cc: stable@dpdk.org
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 07/14] eal: interrupts close epoll fd on shutdown
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (5 preceding siblings ...)
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 06/14] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 08/14] eal: vfio: cleanup the mp sync handle Stephen Hemminger
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Use pthread callbacks to ensure that the epoll fd is closed
when rte_eal_cleanup is called.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/linux/eal/eal_interrupts.c | 26 ++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
index fa08ac4171bd..ae4d3ae3ed60 100644
--- a/lib/librte_eal/linux/eal/eal_interrupts.c
+++ b/lib/librte_eal/linux/eal/eal_interrupts.c
@@ -1032,6 +1032,19 @@ eal_intr_handle_interrupts(int pfd, unsigned totalfds)
 	}
 }
 
+
+/**
+ * Callback at end of intr_thread loop or if thread is cancelled
+ * that closes the epoll file descriptor
+ */
+static void
+eal_intr_thread_close(void *arg)
+{
+	int pfd = (int)(unsigned long)arg;
+
+	close(pfd);
+}
+
 /**
  * It builds/rebuilds up the epoll file descriptor with all the
  * file descriptors being waited on. Then handles the interrupts.
@@ -1061,6 +1074,12 @@ eal_intr_thread_main(__rte_unused void *arg)
 		if (pfd < 0)
 			rte_panic("Cannot create epoll instance\n");
 
+		/* close pfd on thread exit or cancel_pop
+		 * see man page for restrictions on this macro.
+		 */
+		pthread_cleanup_push(eal_intr_thread_close,
+				     (void *)(unsigned long)pfd);
+
 		pipe_event.data.fd = intr_pipe.readfd;
 		/**
 		 * add pipe fd into wait list, this pipe is used to
@@ -1100,11 +1119,8 @@ eal_intr_thread_main(__rte_unused void *arg)
 		/* serve the interrupt */
 		eal_intr_handle_interrupts(pfd, numfds);
 
-		/**
-		 * when we return, we need to rebuild the
-		 * list of fds to monitor.
-		 */
-		close(pfd);
+		/* close pfd and rebuild the list */
+		pthread_cleanup_pop(1);
 	}
 }
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 08/14] eal: vfio: cleanup the mp sync handle
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (6 preceding siblings ...)
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 07/14] eal: interrupts close epoll fd on shutdown Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-04-27 12:12   ` Burakov, Anatoly
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 09/14] eal: close mem config on cleanup Stephen Hemminger
                   ` (10 subsequent siblings)
  18 siblings, 1 reply; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, anatoly.burakov, stable

When rte_eal_cleanup is called the rte_mp_action for VFIO
should be freed.

Fixes: edf73dd33072 ("ipc: handle unsupported IPC in action register")
Cc: anatoly.burakov@intel.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/linux/eal/eal.c              | 3 +++
 lib/librte_eal/linux/eal/eal_vfio.h         | 1 +
 lib/librte_eal/linux/eal/eal_vfio_mp_sync.c | 8 ++++++++
 3 files changed, 12 insertions(+)

diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index eb95f4f0c317..9ad81378f23c 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -1338,6 +1338,9 @@ rte_eal_cleanup(void)
 	}
 
 	rte_service_finalize();
+#ifdef VFIO_PRESENT
+	vfio_mp_sync_cleanup();
+#endif
 	rte_eal_intr_cleanup();
 	rte_eal_alarm_cleanup();
 	rte_mp_channel_cleanup();
diff --git a/lib/librte_eal/linux/eal/eal_vfio.h b/lib/librte_eal/linux/eal/eal_vfio.h
index cb2d35fb1206..bf7408a897a7 100644
--- a/lib/librte_eal/linux/eal/eal_vfio.h
+++ b/lib/librte_eal/linux/eal/eal_vfio.h
@@ -132,6 +132,7 @@ int
 vfio_has_supported_extensions(int vfio_container_fd);
 
 int vfio_mp_sync_setup(void);
+void vfio_mp_sync_cleanup(void);
 
 #define EAL_VFIO_MP "eal_vfio_mp_sync"
 
diff --git a/lib/librte_eal/linux/eal/eal_vfio_mp_sync.c b/lib/librte_eal/linux/eal/eal_vfio_mp_sync.c
index 5f2a5fc1d94e..b8ae9c65892e 100644
--- a/lib/librte_eal/linux/eal/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linux/eal/eal_vfio_mp_sync.c
@@ -120,4 +120,12 @@ vfio_mp_sync_setup(void)
 	return 0;
 }
 
+void
+vfio_mp_sync_cleanup(void)
+{
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return;
+
+	rte_mp_action_unregister(EAL_VFIO_MP);
+}
 #endif
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 08/14] eal: vfio: cleanup the mp sync handle
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 08/14] eal: vfio: cleanup the mp sync handle Stephen Hemminger
@ 2020-04-27 12:12   ` Burakov, Anatoly
  0 siblings, 0 replies; 74+ messages in thread
From: Burakov, Anatoly @ 2020-04-27 12:12 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: stable

On 04-Jan-20 1:33 AM, Stephen Hemminger wrote:
> When rte_eal_cleanup is called the rte_mp_action for VFIO
> should be freed.
> 
> Fixes: edf73dd33072 ("ipc: handle unsupported IPC in action register")
> Cc: anatoly.burakov@intel.com
> Cc: stable@dpdk.org
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 09/14] eal: close mem config on cleanup
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (7 preceding siblings ...)
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 08/14] eal: vfio: cleanup the mp sync handle Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-04-27 12:12   ` Burakov, Anatoly
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 10/14] tap: close netlink socket on device close Stephen Hemminger
                   ` (9 subsequent siblings)
  18 siblings, 1 reply; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Resolves file descriptor left open after rte_eal_cleanup.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/linux/eal/eal.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index 9ad81378f23c..e5c2a24322e9 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -1346,6 +1346,12 @@ rte_eal_cleanup(void)
 	rte_mp_channel_cleanup();
 	eal_cleanup_config(&internal_config);
 	rte_eal_log_cleanup();
+
+	if (mem_cfg_fd != -1) {
+		close(mem_cfg_fd);
+		mem_cfg_fd = -1;
+	}
+
 	return 0;
 }
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 09/14] eal: close mem config on cleanup
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 09/14] eal: close mem config on cleanup Stephen Hemminger
@ 2020-04-27 12:12   ` Burakov, Anatoly
  2020-04-27 17:00     ` Stephen Hemminger
  0 siblings, 1 reply; 74+ messages in thread
From: Burakov, Anatoly @ 2020-04-27 12:12 UTC (permalink / raw)
  To: Stephen Hemminger, dev

On 04-Jan-20 1:33 AM, Stephen Hemminger wrote:
> Resolves file descriptor left open after rte_eal_cleanup.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>   lib/librte_eal/linux/eal/eal.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index 9ad81378f23c..e5c2a24322e9 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -1346,6 +1346,12 @@ rte_eal_cleanup(void)
>   	rte_mp_channel_cleanup();
>   	eal_cleanup_config(&internal_config);
>   	rte_eal_log_cleanup();
> +
> +	if (mem_cfg_fd != -1) {
> +		close(mem_cfg_fd);
> +		mem_cfg_fd = -1;
> +	}
> +
>   	return 0;
>   }
>   
> 

For the patch,

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

However i think it's incomplete, as there are also memory-backing 
fbarrays that are still mapped. Also, secondary processes have their own 
shadow copies of the master page table located in the mem config, so 
those should be destroyed on cleanup too.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 09/14] eal: close mem config on cleanup
  2020-04-27 12:12   ` Burakov, Anatoly
@ 2020-04-27 17:00     ` Stephen Hemminger
  2020-04-28  9:20       ` Burakov, Anatoly
  0 siblings, 1 reply; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-27 17:00 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev

On Mon, 27 Apr 2020 13:12:32 +0100
"Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:

> On 04-Jan-20 1:33 AM, Stephen Hemminger wrote:
> > Resolves file descriptor left open after rte_eal_cleanup.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >   lib/librte_eal/linux/eal/eal.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> > index 9ad81378f23c..e5c2a24322e9 100644
> > --- a/lib/librte_eal/linux/eal/eal.c
> > +++ b/lib/librte_eal/linux/eal/eal.c
> > @@ -1346,6 +1346,12 @@ rte_eal_cleanup(void)
> >   	rte_mp_channel_cleanup();
> >   	eal_cleanup_config(&internal_config);
> >   	rte_eal_log_cleanup();
> > +
> > +	if (mem_cfg_fd != -1) {
> > +		close(mem_cfg_fd);
> > +		mem_cfg_fd = -1;
> > +	}
> > +
> >   	return 0;
> >   }
> >   
> >   
> 
> For the patch,
> 
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> However i think it's incomplete, as there are also memory-backing 
> fbarrays that are still mapped. Also, secondary processes have their own 
> shadow copies of the master page table located in the mem config, so 
> those should be destroyed on cleanup too.
> 

This patch set was targeting things in stages. It is not complete, some of the
cleanups would be hard to do. Just getting the obvious things first.

^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 09/14] eal: close mem config on cleanup
  2020-04-27 17:00     ` Stephen Hemminger
@ 2020-04-28  9:20       ` Burakov, Anatoly
  0 siblings, 0 replies; 74+ messages in thread
From: Burakov, Anatoly @ 2020-04-28  9:20 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On 27-Apr-20 6:00 PM, Stephen Hemminger wrote:
> On Mon, 27 Apr 2020 13:12:32 +0100
> "Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:
> 
>> On 04-Jan-20 1:33 AM, Stephen Hemminger wrote:
>>> Resolves file descriptor left open after rte_eal_cleanup.
>>>
>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>>> ---
>>>    lib/librte_eal/linux/eal/eal.c | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
>>> index 9ad81378f23c..e5c2a24322e9 100644
>>> --- a/lib/librte_eal/linux/eal/eal.c
>>> +++ b/lib/librte_eal/linux/eal/eal.c
>>> @@ -1346,6 +1346,12 @@ rte_eal_cleanup(void)
>>>    	rte_mp_channel_cleanup();
>>>    	eal_cleanup_config(&internal_config);
>>>    	rte_eal_log_cleanup();
>>> +
>>> +	if (mem_cfg_fd != -1) {
>>> +		close(mem_cfg_fd);
>>> +		mem_cfg_fd = -1;
>>> +	}
>>> +
>>>    	return 0;
>>>    }
>>>    
>>>    
>>
>> For the patch,
>>
>> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>
>> However i think it's incomplete, as there are also memory-backing
>> fbarrays that are still mapped. Also, secondary processes have their own
>> shadow copies of the master page table located in the mem config, so
>> those should be destroyed on cleanup too.
>>
> 
> This patch set was targeting things in stages. It is not complete, some of the
> cleanups would be hard to do. Just getting the obvious things first.
> 

Fair enough. You still got my ack :)

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 10/14] tap: close netlink socket on device close
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (8 preceding siblings ...)
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 09/14] eal: close mem config on cleanup Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-04-25 15:52   ` David Marchand
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 11/14] eal: cleanup plugins data Stephen Hemminger
                   ` (8 subsequent siblings)
  18 siblings, 1 reply; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, pascal.mazon, stable

The netlink socket for flow creation was left open and never
closed.

Fixes: bf7b7f437b49 ("net/tap: create netdevice during probing")
Cc: pascal.mazon@6wind.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/rte_eth_tap.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index a13d8d50d7d7..d293dd8eeed5 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1035,6 +1035,11 @@ tap_dev_close(struct rte_eth_dev *dev)
 				&internals->remote_initial_flags);
 	}
 
+	if (internals->nlsk_fd != -1) {
+		close(internals->nlsk_fd);
+		internals->nlsk_fd = -1;
+	}
+
 	if (internals->ka_fd != -1) {
 		close(internals->ka_fd);
 		internals->ka_fd = -1;
@@ -2406,7 +2411,7 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev)
 	TAP_LOG(DEBUG, "Closing %s Ethernet device on numa %u",
 		tuntap_types[internals->type], rte_socket_id());
 
-	if (internals->nlsk_fd) {
+	if (internals->nlsk_fd != -1) {
 		tap_flow_flush(eth_dev, NULL);
 		tap_flow_implicit_flush(internals, NULL);
 		tap_nl_final(internals->nlsk_fd);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 10/14] tap: close netlink socket on device close
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 10/14] tap: close netlink socket on device close Stephen Hemminger
@ 2020-04-25 15:52   ` David Marchand
  0 siblings, 0 replies; 74+ messages in thread
From: David Marchand @ 2020-04-25 15:52 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, dpdk stable

On Sat, Jan 4, 2020 at 2:35 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> The netlink socket for flow creation was left open and never
> closed.
>
> Fixes: bf7b7f437b49 ("net/tap: create netdevice during probing")
> Cc: pascal.mazon@6wind.com
> Cc: stable@dpdk.org
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Afaics, superseded by the "fixes for tap" series recently merged.
http://patchwork.dpdk.org/cover/68602/


-- 
David Marchand


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 11/14] eal: cleanup plugins data
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (9 preceding siblings ...)
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 10/14] tap: close netlink socket on device close Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 12/14] ethdev: raise priority of old driver warning Stephen Hemminger
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When rte_eal_cleanup is called walk through the list of shared
objects loaded, and close them and free the data structure.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_common_options.c | 12 ++++++++++++
 lib/librte_eal/common/eal_options.h        |  1 +
 lib/librte_eal/linux/eal/eal.c             |  1 +
 3 files changed, 14 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index a7f9c5f9bdfa..58c364d896f8 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -308,6 +308,18 @@ eal_plugins_init(void)
 	return 0;
 }
 
+void
+eal_plugins_cleanup(void)
+{
+	struct shared_driver *solib, *tmp;
+
+	TAILQ_FOREACH_SAFE(solib, &solib_list, next, tmp) {
+		if (solib->lib_handle)
+			dlclose(solib->lib_handle);
+		free(solib);
+	}
+}
+
 /*
  * Parse the coremask given as argument (hexadecimal string) and fill
  * the global configuration (core role and core count) with the parsed
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index 9855429e5813..67d3ba0d4491 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -84,5 +84,6 @@ int eal_check_common_options(struct internal_config *internal_cfg);
 void eal_common_usage(void);
 enum rte_proc_type_t eal_proc_type_detect(void);
 int eal_plugins_init(void);
+void eal_plugins_cleanup(void);
 
 #endif /* EAL_OPTIONS_H */
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index e5c2a24322e9..9a00a3ed43ab 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -1344,6 +1344,7 @@ rte_eal_cleanup(void)
 	rte_eal_intr_cleanup();
 	rte_eal_alarm_cleanup();
 	rte_mp_channel_cleanup();
+	eal_plugins_cleanup();
 	eal_cleanup_config(&internal_config);
 	rte_eal_log_cleanup();
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 12/14] ethdev: raise priority of old driver warning
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (10 preceding siblings ...)
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 11/14] eal: cleanup plugins data Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-04-25 15:53   ` Thomas Monjalon
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 13/14] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
                   ` (6 subsequent siblings)
  18 siblings, 1 reply; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The priority of the message about drivers not using new (correct)
behaviour on close was debug. And debug messages are typically surpressed
and never seen.  Raise the priority so that broken drivers are visible
and hopefully get developers to fix.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_ethdev/rte_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 6e9cb243ea80..545687dbc5c9 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1717,7 +1717,7 @@ rte_eth_dev_close(uint16_t port_id)
 		rte_eth_dev_release_port(dev);
 		return;
 	}
-	RTE_ETHDEV_LOG(DEBUG, "Port closing is using an old behaviour.\n"
+	RTE_ETHDEV_LOG(NOTICE, "Port closing is using an old behaviour.\n"
 			"The driver %s should migrate to the new behaviour.\n",
 			dev->device->driver->name);
 	/* old behaviour: only free queue arrays */
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 12/14] ethdev: raise priority of old driver warning
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 12/14] ethdev: raise priority of old driver warning Stephen Hemminger
@ 2020-04-25 15:53   ` Thomas Monjalon
  0 siblings, 0 replies; 74+ messages in thread
From: Thomas Monjalon @ 2020-04-25 15:53 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, ferruh.yigit, arybchenko, david.marchand

04/01/2020 02:33, Stephen Hemminger:
> The priority of the message about drivers not using new (correct)
> behaviour on close was debug. And debug messages are typically surpressed
> and never seen.  Raise the priority so that broken drivers are visible
> and hopefully get developers to fix.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -1717,7 +1717,7 @@ rte_eth_dev_close(uint16_t port_id)
> -	RTE_ETHDEV_LOG(DEBUG, "Port closing is using an old behaviour.\n"
> +	RTE_ETHDEV_LOG(NOTICE, "Port closing is using an old behaviour.\n"

Acked-by: Thomas Monjalon <thomas@monjalon.net>

PS: I did not notice this patch earlier.
Please, next time, Cc maintainers with this git-send-email option:
	--cc-cmd devtools/get-maintainer.sh



^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 13/14] eal: hotplug: cleanup multiprocess resources
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (11 preceding siblings ...)
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 12/14] ethdev: raise priority of old driver warning Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 14/14] eal: malloc: cleanup mp resources Stephen Hemminger
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When rte_eal_cleanup is called, hotplug should unregister the
resources associated with the multi-process server.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/hotplug_mp.c | 5 +++++
 lib/librte_eal/common/hotplug_mp.h | 6 ++++++
 lib/librte_eal/linux/eal/eal.c     | 1 +
 3 files changed, 12 insertions(+)

diff --git a/lib/librte_eal/common/hotplug_mp.c b/lib/librte_eal/common/hotplug_mp.c
index ee791903b3b7..a390c01fd41c 100644
--- a/lib/librte_eal/common/hotplug_mp.c
+++ b/lib/librte_eal/common/hotplug_mp.c
@@ -463,3 +463,8 @@ int eal_mp_dev_hotplug_init(void)
 
 	return 0;
 }
+
+void eal_mp_dev_hotplug_cleanup(void)
+{
+	rte_mp_action_unregister(EAL_DEV_MP_ACTION_REQUEST);
+}
diff --git a/lib/librte_eal/common/hotplug_mp.h b/lib/librte_eal/common/hotplug_mp.h
index 8fcf9b52e24c..4848446c852d 100644
--- a/lib/librte_eal/common/hotplug_mp.h
+++ b/lib/librte_eal/common/hotplug_mp.h
@@ -37,6 +37,12 @@ struct eal_dev_mp_req {
 int
 eal_mp_dev_hotplug_init(void);
 
+/**
+ * Unregister all mp action callbacks for hotplug.
+ */
+void
+eal_mp_dev_hotplug_cleanup(void);
+
 /**
  * This is a synchronous wrapper for secondary process send
  * request to primary process, this is invoked when an attach
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index 9a00a3ed43ab..ef04defbeaa4 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -1343,6 +1343,7 @@ rte_eal_cleanup(void)
 #endif
 	rte_eal_intr_cleanup();
 	rte_eal_alarm_cleanup();
+	eal_mp_dev_hotplug_cleanup();
 	rte_mp_channel_cleanup();
 	eal_plugins_cleanup();
 	eal_cleanup_config(&internal_config);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH 14/14] eal: malloc: cleanup mp resources
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (12 preceding siblings ...)
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 13/14] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
@ 2020-01-04  1:33 ` Stephen Hemminger
  2020-04-27 12:10   ` Burakov, Anatoly
  2020-02-05  9:32 ` [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown David Marchand
                   ` (4 subsequent siblings)
  18 siblings, 1 reply; 74+ messages in thread
From: Stephen Hemminger @ 2020-01-04  1:33 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The mp action resources in malloc should be cleaned up via
rte_eal_cleanup.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/malloc_heap.c |  6 ++++++
 lib/librte_eal/common/malloc_heap.h |  3 +++
 lib/librte_eal/common/malloc_mp.c   | 12 ++++++++++++
 lib/librte_eal/common/malloc_mp.h   |  3 +++
 lib/librte_eal/linux/eal/eal.c      |  1 +
 5 files changed, 25 insertions(+)

diff --git a/lib/librte_eal/common/malloc_heap.c b/lib/librte_eal/common/malloc_heap.c
index 842eb9de75a1..13c673f363a9 100644
--- a/lib/librte_eal/common/malloc_heap.c
+++ b/lib/librte_eal/common/malloc_heap.c
@@ -1362,3 +1362,9 @@ rte_eal_malloc_heap_init(void)
 	/* add all IOVA-contiguous areas to the heap */
 	return rte_memseg_contig_walk(malloc_add_seg, NULL);
 }
+
+void
+rte_eal_malloc_heap_cleanup(void)
+{
+	unregister_mp_requests();
+}
diff --git a/lib/librte_eal/common/malloc_heap.h b/lib/librte_eal/common/malloc_heap.h
index 772736b53f3c..ffad1b61e246 100644
--- a/lib/librte_eal/common/malloc_heap.h
+++ b/lib/librte_eal/common/malloc_heap.h
@@ -100,6 +100,9 @@ malloc_socket_to_heap_id(unsigned int socket_id);
 int
 rte_eal_malloc_heap_init(void);
 
+void
+rte_eal_malloc_heap_cleanup(void);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/common/malloc_mp.c b/lib/librte_eal/common/malloc_mp.c
index 1f212f834993..a9a9e8a45221 100644
--- a/lib/librte_eal/common/malloc_mp.c
+++ b/lib/librte_eal/common/malloc_mp.c
@@ -749,3 +749,15 @@ register_mp_requests(void)
 	}
 	return 0;
 }
+
+void
+unregister_mp_requests(void)
+{
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		rte_mp_action_unregister(MP_ACTION_REQUEST);
+	} else {
+		rte_mp_action_unregister(MP_ACTION_SYNC);
+		rte_mp_action_unregister(MP_ACTION_ROLLBACK);
+		rte_mp_action_unregister(MP_ACTION_RESPONSE);
+	}
+}
diff --git a/lib/librte_eal/common/malloc_mp.h b/lib/librte_eal/common/malloc_mp.h
index 2b86b76f6848..fb3d18c4e458 100644
--- a/lib/librte_eal/common/malloc_mp.h
+++ b/lib/librte_eal/common/malloc_mp.h
@@ -63,6 +63,9 @@ struct malloc_mp_req {
 int
 register_mp_requests(void);
 
+void
+unregister_mp_requests(void);
+
 int
 request_to_primary(struct malloc_mp_req *req);
 
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index ef04defbeaa4..c660f6fac3f2 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -1344,6 +1344,7 @@ rte_eal_cleanup(void)
 	rte_eal_intr_cleanup();
 	rte_eal_alarm_cleanup();
 	eal_mp_dev_hotplug_cleanup();
+	rte_eal_malloc_heap_cleanup();
 	rte_mp_channel_cleanup();
 	eal_plugins_cleanup();
 	eal_cleanup_config(&internal_config);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 14/14] eal: malloc: cleanup mp resources
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 14/14] eal: malloc: cleanup mp resources Stephen Hemminger
@ 2020-04-27 12:10   ` Burakov, Anatoly
  0 siblings, 0 replies; 74+ messages in thread
From: Burakov, Anatoly @ 2020-04-27 12:10 UTC (permalink / raw)
  To: Stephen Hemminger, dev

On 04-Jan-20 1:33 AM, Stephen Hemminger wrote:
> The mp action resources in malloc should be cleaned up via
> rte_eal_cleanup.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (13 preceding siblings ...)
  2020-01-04  1:33 ` [dpdk-dev] [PATCH 14/14] eal: malloc: cleanup mp resources Stephen Hemminger
@ 2020-02-05  9:32 ` David Marchand
  2020-02-05 12:07   ` Stephen Hemminger
  2020-02-06 14:06 ` David Marchand
                   ` (3 subsequent siblings)
  18 siblings, 1 reply; 74+ messages in thread
From: David Marchand @ 2020-02-05  9:32 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Luca Boccassi, Bruce Richardson

Hello Stephen,

On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> Recently started using valgrind with DPDK, and the results
> are not clean.
>
> The DPDK has a function that applications can use to tell it
> to cleanup resources on shutdown (rte_eal_cleanup). But the
> current coverage of that API is spotty. Many internal parts of
> DPDK leave files and allocated memory behind.
>
> This patch set is a start at getting the sub-parts of
> DPDK to cleanup after themselves. These are the easier ones,
> the harder and more critical ones are in the drivers
> and the memory subsystem.
>
> There are no visible API or ABI changes here.

Could you share what you did to run a dpdk application with valgrind?

I tried with testpmd and a 3.15 valgrind (fc30), but I get an init
failure on the cpu flags.

$ LD_LIBRARY_PATH=/home/dmarchan/builds/build-gcc-shared/install/usr/local/lib64
valgrind /home/dmarchan/builds/build-gcc-shared/install/usr/local/bin/dpdk-testpmd
-c 3 --no-huge -m 20 -d librte_mempool_ring.so -d librte_pmd_null.so
-w 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall
--total-num-mbufs=2048 -ia
==10258== Memcheck, a memory error detector
==10258== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==10258== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==10258== Command:
/home/dmarchan/builds/build-gcc-shared/install/usr/local/bin/dpdk-testpmd
-c 3 --no-huge -m 20 -d librte_mempool_ring.so -d librte_pmd_null.so
-w 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall
--total-num-mbufs=2048 -ia
==10258==
ERROR: This system does not support "RDSEED".
Please check that RTE_MACHINE is set correctly.
EAL: FATAL: unsupported cpu type.
EAL: unsupported cpu type.
EAL: Error - exiting with code: 1
  Cause: Cannot init EAL: Operation not supported
==10258==
==10258== HEAP SUMMARY:
==10258==     in use at exit: 1,388 bytes in 49 blocks
==10258==   total heap usage: 97 allocs, 48 frees, 89,426 bytes allocated
==10258==
==10258== LEAK SUMMARY:
==10258==    definitely lost: 0 bytes in 0 blocks
==10258==    indirectly lost: 0 bytes in 0 blocks
==10258==      possibly lost: 0 bytes in 0 blocks
==10258==    still reachable: 1,388 bytes in 49 blocks
==10258==         suppressed: 0 bytes in 0 blocks
==10258== Rerun with --leak-check=full to see details of leaked memory
==10258==
==10258== For lists of detected and suppressed errors, rerun with: -s
==10258== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)


Thanks.

--
David Marchand


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown
  2020-02-05  9:32 ` [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown David Marchand
@ 2020-02-05 12:07   ` Stephen Hemminger
  2020-02-05 12:32     ` David Marchand
  0 siblings, 1 reply; 74+ messages in thread
From: Stephen Hemminger @ 2020-02-05 12:07 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Luca Boccassi, Bruce Richardson

On Wed, 5 Feb 2020 10:32:49 +0100
David Marchand <david.marchand@redhat.com> wrote:

> Hello Stephen,
> 
> On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > Recently started using valgrind with DPDK, and the results
> > are not clean.
> >
> > The DPDK has a function that applications can use to tell it
> > to cleanup resources on shutdown (rte_eal_cleanup). But the
> > current coverage of that API is spotty. Many internal parts of
> > DPDK leave files and allocated memory behind.
> >
> > This patch set is a start at getting the sub-parts of
> > DPDK to cleanup after themselves. These are the easier ones,
> > the harder and more critical ones are in the drivers
> > and the memory subsystem.
> >
> > There are no visible API or ABI changes here.  
> 
> Could you share what you did to run a dpdk application with valgrind?
> 
> I tried with testpmd and a 3.15 valgrind (fc30), but I get an init
> failure on the cpu flags.
> 
> $ LD_LIBRARY_PATH=/home/dmarchan/builds/build-gcc-shared/install/usr/local/lib64
> valgrind /home/dmarchan/builds/build-gcc-shared/install/usr/local/bin/dpdk-testpmd
> -c 3 --no-huge -m 20 -d librte_mempool_ring.so -d librte_pmd_null.so
> -w 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall
> --total-num-mbufs=2048 -ia
> ==10258== Memcheck, a memory error detector
> ==10258== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
> ==10258== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
> ==10258== Command:
> /home/dmarchan/builds/build-gcc-shared/install/usr/local/bin/dpdk-testpmd
> -c 3 --no-huge -m 20 -d librte_mempool_ring.so -d librte_pmd_null.so
> -w 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall
> --total-num-mbufs=2048 -ia
> ==10258==
> ERROR: This system does not support "RDSEED".
> Please check that RTE_MACHINE is set correctly.
> EAL: FATAL: unsupported cpu type.
> EAL: unsupported cpu type.
> EAL: Error - exiting with code: 1
>   Cause: Cannot init EAL: Operation not supported
> ==10258==
> ==10258== HEAP SUMMARY:
> ==10258==     in use at exit: 1,388 bytes in 49 blocks
> ==10258==   total heap usage: 97 allocs, 48 frees, 89,426 bytes allocated
> ==10258==
> ==10258== LEAK SUMMARY:
> ==10258==    definitely lost: 0 bytes in 0 blocks
> ==10258==    indirectly lost: 0 bytes in 0 blocks
> ==10258==      possibly lost: 0 bytes in 0 blocks
> ==10258==    still reachable: 1,388 bytes in 49 blocks
> ==10258==         suppressed: 0 bytes in 0 blocks
> ==10258== Rerun with --leak-check=full to see details of leaked memory
> ==10258==
> ==10258== For lists of detected and suppressed errors, rerun with: -s
> ==10258== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
> 

I am testing with valgrind on ARM.
It should be possible on x86 but you need to dial down the RTE_MACHINE
choice to something valgrind understands.


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown
  2020-02-05 12:07   ` Stephen Hemminger
@ 2020-02-05 12:32     ` David Marchand
  0 siblings, 0 replies; 74+ messages in thread
From: David Marchand @ 2020-02-05 12:32 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Luca Boccassi, Bruce Richardson

On Wed, Feb 5, 2020 at 1:07 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Wed, 5 Feb 2020 10:32:49 +0100
> David Marchand <david.marchand@redhat.com> wrote:
>
> > Hello Stephen,
> >
> > On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> > >
> > > Recently started using valgrind with DPDK, and the results
> > > are not clean.
> > >
> > > The DPDK has a function that applications can use to tell it
> > > to cleanup resources on shutdown (rte_eal_cleanup). But the
> > > current coverage of that API is spotty. Many internal parts of
> > > DPDK leave files and allocated memory behind.
> > >
> > > This patch set is a start at getting the sub-parts of
> > > DPDK to cleanup after themselves. These are the easier ones,
> > > the harder and more critical ones are in the drivers
> > > and the memory subsystem.
> > >
> > > There are no visible API or ABI changes here.
> >
> > Could you share what you did to run a dpdk application with valgrind?
> >
> > I tried with testpmd and a 3.15 valgrind (fc30), but I get an init
> > failure on the cpu flags.
> >
> > $ LD_LIBRARY_PATH=/home/dmarchan/builds/build-gcc-shared/install/usr/local/lib64
> > valgrind /home/dmarchan/builds/build-gcc-shared/install/usr/local/bin/dpdk-testpmd
> > -c 3 --no-huge -m 20 -d librte_mempool_ring.so -d librte_pmd_null.so
> > -w 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall
> > --total-num-mbufs=2048 -ia
> > ==10258== Memcheck, a memory error detector
> > ==10258== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
> > ==10258== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
> > ==10258== Command:
> > /home/dmarchan/builds/build-gcc-shared/install/usr/local/bin/dpdk-testpmd
> > -c 3 --no-huge -m 20 -d librte_mempool_ring.so -d librte_pmd_null.so
> > -w 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall
> > --total-num-mbufs=2048 -ia
> > ==10258==
> > ERROR: This system does not support "RDSEED".
> > Please check that RTE_MACHINE is set correctly.
> > EAL: FATAL: unsupported cpu type.
> > EAL: unsupported cpu type.
> > EAL: Error - exiting with code: 1
> >   Cause: Cannot init EAL: Operation not supported
> > ==10258==
> > ==10258== HEAP SUMMARY:
> > ==10258==     in use at exit: 1,388 bytes in 49 blocks
> > ==10258==   total heap usage: 97 allocs, 48 frees, 89,426 bytes allocated
> > ==10258==
> > ==10258== LEAK SUMMARY:
> > ==10258==    definitely lost: 0 bytes in 0 blocks
> > ==10258==    indirectly lost: 0 bytes in 0 blocks
> > ==10258==      possibly lost: 0 bytes in 0 blocks
> > ==10258==    still reachable: 1,388 bytes in 49 blocks
> > ==10258==         suppressed: 0 bytes in 0 blocks
> > ==10258== Rerun with --leak-check=full to see details of leaked memory
> > ==10258==
> > ==10258== For lists of detected and suppressed errors, rerun with: -s
> > ==10258== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
> >
>
> I am testing with valgrind on ARM.
> It should be possible on x86 but you need to dial down the RTE_MACHINE
> choice to something valgrind understands.
>

Ok, so no black magic in valgrind :-)
Yeah I managed to run with the x86-default target we have in
test-meson-builds.sh.


Thanks.

--
David Marchand


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (14 preceding siblings ...)
  2020-02-05  9:32 ` [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown David Marchand
@ 2020-02-06 14:06 ` David Marchand
  2020-02-07 18:24   ` Stephen Hemminger
  2020-04-25 19:34 ` David Marchand
                   ` (2 subsequent siblings)
  18 siblings, 1 reply; 74+ messages in thread
From: David Marchand @ 2020-02-06 14:06 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> Recently started using valgrind with DPDK, and the results
> are not clean.
>
> The DPDK has a function that applications can use to tell it
> to cleanup resources on shutdown (rte_eal_cleanup). But the
> current coverage of that API is spotty. Many internal parts of
> DPDK leave files and allocated memory behind.
>
> This patch set is a start at getting the sub-parts of
> DPDK to cleanup after themselves. These are the easier ones,
> the harder and more critical ones are in the drivers
> and the memory subsystem.

I am too short on time to check and integrate this big series in rc2,
and from now it would be too risky to take in 20.02.
Can you respin it for 20.05 with FreeBSD fixes too?


Thanks.

-- 
David Marchand


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown
  2020-02-06 14:06 ` David Marchand
@ 2020-02-07 18:24   ` Stephen Hemminger
  0 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-02-07 18:24 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

On Thu, 6 Feb 2020 15:06:56 +0100
David Marchand <david.marchand@redhat.com> wrote:

> On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > Recently started using valgrind with DPDK, and the results
> > are not clean.
> >
> > The DPDK has a function that applications can use to tell it
> > to cleanup resources on shutdown (rte_eal_cleanup). But the
> > current coverage of that API is spotty. Many internal parts of
> > DPDK leave files and allocated memory behind.
> >
> > This patch set is a start at getting the sub-parts of
> > DPDK to cleanup after themselves. These are the easier ones,
> > the harder and more critical ones are in the drivers
> > and the memory subsystem.  
> 
> I am too short on time to check and integrate this big series in rc2,
> and from now it would be too risky to take in 20.02.
> Can you respin it for 20.05 with FreeBSD fixes too?

OK, but if this kind of patch can't be reviewed then
the DPDK process is still broken.

I don't see how FreeBSD matters here. It can be leaky but that
is ok.  I split it out to get review, then you complain it
is too big :-(


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (15 preceding siblings ...)
  2020-02-06 14:06 ` David Marchand
@ 2020-04-25 19:34 ` David Marchand
  2020-04-28 23:14 ` [dpdk-dev] [PATCH v2 0/9] eal: " Stephen Hemminger
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
  18 siblings, 0 replies; 74+ messages in thread
From: David Marchand @ 2020-04-25 19:34 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> Recently started using valgrind with DPDK, and the results
> are not clean.
>
> The DPDK has a function that applications can use to tell it
> to cleanup resources on shutdown (rte_eal_cleanup). But the
> current coverage of that API is spotty. Many internal parts of
> DPDK leave files and allocated memory behind.
>
> This patch set is a start at getting the sub-parts of
> DPDK to cleanup after themselves. These are the easier ones,
> the harder and more critical ones are in the drivers
> and the memory subsystem.
>
> There are no visible API or ABI changes here.

I was about to push the series (except patch 10), but I hit a crash
when passing an invalid option to test-null.sh.

Reproduced with:

Core was generated by
`/home/dmarchan/builds/x86_64-native-linux-gcc+shared+kmods/app/testpmd
-c 0x3 --log-level='.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007fd5231dba64 in pthread_cancel () from /usr/lib64/libpthread.so.0
Missing separate debuginfos, use: dnf debuginfo-install
elfutils-libelf-0.178-7.fc30.x86_64 glibc-2.29-28.fc30.x86_64
jansson-2.12-2.fc30.x86_64 libgcc-9.2.1-1.fc30.x86_64
libpcap-1.9.1-1.fc30.x86_64 numactl-libs-2.0.12-2.fc30.x86_64
zlib-1.2.11-19.fc30.x86_64
(gdb) bt full
#0  0x00007fd5231dba64 in pthread_cancel () from /usr/lib64/libpthread.so.0
No symbol table info available.
#1  0x00007fd52320c586 in rte_eal_cleanup () at
/home/dmarchan/dpdk/lib/librte_eal/linux/eal.c:1339
        i = 1
#2  0x00007fd523215f5e in rte_exit (exit_code=exit_code@entry=1,
format=format@entry=0x47ada4 "Cannot init EAL: %s\n") at
/home/dmarchan/dpdk/lib/librte_eal/linux/eal_debug.c:83
        ap = {{gp_offset = 24, fp_offset = 48, overflow_arg_area =
0x7ffecdf7aa70, reg_save_area = 0x7ffecdf7a9a0}}
#3  0x000000000043535b in main (argc=21, argv=0x7ffecdf7abc8) at
/home/dmarchan/dpdk/app/test-pmd/testpmd.c:3647
        diag = -1
        port_id = <optimized out>
        count = <optimized out>
        ret = <optimized out>
(gdb) f 1
#1  0x00007fd52320c586 in rte_eal_cleanup () at
/home/dmarchan/dpdk/lib/librte_eal/linux/eal.c:1339
1339            pthread_cancel(lcore_config[i].thread_id);
(gdb) p lcore_config[1].thread_id
$1 = 0

rte_eal_cleanup() is called from rte_exit() by testpmd.
But since rte_eal_init() failed at parsing, lcore_config[*].thread_id
are invalid, and we crash on pthread_cancel.
I have no quick idea to fix this, series postponed to rc2.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v2 0/9] eal: cleanup resources on shutdown
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (16 preceding siblings ...)
  2020-04-25 19:34 ` David Marchand
@ 2020-04-28 23:14 ` Stephen Hemminger
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 1/8] eal: log: close on cleanup Stephen Hemminger
                     ` (7 more replies)
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
  18 siblings, 8 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:14 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Started using valgrind with DPDK, and there are lots of leftover
memory and file descriptors. This makes it hard to find application
leaks versus DPDK leaks.

The DPDK has a function that applications can use to tell it
to cleanup resources on shutdown (rte_eal_cleanup). But the
current coverage of that API is spotty. Many internal parts of
DPDK leave files and allocated memory behind.

This patch set is a first step at getting the sub-parts of
DPDK to cleanup after themselves. These are the easier ones,
the harder and more critical ones are in the drivers
and the memory subsystem.

There are no new exposed API or ABI changes here.

v2
 - rebase after 20.05 file renames
 - incorporate review comment feedback
 - hold off some of the more involved patches for later

Stephen Hemminger (8):
  eal: log: close on cleanup
  eal: log: free dynamic state on cleanup
  eal: alarm: close file on cleanup
  eal: cleanup threads
  eal: mp: end the multiprocess thread during cleanup
  eal: vfio: cleanup the mp sync handle
  eal: hotplug: cleanup multiprocess resources
  eal: malloc: cleanup mp resources

 lib/librte_eal/common/eal_common_log.c  | 33 ++++++++++++++++++++++++-
 lib/librte_eal/common/eal_common_proc.c | 17 +++++++++----
 lib/librte_eal/common/eal_private.h     | 20 +++++++++++++++
 lib/librte_eal/common/hotplug_mp.c      |  5 ++++
 lib/librte_eal/common/hotplug_mp.h      |  6 +++++
 lib/librte_eal/common/malloc_heap.c     |  6 +++++
 lib/librte_eal/common/malloc_heap.h     |  3 +++
 lib/librte_eal/common/malloc_mp.c       | 12 +++++++++
 lib/librte_eal/common/malloc_mp.h       |  3 +++
 lib/librte_eal/freebsd/eal.c            |  1 +
 lib/librte_eal/freebsd/eal_alarm.c      | 10 ++++++++
 lib/librte_eal/linux/eal.c              | 20 +++++++++++++++
 lib/librte_eal/linux/eal_alarm.c        | 11 +++++++++
 lib/librte_eal/linux/eal_log.c          | 14 +++++++++++
 lib/librte_eal/linux/eal_vfio.h         |  1 +
 lib/librte_eal/linux/eal_vfio_mp_sync.c |  8 ++++++
 16 files changed, 164 insertions(+), 6 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v2 1/8] eal: log: close on cleanup
  2020-04-28 23:14 ` [dpdk-dev] [PATCH v2 0/9] eal: " Stephen Hemminger
@ 2020-04-28 23:14   ` Stephen Hemminger
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 2/8] eal: log: free dynamic state " Stephen Hemminger
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:14 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When application calls rte_eal_cleanup on shutdown,
the DPDK log should be closed and cleaned up.

Fixes: af75078fece3 ("first public release")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_common_log.c | 12 ++++++++++++
 lib/librte_eal/common/eal_private.h    | 13 +++++++++++++
 lib/librte_eal/linux/eal.c             |  1 +
 lib/librte_eal/linux/eal_log.c         | 14 ++++++++++++++
 4 files changed, 40 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index d7a5f9b6417a..43de798ae96a 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -479,3 +479,15 @@ eal_log_set_default(FILE *default_log)
 		"Debug dataplane logs available - lower performance\n");
 #endif
 }
+
+/*
+ * Called by environment-specific cleanup function.
+ */
+void
+eal_log_cleanup(void)
+{
+	if (default_log_stream) {
+		fclose(default_log_stream);
+		default_log_stream = NULL;
+	}
+}
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index ecf827914fdd..24ddfc6c53f4 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -89,6 +89,12 @@ int rte_eal_memzone_init(void);
  */
 void eal_log_set_default(FILE *default_log);
 
+/**
+ * Common log cleanup function (private to eal).
+ * Closes the default log stream. Called from rte_eal_cleanup().
+ */
+void eal_log_cleanup(void);
+
 /**
  * Fill configuration with number of physical and logical processors
  *
@@ -150,6 +156,13 @@ int rte_eal_timer_init(void);
  */
 int rte_eal_log_init(const char *id, int facility);
 
+/**
+ * Close the default log stream
+ *
+ * This function is private to EAL.
+ */
+void rte_eal_log_cleanup(void);
+
 /**
  * Save the log regexp for later
  */
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index aa72d3650929..73d2c98b012b 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1337,6 +1337,7 @@ rte_eal_cleanup(void)
 	rte_trace_save();
 	eal_trace_fini();
 	eal_cleanup_config(&internal_config);
+	rte_eal_log_cleanup();
 	return 0;
 }
 
diff --git a/lib/librte_eal/linux/eal_log.c b/lib/librte_eal/linux/eal_log.c
index 43c8460bfb07..c407cb3c0edb 100644
--- a/lib/librte_eal/linux/eal_log.c
+++ b/lib/librte_eal/linux/eal_log.c
@@ -37,8 +37,16 @@ console_log_write(__rte_unused void *c, const char *buf, size_t size)
 	return ret;
 }
 
+static int
+console_log_close(__attribute__((unused)) void *c)
+{
+	closelog();
+	return 0;
+}
+
 static cookie_io_functions_t console_log_func = {
 	.write = console_log_write,
+	.close = console_log_close,
 };
 
 /*
@@ -60,3 +68,9 @@ rte_eal_log_init(const char *id, int facility)
 
 	return 0;
 }
+
+void
+rte_eal_log_cleanup(void)
+{
+	eal_log_cleanup();
+}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v2 2/8] eal: log: free dynamic state on cleanup
  2020-04-28 23:14 ` [dpdk-dev] [PATCH v2 0/9] eal: " Stephen Hemminger
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 1/8] eal: log: close on cleanup Stephen Hemminger
@ 2020-04-28 23:14   ` Stephen Hemminger
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 3/8] eal: alarm: close file " Stephen Hemminger
                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:14 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, olivier.matz

When rte_eal_cleanup is called, free all the memory
associated with dynamic log levels and types.

Fixes: c1b5fa94a46f ("eal: support dynamic log types")
Cc: olivier.matz@6wind.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_common_log.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 43de798ae96a..a37cd86f7743 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -14,6 +14,7 @@
 #include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_per_lcore.h>
+#include <rte_tailq.h>
 
 #include "eal_private.h"
 
@@ -54,7 +55,7 @@ struct log_cur_msg {
 };
 
 struct rte_log_dynamic_type {
-	const char *name;
+	char *name;
 	uint32_t loglevel;
 };
 
@@ -486,8 +487,26 @@ eal_log_set_default(FILE *default_log)
 void
 eal_log_cleanup(void)
 {
+	struct rte_eal_opt_loglevel *opt_ll, *tmp;
+	size_t i;
+
 	if (default_log_stream) {
 		fclose(default_log_stream);
 		default_log_stream = NULL;
 	}
+
+	TAILQ_FOREACH_SAFE(opt_ll, &opt_loglevel_list, next, tmp) {
+		if (opt_ll->pattern != NULL)
+			free(opt_ll->pattern);
+		else
+			regfree(&opt_ll->re_match);
+		free(opt_ll);
+	}
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++)
+		free(rte_logs.dynamic_types[i].name);
+
+	rte_logs.dynamic_types_len = 0;
+	free(rte_logs.dynamic_types);
+	rte_logs.dynamic_types = NULL;
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v2 3/8] eal: alarm: close file on cleanup
  2020-04-28 23:14 ` [dpdk-dev] [PATCH v2 0/9] eal: " Stephen Hemminger
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 1/8] eal: log: close on cleanup Stephen Hemminger
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 2/8] eal: log: free dynamic state " Stephen Hemminger
@ 2020-04-28 23:14   ` Stephen Hemminger
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 4/8] eal: cleanup threads Stephen Hemminger
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:14 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, olivier.matz, Bruce Richardson

When rte_eal_cleanup is called, free all the memory
associated with dynamic log levels and types.

Fixes: c1b5fa94a46f ("eal: support dynamic log types")
Cc: olivier.matz@6wind.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_private.h |  7 +++++++
 lib/librte_eal/freebsd/eal.c        |  1 +
 lib/librte_eal/freebsd/eal_alarm.c  | 10 ++++++++++
 lib/librte_eal/linux/eal.c          |  1 +
 lib/librte_eal/linux/eal_alarm.c    | 11 +++++++++++
 5 files changed, 30 insertions(+)

diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 24ddfc6c53f4..71539dae412f 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -201,6 +201,13 @@ int rte_eal_intr_init(void);
  */
 int rte_eal_alarm_init(void);
 
+/**
+ * Cleanup alarm resources.
+ *
+ * This function is private to EAL.
+ */
+void rte_eal_alarm_cleanup(void);
+
 /**
  * Function is to check if the kernel module(like, vfio, vfio_iommu_type1,
  * etc.) loaded.
diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index 540b7d38c5b7..582ff0920af4 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -973,6 +973,7 @@ int
 rte_eal_cleanup(void)
 {
 	rte_service_finalize();
+	rte_eal_alarm_cleanup();
 	rte_mp_channel_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
diff --git a/lib/librte_eal/freebsd/eal_alarm.c b/lib/librte_eal/freebsd/eal_alarm.c
index c38b2e04f811..1602f5c65293 100644
--- a/lib/librte_eal/freebsd/eal_alarm.c
+++ b/lib/librte_eal/freebsd/eal_alarm.c
@@ -61,6 +61,16 @@ rte_eal_alarm_init(void)
 	return 0;
 }
 
+void
+rte_eal_alarm_cleanup(void)
+{
+       if (intr_handle.fd == -1)
+               return;
+
+       close(intr_handle.fd);
+       intr_handle.fd = -1;
+}
+
 static inline int
 timespec_cmp(const struct timespec *now, const struct timespec *at)
 {
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 73d2c98b012b..7458592f4950 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1333,6 +1333,7 @@ rte_eal_cleanup(void)
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_memseg_walk(mark_freeable, NULL);
 	rte_service_finalize();
+	rte_eal_alarm_cleanup();
 	rte_mp_channel_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
diff --git a/lib/librte_eal/linux/eal_alarm.c b/lib/librte_eal/linux/eal_alarm.c
index 3252c6fa5909..f839626dad7d 100644
--- a/lib/librte_eal/linux/eal_alarm.c
+++ b/lib/librte_eal/linux/eal_alarm.c
@@ -6,6 +6,7 @@
 #include <signal.h>
 #include <errno.h>
 #include <string.h>
+#include <unistd.h>
 #include <sys/queue.h>
 #include <sys/time.h>
 #include <sys/timerfd.h>
@@ -74,6 +75,16 @@ rte_eal_alarm_init(void)
 	return -1;
 }
 
+void
+rte_eal_alarm_cleanup(void)
+{
+	if (intr_handle.fd == -1)
+		return;
+
+	close(intr_handle.fd);
+	intr_handle.fd = -1;
+}
+
 static void
 eal_alarm_callback(void *arg __rte_unused)
 {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v2 4/8] eal: cleanup threads
  2020-04-28 23:14 ` [dpdk-dev] [PATCH v2 0/9] eal: " Stephen Hemminger
                     ` (2 preceding siblings ...)
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 3/8] eal: alarm: close file " Stephen Hemminger
@ 2020-04-28 23:14   ` Stephen Hemminger
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 5/8] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:14 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Aaron Conole

When rte_eal_cleanup is called it should stop all the child threads
and close the pipes between threads.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Aaron Conole <aconole@redhat.com>
---
 lib/librte_eal/linux/eal.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 7458592f4950..27b768b15c4a 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1327,11 +1327,24 @@ mark_freeable(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
 int
 rte_eal_cleanup(void)
 {
+	int i;
+
 	/* if we're in a primary process, we need to mark hugepages as freeable
 	 * so that finalization can release them back to the system.
 	 */
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_memseg_walk(mark_freeable, NULL);
+
+	RTE_LCORE_FOREACH_SLAVE(i) {
+		pthread_cancel(lcore_config[i].thread_id);
+		pthread_join(lcore_config[i].thread_id, NULL);
+
+		close(lcore_config[i].pipe_master2slave[0]);
+		close(lcore_config[i].pipe_master2slave[1]);
+		close(lcore_config[i].pipe_slave2master[0]);
+		close(lcore_config[i].pipe_slave2master[1]);
+	}
+
 	rte_service_finalize();
 	rte_eal_alarm_cleanup();
 	rte_mp_channel_cleanup();
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v2 5/8] eal: mp: end the multiprocess thread during cleanup
  2020-04-28 23:14 ` [dpdk-dev] [PATCH v2 0/9] eal: " Stephen Hemminger
                     ` (3 preceding siblings ...)
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 4/8] eal: cleanup threads Stephen Hemminger
@ 2020-04-28 23:14   ` Stephen Hemminger
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 6/8] eal: vfio: cleanup the mp sync handle Stephen Hemminger
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:14 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, qi.z.zhang, stable, Anatoly Burakov

When rte_eal_cleanup is called, all control threads should exit.
For the mp thread, this best handled by closing the mp_socket
and letting the thread see that.

This also fixes potential problems where the mp_socket gets
another hard error, and the thread runs away repeating itself
by reading the same error.

Fixes: 85d6815fa6d0 ("eal: close multi-process socket during cleanup")
Cc: qi.z.zhang@intel.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/eal_common_proc.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
index 935e8fefeba8..f369d8bf6dd8 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -276,8 +276,17 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 	msgh.msg_control = control;
 	msgh.msg_controllen = sizeof(control);
 
+retry:
 	msglen = recvmsg(mp_fd, &msgh, 0);
+
+	/* zero length message means socket was closed */
+	if (msglen == 0)
+		return 0;
+
 	if (msglen < 0) {
+		if (errno == EINTR)
+			goto retry;
+
 		RTE_LOG(ERR, EAL, "recvmsg failed, %s\n", strerror(errno));
 		return -1;
 	}
@@ -305,7 +314,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 		RTE_LOG(ERR, EAL, "invalid received data length\n");
 		return -1;
 	}
-	return 0;
+	return msglen;
 }
 
 static void
@@ -376,10 +385,8 @@ mp_handle(void *arg __rte_unused)
 	struct mp_msg_internal msg;
 	struct sockaddr_un sa;
 
-	while (1) {
-		if (read_msg(&msg, &sa) == 0)
-			process_msg(&msg, &sa);
-	}
+	while (read_msg(&msg, &sa) > 0)
+		process_msg(&msg, &sa);
 
 	return NULL;
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v2 6/8] eal: vfio: cleanup the mp sync handle
  2020-04-28 23:14 ` [dpdk-dev] [PATCH v2 0/9] eal: " Stephen Hemminger
                     ` (4 preceding siblings ...)
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 5/8] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
@ 2020-04-28 23:14   ` Stephen Hemminger
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 7/8] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 8/8] eal: malloc: cleanup mp resources Stephen Hemminger
  7 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:14 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, anatoly.burakov, stable

When rte_eal_cleanup is called the rte_mp_action for VFIO
should be freed.

Fixes: edf73dd33072 ("ipc: handle unsupported IPC in action register")
Cc: anatoly.burakov@intel.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/linux/eal.c              | 3 +++
 lib/librte_eal/linux/eal_vfio.h         | 1 +
 lib/librte_eal/linux/eal_vfio_mp_sync.c | 8 ++++++++
 3 files changed, 12 insertions(+)

diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 27b768b15c4a..7c56dbf49508 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1346,6 +1346,9 @@ rte_eal_cleanup(void)
 	}
 
 	rte_service_finalize();
+#ifdef VFIO_PRESENT
+	vfio_mp_sync_cleanup();
+#endif
 	rte_eal_alarm_cleanup();
 	rte_mp_channel_cleanup();
 	rte_trace_save();
diff --git a/lib/librte_eal/linux/eal_vfio.h b/lib/librte_eal/linux/eal_vfio.h
index cb2d35fb1206..bf7408a897a7 100644
--- a/lib/librte_eal/linux/eal_vfio.h
+++ b/lib/librte_eal/linux/eal_vfio.h
@@ -132,6 +132,7 @@ int
 vfio_has_supported_extensions(int vfio_container_fd);
 
 int vfio_mp_sync_setup(void);
+void vfio_mp_sync_cleanup(void);
 
 #define EAL_VFIO_MP "eal_vfio_mp_sync"
 
diff --git a/lib/librte_eal/linux/eal_vfio_mp_sync.c b/lib/librte_eal/linux/eal_vfio_mp_sync.c
index 5f2a5fc1d94e..b8ae9c65892e 100644
--- a/lib/librte_eal/linux/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linux/eal_vfio_mp_sync.c
@@ -120,4 +120,12 @@ vfio_mp_sync_setup(void)
 	return 0;
 }
 
+void
+vfio_mp_sync_cleanup(void)
+{
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return;
+
+	rte_mp_action_unregister(EAL_VFIO_MP);
+}
 #endif
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v2 7/8] eal: hotplug: cleanup multiprocess resources
  2020-04-28 23:14 ` [dpdk-dev] [PATCH v2 0/9] eal: " Stephen Hemminger
                     ` (5 preceding siblings ...)
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 6/8] eal: vfio: cleanup the mp sync handle Stephen Hemminger
@ 2020-04-28 23:14   ` Stephen Hemminger
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 8/8] eal: malloc: cleanup mp resources Stephen Hemminger
  7 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:14 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When rte_eal_cleanup is called, hotplug should unregister the
resources associated with the multi-process server.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/hotplug_mp.c | 5 +++++
 lib/librte_eal/common/hotplug_mp.h | 6 ++++++
 lib/librte_eal/linux/eal.c         | 1 +
 3 files changed, 12 insertions(+)

diff --git a/lib/librte_eal/common/hotplug_mp.c b/lib/librte_eal/common/hotplug_mp.c
index ee791903b3b7..a390c01fd41c 100644
--- a/lib/librte_eal/common/hotplug_mp.c
+++ b/lib/librte_eal/common/hotplug_mp.c
@@ -463,3 +463,8 @@ int eal_mp_dev_hotplug_init(void)
 
 	return 0;
 }
+
+void eal_mp_dev_hotplug_cleanup(void)
+{
+	rte_mp_action_unregister(EAL_DEV_MP_ACTION_REQUEST);
+}
diff --git a/lib/librte_eal/common/hotplug_mp.h b/lib/librte_eal/common/hotplug_mp.h
index 8fcf9b52e24c..4848446c852d 100644
--- a/lib/librte_eal/common/hotplug_mp.h
+++ b/lib/librte_eal/common/hotplug_mp.h
@@ -37,6 +37,12 @@ struct eal_dev_mp_req {
 int
 eal_mp_dev_hotplug_init(void);
 
+/**
+ * Unregister all mp action callbacks for hotplug.
+ */
+void
+eal_mp_dev_hotplug_cleanup(void);
+
 /**
  * This is a synchronous wrapper for secondary process send
  * request to primary process, this is invoked when an attach
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 7c56dbf49508..ffb0678b864a 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1350,6 +1350,7 @@ rte_eal_cleanup(void)
 	vfio_mp_sync_cleanup();
 #endif
 	rte_eal_alarm_cleanup();
+	eal_mp_dev_hotplug_cleanup();
 	rte_mp_channel_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v2 8/8] eal: malloc: cleanup mp resources
  2020-04-28 23:14 ` [dpdk-dev] [PATCH v2 0/9] eal: " Stephen Hemminger
                     ` (6 preceding siblings ...)
  2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 7/8] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
@ 2020-04-28 23:14   ` Stephen Hemminger
  7 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:14 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Anatoly Burakov

The mp action resources in malloc should be cleaned up via
rte_eal_cleanup.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/malloc_heap.c |  6 ++++++
 lib/librte_eal/common/malloc_heap.h |  3 +++
 lib/librte_eal/common/malloc_mp.c   | 12 ++++++++++++
 lib/librte_eal/common/malloc_mp.h   |  3 +++
 lib/librte_eal/linux/eal.c          |  1 +
 5 files changed, 25 insertions(+)

diff --git a/lib/librte_eal/common/malloc_heap.c b/lib/librte_eal/common/malloc_heap.c
index 842eb9de75a1..13c673f363a9 100644
--- a/lib/librte_eal/common/malloc_heap.c
+++ b/lib/librte_eal/common/malloc_heap.c
@@ -1362,3 +1362,9 @@ rte_eal_malloc_heap_init(void)
 	/* add all IOVA-contiguous areas to the heap */
 	return rte_memseg_contig_walk(malloc_add_seg, NULL);
 }
+
+void
+rte_eal_malloc_heap_cleanup(void)
+{
+	unregister_mp_requests();
+}
diff --git a/lib/librte_eal/common/malloc_heap.h b/lib/librte_eal/common/malloc_heap.h
index 772736b53f3c..ffad1b61e246 100644
--- a/lib/librte_eal/common/malloc_heap.h
+++ b/lib/librte_eal/common/malloc_heap.h
@@ -100,6 +100,9 @@ malloc_socket_to_heap_id(unsigned int socket_id);
 int
 rte_eal_malloc_heap_init(void);
 
+void
+rte_eal_malloc_heap_cleanup(void);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/common/malloc_mp.c b/lib/librte_eal/common/malloc_mp.c
index 1f212f834993..a9a9e8a45221 100644
--- a/lib/librte_eal/common/malloc_mp.c
+++ b/lib/librte_eal/common/malloc_mp.c
@@ -749,3 +749,15 @@ register_mp_requests(void)
 	}
 	return 0;
 }
+
+void
+unregister_mp_requests(void)
+{
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		rte_mp_action_unregister(MP_ACTION_REQUEST);
+	} else {
+		rte_mp_action_unregister(MP_ACTION_SYNC);
+		rte_mp_action_unregister(MP_ACTION_ROLLBACK);
+		rte_mp_action_unregister(MP_ACTION_RESPONSE);
+	}
+}
diff --git a/lib/librte_eal/common/malloc_mp.h b/lib/librte_eal/common/malloc_mp.h
index 2b86b76f6848..fb3d18c4e458 100644
--- a/lib/librte_eal/common/malloc_mp.h
+++ b/lib/librte_eal/common/malloc_mp.h
@@ -63,6 +63,9 @@ struct malloc_mp_req {
 int
 register_mp_requests(void);
 
+void
+unregister_mp_requests(void);
+
 int
 request_to_primary(struct malloc_mp_req *req);
 
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index ffb0678b864a..abd478c9ceb0 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1351,6 +1351,7 @@ rte_eal_cleanup(void)
 #endif
 	rte_eal_alarm_cleanup();
 	eal_mp_dev_hotplug_cleanup();
+	rte_eal_malloc_heap_cleanup();
 	rte_mp_channel_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown
  2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
                   ` (17 preceding siblings ...)
  2020-04-28 23:14 ` [dpdk-dev] [PATCH v2 0/9] eal: " Stephen Hemminger
@ 2020-04-28 23:58 ` Stephen Hemminger
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 1/8] eal: log: close on cleanup Stephen Hemminger
                     ` (11 more replies)
  18 siblings, 12 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:58 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Started using valgrind with DPDK, and there are lots of leftover
memory and file descriptors. This makes it hard to find application
leaks versus DPDK leaks.

The DPDK has a function that applications can use to tell it
to cleanup resources on shutdown (rte_eal_cleanup). But the
current coverage of that API is spotty. Many internal parts of
DPDK leave files and allocated memory behind.

This patch set is a first step at getting the sub-parts of
DPDK to cleanup after themselves. These are the easier ones,
the harder and more critical ones are in the drivers
and the memory subsystem.

There are no new exposed API or ABI changes here.

v3
 - fix a couple of minor checkpatch complaints

v2
 - rebase after 20.05 file renames
 - incorporate review comment feedback
 - hold off some of the more involved patches for later

Stephen Hemminger (8):
  eal: log: close on cleanup
  eal: log: free dynamic state on cleanup
  eal: alarm: close file on cleanup
  eal: cleanup threads
  eal: mp: end the multiprocess thread during cleanup
  eal: vfio: cleanup the mp sync handle
  eal: hotplug: cleanup multiprocess resources
  eal: malloc: cleanup mp resources

 lib/librte_eal/common/eal_common_log.c  | 33 ++++++++++++++++++++++++-
 lib/librte_eal/common/eal_common_proc.c | 17 +++++++++----
 lib/librte_eal/common/eal_private.h     | 20 +++++++++++++++
 lib/librte_eal/common/hotplug_mp.c      |  5 ++++
 lib/librte_eal/common/hotplug_mp.h      |  6 +++++
 lib/librte_eal/common/malloc_heap.c     |  6 +++++
 lib/librte_eal/common/malloc_heap.h     |  3 +++
 lib/librte_eal/common/malloc_mp.c       | 12 +++++++++
 lib/librte_eal/common/malloc_mp.h       |  3 +++
 lib/librte_eal/freebsd/eal.c            |  1 +
 lib/librte_eal/freebsd/eal_alarm.c      | 10 ++++++++
 lib/librte_eal/linux/eal.c              | 20 +++++++++++++++
 lib/librte_eal/linux/eal_alarm.c        | 11 +++++++++
 lib/librte_eal/linux/eal_log.c          | 14 +++++++++++
 lib/librte_eal/linux/eal_vfio.h         |  1 +
 lib/librte_eal/linux/eal_vfio_mp_sync.c |  8 ++++++
 16 files changed, 164 insertions(+), 6 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v3 1/8] eal: log: close on cleanup
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
@ 2020-04-28 23:58   ` Stephen Hemminger
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 2/8] eal: log: free dynamic state " Stephen Hemminger
                     ` (10 subsequent siblings)
  11 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:58 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When application calls rte_eal_cleanup on shutdown,
the DPDK log should be closed and cleaned up.

Fixes: af75078fece3 ("first public release")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_common_log.c | 12 ++++++++++++
 lib/librte_eal/common/eal_private.h    | 13 +++++++++++++
 lib/librte_eal/linux/eal.c             |  1 +
 lib/librte_eal/linux/eal_log.c         | 14 ++++++++++++++
 4 files changed, 40 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index d7a5f9b6417a..43de798ae96a 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -479,3 +479,15 @@ eal_log_set_default(FILE *default_log)
 		"Debug dataplane logs available - lower performance\n");
 #endif
 }
+
+/*
+ * Called by environment-specific cleanup function.
+ */
+void
+eal_log_cleanup(void)
+{
+	if (default_log_stream) {
+		fclose(default_log_stream);
+		default_log_stream = NULL;
+	}
+}
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index ecf827914fdd..24ddfc6c53f4 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -89,6 +89,12 @@ int rte_eal_memzone_init(void);
  */
 void eal_log_set_default(FILE *default_log);
 
+/**
+ * Common log cleanup function (private to eal).
+ * Closes the default log stream. Called from rte_eal_cleanup().
+ */
+void eal_log_cleanup(void);
+
 /**
  * Fill configuration with number of physical and logical processors
  *
@@ -150,6 +156,13 @@ int rte_eal_timer_init(void);
  */
 int rte_eal_log_init(const char *id, int facility);
 
+/**
+ * Close the default log stream
+ *
+ * This function is private to EAL.
+ */
+void rte_eal_log_cleanup(void);
+
 /**
  * Save the log regexp for later
  */
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index aa72d3650929..73d2c98b012b 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1337,6 +1337,7 @@ rte_eal_cleanup(void)
 	rte_trace_save();
 	eal_trace_fini();
 	eal_cleanup_config(&internal_config);
+	rte_eal_log_cleanup();
 	return 0;
 }
 
diff --git a/lib/librte_eal/linux/eal_log.c b/lib/librte_eal/linux/eal_log.c
index 43c8460bfb07..258a16ff2498 100644
--- a/lib/librte_eal/linux/eal_log.c
+++ b/lib/librte_eal/linux/eal_log.c
@@ -37,8 +37,16 @@ console_log_write(__rte_unused void *c, const char *buf, size_t size)
 	return ret;
 }
 
+static int
+console_log_close(__rte_unused void *c)
+{
+	closelog();
+	return 0;
+}
+
 static cookie_io_functions_t console_log_func = {
 	.write = console_log_write,
+	.close = console_log_close,
 };
 
 /*
@@ -60,3 +68,9 @@ rte_eal_log_init(const char *id, int facility)
 
 	return 0;
 }
+
+void
+rte_eal_log_cleanup(void)
+{
+	eal_log_cleanup();
+}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v3 2/8] eal: log: free dynamic state on cleanup
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 1/8] eal: log: close on cleanup Stephen Hemminger
@ 2020-04-28 23:58   ` Stephen Hemminger
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 3/8] eal: alarm: close file " Stephen Hemminger
                     ` (9 subsequent siblings)
  11 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:58 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, olivier.matz

When rte_eal_cleanup is called, free all the memory
associated with dynamic log levels and types.

Fixes: c1b5fa94a46f ("eal: support dynamic log types")
Cc: olivier.matz@6wind.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_common_log.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 43de798ae96a..a37cd86f7743 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -14,6 +14,7 @@
 #include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_per_lcore.h>
+#include <rte_tailq.h>
 
 #include "eal_private.h"
 
@@ -54,7 +55,7 @@ struct log_cur_msg {
 };
 
 struct rte_log_dynamic_type {
-	const char *name;
+	char *name;
 	uint32_t loglevel;
 };
 
@@ -486,8 +487,26 @@ eal_log_set_default(FILE *default_log)
 void
 eal_log_cleanup(void)
 {
+	struct rte_eal_opt_loglevel *opt_ll, *tmp;
+	size_t i;
+
 	if (default_log_stream) {
 		fclose(default_log_stream);
 		default_log_stream = NULL;
 	}
+
+	TAILQ_FOREACH_SAFE(opt_ll, &opt_loglevel_list, next, tmp) {
+		if (opt_ll->pattern != NULL)
+			free(opt_ll->pattern);
+		else
+			regfree(&opt_ll->re_match);
+		free(opt_ll);
+	}
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++)
+		free(rte_logs.dynamic_types[i].name);
+
+	rte_logs.dynamic_types_len = 0;
+	free(rte_logs.dynamic_types);
+	rte_logs.dynamic_types = NULL;
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v3 3/8] eal: alarm: close file on cleanup
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 1/8] eal: log: close on cleanup Stephen Hemminger
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 2/8] eal: log: free dynamic state " Stephen Hemminger
@ 2020-04-28 23:58   ` Stephen Hemminger
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 4/8] eal: cleanup threads Stephen Hemminger
                     ` (8 subsequent siblings)
  11 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:58 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, olivier.matz, Bruce Richardson

When rte_eal_cleanup is called, free all the memory
associated with dynamic log levels and types.

Fixes: c1b5fa94a46f ("eal: support dynamic log types")
Cc: olivier.matz@6wind.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_private.h |  7 +++++++
 lib/librte_eal/freebsd/eal.c        |  1 +
 lib/librte_eal/freebsd/eal_alarm.c  | 10 ++++++++++
 lib/librte_eal/linux/eal.c          |  1 +
 lib/librte_eal/linux/eal_alarm.c    | 11 +++++++++++
 5 files changed, 30 insertions(+)

diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 24ddfc6c53f4..71539dae412f 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -201,6 +201,13 @@ int rte_eal_intr_init(void);
  */
 int rte_eal_alarm_init(void);
 
+/**
+ * Cleanup alarm resources.
+ *
+ * This function is private to EAL.
+ */
+void rte_eal_alarm_cleanup(void);
+
 /**
  * Function is to check if the kernel module(like, vfio, vfio_iommu_type1,
  * etc.) loaded.
diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index 540b7d38c5b7..582ff0920af4 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -973,6 +973,7 @@ int
 rte_eal_cleanup(void)
 {
 	rte_service_finalize();
+	rte_eal_alarm_cleanup();
 	rte_mp_channel_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
diff --git a/lib/librte_eal/freebsd/eal_alarm.c b/lib/librte_eal/freebsd/eal_alarm.c
index c38b2e04f811..b2089d0b53df 100644
--- a/lib/librte_eal/freebsd/eal_alarm.c
+++ b/lib/librte_eal/freebsd/eal_alarm.c
@@ -61,6 +61,16 @@ rte_eal_alarm_init(void)
 	return 0;
 }
 
+void
+rte_eal_alarm_cleanup(void)
+{
+	if (intr_handle.fd == -1)
+		return;
+
+	close(intr_handle.fd);
+	intr_handle.fd = -1;
+}
+
 static inline int
 timespec_cmp(const struct timespec *now, const struct timespec *at)
 {
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 73d2c98b012b..7458592f4950 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1333,6 +1333,7 @@ rte_eal_cleanup(void)
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_memseg_walk(mark_freeable, NULL);
 	rte_service_finalize();
+	rte_eal_alarm_cleanup();
 	rte_mp_channel_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
diff --git a/lib/librte_eal/linux/eal_alarm.c b/lib/librte_eal/linux/eal_alarm.c
index 3252c6fa5909..f839626dad7d 100644
--- a/lib/librte_eal/linux/eal_alarm.c
+++ b/lib/librte_eal/linux/eal_alarm.c
@@ -6,6 +6,7 @@
 #include <signal.h>
 #include <errno.h>
 #include <string.h>
+#include <unistd.h>
 #include <sys/queue.h>
 #include <sys/time.h>
 #include <sys/timerfd.h>
@@ -74,6 +75,16 @@ rte_eal_alarm_init(void)
 	return -1;
 }
 
+void
+rte_eal_alarm_cleanup(void)
+{
+	if (intr_handle.fd == -1)
+		return;
+
+	close(intr_handle.fd);
+	intr_handle.fd = -1;
+}
+
 static void
 eal_alarm_callback(void *arg __rte_unused)
 {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v3 4/8] eal: cleanup threads
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
                     ` (2 preceding siblings ...)
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 3/8] eal: alarm: close file " Stephen Hemminger
@ 2020-04-28 23:58   ` Stephen Hemminger
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 5/8] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
                     ` (7 subsequent siblings)
  11 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:58 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Aaron Conole

When rte_eal_cleanup is called it should stop all the child threads
and close the pipes between threads.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Aaron Conole <aconole@redhat.com>
---
 lib/librte_eal/linux/eal.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 7458592f4950..27b768b15c4a 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1327,11 +1327,24 @@ mark_freeable(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
 int
 rte_eal_cleanup(void)
 {
+	int i;
+
 	/* if we're in a primary process, we need to mark hugepages as freeable
 	 * so that finalization can release them back to the system.
 	 */
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_memseg_walk(mark_freeable, NULL);
+
+	RTE_LCORE_FOREACH_SLAVE(i) {
+		pthread_cancel(lcore_config[i].thread_id);
+		pthread_join(lcore_config[i].thread_id, NULL);
+
+		close(lcore_config[i].pipe_master2slave[0]);
+		close(lcore_config[i].pipe_master2slave[1]);
+		close(lcore_config[i].pipe_slave2master[0]);
+		close(lcore_config[i].pipe_slave2master[1]);
+	}
+
 	rte_service_finalize();
 	rte_eal_alarm_cleanup();
 	rte_mp_channel_cleanup();
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v3 5/8] eal: mp: end the multiprocess thread during cleanup
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
                     ` (3 preceding siblings ...)
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 4/8] eal: cleanup threads Stephen Hemminger
@ 2020-04-28 23:58   ` Stephen Hemminger
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 6/8] eal: vfio: cleanup the mp sync handle Stephen Hemminger
                     ` (6 subsequent siblings)
  11 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:58 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, qi.z.zhang, stable, Anatoly Burakov

When rte_eal_cleanup is called, all control threads should exit.
For the mp thread, this best handled by closing the mp_socket
and letting the thread see that.

This also fixes potential problems where the mp_socket gets
another hard error, and the thread runs away repeating itself
by reading the same error.

Fixes: 85d6815fa6d0 ("eal: close multi-process socket during cleanup")
Cc: qi.z.zhang@intel.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/eal_common_proc.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
index 935e8fefeba8..f369d8bf6dd8 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -276,8 +276,17 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 	msgh.msg_control = control;
 	msgh.msg_controllen = sizeof(control);
 
+retry:
 	msglen = recvmsg(mp_fd, &msgh, 0);
+
+	/* zero length message means socket was closed */
+	if (msglen == 0)
+		return 0;
+
 	if (msglen < 0) {
+		if (errno == EINTR)
+			goto retry;
+
 		RTE_LOG(ERR, EAL, "recvmsg failed, %s\n", strerror(errno));
 		return -1;
 	}
@@ -305,7 +314,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 		RTE_LOG(ERR, EAL, "invalid received data length\n");
 		return -1;
 	}
-	return 0;
+	return msglen;
 }
 
 static void
@@ -376,10 +385,8 @@ mp_handle(void *arg __rte_unused)
 	struct mp_msg_internal msg;
 	struct sockaddr_un sa;
 
-	while (1) {
-		if (read_msg(&msg, &sa) == 0)
-			process_msg(&msg, &sa);
-	}
+	while (read_msg(&msg, &sa) > 0)
+		process_msg(&msg, &sa);
 
 	return NULL;
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v3 6/8] eal: vfio: cleanup the mp sync handle
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
                     ` (4 preceding siblings ...)
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 5/8] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
@ 2020-04-28 23:58   ` Stephen Hemminger
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 7/8] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
                     ` (5 subsequent siblings)
  11 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:58 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, anatoly.burakov, stable

When rte_eal_cleanup is called the rte_mp_action for VFIO
should be freed.

Fixes: edf73dd33072 ("ipc: handle unsupported IPC in action register")
Cc: anatoly.burakov@intel.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/linux/eal.c              | 3 +++
 lib/librte_eal/linux/eal_vfio.h         | 1 +
 lib/librte_eal/linux/eal_vfio_mp_sync.c | 8 ++++++++
 3 files changed, 12 insertions(+)

diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 27b768b15c4a..7c56dbf49508 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1346,6 +1346,9 @@ rte_eal_cleanup(void)
 	}
 
 	rte_service_finalize();
+#ifdef VFIO_PRESENT
+	vfio_mp_sync_cleanup();
+#endif
 	rte_eal_alarm_cleanup();
 	rte_mp_channel_cleanup();
 	rte_trace_save();
diff --git a/lib/librte_eal/linux/eal_vfio.h b/lib/librte_eal/linux/eal_vfio.h
index cb2d35fb1206..bf7408a897a7 100644
--- a/lib/librte_eal/linux/eal_vfio.h
+++ b/lib/librte_eal/linux/eal_vfio.h
@@ -132,6 +132,7 @@ int
 vfio_has_supported_extensions(int vfio_container_fd);
 
 int vfio_mp_sync_setup(void);
+void vfio_mp_sync_cleanup(void);
 
 #define EAL_VFIO_MP "eal_vfio_mp_sync"
 
diff --git a/lib/librte_eal/linux/eal_vfio_mp_sync.c b/lib/librte_eal/linux/eal_vfio_mp_sync.c
index 5f2a5fc1d94e..b8ae9c65892e 100644
--- a/lib/librte_eal/linux/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linux/eal_vfio_mp_sync.c
@@ -120,4 +120,12 @@ vfio_mp_sync_setup(void)
 	return 0;
 }
 
+void
+vfio_mp_sync_cleanup(void)
+{
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return;
+
+	rte_mp_action_unregister(EAL_VFIO_MP);
+}
 #endif
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v3 7/8] eal: hotplug: cleanup multiprocess resources
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
                     ` (5 preceding siblings ...)
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 6/8] eal: vfio: cleanup the mp sync handle Stephen Hemminger
@ 2020-04-28 23:58   ` Stephen Hemminger
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 8/8] eal: malloc: cleanup mp resources Stephen Hemminger
                     ` (4 subsequent siblings)
  11 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:58 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When rte_eal_cleanup is called, hotplug should unregister the
resources associated with the multi-process server.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/hotplug_mp.c | 5 +++++
 lib/librte_eal/common/hotplug_mp.h | 6 ++++++
 lib/librte_eal/linux/eal.c         | 1 +
 3 files changed, 12 insertions(+)

diff --git a/lib/librte_eal/common/hotplug_mp.c b/lib/librte_eal/common/hotplug_mp.c
index ee791903b3b7..a390c01fd41c 100644
--- a/lib/librte_eal/common/hotplug_mp.c
+++ b/lib/librte_eal/common/hotplug_mp.c
@@ -463,3 +463,8 @@ int eal_mp_dev_hotplug_init(void)
 
 	return 0;
 }
+
+void eal_mp_dev_hotplug_cleanup(void)
+{
+	rte_mp_action_unregister(EAL_DEV_MP_ACTION_REQUEST);
+}
diff --git a/lib/librte_eal/common/hotplug_mp.h b/lib/librte_eal/common/hotplug_mp.h
index 8fcf9b52e24c..4848446c852d 100644
--- a/lib/librte_eal/common/hotplug_mp.h
+++ b/lib/librte_eal/common/hotplug_mp.h
@@ -37,6 +37,12 @@ struct eal_dev_mp_req {
 int
 eal_mp_dev_hotplug_init(void);
 
+/**
+ * Unregister all mp action callbacks for hotplug.
+ */
+void
+eal_mp_dev_hotplug_cleanup(void);
+
 /**
  * This is a synchronous wrapper for secondary process send
  * request to primary process, this is invoked when an attach
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 7c56dbf49508..ffb0678b864a 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1350,6 +1350,7 @@ rte_eal_cleanup(void)
 	vfio_mp_sync_cleanup();
 #endif
 	rte_eal_alarm_cleanup();
+	eal_mp_dev_hotplug_cleanup();
 	rte_mp_channel_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [dpdk-dev] [PATCH v3 8/8] eal: malloc: cleanup mp resources
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
                     ` (6 preceding siblings ...)
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 7/8] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
@ 2020-04-28 23:58   ` Stephen Hemminger
  2020-05-03 17:21   ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown David Marchand
                     ` (3 subsequent siblings)
  11 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2020-04-28 23:58 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Anatoly Burakov

The mp action resources in malloc should be cleaned up via
rte_eal_cleanup.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/malloc_heap.c |  6 ++++++
 lib/librte_eal/common/malloc_heap.h |  3 +++
 lib/librte_eal/common/malloc_mp.c   | 12 ++++++++++++
 lib/librte_eal/common/malloc_mp.h   |  3 +++
 lib/librte_eal/linux/eal.c          |  1 +
 5 files changed, 25 insertions(+)

diff --git a/lib/librte_eal/common/malloc_heap.c b/lib/librte_eal/common/malloc_heap.c
index 842eb9de75a1..13c673f363a9 100644
--- a/lib/librte_eal/common/malloc_heap.c
+++ b/lib/librte_eal/common/malloc_heap.c
@@ -1362,3 +1362,9 @@ rte_eal_malloc_heap_init(void)
 	/* add all IOVA-contiguous areas to the heap */
 	return rte_memseg_contig_walk(malloc_add_seg, NULL);
 }
+
+void
+rte_eal_malloc_heap_cleanup(void)
+{
+	unregister_mp_requests();
+}
diff --git a/lib/librte_eal/common/malloc_heap.h b/lib/librte_eal/common/malloc_heap.h
index 772736b53f3c..ffad1b61e246 100644
--- a/lib/librte_eal/common/malloc_heap.h
+++ b/lib/librte_eal/common/malloc_heap.h
@@ -100,6 +100,9 @@ malloc_socket_to_heap_id(unsigned int socket_id);
 int
 rte_eal_malloc_heap_init(void);
 
+void
+rte_eal_malloc_heap_cleanup(void);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/common/malloc_mp.c b/lib/librte_eal/common/malloc_mp.c
index 1f212f834993..a9a9e8a45221 100644
--- a/lib/librte_eal/common/malloc_mp.c
+++ b/lib/librte_eal/common/malloc_mp.c
@@ -749,3 +749,15 @@ register_mp_requests(void)
 	}
 	return 0;
 }
+
+void
+unregister_mp_requests(void)
+{
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		rte_mp_action_unregister(MP_ACTION_REQUEST);
+	} else {
+		rte_mp_action_unregister(MP_ACTION_SYNC);
+		rte_mp_action_unregister(MP_ACTION_ROLLBACK);
+		rte_mp_action_unregister(MP_ACTION_RESPONSE);
+	}
+}
diff --git a/lib/librte_eal/common/malloc_mp.h b/lib/librte_eal/common/malloc_mp.h
index 2b86b76f6848..fb3d18c4e458 100644
--- a/lib/librte_eal/common/malloc_mp.h
+++ b/lib/librte_eal/common/malloc_mp.h
@@ -63,6 +63,9 @@ struct malloc_mp_req {
 int
 register_mp_requests(void);
 
+void
+unregister_mp_requests(void);
+
 int
 request_to_primary(struct malloc_mp_req *req);
 
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index ffb0678b864a..abd478c9ceb0 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1351,6 +1351,7 @@ rte_eal_cleanup(void)
 #endif
 	rte_eal_alarm_cleanup();
 	eal_mp_dev_hotplug_cleanup();
+	rte_eal_malloc_heap_cleanup();
 	rte_mp_channel_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
-- 
2.20.1


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
                     ` (7 preceding siblings ...)
  2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 8/8] eal: malloc: cleanup mp resources Stephen Hemminger
@ 2020-05-03 17:21   ` David Marchand
  2020-10-19 22:24     ` Thomas Monjalon
  2021-11-13  0:28   ` [PATCH v4 0/5] cleanup more stuff " Stephen Hemminger
                     ` (2 subsequent siblings)
  11 siblings, 1 reply; 74+ messages in thread
From: David Marchand @ 2020-05-03 17:21 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Wed, Apr 29, 2020 at 1:58 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> Started using valgrind with DPDK, and there are lots of leftover
> memory and file descriptors. This makes it hard to find application
> leaks versus DPDK leaks.
>
> The DPDK has a function that applications can use to tell it
> to cleanup resources on shutdown (rte_eal_cleanup). But the
> current coverage of that API is spotty. Many internal parts of
> DPDK leave files and allocated memory behind.
>
> This patch set is a first step at getting the sub-parts of
> DPDK to cleanup after themselves. These are the easier ones,
> the harder and more critical ones are in the drivers
> and the memory subsystem.
>
> There are no new exposed API or ABI changes here.
>
> v3
>  - fix a couple of minor checkpatch complaints
>
> v2
>  - rebase after 20.05 file renames
>  - incorporate review comment feedback
>  - hold off some of the more involved patches for later

Same segfault as v1.

$ ./devtools/test-null.sh ./build/app/dpdk-testpmd 0x3 --plop
./build/app/dpdk-testpmd: unrecognized option '--plop'
EAL: Detected 8 lcore(s)
EAL: Detected 1 NUMA nodes

Usage: ./build/app/dpdk-testpmd [options]

(snip)

EAL: FATAL: Invalid 'command line' arguments.
EAL: Invalid 'command line' arguments.
EAL: Error - exiting with code: 1
  Cause: Cannot init EAL: Invalid argument
./devtools/test-null.sh: line 32: 23134 Broken pipe             (
sleep 1 && echo stop )
     23135 Segmentation fault      (core dumped) | $testpmd -c
$coremask --no-huge -m 20 $libs -w 0:0.0 --vdev net_null1 --vdev
net_null2 $eal_options -- --no-mlockall --total-num-mbufs=2048
$testpmd_options -ia


-- 
David Marchand


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown
  2020-05-03 17:21   ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown David Marchand
@ 2020-10-19 22:24     ` Thomas Monjalon
  2021-03-24 21:30       ` Thomas Monjalon
  0 siblings, 1 reply; 74+ messages in thread
From: Thomas Monjalon @ 2020-10-19 22:24 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: dev, David Marchand, ferruh.yigit, bruce.richardson, andrew.rybchenko

That's a pity this patchset is not concluded.
Please Stephen, could you respin with a fix?


03/05/2020 19:21, David Marchand:
> On Wed, Apr 29, 2020 at 1:58 AM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > Started using valgrind with DPDK, and there are lots of leftover
> > memory and file descriptors. This makes it hard to find application
> > leaks versus DPDK leaks.
> >
> > The DPDK has a function that applications can use to tell it
> > to cleanup resources on shutdown (rte_eal_cleanup). But the
> > current coverage of that API is spotty. Many internal parts of
> > DPDK leave files and allocated memory behind.
> >
> > This patch set is a first step at getting the sub-parts of
> > DPDK to cleanup after themselves. These are the easier ones,
> > the harder and more critical ones are in the drivers
> > and the memory subsystem.
> >
> > There are no new exposed API or ABI changes here.
> >
> > v3
> >  - fix a couple of minor checkpatch complaints
> >
> > v2
> >  - rebase after 20.05 file renames
> >  - incorporate review comment feedback
> >  - hold off some of the more involved patches for later
> 
> Same segfault as v1.
> 
> $ ./devtools/test-null.sh ./build/app/dpdk-testpmd 0x3 --plop
> ./build/app/dpdk-testpmd: unrecognized option '--plop'
> EAL: Detected 8 lcore(s)
> EAL: Detected 1 NUMA nodes
> 
> Usage: ./build/app/dpdk-testpmd [options]
> 
> (snip)
> 
> EAL: FATAL: Invalid 'command line' arguments.
> EAL: Invalid 'command line' arguments.
> EAL: Error - exiting with code: 1
>   Cause: Cannot init EAL: Invalid argument
> ./devtools/test-null.sh: line 32: 23134 Broken pipe             (
> sleep 1 && echo stop )
>      23135 Segmentation fault      (core dumped) | $testpmd -c
> $coremask --no-huge -m 20 $libs -w 0:0.0 --vdev net_null1 --vdev
> net_null2 $eal_options -- --no-mlockall --total-num-mbufs=2048
> $testpmd_options -ia
> 
> 
> 






^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown
  2020-10-19 22:24     ` Thomas Monjalon
@ 2021-03-24 21:30       ` Thomas Monjalon
  0 siblings, 0 replies; 74+ messages in thread
From: Thomas Monjalon @ 2021-03-24 21:30 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: dev, David Marchand, ferruh.yigit, bruce.richardson, andrew.rybchenko

Ping

20/10/2020 00:24, Thomas Monjalon:
> That's a pity this patchset is not concluded.
> Please Stephen, could you respin with a fix?
> 
> 
> 03/05/2020 19:21, David Marchand:
> > On Wed, Apr 29, 2020 at 1:58 AM Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> > >
> > > Started using valgrind with DPDK, and there are lots of leftover
> > > memory and file descriptors. This makes it hard to find application
> > > leaks versus DPDK leaks.
> > >
> > > The DPDK has a function that applications can use to tell it
> > > to cleanup resources on shutdown (rte_eal_cleanup). But the
> > > current coverage of that API is spotty. Many internal parts of
> > > DPDK leave files and allocated memory behind.
> > >
> > > This patch set is a first step at getting the sub-parts of
> > > DPDK to cleanup after themselves. These are the easier ones,
> > > the harder and more critical ones are in the drivers
> > > and the memory subsystem.
> > >
> > > There are no new exposed API or ABI changes here.
> > >
> > > v3
> > >  - fix a couple of minor checkpatch complaints
> > >
> > > v2
> > >  - rebase after 20.05 file renames
> > >  - incorporate review comment feedback
> > >  - hold off some of the more involved patches for later
> > 
> > Same segfault as v1.
> > 
> > $ ./devtools/test-null.sh ./build/app/dpdk-testpmd 0x3 --plop
> > ./build/app/dpdk-testpmd: unrecognized option '--plop'
> > EAL: Detected 8 lcore(s)
> > EAL: Detected 1 NUMA nodes
> > 
> > Usage: ./build/app/dpdk-testpmd [options]
> > 
> > (snip)
> > 
> > EAL: FATAL: Invalid 'command line' arguments.
> > EAL: Invalid 'command line' arguments.
> > EAL: Error - exiting with code: 1
> >   Cause: Cannot init EAL: Invalid argument
> > ./devtools/test-null.sh: line 32: 23134 Broken pipe             (
> > sleep 1 && echo stop )
> >      23135 Segmentation fault      (core dumped) | $testpmd -c
> > $coremask --no-huge -m 20 $libs -w 0:0.0 --vdev net_null1 --vdev
> > net_null2 $eal_options -- --no-mlockall --total-num-mbufs=2048
> > $testpmd_options -ia
> > 
> > 
> > 
> 
> 
> 
> 
> 
> 






^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v4 0/5] cleanup more stuff on shutdown
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
                     ` (8 preceding siblings ...)
  2020-05-03 17:21   ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown David Marchand
@ 2021-11-13  0:28   ` Stephen Hemminger
  2021-11-13  0:28     ` [PATCH v4 1/5] eal: close log in eal_cleanup Stephen Hemminger
                       ` (4 more replies)
  2021-11-13  3:32   ` [PATCH v5 0/5] cleanup DPDK resources via eal_cleanup Stephen Hemminger
  2021-11-13 17:22   ` [PATCH v6 0/5] cleanup more resources on eal_cleanup Stephen Hemminger
  11 siblings, 5 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13  0:28 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Started using valgrind with DPDK, and there are lots of leftover
memory and file descriptors. This makes it hard to find application
leaks versus DPDK leaks.

The DPDK has a function that applications can use to tell it
to cleanup resources on shutdown (rte_eal_cleanup). But the
current coverage of that API is spotty. Many internal parts of
DPDK leave files and allocated memory behind.

This patch set is a first step at getting the sub-parts of
DPDK to cleanup after themselves. These are the easier ones,
the harder and more critical ones are in the drivers
and the memory subsystem.

There should be no new exposed API or ABI changes here.

v4
 - rebase to 20.11-rc
 - drop one patch (alarm cleanup is implemented)
 - drop patch that ends worker threads on cleanup.
   the test is calling rte_exit/eal_cleanup in a forked process.
   (could argue this is a test bug)!

v3
 - fix a couple of minor checkpatch complaints

v2
 - rebase after 20.05 file renames
 - incorporate review comment feedback
 - hold off some of the more involved patches for later

Stephen Hemminger (5):
  eal: close log in eal_cleanup
  eal: mp: end the multiprocess thread during cleanup
  eal: vfio: cleanup the mp sync handle
  eal: hotplug: cleanup multiprocess resources
  eal: malloc: cleanup mp resources

 lib/eal/common/eal_common_log.c  | 13 +++++++++++++
 lib/eal/common/eal_common_proc.c | 20 +++++++++++++++++---
 lib/eal/common/eal_private.h     |  7 +++++++
 lib/eal/common/hotplug_mp.c      |  5 +++++
 lib/eal/common/hotplug_mp.h      |  6 ++++++
 lib/eal/common/malloc_heap.c     |  6 ++++++
 lib/eal/common/malloc_heap.h     |  3 +++
 lib/eal/common/malloc_mp.c       | 12 ++++++++++++
 lib/eal/common/malloc_mp.h       |  3 +++
 lib/eal/linux/eal.c              |  7 +++++++
 lib/eal/linux/eal_log.c          |  8 ++++++++
 lib/eal/linux/eal_vfio.h         |  1 +
 lib/eal/linux/eal_vfio_mp_sync.c |  8 ++++++++
 13 files changed, 96 insertions(+), 3 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v4 1/5] eal: close log in eal_cleanup
  2021-11-13  0:28   ` [PATCH v4 0/5] cleanup more stuff " Stephen Hemminger
@ 2021-11-13  0:28     ` Stephen Hemminger
  2021-11-13  0:28     ` [PATCH v4 2/5] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13  0:28 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When application calls rte_eal_cleanup on shutdown,
the DPDK log should be closed and cleaned up.

This helps reduce false reports from tools like ASAN
and valgrind that track memory leaks.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/eal_common_log.c | 13 +++++++++++++
 lib/eal/common/eal_private.h    |  7 +++++++
 lib/eal/linux/eal.c             |  1 +
 lib/eal/linux/eal_log.c         |  8 ++++++++
 4 files changed, 29 insertions(+)

diff --git a/lib/eal/common/eal_common_log.c b/lib/eal/common/eal_common_log.c
index 1be35f539727..cbd0b851f216 100644
--- a/lib/eal/common/eal_common_log.c
+++ b/lib/eal/common/eal_common_log.c
@@ -18,6 +18,7 @@
 #include <rte_per_lcore.h>
 
 #include "eal_log.h"
+#include "eal_private.h"
 
 struct rte_log_dynamic_type {
 	const char *name;
@@ -535,3 +536,15 @@ eal_log_set_default(FILE *default_log)
 		"Debug dataplane logs available - lower performance\n");
 #endif
 }
+
+/*
+ * Called by eal_cleanup
+ */
+void
+rte_eal_log_cleanup(void)
+{
+	if (default_log_stream) {
+		fclose(default_log_stream);
+		default_log_stream = NULL;
+	}
+}
diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
index 36bcc0b5a492..2fbad534b938 100644
--- a/lib/eal/common/eal_private.h
+++ b/lib/eal/common/eal_private.h
@@ -152,6 +152,13 @@ int rte_eal_tailqs_init(void);
  */
 int rte_eal_intr_init(void);
 
+/**
+ * Close the default log stream
+ *
+ * This function is private to EAL.
+ */
+void rte_eal_log_cleanup(void);
+
 /**
  * Init alarm mechanism. This is to allow a callback be called after
  * specific time.
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 60b49248388e..b67030b2792e 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1370,6 +1370,7 @@ rte_eal_cleanup(void)
 	rte_trace_save();
 	eal_trace_fini();
 	eal_cleanup_config(internal_conf);
+	rte_eal_log_cleanup();
 	return 0;
 }
 
diff --git a/lib/eal/linux/eal_log.c b/lib/eal/linux/eal_log.c
index c0aa1007c4df..5a795ac9ebe3 100644
--- a/lib/eal/linux/eal_log.c
+++ b/lib/eal/linux/eal_log.c
@@ -37,8 +37,16 @@ console_log_write(__rte_unused void *c, const char *buf, size_t size)
 	return ret;
 }
 
+static int
+console_log_close(__rte_unused void *c)
+{
+	closelog();
+	return 0;
+}
+
 static cookie_io_functions_t console_log_func = {
 	.write = console_log_write,
+	.close = console_log_close,
 };
 
 /*
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v4 2/5] eal: mp: end the multiprocess thread during cleanup
  2021-11-13  0:28   ` [PATCH v4 0/5] cleanup more stuff " Stephen Hemminger
  2021-11-13  0:28     ` [PATCH v4 1/5] eal: close log in eal_cleanup Stephen Hemminger
@ 2021-11-13  0:28     ` Stephen Hemminger
  2021-11-13  0:28     ` [PATCH v4 3/5] eal: vfio: cleanup the mp sync handle Stephen Hemminger
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13  0:28 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, qi.z.zhang, Anatoly Burakov

When rte_eal_cleanup is called, all control threads should exit.
For the mp thread, this best handled by closing the mp_socket
and letting the thread see that.

This also fixes potential problems where the mp_socket gets
another hard error, and the thread runs away repeating itself
by reading the same error.

Fixes: 85d6815fa6d0 ("eal: close multi-process socket during cleanup")
Cc: qi.z.zhang@intel.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/common/eal_common_proc.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
index ebd0f6673b8b..b33d58ea0a0f 100644
--- a/lib/eal/common/eal_common_proc.c
+++ b/lib/eal/common/eal_common_proc.c
@@ -282,8 +282,17 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 	msgh.msg_control = control;
 	msgh.msg_controllen = sizeof(control);
 
+retry:
 	msglen = recvmsg(mp_fd, &msgh, 0);
+
+	/* zero length message means socket was closed */
+	if (msglen == 0)
+		return 0;
+
 	if (msglen < 0) {
+		if (errno == EINTR)
+			goto retry;
+
 		RTE_LOG(ERR, EAL, "recvmsg failed, %s\n", strerror(errno));
 		return -1;
 	}
@@ -311,7 +320,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 		RTE_LOG(ERR, EAL, "invalid received data length\n");
 		return -1;
 	}
-	return 0;
+	return msglen;
 }
 
 static void
@@ -385,8 +394,13 @@ mp_handle(void *arg __rte_unused)
 	struct sockaddr_un sa;
 
 	while (mp_fd >= 0) {
-		if (read_msg(&msg, &sa) == 0)
-			process_msg(&msg, &sa);
+		int ret;
+
+		ret = read_msg(&msg, &sa);
+		if (ret <= 0)
+			break;
+
+		process_msg(&msg, &sa);
 	}
 
 	return NULL;
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v4 3/5] eal: vfio: cleanup the mp sync handle
  2021-11-13  0:28   ` [PATCH v4 0/5] cleanup more stuff " Stephen Hemminger
  2021-11-13  0:28     ` [PATCH v4 1/5] eal: close log in eal_cleanup Stephen Hemminger
  2021-11-13  0:28     ` [PATCH v4 2/5] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
@ 2021-11-13  0:28     ` Stephen Hemminger
  2021-11-13  0:28     ` [PATCH v4 4/5] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
  2021-11-13  0:28     ` [PATCH v4 5/5] eal: malloc: cleanup mp resources Stephen Hemminger
  4 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13  0:28 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, anatoly.burakov, stable

When rte_eal_cleanup is called the rte_mp_action for VFIO
should be freed.

Fixes: edf73dd33072 ("ipc: handle unsupported IPC in action register")
Cc: anatoly.burakov@intel.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/linux/eal.c              | 4 ++++
 lib/eal/linux/eal_vfio.h         | 1 +
 lib/eal/linux/eal_vfio_mp_sync.c | 8 ++++++++
 3 files changed, 13 insertions(+)

diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index b67030b2792e..ad8f130b509d 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1362,7 +1362,11 @@ rte_eal_cleanup(void)
 
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_memseg_walk(mark_freeable, NULL);
+
 	rte_service_finalize();
+#ifdef VFIO_PRESENT
+	vfio_mp_sync_cleanup();
+#endif
 	rte_mp_channel_cleanup();
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
diff --git a/lib/eal/linux/eal_vfio.h b/lib/eal/linux/eal_vfio.h
index 6ebaca6a0cbf..921ee595388c 100644
--- a/lib/eal/linux/eal_vfio.h
+++ b/lib/eal/linux/eal_vfio.h
@@ -133,6 +133,7 @@ int
 vfio_has_supported_extensions(int vfio_container_fd);
 
 int vfio_mp_sync_setup(void);
+void vfio_mp_sync_cleanup(void);
 
 #define EAL_VFIO_MP "eal_vfio_mp_sync"
 
diff --git a/lib/eal/linux/eal_vfio_mp_sync.c b/lib/eal/linux/eal_vfio_mp_sync.c
index a2accfab3a38..d12bbaee648b 100644
--- a/lib/eal/linux/eal_vfio_mp_sync.c
+++ b/lib/eal/linux/eal_vfio_mp_sync.c
@@ -120,4 +120,12 @@ vfio_mp_sync_setup(void)
 	return 0;
 }
 
+void
+vfio_mp_sync_cleanup(void)
+{
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return;
+
+	rte_mp_action_unregister(EAL_VFIO_MP);
+}
 #endif
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v4 4/5] eal: hotplug: cleanup multiprocess resources
  2021-11-13  0:28   ` [PATCH v4 0/5] cleanup more stuff " Stephen Hemminger
                       ` (2 preceding siblings ...)
  2021-11-13  0:28     ` [PATCH v4 3/5] eal: vfio: cleanup the mp sync handle Stephen Hemminger
@ 2021-11-13  0:28     ` Stephen Hemminger
  2021-11-13  0:28     ` [PATCH v4 5/5] eal: malloc: cleanup mp resources Stephen Hemminger
  4 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13  0:28 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When rte_eal_cleanup is called, hotplug should unregister the
resources associated with the multi-process server.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/hotplug_mp.c | 5 +++++
 lib/eal/common/hotplug_mp.h | 6 ++++++
 lib/eal/linux/eal.c         | 1 +
 3 files changed, 12 insertions(+)

diff --git a/lib/eal/common/hotplug_mp.c b/lib/eal/common/hotplug_mp.c
index bf55056f59c0..bde0de196ef5 100644
--- a/lib/eal/common/hotplug_mp.c
+++ b/lib/eal/common/hotplug_mp.c
@@ -462,3 +462,8 @@ int eal_mp_dev_hotplug_init(void)
 
 	return 0;
 }
+
+void eal_mp_dev_hotplug_cleanup(void)
+{
+	rte_mp_action_unregister(EAL_DEV_MP_ACTION_REQUEST);
+}
diff --git a/lib/eal/common/hotplug_mp.h b/lib/eal/common/hotplug_mp.h
index 8fcf9b52e24c..4848446c852d 100644
--- a/lib/eal/common/hotplug_mp.h
+++ b/lib/eal/common/hotplug_mp.h
@@ -37,6 +37,12 @@ struct eal_dev_mp_req {
 int
 eal_mp_dev_hotplug_init(void);
 
+/**
+ * Unregister all mp action callbacks for hotplug.
+ */
+void
+eal_mp_dev_hotplug_cleanup(void);
+
 /**
  * This is a synchronous wrapper for secondary process send
  * request to primary process, this is invoked when an attach
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index ad8f130b509d..1a13a3474e38 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1370,6 +1370,7 @@ rte_eal_cleanup(void)
 	rte_mp_channel_cleanup();
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
+	eal_mp_dev_hotplug_cleanup();
 	rte_eal_alarm_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v4 5/5] eal: malloc: cleanup mp resources
  2021-11-13  0:28   ` [PATCH v4 0/5] cleanup more stuff " Stephen Hemminger
                       ` (3 preceding siblings ...)
  2021-11-13  0:28     ` [PATCH v4 4/5] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
@ 2021-11-13  0:28     ` Stephen Hemminger
  4 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13  0:28 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Anatoly Burakov

The mp action resources in malloc should be cleaned up via
rte_eal_cleanup.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/common/malloc_heap.c |  6 ++++++
 lib/eal/common/malloc_heap.h |  3 +++
 lib/eal/common/malloc_mp.c   | 12 ++++++++++++
 lib/eal/common/malloc_mp.h   |  3 +++
 lib/eal/linux/eal.c          |  1 +
 5 files changed, 25 insertions(+)

diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index 55aad2711b39..666f770fbf6d 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -1403,3 +1403,9 @@ rte_eal_malloc_heap_init(void)
 	/* add all IOVA-contiguous areas to the heap */
 	return rte_memseg_contig_walk(malloc_add_seg, NULL);
 }
+
+void
+rte_eal_malloc_heap_cleanup(void)
+{
+	unregister_mp_requests();
+}
diff --git a/lib/eal/common/malloc_heap.h b/lib/eal/common/malloc_heap.h
index 3a29d024b4ca..3b2fbc0aa059 100644
--- a/lib/eal/common/malloc_heap.h
+++ b/lib/eal/common/malloc_heap.h
@@ -85,4 +85,7 @@ malloc_socket_to_heap_id(unsigned int socket_id);
 int
 rte_eal_malloc_heap_init(void);
 
+void
+rte_eal_malloc_heap_cleanup(void);
+
 #endif /* MALLOC_HEAP_H_ */
diff --git a/lib/eal/common/malloc_mp.c b/lib/eal/common/malloc_mp.c
index 2e597a17a2f2..5f6f275b7859 100644
--- a/lib/eal/common/malloc_mp.c
+++ b/lib/eal/common/malloc_mp.c
@@ -806,3 +806,15 @@ register_mp_requests(void)
 	}
 	return 0;
 }
+
+void
+unregister_mp_requests(void)
+{
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		rte_mp_action_unregister(MP_ACTION_REQUEST);
+	} else {
+		rte_mp_action_unregister(MP_ACTION_SYNC);
+		rte_mp_action_unregister(MP_ACTION_ROLLBACK);
+		rte_mp_action_unregister(MP_ACTION_RESPONSE);
+	}
+}
diff --git a/lib/eal/common/malloc_mp.h b/lib/eal/common/malloc_mp.h
index 0095062b72f2..c806f7beafe0 100644
--- a/lib/eal/common/malloc_mp.h
+++ b/lib/eal/common/malloc_mp.h
@@ -63,6 +63,9 @@ struct malloc_mp_req {
 int
 register_mp_requests(void);
 
+void
+unregister_mp_requests(void);
+
 int
 request_to_primary(struct malloc_mp_req *req);
 
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 1a13a3474e38..ea663fe02b93 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1371,6 +1371,7 @@ rte_eal_cleanup(void)
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
 	eal_mp_dev_hotplug_cleanup();
+	rte_eal_malloc_heap_cleanup();
 	rte_eal_alarm_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v5 0/5] cleanup DPDK resources via eal_cleanup
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
                     ` (9 preceding siblings ...)
  2021-11-13  0:28   ` [PATCH v4 0/5] cleanup more stuff " Stephen Hemminger
@ 2021-11-13  3:32   ` Stephen Hemminger
  2021-11-13  3:32     ` [PATCH v5 1/5] eal: close log in eal_cleanup Stephen Hemminger
                       ` (4 more replies)
  2021-11-13 17:22   ` [PATCH v6 0/5] cleanup more resources on eal_cleanup Stephen Hemminger
  11 siblings, 5 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13  3:32 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When testing using ASAN or valgrind with DPDK; there are lots of leftover
memory and file descriptors. This makes it hard to find application
leaks versus internal DPDK leaks.

The DPDK has a function that applications can use to tell it
to cleanup resources on shutdown (rte_eal_cleanup). But the
current coverage of that API is spotty. Many internal parts of
DPDK leave files and allocated memory behind.

This patch set is a first step at getting the sub-parts of
DPDK to cleanup after themselves. These are the easier ones,
the harder and more critical ones are in the drivers
and the memory subsystem.

There should be no new exposed API or ABI changes here.

v5
 - add stub for windows build in rte_malloc cleanup

v4
 - rebase to 20.11-rc
 - drop one patch (alarm cleanup is implemented)
 - drop patch that ends worker threads on cleanup.
   the test is calling rte_exit/eal_cleanup in a forked process.
   (could argue this is a test bug)!

v3
 - fix a couple of minor checkpatch complaints

v2
 - rebase after 20.05 file renames
 - incorporate review comment feedback
 - hold off some of the more involved patches for later

Stephen Hemminger (5):
  eal: close log in eal_cleanup
  eal: mp: end the multiprocess thread during cleanup
  eal: vfio: cleanup the mp sync handle
  eal: hotplug: cleanup multiprocess resources
  eal: malloc: cleanup mp resources

 lib/eal/common/eal_common_log.c  | 13 +++++++++++++
 lib/eal/common/eal_common_proc.c | 20 +++++++++++++++++---
 lib/eal/common/eal_private.h     |  7 +++++++
 lib/eal/common/hotplug_mp.c      |  5 +++++
 lib/eal/common/hotplug_mp.h      |  6 ++++++
 lib/eal/common/malloc_heap.c     |  6 ++++++
 lib/eal/common/malloc_heap.h     |  3 +++
 lib/eal/common/malloc_mp.c       | 12 ++++++++++++
 lib/eal/common/malloc_mp.h       |  3 +++
 lib/eal/linux/eal.c              |  7 +++++++
 lib/eal/linux/eal_log.c          |  8 ++++++++
 lib/eal/linux/eal_vfio.h         |  1 +
 lib/eal/linux/eal_vfio_mp_sync.c |  8 ++++++++
 lib/eal/windows/eal_mp.c         |  7 +++++++
 14 files changed, 103 insertions(+), 3 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v5 1/5] eal: close log in eal_cleanup
  2021-11-13  3:32   ` [PATCH v5 0/5] cleanup DPDK resources via eal_cleanup Stephen Hemminger
@ 2021-11-13  3:32     ` Stephen Hemminger
  2021-11-13  3:32     ` [PATCH v5 2/5] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13  3:32 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When application calls rte_eal_cleanup on shutdown,
the DPDK log should be closed and cleaned up.

This helps reduce false reports from tools like ASAN
and valgrind that track memory leaks.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/eal_common_log.c | 13 +++++++++++++
 lib/eal/common/eal_private.h    |  7 +++++++
 lib/eal/linux/eal.c             |  1 +
 lib/eal/linux/eal_log.c         |  8 ++++++++
 4 files changed, 29 insertions(+)

diff --git a/lib/eal/common/eal_common_log.c b/lib/eal/common/eal_common_log.c
index 1be35f539727..cbd0b851f216 100644
--- a/lib/eal/common/eal_common_log.c
+++ b/lib/eal/common/eal_common_log.c
@@ -18,6 +18,7 @@
 #include <rte_per_lcore.h>
 
 #include "eal_log.h"
+#include "eal_private.h"
 
 struct rte_log_dynamic_type {
 	const char *name;
@@ -535,3 +536,15 @@ eal_log_set_default(FILE *default_log)
 		"Debug dataplane logs available - lower performance\n");
 #endif
 }
+
+/*
+ * Called by eal_cleanup
+ */
+void
+rte_eal_log_cleanup(void)
+{
+	if (default_log_stream) {
+		fclose(default_log_stream);
+		default_log_stream = NULL;
+	}
+}
diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
index 36bcc0b5a492..2fbad534b938 100644
--- a/lib/eal/common/eal_private.h
+++ b/lib/eal/common/eal_private.h
@@ -152,6 +152,13 @@ int rte_eal_tailqs_init(void);
  */
 int rte_eal_intr_init(void);
 
+/**
+ * Close the default log stream
+ *
+ * This function is private to EAL.
+ */
+void rte_eal_log_cleanup(void);
+
 /**
  * Init alarm mechanism. This is to allow a callback be called after
  * specific time.
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 60b49248388e..b67030b2792e 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1370,6 +1370,7 @@ rte_eal_cleanup(void)
 	rte_trace_save();
 	eal_trace_fini();
 	eal_cleanup_config(internal_conf);
+	rte_eal_log_cleanup();
 	return 0;
 }
 
diff --git a/lib/eal/linux/eal_log.c b/lib/eal/linux/eal_log.c
index c0aa1007c4df..5a795ac9ebe3 100644
--- a/lib/eal/linux/eal_log.c
+++ b/lib/eal/linux/eal_log.c
@@ -37,8 +37,16 @@ console_log_write(__rte_unused void *c, const char *buf, size_t size)
 	return ret;
 }
 
+static int
+console_log_close(__rte_unused void *c)
+{
+	closelog();
+	return 0;
+}
+
 static cookie_io_functions_t console_log_func = {
 	.write = console_log_write,
+	.close = console_log_close,
 };
 
 /*
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v5 2/5] eal: mp: end the multiprocess thread during cleanup
  2021-11-13  3:32   ` [PATCH v5 0/5] cleanup DPDK resources via eal_cleanup Stephen Hemminger
  2021-11-13  3:32     ` [PATCH v5 1/5] eal: close log in eal_cleanup Stephen Hemminger
@ 2021-11-13  3:32     ` Stephen Hemminger
  2021-11-13  3:32     ` [PATCH v5 3/5] eal: vfio: cleanup the mp sync handle Stephen Hemminger
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13  3:32 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, qi.z.zhang, Anatoly Burakov

When rte_eal_cleanup is called, all control threads should exit.
For the mp thread, this best handled by closing the mp_socket
and letting the thread see that.

This also fixes potential problems where the mp_socket gets
another hard error, and the thread runs away repeating itself
by reading the same error.

Fixes: 85d6815fa6d0 ("eal: close multi-process socket during cleanup")
Cc: qi.z.zhang@intel.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/common/eal_common_proc.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
index ebd0f6673b8b..b33d58ea0a0f 100644
--- a/lib/eal/common/eal_common_proc.c
+++ b/lib/eal/common/eal_common_proc.c
@@ -282,8 +282,17 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 	msgh.msg_control = control;
 	msgh.msg_controllen = sizeof(control);
 
+retry:
 	msglen = recvmsg(mp_fd, &msgh, 0);
+
+	/* zero length message means socket was closed */
+	if (msglen == 0)
+		return 0;
+
 	if (msglen < 0) {
+		if (errno == EINTR)
+			goto retry;
+
 		RTE_LOG(ERR, EAL, "recvmsg failed, %s\n", strerror(errno));
 		return -1;
 	}
@@ -311,7 +320,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 		RTE_LOG(ERR, EAL, "invalid received data length\n");
 		return -1;
 	}
-	return 0;
+	return msglen;
 }
 
 static void
@@ -385,8 +394,13 @@ mp_handle(void *arg __rte_unused)
 	struct sockaddr_un sa;
 
 	while (mp_fd >= 0) {
-		if (read_msg(&msg, &sa) == 0)
-			process_msg(&msg, &sa);
+		int ret;
+
+		ret = read_msg(&msg, &sa);
+		if (ret <= 0)
+			break;
+
+		process_msg(&msg, &sa);
 	}
 
 	return NULL;
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v5 3/5] eal: vfio: cleanup the mp sync handle
  2021-11-13  3:32   ` [PATCH v5 0/5] cleanup DPDK resources via eal_cleanup Stephen Hemminger
  2021-11-13  3:32     ` [PATCH v5 1/5] eal: close log in eal_cleanup Stephen Hemminger
  2021-11-13  3:32     ` [PATCH v5 2/5] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
@ 2021-11-13  3:32     ` Stephen Hemminger
  2021-11-13  3:32     ` [PATCH v5 4/5] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
  2021-11-13  3:32     ` [PATCH v5 5/5] eal: malloc: cleanup mp resources Stephen Hemminger
  4 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13  3:32 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, anatoly.burakov, stable

When rte_eal_cleanup is called the rte_mp_action for VFIO
should be freed.

Fixes: edf73dd33072 ("ipc: handle unsupported IPC in action register")
Cc: anatoly.burakov@intel.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/linux/eal.c              | 4 ++++
 lib/eal/linux/eal_vfio.h         | 1 +
 lib/eal/linux/eal_vfio_mp_sync.c | 8 ++++++++
 3 files changed, 13 insertions(+)

diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index b67030b2792e..ad8f130b509d 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1362,7 +1362,11 @@ rte_eal_cleanup(void)
 
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_memseg_walk(mark_freeable, NULL);
+
 	rte_service_finalize();
+#ifdef VFIO_PRESENT
+	vfio_mp_sync_cleanup();
+#endif
 	rte_mp_channel_cleanup();
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
diff --git a/lib/eal/linux/eal_vfio.h b/lib/eal/linux/eal_vfio.h
index 6ebaca6a0cbf..921ee595388c 100644
--- a/lib/eal/linux/eal_vfio.h
+++ b/lib/eal/linux/eal_vfio.h
@@ -133,6 +133,7 @@ int
 vfio_has_supported_extensions(int vfio_container_fd);
 
 int vfio_mp_sync_setup(void);
+void vfio_mp_sync_cleanup(void);
 
 #define EAL_VFIO_MP "eal_vfio_mp_sync"
 
diff --git a/lib/eal/linux/eal_vfio_mp_sync.c b/lib/eal/linux/eal_vfio_mp_sync.c
index a2accfab3a38..d12bbaee648b 100644
--- a/lib/eal/linux/eal_vfio_mp_sync.c
+++ b/lib/eal/linux/eal_vfio_mp_sync.c
@@ -120,4 +120,12 @@ vfio_mp_sync_setup(void)
 	return 0;
 }
 
+void
+vfio_mp_sync_cleanup(void)
+{
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return;
+
+	rte_mp_action_unregister(EAL_VFIO_MP);
+}
 #endif
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v5 4/5] eal: hotplug: cleanup multiprocess resources
  2021-11-13  3:32   ` [PATCH v5 0/5] cleanup DPDK resources via eal_cleanup Stephen Hemminger
                       ` (2 preceding siblings ...)
  2021-11-13  3:32     ` [PATCH v5 3/5] eal: vfio: cleanup the mp sync handle Stephen Hemminger
@ 2021-11-13  3:32     ` Stephen Hemminger
  2021-11-13  3:32     ` [PATCH v5 5/5] eal: malloc: cleanup mp resources Stephen Hemminger
  4 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13  3:32 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When rte_eal_cleanup is called, hotplug should unregister the
resources associated with the multi-process server.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/hotplug_mp.c | 5 +++++
 lib/eal/common/hotplug_mp.h | 6 ++++++
 lib/eal/linux/eal.c         | 1 +
 3 files changed, 12 insertions(+)

diff --git a/lib/eal/common/hotplug_mp.c b/lib/eal/common/hotplug_mp.c
index bf55056f59c0..bde0de196ef5 100644
--- a/lib/eal/common/hotplug_mp.c
+++ b/lib/eal/common/hotplug_mp.c
@@ -462,3 +462,8 @@ int eal_mp_dev_hotplug_init(void)
 
 	return 0;
 }
+
+void eal_mp_dev_hotplug_cleanup(void)
+{
+	rte_mp_action_unregister(EAL_DEV_MP_ACTION_REQUEST);
+}
diff --git a/lib/eal/common/hotplug_mp.h b/lib/eal/common/hotplug_mp.h
index 8fcf9b52e24c..4848446c852d 100644
--- a/lib/eal/common/hotplug_mp.h
+++ b/lib/eal/common/hotplug_mp.h
@@ -37,6 +37,12 @@ struct eal_dev_mp_req {
 int
 eal_mp_dev_hotplug_init(void);
 
+/**
+ * Unregister all mp action callbacks for hotplug.
+ */
+void
+eal_mp_dev_hotplug_cleanup(void);
+
 /**
  * This is a synchronous wrapper for secondary process send
  * request to primary process, this is invoked when an attach
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index ad8f130b509d..1a13a3474e38 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1370,6 +1370,7 @@ rte_eal_cleanup(void)
 	rte_mp_channel_cleanup();
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
+	eal_mp_dev_hotplug_cleanup();
 	rte_eal_alarm_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v5 5/5] eal: malloc: cleanup mp resources
  2021-11-13  3:32   ` [PATCH v5 0/5] cleanup DPDK resources via eal_cleanup Stephen Hemminger
                       ` (3 preceding siblings ...)
  2021-11-13  3:32     ` [PATCH v5 4/5] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
@ 2021-11-13  3:32     ` Stephen Hemminger
  4 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13  3:32 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, Anatoly Burakov, Dmitry Kozlyuk,
	Narcisa Ana Maria Vasile, Dmitry Malloy, Pallavi Kadam

The mp action resources in malloc should be cleaned up via
rte_eal_cleanup.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/common/malloc_heap.c |  6 ++++++
 lib/eal/common/malloc_heap.h |  3 +++
 lib/eal/common/malloc_mp.c   | 12 ++++++++++++
 lib/eal/common/malloc_mp.h   |  3 +++
 lib/eal/linux/eal.c          |  1 +
 lib/eal/windows/eal_mp.c     |  7 +++++++
 6 files changed, 32 insertions(+)

diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index 55aad2711b39..666f770fbf6d 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -1403,3 +1403,9 @@ rte_eal_malloc_heap_init(void)
 	/* add all IOVA-contiguous areas to the heap */
 	return rte_memseg_contig_walk(malloc_add_seg, NULL);
 }
+
+void
+rte_eal_malloc_heap_cleanup(void)
+{
+	unregister_mp_requests();
+}
diff --git a/lib/eal/common/malloc_heap.h b/lib/eal/common/malloc_heap.h
index 3a29d024b4ca..3b2fbc0aa059 100644
--- a/lib/eal/common/malloc_heap.h
+++ b/lib/eal/common/malloc_heap.h
@@ -85,4 +85,7 @@ malloc_socket_to_heap_id(unsigned int socket_id);
 int
 rte_eal_malloc_heap_init(void);
 
+void
+rte_eal_malloc_heap_cleanup(void);
+
 #endif /* MALLOC_HEAP_H_ */
diff --git a/lib/eal/common/malloc_mp.c b/lib/eal/common/malloc_mp.c
index 2e597a17a2f2..5f6f275b7859 100644
--- a/lib/eal/common/malloc_mp.c
+++ b/lib/eal/common/malloc_mp.c
@@ -806,3 +806,15 @@ register_mp_requests(void)
 	}
 	return 0;
 }
+
+void
+unregister_mp_requests(void)
+{
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		rte_mp_action_unregister(MP_ACTION_REQUEST);
+	} else {
+		rte_mp_action_unregister(MP_ACTION_SYNC);
+		rte_mp_action_unregister(MP_ACTION_ROLLBACK);
+		rte_mp_action_unregister(MP_ACTION_RESPONSE);
+	}
+}
diff --git a/lib/eal/common/malloc_mp.h b/lib/eal/common/malloc_mp.h
index 0095062b72f2..c806f7beafe0 100644
--- a/lib/eal/common/malloc_mp.h
+++ b/lib/eal/common/malloc_mp.h
@@ -63,6 +63,9 @@ struct malloc_mp_req {
 int
 register_mp_requests(void);
 
+void
+unregister_mp_requests(void);
+
 int
 request_to_primary(struct malloc_mp_req *req);
 
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 1a13a3474e38..ea663fe02b93 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1371,6 +1371,7 @@ rte_eal_cleanup(void)
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
 	eal_mp_dev_hotplug_cleanup();
+	rte_eal_malloc_heap_cleanup();
 	rte_eal_alarm_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
diff --git a/lib/eal/windows/eal_mp.c b/lib/eal/windows/eal_mp.c
index f2fa9366f668..a9ebea1da257 100644
--- a/lib/eal/windows/eal_mp.c
+++ b/lib/eal/windows/eal_mp.c
@@ -87,6 +87,13 @@ register_mp_requests(void)
 	return 0;
 }
 
+void
+register_mp_requests(void)
+{
+	/* Non-stub function succeeds if multi-process is not supported. */
+	EAL_LOG_STUB();
+}
+
 int
 request_to_primary(struct malloc_mp_req *req)
 {
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v6 0/5] cleanup more resources on eal_cleanup
  2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
                     ` (10 preceding siblings ...)
  2021-11-13  3:32   ` [PATCH v5 0/5] cleanup DPDK resources via eal_cleanup Stephen Hemminger
@ 2021-11-13 17:22   ` Stephen Hemminger
  2021-11-13 17:22     ` [PATCH v6 1/5] eal: close log in eal_cleanup Stephen Hemminger
                       ` (5 more replies)
  11 siblings, 6 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13 17:22 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When testing using ASAN or valgrind with DPDK; there are lots of leftover
memory and file descriptors. This makes it hard to find application
leaks versus internal DPDK leaks.

The DPDK has a function that applications can use to tell it
to cleanup resources on shutdown (rte_eal_cleanup). But the
current coverage of that API is spotty. Many internal parts of
DPDK leave files and allocated memory behind.

This patch set is a first step at getting the sub-parts of
DPDK to cleanup after themselves. These are the easier ones,
the harder and more critical ones are in the drivers
and the memory subsystem.

There should be no new exposed API or ABI changes here.

v6 - fix windows stub 
v5 - add stub for windows build in rte_malloc cleanup
v4
 - rebase to 20.11-rc
 - drop one patch (alarm cleanup is implemented)
 - drop patch that ends worker threads on cleanup.
   the test is calling rte_exit/eal_cleanup in a forked process.
   (could argue this is a test bug)!


Stephen Hemminger (5):
  eal: close log in eal_cleanup
  eal: mp: end the multiprocess thread during cleanup
  eal: vfio: cleanup the mp sync handle
  eal: hotplug: cleanup multiprocess resources
  eal: malloc: cleanup mp resources

 lib/eal/common/eal_common_log.c  | 13 +++++++++++++
 lib/eal/common/eal_common_proc.c | 20 +++++++++++++++++---
 lib/eal/common/eal_private.h     |  7 +++++++
 lib/eal/common/hotplug_mp.c      |  5 +++++
 lib/eal/common/hotplug_mp.h      |  6 ++++++
 lib/eal/common/malloc_heap.c     |  6 ++++++
 lib/eal/common/malloc_heap.h     |  3 +++
 lib/eal/common/malloc_mp.c       | 12 ++++++++++++
 lib/eal/common/malloc_mp.h       |  3 +++
 lib/eal/linux/eal.c              |  7 +++++++
 lib/eal/linux/eal_log.c          |  8 ++++++++
 lib/eal/linux/eal_vfio.h         |  1 +
 lib/eal/linux/eal_vfio_mp_sync.c |  8 ++++++++
 lib/eal/windows/eal_mp.c         |  7 +++++++
 14 files changed, 103 insertions(+), 3 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v6 1/5] eal: close log in eal_cleanup
  2021-11-13 17:22   ` [PATCH v6 0/5] cleanup more resources on eal_cleanup Stephen Hemminger
@ 2021-11-13 17:22     ` Stephen Hemminger
  2021-11-13 17:22     ` [PATCH v6 2/5] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
                       ` (4 subsequent siblings)
  5 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13 17:22 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When application calls rte_eal_cleanup on shutdown,
the DPDK log should be closed and cleaned up.

This helps reduce false reports from tools like ASAN
and valgrind that track memory leaks.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/eal_common_log.c | 13 +++++++++++++
 lib/eal/common/eal_private.h    |  7 +++++++
 lib/eal/linux/eal.c             |  1 +
 lib/eal/linux/eal_log.c         |  8 ++++++++
 4 files changed, 29 insertions(+)

diff --git a/lib/eal/common/eal_common_log.c b/lib/eal/common/eal_common_log.c
index 1be35f539727..cbd0b851f216 100644
--- a/lib/eal/common/eal_common_log.c
+++ b/lib/eal/common/eal_common_log.c
@@ -18,6 +18,7 @@
 #include <rte_per_lcore.h>
 
 #include "eal_log.h"
+#include "eal_private.h"
 
 struct rte_log_dynamic_type {
 	const char *name;
@@ -535,3 +536,15 @@ eal_log_set_default(FILE *default_log)
 		"Debug dataplane logs available - lower performance\n");
 #endif
 }
+
+/*
+ * Called by eal_cleanup
+ */
+void
+rte_eal_log_cleanup(void)
+{
+	if (default_log_stream) {
+		fclose(default_log_stream);
+		default_log_stream = NULL;
+	}
+}
diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
index 36bcc0b5a492..2fbad534b938 100644
--- a/lib/eal/common/eal_private.h
+++ b/lib/eal/common/eal_private.h
@@ -152,6 +152,13 @@ int rte_eal_tailqs_init(void);
  */
 int rte_eal_intr_init(void);
 
+/**
+ * Close the default log stream
+ *
+ * This function is private to EAL.
+ */
+void rte_eal_log_cleanup(void);
+
 /**
  * Init alarm mechanism. This is to allow a callback be called after
  * specific time.
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 60b49248388e..b67030b2792e 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1370,6 +1370,7 @@ rte_eal_cleanup(void)
 	rte_trace_save();
 	eal_trace_fini();
 	eal_cleanup_config(internal_conf);
+	rte_eal_log_cleanup();
 	return 0;
 }
 
diff --git a/lib/eal/linux/eal_log.c b/lib/eal/linux/eal_log.c
index c0aa1007c4df..5a795ac9ebe3 100644
--- a/lib/eal/linux/eal_log.c
+++ b/lib/eal/linux/eal_log.c
@@ -37,8 +37,16 @@ console_log_write(__rte_unused void *c, const char *buf, size_t size)
 	return ret;
 }
 
+static int
+console_log_close(__rte_unused void *c)
+{
+	closelog();
+	return 0;
+}
+
 static cookie_io_functions_t console_log_func = {
 	.write = console_log_write,
+	.close = console_log_close,
 };
 
 /*
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v6 2/5] eal: mp: end the multiprocess thread during cleanup
  2021-11-13 17:22   ` [PATCH v6 0/5] cleanup more resources on eal_cleanup Stephen Hemminger
  2021-11-13 17:22     ` [PATCH v6 1/5] eal: close log in eal_cleanup Stephen Hemminger
@ 2021-11-13 17:22     ` Stephen Hemminger
  2021-11-13 17:22     ` [PATCH v6 3/5] eal: vfio: cleanup the mp sync handle Stephen Hemminger
                       ` (3 subsequent siblings)
  5 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13 17:22 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, qi.z.zhang, Anatoly Burakov

When rte_eal_cleanup is called, all control threads should exit.
For the mp thread, this best handled by closing the mp_socket
and letting the thread see that.

This also fixes potential problems where the mp_socket gets
another hard error, and the thread runs away repeating itself
by reading the same error.

Fixes: 85d6815fa6d0 ("eal: close multi-process socket during cleanup")
Cc: qi.z.zhang@intel.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/common/eal_common_proc.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
index ebd0f6673b8b..b33d58ea0a0f 100644
--- a/lib/eal/common/eal_common_proc.c
+++ b/lib/eal/common/eal_common_proc.c
@@ -282,8 +282,17 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 	msgh.msg_control = control;
 	msgh.msg_controllen = sizeof(control);
 
+retry:
 	msglen = recvmsg(mp_fd, &msgh, 0);
+
+	/* zero length message means socket was closed */
+	if (msglen == 0)
+		return 0;
+
 	if (msglen < 0) {
+		if (errno == EINTR)
+			goto retry;
+
 		RTE_LOG(ERR, EAL, "recvmsg failed, %s\n", strerror(errno));
 		return -1;
 	}
@@ -311,7 +320,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 		RTE_LOG(ERR, EAL, "invalid received data length\n");
 		return -1;
 	}
-	return 0;
+	return msglen;
 }
 
 static void
@@ -385,8 +394,13 @@ mp_handle(void *arg __rte_unused)
 	struct sockaddr_un sa;
 
 	while (mp_fd >= 0) {
-		if (read_msg(&msg, &sa) == 0)
-			process_msg(&msg, &sa);
+		int ret;
+
+		ret = read_msg(&msg, &sa);
+		if (ret <= 0)
+			break;
+
+		process_msg(&msg, &sa);
 	}
 
 	return NULL;
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v6 3/5] eal: vfio: cleanup the mp sync handle
  2021-11-13 17:22   ` [PATCH v6 0/5] cleanup more resources on eal_cleanup Stephen Hemminger
  2021-11-13 17:22     ` [PATCH v6 1/5] eal: close log in eal_cleanup Stephen Hemminger
  2021-11-13 17:22     ` [PATCH v6 2/5] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
@ 2021-11-13 17:22     ` Stephen Hemminger
  2021-11-13 17:22     ` [PATCH v6 4/5] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
                       ` (2 subsequent siblings)
  5 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13 17:22 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, anatoly.burakov, stable

When rte_eal_cleanup is called the rte_mp_action for VFIO
should be freed.

Fixes: edf73dd33072 ("ipc: handle unsupported IPC in action register")
Cc: anatoly.burakov@intel.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/linux/eal.c              | 4 ++++
 lib/eal/linux/eal_vfio.h         | 1 +
 lib/eal/linux/eal_vfio_mp_sync.c | 8 ++++++++
 3 files changed, 13 insertions(+)

diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index b67030b2792e..ad8f130b509d 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1362,7 +1362,11 @@ rte_eal_cleanup(void)
 
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_memseg_walk(mark_freeable, NULL);
+
 	rte_service_finalize();
+#ifdef VFIO_PRESENT
+	vfio_mp_sync_cleanup();
+#endif
 	rte_mp_channel_cleanup();
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
diff --git a/lib/eal/linux/eal_vfio.h b/lib/eal/linux/eal_vfio.h
index 6ebaca6a0cbf..921ee595388c 100644
--- a/lib/eal/linux/eal_vfio.h
+++ b/lib/eal/linux/eal_vfio.h
@@ -133,6 +133,7 @@ int
 vfio_has_supported_extensions(int vfio_container_fd);
 
 int vfio_mp_sync_setup(void);
+void vfio_mp_sync_cleanup(void);
 
 #define EAL_VFIO_MP "eal_vfio_mp_sync"
 
diff --git a/lib/eal/linux/eal_vfio_mp_sync.c b/lib/eal/linux/eal_vfio_mp_sync.c
index a2accfab3a38..d12bbaee648b 100644
--- a/lib/eal/linux/eal_vfio_mp_sync.c
+++ b/lib/eal/linux/eal_vfio_mp_sync.c
@@ -120,4 +120,12 @@ vfio_mp_sync_setup(void)
 	return 0;
 }
 
+void
+vfio_mp_sync_cleanup(void)
+{
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return;
+
+	rte_mp_action_unregister(EAL_VFIO_MP);
+}
 #endif
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v6 4/5] eal: hotplug: cleanup multiprocess resources
  2021-11-13 17:22   ` [PATCH v6 0/5] cleanup more resources on eal_cleanup Stephen Hemminger
                       ` (2 preceding siblings ...)
  2021-11-13 17:22     ` [PATCH v6 3/5] eal: vfio: cleanup the mp sync handle Stephen Hemminger
@ 2021-11-13 17:22     ` Stephen Hemminger
  2021-11-13 17:22     ` [PATCH v6 5/5] eal: malloc: cleanup mp resources Stephen Hemminger
  2022-02-11 20:17     ` [PATCH v6 0/5] cleanup more resources on eal_cleanup Thomas Monjalon
  5 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13 17:22 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

When rte_eal_cleanup is called, hotplug should unregister the
resources associated with the multi-process server.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/hotplug_mp.c | 5 +++++
 lib/eal/common/hotplug_mp.h | 6 ++++++
 lib/eal/linux/eal.c         | 1 +
 3 files changed, 12 insertions(+)

diff --git a/lib/eal/common/hotplug_mp.c b/lib/eal/common/hotplug_mp.c
index bf55056f59c0..bde0de196ef5 100644
--- a/lib/eal/common/hotplug_mp.c
+++ b/lib/eal/common/hotplug_mp.c
@@ -462,3 +462,8 @@ int eal_mp_dev_hotplug_init(void)
 
 	return 0;
 }
+
+void eal_mp_dev_hotplug_cleanup(void)
+{
+	rte_mp_action_unregister(EAL_DEV_MP_ACTION_REQUEST);
+}
diff --git a/lib/eal/common/hotplug_mp.h b/lib/eal/common/hotplug_mp.h
index 8fcf9b52e24c..4848446c852d 100644
--- a/lib/eal/common/hotplug_mp.h
+++ b/lib/eal/common/hotplug_mp.h
@@ -37,6 +37,12 @@ struct eal_dev_mp_req {
 int
 eal_mp_dev_hotplug_init(void);
 
+/**
+ * Unregister all mp action callbacks for hotplug.
+ */
+void
+eal_mp_dev_hotplug_cleanup(void);
+
 /**
  * This is a synchronous wrapper for secondary process send
  * request to primary process, this is invoked when an attach
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index ad8f130b509d..1a13a3474e38 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1370,6 +1370,7 @@ rte_eal_cleanup(void)
 	rte_mp_channel_cleanup();
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
+	eal_mp_dev_hotplug_cleanup();
 	rte_eal_alarm_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* [PATCH v6 5/5] eal: malloc: cleanup mp resources
  2021-11-13 17:22   ` [PATCH v6 0/5] cleanup more resources on eal_cleanup Stephen Hemminger
                       ` (3 preceding siblings ...)
  2021-11-13 17:22     ` [PATCH v6 4/5] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
@ 2021-11-13 17:22     ` Stephen Hemminger
  2022-02-11 20:17     ` [PATCH v6 0/5] cleanup more resources on eal_cleanup Thomas Monjalon
  5 siblings, 0 replies; 74+ messages in thread
From: Stephen Hemminger @ 2021-11-13 17:22 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, Anatoly Burakov, Dmitry Kozlyuk,
	Narcisa Ana Maria Vasile, Dmitry Malloy, Pallavi Kadam

The mp action resources in malloc should be cleaned up via
rte_eal_cleanup.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/common/malloc_heap.c |  6 ++++++
 lib/eal/common/malloc_heap.h |  3 +++
 lib/eal/common/malloc_mp.c   | 12 ++++++++++++
 lib/eal/common/malloc_mp.h   |  3 +++
 lib/eal/linux/eal.c          |  1 +
 lib/eal/windows/eal_mp.c     |  7 +++++++
 6 files changed, 32 insertions(+)

diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index 55aad2711b39..666f770fbf6d 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -1403,3 +1403,9 @@ rte_eal_malloc_heap_init(void)
 	/* add all IOVA-contiguous areas to the heap */
 	return rte_memseg_contig_walk(malloc_add_seg, NULL);
 }
+
+void
+rte_eal_malloc_heap_cleanup(void)
+{
+	unregister_mp_requests();
+}
diff --git a/lib/eal/common/malloc_heap.h b/lib/eal/common/malloc_heap.h
index 3a29d024b4ca..3b2fbc0aa059 100644
--- a/lib/eal/common/malloc_heap.h
+++ b/lib/eal/common/malloc_heap.h
@@ -85,4 +85,7 @@ malloc_socket_to_heap_id(unsigned int socket_id);
 int
 rte_eal_malloc_heap_init(void);
 
+void
+rte_eal_malloc_heap_cleanup(void);
+
 #endif /* MALLOC_HEAP_H_ */
diff --git a/lib/eal/common/malloc_mp.c b/lib/eal/common/malloc_mp.c
index 2e597a17a2f2..5f6f275b7859 100644
--- a/lib/eal/common/malloc_mp.c
+++ b/lib/eal/common/malloc_mp.c
@@ -806,3 +806,15 @@ register_mp_requests(void)
 	}
 	return 0;
 }
+
+void
+unregister_mp_requests(void)
+{
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		rte_mp_action_unregister(MP_ACTION_REQUEST);
+	} else {
+		rte_mp_action_unregister(MP_ACTION_SYNC);
+		rte_mp_action_unregister(MP_ACTION_ROLLBACK);
+		rte_mp_action_unregister(MP_ACTION_RESPONSE);
+	}
+}
diff --git a/lib/eal/common/malloc_mp.h b/lib/eal/common/malloc_mp.h
index 0095062b72f2..c806f7beafe0 100644
--- a/lib/eal/common/malloc_mp.h
+++ b/lib/eal/common/malloc_mp.h
@@ -63,6 +63,9 @@ struct malloc_mp_req {
 int
 register_mp_requests(void);
 
+void
+unregister_mp_requests(void);
+
 int
 request_to_primary(struct malloc_mp_req *req);
 
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 1a13a3474e38..ea663fe02b93 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1371,6 +1371,7 @@ rte_eal_cleanup(void)
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
 	eal_mp_dev_hotplug_cleanup();
+	rte_eal_malloc_heap_cleanup();
 	rte_eal_alarm_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
diff --git a/lib/eal/windows/eal_mp.c b/lib/eal/windows/eal_mp.c
index f2fa9366f668..76413f49a393 100644
--- a/lib/eal/windows/eal_mp.c
+++ b/lib/eal/windows/eal_mp.c
@@ -87,6 +87,13 @@ register_mp_requests(void)
 	return 0;
 }
 
+void
+unregister_mp_requests(void)
+{
+	/* Non-stub function succeeds if multi-process is not supported. */
+	EAL_LOG_STUB();
+}
+
 int
 request_to_primary(struct malloc_mp_req *req)
 {
-- 
2.30.2


^ permalink raw reply	[flat|nested] 74+ messages in thread

* Re: [PATCH v6 0/5] cleanup more resources on eal_cleanup
  2021-11-13 17:22   ` [PATCH v6 0/5] cleanup more resources on eal_cleanup Stephen Hemminger
                       ` (4 preceding siblings ...)
  2021-11-13 17:22     ` [PATCH v6 5/5] eal: malloc: cleanup mp resources Stephen Hemminger
@ 2022-02-11 20:17     ` Thomas Monjalon
  5 siblings, 0 replies; 74+ messages in thread
From: Thomas Monjalon @ 2022-02-11 20:17 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

13/11/2021 18:22, Stephen Hemminger:
> Stephen Hemminger (5):
>   eal: close log in eal_cleanup
>   eal: mp: end the multiprocess thread during cleanup
>   eal: vfio: cleanup the mp sync handle
>   eal: hotplug: cleanup multiprocess resources
>   eal: malloc: cleanup mp resources

Applied, thanks.




^ permalink raw reply	[flat|nested] 74+ messages in thread

end of thread, other threads:[~2022-02-11 20:17 UTC | newest]

Thread overview: 74+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-04  1:33 [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown Stephen Hemminger
2020-01-04  1:33 ` [dpdk-dev] [PATCH 01/14] eal: log: close on cleanup Stephen Hemminger
2020-01-04  1:33 ` [dpdk-dev] [PATCH 02/14] eal: log: free dynamic state " Stephen Hemminger
2020-04-25 16:26   ` David Marchand
2020-01-04  1:33 ` [dpdk-dev] [PATCH 03/14] eal: alarm: close timerfd on eal cleanup Stephen Hemminger
2020-04-25 16:32   ` David Marchand
2020-01-04  1:33 ` [dpdk-dev] [PATCH 04/14] eal: cleanup threads Stephen Hemminger
2020-02-04 14:48   ` Aaron Conole
2020-01-04  1:33 ` [dpdk-dev] [PATCH 05/14] eal: intr: cleanup resources Stephen Hemminger
2020-04-25 16:49   ` David Marchand
2020-04-26 16:24     ` Stephen Hemminger
2020-01-04  1:33 ` [dpdk-dev] [PATCH 06/14] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
2020-04-27 15:37   ` Burakov, Anatoly
2020-01-04  1:33 ` [dpdk-dev] [PATCH 07/14] eal: interrupts close epoll fd on shutdown Stephen Hemminger
2020-01-04  1:33 ` [dpdk-dev] [PATCH 08/14] eal: vfio: cleanup the mp sync handle Stephen Hemminger
2020-04-27 12:12   ` Burakov, Anatoly
2020-01-04  1:33 ` [dpdk-dev] [PATCH 09/14] eal: close mem config on cleanup Stephen Hemminger
2020-04-27 12:12   ` Burakov, Anatoly
2020-04-27 17:00     ` Stephen Hemminger
2020-04-28  9:20       ` Burakov, Anatoly
2020-01-04  1:33 ` [dpdk-dev] [PATCH 10/14] tap: close netlink socket on device close Stephen Hemminger
2020-04-25 15:52   ` David Marchand
2020-01-04  1:33 ` [dpdk-dev] [PATCH 11/14] eal: cleanup plugins data Stephen Hemminger
2020-01-04  1:33 ` [dpdk-dev] [PATCH 12/14] ethdev: raise priority of old driver warning Stephen Hemminger
2020-04-25 15:53   ` Thomas Monjalon
2020-01-04  1:33 ` [dpdk-dev] [PATCH 13/14] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
2020-01-04  1:33 ` [dpdk-dev] [PATCH 14/14] eal: malloc: cleanup mp resources Stephen Hemminger
2020-04-27 12:10   ` Burakov, Anatoly
2020-02-05  9:32 ` [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown David Marchand
2020-02-05 12:07   ` Stephen Hemminger
2020-02-05 12:32     ` David Marchand
2020-02-06 14:06 ` David Marchand
2020-02-07 18:24   ` Stephen Hemminger
2020-04-25 19:34 ` David Marchand
2020-04-28 23:14 ` [dpdk-dev] [PATCH v2 0/9] eal: " Stephen Hemminger
2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 1/8] eal: log: close on cleanup Stephen Hemminger
2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 2/8] eal: log: free dynamic state " Stephen Hemminger
2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 3/8] eal: alarm: close file " Stephen Hemminger
2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 4/8] eal: cleanup threads Stephen Hemminger
2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 5/8] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 6/8] eal: vfio: cleanup the mp sync handle Stephen Hemminger
2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 7/8] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
2020-04-28 23:14   ` [dpdk-dev] [PATCH v2 8/8] eal: malloc: cleanup mp resources Stephen Hemminger
2020-04-28 23:58 ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown Stephen Hemminger
2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 1/8] eal: log: close on cleanup Stephen Hemminger
2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 2/8] eal: log: free dynamic state " Stephen Hemminger
2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 3/8] eal: alarm: close file " Stephen Hemminger
2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 4/8] eal: cleanup threads Stephen Hemminger
2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 5/8] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 6/8] eal: vfio: cleanup the mp sync handle Stephen Hemminger
2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 7/8] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
2020-04-28 23:58   ` [dpdk-dev] [PATCH v3 8/8] eal: malloc: cleanup mp resources Stephen Hemminger
2020-05-03 17:21   ` [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown David Marchand
2020-10-19 22:24     ` Thomas Monjalon
2021-03-24 21:30       ` Thomas Monjalon
2021-11-13  0:28   ` [PATCH v4 0/5] cleanup more stuff " Stephen Hemminger
2021-11-13  0:28     ` [PATCH v4 1/5] eal: close log in eal_cleanup Stephen Hemminger
2021-11-13  0:28     ` [PATCH v4 2/5] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
2021-11-13  0:28     ` [PATCH v4 3/5] eal: vfio: cleanup the mp sync handle Stephen Hemminger
2021-11-13  0:28     ` [PATCH v4 4/5] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
2021-11-13  0:28     ` [PATCH v4 5/5] eal: malloc: cleanup mp resources Stephen Hemminger
2021-11-13  3:32   ` [PATCH v5 0/5] cleanup DPDK resources via eal_cleanup Stephen Hemminger
2021-11-13  3:32     ` [PATCH v5 1/5] eal: close log in eal_cleanup Stephen Hemminger
2021-11-13  3:32     ` [PATCH v5 2/5] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
2021-11-13  3:32     ` [PATCH v5 3/5] eal: vfio: cleanup the mp sync handle Stephen Hemminger
2021-11-13  3:32     ` [PATCH v5 4/5] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
2021-11-13  3:32     ` [PATCH v5 5/5] eal: malloc: cleanup mp resources Stephen Hemminger
2021-11-13 17:22   ` [PATCH v6 0/5] cleanup more resources on eal_cleanup Stephen Hemminger
2021-11-13 17:22     ` [PATCH v6 1/5] eal: close log in eal_cleanup Stephen Hemminger
2021-11-13 17:22     ` [PATCH v6 2/5] eal: mp: end the multiprocess thread during cleanup Stephen Hemminger
2021-11-13 17:22     ` [PATCH v6 3/5] eal: vfio: cleanup the mp sync handle Stephen Hemminger
2021-11-13 17:22     ` [PATCH v6 4/5] eal: hotplug: cleanup multiprocess resources Stephen Hemminger
2021-11-13 17:22     ` [PATCH v6 5/5] eal: malloc: cleanup mp resources Stephen Hemminger
2022-02-11 20:17     ` [PATCH v6 0/5] cleanup more resources on eal_cleanup Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).