[dpdk-stable] [PATCH backport 0/4] service finalize and co

[dpdk-stable] [PATCH backport 0/4] service finalize and co

[Harry van Haaren]

This patchset targets 17.11 stable, backported fix from upstream commit:
da23f0a service: fix memory leak with new function

This patchset is a backport of a patches to allow
secondary processes to cleaup hugepage allocations that
occur during rte_eal_init().

The two applications proc_info and pdump are reworked to call
the cleanup function to avoid leaking hugepage memory. Without
this patchset, these secondary processes leak hugepage memory,
eventually depleting it.

The 4th patch in the series is a doc/release_notes patch,
which notes that secondary processes should call this before
quitting.

Yuanhan, I'm not sure where the doc patches should go to note
the above - feel free to move to a different section if required.

-Harry

Harry van Haaren (1):
  doc/release notes: add note to 17.11 on cleanup

Vipin Varghese (3):
  service: fix memory leak with new function
  app/pdump: fix the memory leak by rte_service_init
  app/procinfo: fix memory leak by rte_service_init

 app/pdump/main.c                            |  3 +++
 app/proc_info/main.c                        |  3 +++
 doc/guides/rel_notes/release_17_11.rst      | 13 +++++++++++++
 lib/librte_eal/common/include/rte_service.h | 11 +++++++++++
 lib/librte_eal/common/rte_service.c         | 14 ++++++++++++++
 lib/librte_eal/rte_eal_version.map          |  1 +
 6 files changed, 45 insertions(+)

-- 
2.7.4

[dpdk-stable] [PATCH 1/4] service: fix memory leak with new function

[Harry van Haaren]

From: Vipin Varghese <vipin.varghese@intel.com>

Backported fix from upstream commit:
da23f0a service: fix memory leak with new function

The rte_service_finalize routine checks if service is initialized
or not. If yes; releases internal memory for services and lcore
states are freed. This routine is to be invoked at end of application
termination.

Fixes: 21698354c832 ("service: introduce service cores concept")
Cc: stable@dpdk.org

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>

---

As I only backported, I didn't add signoff - its Vipin's code.
If that's not correct, please advise and I can update it.
---
 lib/librte_eal/common/include/rte_service.h | 11 +++++++++++
 lib/librte_eal/common/rte_service.c         | 14 ++++++++++++++
 lib/librte_eal/rte_eal_version.map          |  1 +
 3 files changed, 26 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h
index 80b90fa..b4cdf40 100644
--- a/lib/librte_eal/common/include/rte_service.h
+++ b/lib/librte_eal/common/include/rte_service.h
@@ -423,6 +423,17 @@ int32_t rte_service_lcore_count_services(uint32_t lcore);
  */
 int32_t rte_service_dump(FILE *f, uint32_t id);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Free up the memory that has been initialized. This routine
+ * is to be invoked prior to process termination.
+ *
+ * @retval None
+ */
+void rte_service_finalize(void);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index 1f92294..0f101f6 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -143,6 +143,20 @@ int32_t rte_service_init(void)
 	return -ENOMEM;
 }
 
+void rte_service_finalize(void)
+{
+	if (!rte_service_library_initialized)
+		return;
+
+	if (rte_services)
+		rte_free(rte_services);
+
+	if (lcore_states)
+		rte_free(lcore_states);
+
+	rte_service_library_initialized = 0;
+}
+
 /* returns 1 if service is registered and has not been unregistered
  * Returns 0 if service never registered, or has been unregistered
  */
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index f4f46c1..edc8e9e 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -212,6 +212,7 @@ EXPERIMENTAL {
 	rte_service_component_unregister;
 	rte_service_component_runstate_set;
 	rte_service_dump;
+	rte_service_finalize;
 	rte_service_get_by_id;
 	rte_service_get_by_name;
 	rte_service_get_count;
-- 
2.7.4

[dpdk-stable] [PATCH 2/4] app/pdump: fix the memory leak by rte_service_init

[Harry van Haaren]

From: Vipin Varghese <vipin.varghese@intel.com>

When pdump is run multiple times against any primary application,
it consumes huge page memory by rte_service_init. This is not freed
at exit of application.

Invoking rte_service_finalize to free memory and prevent memory leak.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
---
 app/pdump/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index 66272f5..6f71c69 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -54,6 +54,7 @@
 #include <rte_mempool.h>
 #include <rte_ring.h>
 #include <rte_pdump.h>
+#include <rte_service.h>
 
 #define CMD_LINE_OPT_PDUMP "pdump"
 #define PDUMP_PORT_ARG "port"
@@ -911,5 +912,7 @@ main(int argc, char **argv)
 	/* dump debug stats */
 	print_pdump_stats();
 
+	rte_service_finalize();
+
 	return 0;
 }
-- 
2.7.4

[dpdk-stable] [PATCH 3/4] app/procinfo: fix memory leak by rte_service_init

[Harry van Haaren]

From: Vipin Varghese <vipin.varghese@intel.com>

When procinfo is run multiple times against primary application,
it consumes huge page memory by rte_service_init. Which is not
released at exit of application.

Invoking rte_service_finalize to free memory and prevent memory leak.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
---
 app/proc_info/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index 64fbbd0..cd30b3c 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -58,6 +58,7 @@
 #include <rte_branch_prediction.h>
 #include <rte_string_fns.h>
 #include <rte_metrics.h>
+#include <rte_service.h>
 
 /* Maximum long option length for option parsing. */
 #define MAX_LONG_OPT_SZ 64
@@ -689,5 +690,7 @@ main(int argc, char **argv)
 	if (enable_metrics)
 		metrics_display(RTE_METRICS_GLOBAL);
 
+	rte_service_finalize();
+
 	return 0;
 }
-- 
2.7.4

[dpdk-stable] [PATCH 4/4] doc/release notes: add note to 17.11 on cleanup

[Harry van Haaren]

This adds a note in the 17.11 release notes, which applies
only to 17.11-stable backport of the rte_service_finalize()
function. The note should be included in 17.11 notes somewhere.

The fix has a minimal code change in order to workaround the issue.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---

Cc: vipin.varghese@intel.com

 doc/guides/rel_notes/release_17_11.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/doc/guides/rel_notes/release_17_11.rst b/doc/guides/rel_notes/release_17_11.rst
index 016a08c..acc51b8 100644
--- a/doc/guides/rel_notes/release_17_11.rst
+++ b/doc/guides/rel_notes/release_17_11.rst
@@ -303,6 +303,19 @@ Resolved Issues
   atomic locking of the services has been fixed and refactored for readability.
 
 
+Known Issues
+------------
+
+* **Secondary processes must call rte_service_finalize to cleanup on exit**
+
+  As ``rte_eal_init()`` now allocates hugepage memory to enable service-cores
+  functionality, there is a requirement for applications to call
+  ``rte_service_finalize()`` to free this memory. Primary processes will
+  re-initialize hugepages - so this does not impact them significantly.
+  Secondary processes which start and stop frequently must call this function,
+  in order to not leak hugepage memory.
+
+
 API Changes
 -----------
 
-- 
2.7.4