* [dpdk-dev] [PATCH 1/2] lib/metrics: fix to reset the init flag
@ 2020-05-13 10:36 Hemant Agrawal
2020-05-13 10:36 ` [dpdk-dev] [PATCH 2/2] test: support cleanup in bitrate and latency test Hemant Agrawal
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Hemant Agrawal @ 2020-05-13 10:36 UTC (permalink / raw)
To: dev; +Cc: bruce.richardson, Hemant Agrawal
metrics_initialized shall be reset in deinit function
This is currently causing issue in running
metrics_autotest mulutiple times
Fixes: 07c1b6925b65 ("telemetry: invert dependency on metrics library")
Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
lib/librte_metrics/rte_metrics.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
index e07670219..f570cf226 100644
--- a/lib/librte_metrics/rte_metrics.c
+++ b/lib/librte_metrics/rte_metrics.c
@@ -96,6 +96,8 @@ rte_metrics_deinit(void)
stats = memzone->addr;
memset(stats, 0, sizeof(struct rte_metrics_data_s));
+ metrics_initialized = 0;
+
return rte_memzone_free(memzone);
}
--
2.17.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH 2/2] test: support cleanup in bitrate and latency test
2020-05-13 10:36 [dpdk-dev] [PATCH 1/2] lib/metrics: fix to reset the init flag Hemant Agrawal
@ 2020-05-13 10:36 ` Hemant Agrawal
2020-05-19 9:31 ` [dpdk-dev] [PATCH 1/2] lib/metrics: fix to reset the init flag David Marchand
2020-05-19 10:52 ` [dpdk-dev] [PATCH v2 " Hemant Agrawal
2 siblings, 0 replies; 9+ messages in thread
From: Hemant Agrawal @ 2020-05-13 10:36 UTC (permalink / raw)
To: dev; +Cc: bruce.richardson, Hemant Agrawal
both bitratestats_autotest latency test initializes
the metrics library. It should be cleaned during exit.
Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
app/test/test_bitratestats.c | 14 ++++++++++++++
app/test/test_latencystats.c | 3 +++
2 files changed, 17 insertions(+)
diff --git a/app/test/test_bitratestats.c b/app/test/test_bitratestats.c
index 3a7d9c037..39d7f734d 100644
--- a/app/test/test_bitratestats.c
+++ b/app/test/test_bitratestats.c
@@ -32,6 +32,18 @@ test_stats_bitrate_create(void)
return TEST_SUCCESS;
}
+/* To test free the resources from bitrate_reg test */
+static int
+test_stats_bitrate_free(void)
+{
+ int ret = 0;
+
+ ret = rte_metrics_deinit();
+ TEST_ASSERT(ret >= 0, "Test Failed: rte_metrics_deinit failed");
+
+ return TEST_SUCCESS;
+}
+
/* To test bit rate registration */
static int
test_stats_bitrate_reg(void)
@@ -214,6 +226,8 @@ unit_test_suite bitratestats_testsuite = {
*/
TEST_CASE_ST(test_bit_packet_forward, NULL,
test_stats_bitrate_calc),
+ /* TEST CASE 9: Test to do the cleanup w.r.t create */
+ TEST_CASE(test_stats_bitrate_free),
TEST_CASES_END()
}
};
diff --git a/app/test/test_latencystats.c b/app/test/test_latencystats.c
index 968e0bc47..427339904 100644
--- a/app/test/test_latencystats.c
+++ b/app/test/test_latencystats.c
@@ -60,6 +60,9 @@ static int test_latency_uninit(void)
ret = rte_latencystats_uninit();
TEST_ASSERT(ret >= 0, "Test Failed: rte_latencystats_uninit failed");
+ ret = rte_metrics_deinit();
+ TEST_ASSERT(ret >= 0, "Test Failed: rte_metrics_deinit failed");
+
return TEST_SUCCESS;
}
--
2.17.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] lib/metrics: fix to reset the init flag
2020-05-13 10:36 [dpdk-dev] [PATCH 1/2] lib/metrics: fix to reset the init flag Hemant Agrawal
2020-05-13 10:36 ` [dpdk-dev] [PATCH 2/2] test: support cleanup in bitrate and latency test Hemant Agrawal
@ 2020-05-19 9:31 ` David Marchand
2020-05-19 9:50 ` Hemant Agrawal
2020-05-19 10:52 ` [dpdk-dev] [PATCH v2 " Hemant Agrawal
2 siblings, 1 reply; 9+ messages in thread
From: David Marchand @ 2020-05-19 9:31 UTC (permalink / raw)
To: Hemant Agrawal, Bruce Richardson; +Cc: dev
On Wed, May 13, 2020 at 12:39 PM Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
>
> metrics_initialized shall be reset in deinit function
> This is currently causing issue in running
> metrics_autotest mulutiple times
>
> Fixes: 07c1b6925b65 ("telemetry: invert dependency on metrics library")
>
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
> lib/librte_metrics/rte_metrics.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
> index e07670219..f570cf226 100644
> --- a/lib/librte_metrics/rte_metrics.c
> +++ b/lib/librte_metrics/rte_metrics.c
> @@ -96,6 +96,8 @@ rte_metrics_deinit(void)
> stats = memzone->addr;
> memset(stats, 0, sizeof(struct rte_metrics_data_s));
>
> + metrics_initialized = 0;
> +
> return rte_memzone_free(memzone);
Should this flag be reset only if rte_memzone_free succeeds?
--
David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] lib/metrics: fix to reset the init flag
2020-05-19 9:31 ` [dpdk-dev] [PATCH 1/2] lib/metrics: fix to reset the init flag David Marchand
@ 2020-05-19 9:50 ` Hemant Agrawal
0 siblings, 0 replies; 9+ messages in thread
From: Hemant Agrawal @ 2020-05-19 9:50 UTC (permalink / raw)
To: David Marchand, Bruce Richardson; +Cc: dev
> On Wed, May 13, 2020 at 12:39 PM Hemant Agrawal
> <hemant.agrawal@nxp.com> wrote:
> >
> > metrics_initialized shall be reset in deinit function This is
> > currently causing issue in running metrics_autotest mulutiple times
> >
> > Fixes: 07c1b6925b65 ("telemetry: invert dependency on metrics
> > library")
> >
> > Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> > ---
> > lib/librte_metrics/rte_metrics.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/librte_metrics/rte_metrics.c
> > b/lib/librte_metrics/rte_metrics.c
> > index e07670219..f570cf226 100644
> > --- a/lib/librte_metrics/rte_metrics.c
> > +++ b/lib/librte_metrics/rte_metrics.c
> > @@ -96,6 +96,8 @@ rte_metrics_deinit(void)
> > stats = memzone->addr;
> > memset(stats, 0, sizeof(struct rte_metrics_data_s));
> >
> > + metrics_initialized = 0;
> > +
> > return rte_memzone_free(memzone);
>
> Should this flag be reset only if rte_memzone_free succeeds?
>
[Hemant] I thought about it but I did not do it for following reasons.
1. If the memzone is not freed, It will not be initialized next time due to following check in init routine.
memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
if (memzone != NULL)
return;
2. Most applications have very weak error handling in de-init/cleanup parts.
Having said that. It can be changed to do it only on the success of rte_memzone_free
>
> --
> David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH v2 1/2] lib/metrics: fix to reset the init flag
2020-05-13 10:36 [dpdk-dev] [PATCH 1/2] lib/metrics: fix to reset the init flag Hemant Agrawal
2020-05-13 10:36 ` [dpdk-dev] [PATCH 2/2] test: support cleanup in bitrate and latency test Hemant Agrawal
2020-05-19 9:31 ` [dpdk-dev] [PATCH 1/2] lib/metrics: fix to reset the init flag David Marchand
@ 2020-05-19 10:52 ` Hemant Agrawal
2020-05-19 10:52 ` [dpdk-dev] [PATCH v2 2/2] test: support cleanup in bitrate and latency test Hemant Agrawal
` (2 more replies)
2 siblings, 3 replies; 9+ messages in thread
From: Hemant Agrawal @ 2020-05-19 10:52 UTC (permalink / raw)
To: dev, david.marchand; +Cc: Hemant Agrawal
metrics_initialized shall be reset in deinit function
This is currently causing issue in running
metrics_autotest mulutiple times
Fixes: 07c1b6925b65 ("telemetry: invert dependency on metrics library")
Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
lib/librte_metrics/rte_metrics.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
index e07670219f..dba6409c27 100644
--- a/lib/librte_metrics/rte_metrics.c
+++ b/lib/librte_metrics/rte_metrics.c
@@ -85,6 +85,7 @@ rte_metrics_deinit(void)
{
struct rte_metrics_data_s *stats;
const struct rte_memzone *memzone;
+ int ret = 0;
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return -EINVAL;
@@ -96,8 +97,10 @@ rte_metrics_deinit(void)
stats = memzone->addr;
memset(stats, 0, sizeof(struct rte_metrics_data_s));
- return rte_memzone_free(memzone);
-
+ ret = rte_memzone_free(memzone);
+ if (ret == 0)
+ metrics_initialized = 0;
+ return ret;
}
int
--
2.17.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH v2 2/2] test: support cleanup in bitrate and latency test
2020-05-19 10:52 ` [dpdk-dev] [PATCH v2 " Hemant Agrawal
@ 2020-05-19 10:52 ` Hemant Agrawal
2020-05-19 15:24 ` Stephen Hemminger
2020-05-19 12:12 ` [dpdk-dev] [PATCH v2 1/2] lib/metrics: fix to reset the init flag David Marchand
2020-05-19 15:23 ` Stephen Hemminger
2 siblings, 1 reply; 9+ messages in thread
From: Hemant Agrawal @ 2020-05-19 10:52 UTC (permalink / raw)
To: dev, david.marchand; +Cc: Hemant Agrawal
both bitratestats_autotest latency test initializes
the metrics library. It should be cleaned during exit.
Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
app/test/test_bitratestats.c | 14 ++++++++++++++
app/test/test_latencystats.c | 3 +++
2 files changed, 17 insertions(+)
diff --git a/app/test/test_bitratestats.c b/app/test/test_bitratestats.c
index 3a7d9c037a..39d7f734d4 100644
--- a/app/test/test_bitratestats.c
+++ b/app/test/test_bitratestats.c
@@ -32,6 +32,18 @@ test_stats_bitrate_create(void)
return TEST_SUCCESS;
}
+/* To test free the resources from bitrate_reg test */
+static int
+test_stats_bitrate_free(void)
+{
+ int ret = 0;
+
+ ret = rte_metrics_deinit();
+ TEST_ASSERT(ret >= 0, "Test Failed: rte_metrics_deinit failed");
+
+ return TEST_SUCCESS;
+}
+
/* To test bit rate registration */
static int
test_stats_bitrate_reg(void)
@@ -214,6 +226,8 @@ unit_test_suite bitratestats_testsuite = {
*/
TEST_CASE_ST(test_bit_packet_forward, NULL,
test_stats_bitrate_calc),
+ /* TEST CASE 9: Test to do the cleanup w.r.t create */
+ TEST_CASE(test_stats_bitrate_free),
TEST_CASES_END()
}
};
diff --git a/app/test/test_latencystats.c b/app/test/test_latencystats.c
index 968e0bc470..427339904d 100644
--- a/app/test/test_latencystats.c
+++ b/app/test/test_latencystats.c
@@ -60,6 +60,9 @@ static int test_latency_uninit(void)
ret = rte_latencystats_uninit();
TEST_ASSERT(ret >= 0, "Test Failed: rte_latencystats_uninit failed");
+ ret = rte_metrics_deinit();
+ TEST_ASSERT(ret >= 0, "Test Failed: rte_metrics_deinit failed");
+
return TEST_SUCCESS;
}
--
2.17.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2 1/2] lib/metrics: fix to reset the init flag
2020-05-19 10:52 ` [dpdk-dev] [PATCH v2 " Hemant Agrawal
2020-05-19 10:52 ` [dpdk-dev] [PATCH v2 2/2] test: support cleanup in bitrate and latency test Hemant Agrawal
@ 2020-05-19 12:12 ` David Marchand
2020-05-19 15:23 ` Stephen Hemminger
2 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2020-05-19 12:12 UTC (permalink / raw)
To: Hemant Agrawal; +Cc: dev
On Tue, May 19, 2020 at 12:55 PM Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
>
> metrics_initialized shall be reset in deinit function
> This is currently causing issue in running
> metrics_autotest mulutiple times
>
> Fixes: 07c1b6925b65 ("telemetry: invert dependency on metrics library")
>
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
For the series,
Acked-by: David Marchand <david.marchand@redhat.com>
Applied, thanks.
--
David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2 1/2] lib/metrics: fix to reset the init flag
2020-05-19 10:52 ` [dpdk-dev] [PATCH v2 " Hemant Agrawal
2020-05-19 10:52 ` [dpdk-dev] [PATCH v2 2/2] test: support cleanup in bitrate and latency test Hemant Agrawal
2020-05-19 12:12 ` [dpdk-dev] [PATCH v2 1/2] lib/metrics: fix to reset the init flag David Marchand
@ 2020-05-19 15:23 ` Stephen Hemminger
2 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2020-05-19 15:23 UTC (permalink / raw)
To: Hemant Agrawal; +Cc: dev, david.marchand
On Tue, 19 May 2020 16:22:57 +0530
Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
> diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
> index e07670219f..dba6409c27 100644
> --- a/lib/librte_metrics/rte_metrics.c
> +++ b/lib/librte_metrics/rte_metrics.c
> @@ -85,6 +85,7 @@ rte_metrics_deinit(void)
> {
> struct rte_metrics_data_s *stats;
> const struct rte_memzone *memzone;
> + int ret = 0;
>
Why do gratuitous initialization? It blocks compiler from finding buggy
code.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2 2/2] test: support cleanup in bitrate and latency test
2020-05-19 10:52 ` [dpdk-dev] [PATCH v2 2/2] test: support cleanup in bitrate and latency test Hemant Agrawal
@ 2020-05-19 15:24 ` Stephen Hemminger
0 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2020-05-19 15:24 UTC (permalink / raw)
To: Hemant Agrawal; +Cc: dev, david.marchand
On Tue, 19 May 2020 16:22:58 +0530
Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
> +/* To test free the resources from bitrate_reg test */
> +static int
> +test_stats_bitrate_free(void)
> +{
> + int ret = 0;
> +
> + ret = rte_metrics_deinit();
Coverity will complain about extra initialization like this
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-05-19 15:24 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-13 10:36 [dpdk-dev] [PATCH 1/2] lib/metrics: fix to reset the init flag Hemant Agrawal
2020-05-13 10:36 ` [dpdk-dev] [PATCH 2/2] test: support cleanup in bitrate and latency test Hemant Agrawal
2020-05-19 9:31 ` [dpdk-dev] [PATCH 1/2] lib/metrics: fix to reset the init flag David Marchand
2020-05-19 9:50 ` Hemant Agrawal
2020-05-19 10:52 ` [dpdk-dev] [PATCH v2 " Hemant Agrawal
2020-05-19 10:52 ` [dpdk-dev] [PATCH v2 2/2] test: support cleanup in bitrate and latency test Hemant Agrawal
2020-05-19 15:24 ` Stephen Hemminger
2020-05-19 12:12 ` [dpdk-dev] [PATCH v2 1/2] lib/metrics: fix to reset the init flag David Marchand
2020-05-19 15:23 ` Stephen Hemminger
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).