* [dpdk-stable] [PATCH v2 2/2] test/metrics: first test case fails on continuous
2019-03-01 10:07 ` [dpdk-stable] [PATCH v2 " Harman Kalra
@ 2019-03-01 10:07 ` Harman Kalra
2019-06-27 10:59 ` [dpdk-stable] [dpdk-dev] [PATCH v2 1/2] metrics: new API to deinitialise metrics library Pattan, Reshma
2019-06-27 11:03 ` Pattan, Reshma
2 siblings, 0 replies; 10+ messages in thread
From: Harman Kalra @ 2019-03-01 10:07 UTC (permalink / raw)
To: remy.horton, anatoly.burakov, marko.kovacevic, john.mcnamara
Cc: dev, stable, 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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/2] metrics: new API to deinitialise metrics library
2019-03-01 10:07 ` [dpdk-stable] [PATCH v2 " Harman Kalra
2019-03-01 10:07 ` [dpdk-stable] [PATCH v2 2/2] test/metrics: first test case fails on continuous Harman Kalra
@ 2019-06-27 10:59 ` Pattan, Reshma
2019-06-27 11:03 ` Pattan, Reshma
2 siblings, 0 replies; 10+ messages in thread
From: Pattan, Reshma @ 2019-06-27 10:59 UTC (permalink / raw)
To: Harman Kalra, Burakov, Anatoly, Kovacevic, Marko, Mcnamara, John
Cc: dev, stable
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Harman Kalra
> Sent: Friday, March 1, 2019 10:08 AM
> To: Horton, Remy <remy.horton@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Harman Kalra <hkalra@marvell.com>
> Subject: [dpdk-dev] [PATCH v2 1/2] metrics: new API to deinitialise metrics
> library
>
> 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
> +------------------------
1)
The underline should be same length as heading. Below is the error from "make doc-guides-html"
sphinx processing guides-html...
metrics_lib.rst:158: WARNING: Title underline too short.
Deinitialising the library
------------------------
metrics_lib.rst:158: WARNING: Title underline too short.
2) Need to fix below
./devtools/check-git-log.sh
Wrong tag:
Fixes: observed an issue while running 'metrics_autotest'
Wrong 'Fixes' reference:
Fixes: observed an issue while running 'metrics_autotest'
If you want to add Fixes line it should be of below form
Fixes: <commitd> ("commit heading")
Ex: Fixes: d7a0da3c0043 ("mempool/octeontx2: add fast path mempool ops")
Otherwise,
Tested-by : Reshma Pattan <reshma.pattan@intel.com>
Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
Acked-by: Reshma Pattan <reshma.pattan@intel.com>
Please keep these tags for next versions, otherwise it can easily miss from being applied.
Thanks,
Reshma
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/2] metrics: new API to deinitialise metrics library
2019-03-01 10:07 ` [dpdk-stable] [PATCH v2 " Harman Kalra
2019-03-01 10:07 ` [dpdk-stable] [PATCH v2 2/2] test/metrics: first test case fails on continuous Harman Kalra
2019-06-27 10:59 ` [dpdk-stable] [dpdk-dev] [PATCH v2 1/2] metrics: new API to deinitialise metrics library Pattan, Reshma
@ 2019-06-27 11:03 ` Pattan, Reshma
2019-07-10 10:52 ` [dpdk-stable] [PATCH v3 " Harman Kalra
2 siblings, 1 reply; 10+ messages in thread
From: Pattan, Reshma @ 2019-06-27 11:03 UTC (permalink / raw)
To: Harman Kalra, Burakov, Anatoly, Kovacevic, Marko, Mcnamara, John
Cc: dev, stable
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Harman Kalra
> Sent: Friday, March 1, 2019 10:08 AM
> To: Horton, Remy <remy.horton@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Harman Kalra <hkalra@marvell.com>
> Subject: [dpdk-dev] [PATCH v2 1/2] metrics: new API to deinitialise metrics
> library
>
> 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'
Need to fix this
./devtools/check-git-log.sh
Wrong tag:
Fixes: observed an issue while running 'metrics_autotest'
Wrong 'Fixes' reference:
Fixes: observed an issue while running 'metrics_autotest'
No need of of Fixes line, so just correct the sentence,
Tested-by : Reshma Pattan <reshma.pattan@intel.com>
Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
Acked-by: Reshma Pattan <reshma.pattan@intel.com>
Please keep these tags for next versions.
Thanks,
Reshma
^ permalink raw reply [flat|nested] 10+ messages in thread* [dpdk-stable] [PATCH v3 1/2] metrics: new API to deinitialise metrics library
2019-06-27 11:03 ` Pattan, Reshma
@ 2019-07-10 10:52 ` Harman Kalra
2019-07-10 10:52 ` [dpdk-stable] [PATCH v3 2/2] test/metrics: fix metrics autotest failure Harman Kalra
2019-07-10 22:08 ` [dpdk-stable] [PATCH v3 1/2] metrics: new API to deinitialise metrics library Thomas Monjalon
0 siblings, 2 replies; 10+ messages in thread
From: Harman Kalra @ 2019-07-10 10:52 UTC (permalink / raw)
To: remy.horton, reshma.pattan, anatoly.burakov, marko.kovacevic,
john.mcnamara
Cc: dev, Harman Kalra, stable
Once the library usage is over, it must be deinitialized which
will free the shared memory reserved during initialization.
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.
v3:
* Fixed check-git-log.sh and make doc-guides-html issues.
doc/guides/prog_guide/metrics_lib.rst | 14 ++++++++++++++
lib/librte_metrics/rte_metrics.c | 20 ++++++++++++++++++++
lib/librte_metrics/rte_metrics.h | 18 ++++++++++++++++++
lib/librte_metrics/rte_metrics_version.map | 6 ++++++
4 files changed, 58 insertions(+)
diff --git a/doc/guides/prog_guide/metrics_lib.rst b/doc/guides/prog_guide/metrics_lib.rst
index 89bc7d68f..eca855d60 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..df5e32c59 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_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..77bffe08e 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,23 @@ 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
+ */
+__rte_experimental
+int 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
^ permalink raw reply [flat|nested] 10+ messages in thread* [dpdk-stable] [PATCH v3 2/2] test/metrics: fix metrics autotest failure
2019-07-10 10:52 ` [dpdk-stable] [PATCH v3 " Harman Kalra
@ 2019-07-10 10:52 ` Harman Kalra
2019-07-10 22:08 ` [dpdk-stable] [PATCH v3 1/2] metrics: new API to deinitialise metrics library Thomas Monjalon
1 sibling, 0 replies; 10+ messages in thread
From: Harman Kalra @ 2019-07-10 10:52 UTC (permalink / raw)
To: remy.horton, reshma.pattan, anatoly.burakov, marko.kovacevic,
john.mcnamara
Cc: dev, Harman Kalra, stable
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.
Fixes: cd3804242901 ("test/metrics: add unit tests for metrics library")
Cc: stable@dpdk.org
Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [dpdk-stable] [PATCH v3 1/2] metrics: new API to deinitialise metrics library
2019-07-10 10:52 ` [dpdk-stable] [PATCH v3 " Harman Kalra
2019-07-10 10:52 ` [dpdk-stable] [PATCH v3 2/2] test/metrics: fix metrics autotest failure Harman Kalra
@ 2019-07-10 22:08 ` Thomas Monjalon
2019-07-11 8:12 ` Harman Kalra
1 sibling, 1 reply; 10+ messages in thread
From: Thomas Monjalon @ 2019-07-10 22:08 UTC (permalink / raw)
To: Harman Kalra
Cc: stable, remy.horton, reshma.pattan, anatoly.burakov,
marko.kovacevic, john.mcnamara, dev, jerinj
10/07/2019 12:52, Harman Kalra:
> Once the library usage is over, it must be deinitialized which
> will free the shared memory reserved during initialization.
>
> 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.
> v3:
> * Fixed check-git-log.sh and make doc-guides-html issues.
Why don't you take our comments into account?
Reminder 1:
"
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.
"
Reminder 2:
"
Tested-by : Reshma Pattan <reshma.pattan@intel.com>
Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
Acked-by: Reshma Pattan <reshma.pattan@intel.com>
Please keep these tags for next versions.
"
Reminder 3:
"
I would vote for not backporting this new API.
"
In case it is not clear, this comment means you should not Cc stable@dpdk.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-stable] [PATCH v3 1/2] metrics: new API to deinitialise metrics library
2019-07-10 22:08 ` [dpdk-stable] [PATCH v3 1/2] metrics: new API to deinitialise metrics library Thomas Monjalon
@ 2019-07-11 8:12 ` Harman Kalra
2019-07-11 8:34 ` Thomas Monjalon
0 siblings, 1 reply; 10+ messages in thread
From: Harman Kalra @ 2019-07-11 8:12 UTC (permalink / raw)
To: Thomas Monjalon
Cc: stable, remy.horton, reshma.pattan, anatoly.burakov,
marko.kovacevic, john.mcnamara, dev, Jerin Jacob Kollanukkaran
On Thu, Jul 11, 2019 at 12:08:09AM +0200, Thomas Monjalon wrote:
> 10/07/2019 12:52, Harman Kalra:
> > Once the library usage is over, it must be deinitialized which
> > will free the shared memory reserved during initialization.
> >
> > 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.
> > v3:
> > * Fixed check-git-log.sh and make doc-guides-html issues.
>
> Why don't you take our comments into account?
Sorry, I did not know that I have to copy ACKs and other tags.
Will surely take care from next time onwards.
>
> Reminder 1:
> "
> 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.
> "
>
> Reminder 2:
> "
> Tested-by : Reshma Pattan <reshma.pattan@intel.com>
> Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
> Acked-by: Reshma Pattan <reshma.pattan@intel.com>
>
> Please keep these tags for next versions.
> "
Will include these in version 4.
>
> Reminder 3:
> "
> I would vote for not backporting this new API.
> "
>
> In case it is not clear, this comment means you should not Cc stable@dpdk.org
I added CC because of the following comment from Remy:
"
On Mon, Feb 25, 2019 at 12:21:21PM +0000, Remy Horton wrote:
> External Email
>
> ----------------------------------------------------------------------
> This patchset is in that grey area between new feature and bugfix so
> it
> might need to be CC'd to stable@dpdk.org
"
Shall I remove it now?
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-stable] [PATCH v3 1/2] metrics: new API to deinitialise metrics library
2019-07-11 8:12 ` Harman Kalra
@ 2019-07-11 8:34 ` Thomas Monjalon
0 siblings, 0 replies; 10+ messages in thread
From: Thomas Monjalon @ 2019-07-11 8:34 UTC (permalink / raw)
To: Harman Kalra
Cc: stable, remy.horton, reshma.pattan, anatoly.burakov,
marko.kovacevic, john.mcnamara, dev, Jerin Jacob Kollanukkaran
11/07/2019 10:12, Harman Kalra:
> On Thu, Jul 11, 2019 at 12:08:09AM +0200, Thomas Monjalon wrote:
> > "
> > I would vote for not backporting this new API.
> > "
> >
> > In case it is not clear, this comment means you should not Cc stable@dpdk.org
>
> I added CC because of the following comment from Remy:
> "
> On Mon, Feb 25, 2019 at 12:21:21PM +0000, Remy Horton wrote:
> > This patchset is in that grey area between new feature and bugfix so
> > it might need to be CC'd to stable@dpdk.org
> "
>
> Shall I remove it now?
Yes, this is my opinion and we did not get any other opinion.
I don't think the issue is critical enough to backport a new API.
^ permalink raw reply [flat|nested] 10+ messages in thread