[dpdk-stable] [PATCH v2 1/2] metrics: new API to deinitialise metrics library

[dpdk-stable] [PATCH v2 1/2] metrics: new API to deinitialise metrics library

[Harman Kalra]

Once the library usage is over, it must be deinitialized which
will free the shared memory reserved during initialization.

Fixes: observed an issue while running 'metrics_autotest'
continuously without quiting. For the first run 'metrics_autotest'
passes all test cases but second run onwards first test case
fails because metrics library is already initialized during
first run.
Cc: stable@dpdk.org

Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
v2:
* Adding stable@dpdk.org into cc as this patch falls between bug fix
and new feature.

 doc/guides/prog_guide/metrics_lib.rst      | 14 ++++++++++++++
 lib/librte_metrics/rte_metrics.c           | 20 ++++++++++++++++++++
 lib/librte_metrics/rte_metrics.h           | 17 +++++++++++++++++
 lib/librte_metrics/rte_metrics_version.map |  6 ++++++
 4 files changed, 57 insertions(+)

diff --git a/doc/guides/prog_guide/metrics_lib.rst b/doc/guides/prog_guide/metrics_lib.rst
index e68e4e743..08e107df3 100644
--- a/doc/guides/prog_guide/metrics_lib.rst
+++ b/doc/guides/prog_guide/metrics_lib.rst
@@ -154,6 +154,20 @@ print out all metrics for a given port:
     }
 
 
+Deinitialising the library
+------------------------
+
+Once the library usage is done, it must be deinitialized by calling
+``rte_metrics_deinit()`` which will free the shared memory reserved
+during initialization.
+
+.. code-block:: c
+
+    err = rte_metrics_deinit(void);
+
+If the return value is negative, it means deinitialization failed.
+This function **must** be called from a primary process.
+
 Bit-rate statistics library
 ---------------------------
 
diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
index 99a96b651..0c816a1fc 100644
--- a/lib/librte_metrics/rte_metrics.c
+++ b/lib/librte_metrics/rte_metrics.c
@@ -76,6 +76,26 @@ rte_metrics_init(int socket_id)
 	rte_spinlock_init(&stats->lock);
 }
 
+int __rte_experimental
+rte_metrics_deinit(void)
+{
+	struct rte_metrics_data_s *stats;
+	const struct rte_memzone *memzone;
+
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -EINVAL;
+
+	memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
+	if (memzone == NULL)
+		return -EIO;
+
+	stats = memzone->addr;
+	memset(stats, 0, sizeof(struct rte_metrics_data_s));
+
+	return rte_memzone_free(memzone);
+
+}
+
 int
 rte_metrics_reg_name(const char *name)
 {
diff --git a/lib/librte_metrics/rte_metrics.h b/lib/librte_metrics/rte_metrics.h
index 67a60fadd..0957a94b6 100644
--- a/lib/librte_metrics/rte_metrics.h
+++ b/lib/librte_metrics/rte_metrics.h
@@ -24,6 +24,7 @@
 #define _RTE_METRICS_H_
 
 #include <stdint.h>
+#include <rte_compat.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -80,6 +81,22 @@ struct rte_metric_value {
  */
 void rte_metrics_init(int socket_id);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Deinitialize metric module. This function must be called from
+ * a primary process after all the metrics usage is over, to
+ *  release the shared memory.
+ *
+ * @return
+ *  -EINVAL - invalid parameter.
+ *  -EIO: Error, unable to access metrics shared memory
+ *    (rte_metrics_init() not called)
+ *  0 - success
+ */
+int __rte_experimental rte_metrics_deinit(void);
+
 /**
  * Register a metric, making it available as a reporting parameter.
  *
diff --git a/lib/librte_metrics/rte_metrics_version.map b/lib/librte_metrics/rte_metrics_version.map
index 4c5234cd1..6ac99a44a 100644
--- a/lib/librte_metrics/rte_metrics_version.map
+++ b/lib/librte_metrics/rte_metrics_version.map
@@ -11,3 +11,9 @@ DPDK_17.05 {
 
 	local: *;
 };
+
+EXPERIMENTAL {
+	global:
+
+	rte_metrics_deinit;
+};
-- 
2.18.0

[dpdk-stable] [PATCH v2 2/2] test/metrics: first test case fails on continuous execution

[Harman Kalra]

Issue is observed while running 'metrics_autotest' continuously
without quiting. During first execution all test cases pass but
second run onwards first test case fails as library is already
initialized.

To resolve, introduced a new API to deinitialise the library
after all test cases are executed.

Cc: stable@dpdk.org

Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
v2:
* Adding stable@dpdk.org into cc as patch 1/2 of same patchset falls
between bug fix and new feature.

 app/test/test_metrics.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/app/test/test_metrics.c b/app/test/test_metrics.c
index 3c2f36b8a..78b3936ee 100644
--- a/app/test/test_metrics.c
+++ b/app/test/test_metrics.c
@@ -28,6 +28,19 @@ test_metrics_init(void)
 	return TEST_SUCCESS;
 }
 
+/* Deinitialize metric module. This function must be called
+ * from a primary process after metrics usage is over
+ */
+static int
+test_metrics_deinitialize(void)
+{
+	int err = 0;
+	err = rte_metrics_deinit();
+	TEST_ASSERT(err == 0, "%s, %d", __func__, __LINE__);
+
+	return TEST_SUCCESS;
+}
+
  /* Test Case to check failures when memzone init is not done */
 static int
 test_metrics_without_init(void)
@@ -300,6 +313,10 @@ static struct unit_test_suite metrics_testsuite  = {
 		 * arraylist, count size
 		 */
 		TEST_CASE(test_metrics_get_values),
+
+		/* TEST CASE 8: Test to unregister metrics*/
+		TEST_CASE(test_metrics_deinitialize),
+
 		TEST_CASES_END()
 	}
 };
-- 
2.18.0

Re: [dpdk-stable] [PATCH v2 1/2] metrics: new API to deinitialise metrics library

[Thomas Monjalon]

01/03/2019 11:00, Harman Kalra:
> Once the library usage is over, it must be deinitialized which
> will free the shared memory reserved during initialization.
> 
> Fixes: observed an issue while running 'metrics_autotest'
> continuously without quiting. For the first run 'metrics_autotest'
> passes all test cases but second run onwards first test case
> fails because metrics library is already initialized during
> first run.
> Cc: stable@dpdk.org
> 
> Signed-off-by: Harman Kalra <hkalra@marvell.com>

I was waiting for an ack on this patch,
and realized that there was one already on v1.
When sending v2, you should have reported the Ack.
And to ease review between versions, please keep all the versions
in the same thread with --in-reply-to.
Would be nice if old contributors from Marvell would guide you.
Thanks

> ---
> v2:
> * Adding stable@dpdk.org into cc as this patch falls between bug fix
> and new feature.

I would vote for not backporting this new API.
Any other opinion?

[dpdk-stable] [PATCH v2 1/2] metrics: new API to deinitialise metrics library

[Harman Kalra]

Once the library usage is over, it must be deinitialized which
will free the shared memory reserved during initialization.

Fixes: observed an issue while running 'metrics_autotest'
continuously without quiting. For the first run 'metrics_autotest'
passes all test cases but second run onwards first test case
fails because metrics library is already initialized during
first run.
Cc: stable@dpdk.org

Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
v2:
* Adding stable@dpdk.org into cc as this patch falls between bug fix
and new feature.

 doc/guides/prog_guide/metrics_lib.rst      | 14 ++++++++++++++
 lib/librte_metrics/rte_metrics.c           | 20 ++++++++++++++++++++
 lib/librte_metrics/rte_metrics.h           | 17 +++++++++++++++++
 lib/librte_metrics/rte_metrics_version.map |  6 ++++++
 4 files changed, 57 insertions(+)

diff --git a/doc/guides/prog_guide/metrics_lib.rst b/doc/guides/prog_guide/metrics_lib.rst
index e68e4e743..08e107df3 100644
--- a/doc/guides/prog_guide/metrics_lib.rst
+++ b/doc/guides/prog_guide/metrics_lib.rst
@@ -154,6 +154,20 @@ print out all metrics for a given port:
     }
 
 
+Deinitialising the library
+------------------------
+
+Once the library usage is done, it must be deinitialized by calling
+``rte_metrics_deinit()`` which will free the shared memory reserved
+during initialization.
+
+.. code-block:: c
+
+    err = rte_metrics_deinit(void);
+
+If the return value is negative, it means deinitialization failed.
+This function **must** be called from a primary process.
+
 Bit-rate statistics library
 ---------------------------
 
diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
index 99a96b651..0c816a1fc 100644
--- a/lib/librte_metrics/rte_metrics.c
+++ b/lib/librte_metrics/rte_metrics.c
@@ -76,6 +76,26 @@ rte_metrics_init(int socket_id)
 	rte_spinlock_init(&stats->lock);
 }
 
+int __rte_experimental
+rte_metrics_deinit(void)
+{
+	struct rte_metrics_data_s *stats;
+	const struct rte_memzone *memzone;
+
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -EINVAL;
+
+	memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
+	if (memzone == NULL)
+		return -EIO;
+
+	stats = memzone->addr;
+	memset(stats, 0, sizeof(struct rte_metrics_data_s));
+
+	return rte_memzone_free(memzone);
+
+}
+
 int
 rte_metrics_reg_name(const char *name)
 {
diff --git a/lib/librte_metrics/rte_metrics.h b/lib/librte_metrics/rte_metrics.h
index 67a60fadd..0957a94b6 100644
--- a/lib/librte_metrics/rte_metrics.h
+++ b/lib/librte_metrics/rte_metrics.h
@@ -24,6 +24,7 @@
 #define _RTE_METRICS_H_
 
 #include <stdint.h>
+#include <rte_compat.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -80,6 +81,22 @@ struct rte_metric_value {
  */
 void rte_metrics_init(int socket_id);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Deinitialize metric module. This function must be called from
+ * a primary process after all the metrics usage is over, to
+ *  release the shared memory.
+ *
+ * @return
+ *  -EINVAL - invalid parameter.
+ *  -EIO: Error, unable to access metrics shared memory
+ *    (rte_metrics_init() not called)
+ *  0 - success
+ */
+int __rte_experimental rte_metrics_deinit(void);
+
 /**
  * Register a metric, making it available as a reporting parameter.
  *
diff --git a/lib/librte_metrics/rte_metrics_version.map b/lib/librte_metrics/rte_metrics_version.map
index 4c5234cd1..6ac99a44a 100644
--- a/lib/librte_metrics/rte_metrics_version.map
+++ b/lib/librte_metrics/rte_metrics_version.map
@@ -11,3 +11,9 @@ DPDK_17.05 {
 
 	local: *;
 };
+
+EXPERIMENTAL {
+	global:
+
+	rte_metrics_deinit;
+};
-- 
2.18.0