* [dpdk-dev] [PATCH] metrics: make number of metrics names configurable
@ 2020-07-02 17:28 Stephen Hemminger
2020-07-28 5:14 ` Stephen Hemminger
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Stephen Hemminger @ 2020-07-02 17:28 UTC (permalink / raw)
To: Anatoly Burakov; +Cc: dev, Stephen Hemminger, remy.horton, ciara.power
The maximum number of metrics is hardcoded at 256.
This severely limits the usefulness of the library.
It should be configurable like other limits in DPDK.
Fixes: 349950ddb9c5 ("metrics: add information metrics library")
Cc: remy.horton@intel.com
Cc: ciara.power@intel.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
config/common_base | 1 +
config/meson.build | 2 +-
lib/librte_metrics/rte_metrics.h | 1 -
3 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/config/common_base b/config/common_base
index fe30c515e5a3..f0212faec80c 100644
--- a/config/common_base
+++ b/config/common_base
@@ -917,6 +917,7 @@ CONFIG_RTE_LIBRTE_JOBSTATS=y
# Compile the device metrics library
#
CONFIG_RTE_LIBRTE_METRICS=y
+CONFIG_RTE_METRICS_MAX_METRICS=256
#
# Compile the bitrate statistics library
diff --git a/config/meson.build b/config/meson.build
index 351e268c1f5b..cc8cb8fbf2f0 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -238,7 +238,7 @@ dpdk_conf.set('RTE_ENABLE_TRACE_FP', get_option('enable_trace_fp'))
dpdk_conf.set('RTE_MAX_VFIO_GROUPS', 64)
dpdk_conf.set('RTE_DRIVER_MEMPOOL_BUCKET_SIZE_KB', 64)
dpdk_conf.set('RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', true)
-
+dpdk_conf.set('RTE_METRICS_MAX_METRICS', 256)
compile_time_cpuflags = []
subdir(arch_subdir)
diff --git a/lib/librte_metrics/rte_metrics.h b/lib/librte_metrics/rte_metrics.h
index fbe64ddf2b47..40f015b8bb93 100644
--- a/lib/librte_metrics/rte_metrics.h
+++ b/lib/librte_metrics/rte_metrics.h
@@ -34,7 +34,6 @@ extern int metrics_initialized;
/** Maximum length of metric name (including null-terminator) */
#define RTE_METRICS_MAX_NAME_LEN 64
-#define RTE_METRICS_MAX_METRICS 256
/**
* Global metric special id.
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [dpdk-dev] [PATCH] metrics: make number of metrics names configurable
2020-07-02 17:28 [dpdk-dev] [PATCH] metrics: make number of metrics names configurable Stephen Hemminger
@ 2020-07-28 5:14 ` Stephen Hemminger
2020-07-28 10:24 ` Bruce Richardson
2020-09-04 22:31 ` [dpdk-dev] [PATCH] rte_metrics: move maximum number of metrics into rte_config Stephen Hemminger
2 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2020-07-28 5:14 UTC (permalink / raw)
To: Anatoly Burakov; +Cc: dev, remy.horton, ciara.power
On Thu, 2 Jul 2020 10:28:52 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:
> The maximum number of metrics is hardcoded at 256.
> This severely limits the usefulness of the library.
> It should be configurable like other limits in DPDK.
>
> Fixes: 349950ddb9c5 ("metrics: add information metrics library")
> Cc: remy.horton@intel.com
> Cc: ciara.power@intel.com
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Ping, this should be addressed, and doesn't require much effort
to review.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [dpdk-dev] [PATCH] metrics: make number of metrics names configurable
2020-07-02 17:28 [dpdk-dev] [PATCH] metrics: make number of metrics names configurable Stephen Hemminger
2020-07-28 5:14 ` Stephen Hemminger
@ 2020-07-28 10:24 ` Bruce Richardson
2020-07-28 19:59 ` Stephen Hemminger
2020-09-04 22:31 ` [dpdk-dev] [PATCH] rte_metrics: move maximum number of metrics into rte_config Stephen Hemminger
2 siblings, 1 reply; 10+ messages in thread
From: Bruce Richardson @ 2020-07-28 10:24 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Anatoly Burakov, dev, remy.horton, ciara.power
On Thu, Jul 02, 2020 at 10:28:52AM -0700, Stephen Hemminger wrote:
> The maximum number of metrics is hardcoded at 256.
> This severely limits the usefulness of the library.
> It should be configurable like other limits in DPDK.
>
> Fixes: 349950ddb9c5 ("metrics: add information metrics library")
> Cc: remy.horton@intel.com
> Cc: ciara.power@intel.com
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> config/common_base | 1 +
> config/meson.build | 2 +-
> lib/librte_metrics/rte_metrics.h | 1 -
> 3 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/config/common_base b/config/common_base
> index fe30c515e5a3..f0212faec80c 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -917,6 +917,7 @@ CONFIG_RTE_LIBRTE_JOBSTATS=y
> # Compile the device metrics library
> #
> CONFIG_RTE_LIBRTE_METRICS=y
> +CONFIG_RTE_METRICS_MAX_METRICS=256
>
> #
> # Compile the bitrate statistics library
> diff --git a/config/meson.build b/config/meson.build
> index 351e268c1f5b..cc8cb8fbf2f0 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -238,7 +238,7 @@ dpdk_conf.set('RTE_ENABLE_TRACE_FP', get_option('enable_trace_fp'))
> dpdk_conf.set('RTE_MAX_VFIO_GROUPS', 64)
> dpdk_conf.set('RTE_DRIVER_MEMPOOL_BUCKET_SIZE_KB', 64)
> dpdk_conf.set('RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', true)
> -
> +dpdk_conf.set('RTE_METRICS_MAX_METRICS', 256)
>
The meson.build file should really just be used for computed values, I
think. For build-time constants like this it's probably better put in
config/rte_config.h file.
Regards,
/Bruce
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [dpdk-dev] [PATCH] metrics: make number of metrics names configurable
2020-07-28 10:24 ` Bruce Richardson
@ 2020-07-28 19:59 ` Stephen Hemminger
2020-07-29 9:47 ` Bruce Richardson
0 siblings, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2020-07-28 19:59 UTC (permalink / raw)
To: Bruce Richardson; +Cc: Anatoly Burakov, dev, remy.horton, ciara.power
On Tue, 28 Jul 2020 11:24:58 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:
> On Thu, Jul 02, 2020 at 10:28:52AM -0700, Stephen Hemminger wrote:
> > The maximum number of metrics is hardcoded at 256.
> > This severely limits the usefulness of the library.
> > It should be configurable like other limits in DPDK.
> >
> > Fixes: 349950ddb9c5 ("metrics: add information metrics library")
> > Cc: remy.horton@intel.com
> > Cc: ciara.power@intel.com
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > config/common_base | 1 +
> > config/meson.build | 2 +-
> > lib/librte_metrics/rte_metrics.h | 1 -
> > 3 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/config/common_base b/config/common_base
> > index fe30c515e5a3..f0212faec80c 100644
> > --- a/config/common_base
> > +++ b/config/common_base
> > @@ -917,6 +917,7 @@ CONFIG_RTE_LIBRTE_JOBSTATS=y
> > # Compile the device metrics library
> > #
> > CONFIG_RTE_LIBRTE_METRICS=y
> > +CONFIG_RTE_METRICS_MAX_METRICS=256
> >
> > #
> > # Compile the bitrate statistics library
> > diff --git a/config/meson.build b/config/meson.build
> > index 351e268c1f5b..cc8cb8fbf2f0 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -238,7 +238,7 @@ dpdk_conf.set('RTE_ENABLE_TRACE_FP', get_option('enable_trace_fp'))
> > dpdk_conf.set('RTE_MAX_VFIO_GROUPS', 64)
> > dpdk_conf.set('RTE_DRIVER_MEMPOOL_BUCKET_SIZE_KB', 64)
> > dpdk_conf.set('RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', true)
> > -
> > +dpdk_conf.set('RTE_METRICS_MAX_METRICS', 256)
> >
>
> The meson.build file should really just be used for computed values, I
> think. For build-time constants like this it's probably better put in
> config/rte_config.h file.
config/rte_config.h is generated isn't it?
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [dpdk-dev] [PATCH] metrics: make number of metrics names configurable
2020-07-28 19:59 ` Stephen Hemminger
@ 2020-07-29 9:47 ` Bruce Richardson
0 siblings, 0 replies; 10+ messages in thread
From: Bruce Richardson @ 2020-07-29 9:47 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Anatoly Burakov, dev, remy.horton, ciara.power
On Tue, Jul 28, 2020 at 12:59:22PM -0700, Stephen Hemminger wrote:
> On Tue, 28 Jul 2020 11:24:58 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
>
> > On Thu, Jul 02, 2020 at 10:28:52AM -0700, Stephen Hemminger wrote:
> > > The maximum number of metrics is hardcoded at 256.
> > > This severely limits the usefulness of the library.
> > > It should be configurable like other limits in DPDK.
> > >
> > > Fixes: 349950ddb9c5 ("metrics: add information metrics library")
> > > Cc: remy.horton@intel.com
> > > Cc: ciara.power@intel.com
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> > > config/common_base | 1 +
> > > config/meson.build | 2 +-
> > > lib/librte_metrics/rte_metrics.h | 1 -
> > > 3 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/config/common_base b/config/common_base
> > > index fe30c515e5a3..f0212faec80c 100644
> > > --- a/config/common_base
> > > +++ b/config/common_base
> > > @@ -917,6 +917,7 @@ CONFIG_RTE_LIBRTE_JOBSTATS=y
> > > # Compile the device metrics library
> > > #
> > > CONFIG_RTE_LIBRTE_METRICS=y
> > > +CONFIG_RTE_METRICS_MAX_METRICS=256
> > >
> > > #
> > > # Compile the bitrate statistics library
> > > diff --git a/config/meson.build b/config/meson.build
> > > index 351e268c1f5b..cc8cb8fbf2f0 100644
> > > --- a/config/meson.build
> > > +++ b/config/meson.build
> > > @@ -238,7 +238,7 @@ dpdk_conf.set('RTE_ENABLE_TRACE_FP', get_option('enable_trace_fp'))
> > > dpdk_conf.set('RTE_MAX_VFIO_GROUPS', 64)
> > > dpdk_conf.set('RTE_DRIVER_MEMPOOL_BUCKET_SIZE_KB', 64)
> > > dpdk_conf.set('RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', true)
> > > -
> > > +dpdk_conf.set('RTE_METRICS_MAX_METRICS', 256)
> > >
> >
> > The meson.build file should really just be used for computed values, I
> > think. For build-time constants like this it's probably better put in
> > config/rte_config.h file.
>
> config/rte_config.h is generated isn't it?
>
For make builds it is, for meson there is a static rte_config.h which
includes the dynamically-generated rte_build_config.h.
While nothing mandates this design, and we can change it if people find it
confusing, I though it worthwhile to have a config header file for stuff
like this that is not normally configurable, but some folks might want to
tweak in custom builds. For 20.11 I'll try and remember to update the docs
to cover it a bit, so that people are more aware its there.
/Bruce
^ permalink raw reply [flat|nested] 10+ messages in thread
* [dpdk-dev] [PATCH] rte_metrics: move maximum number of metrics into rte_config
2020-07-02 17:28 [dpdk-dev] [PATCH] metrics: make number of metrics names configurable Stephen Hemminger
2020-07-28 5:14 ` Stephen Hemminger
2020-07-28 10:24 ` Bruce Richardson
@ 2020-09-04 22:31 ` Stephen Hemminger
2020-09-05 3:11 ` Stephen Hemminger
2020-10-20 11:50 ` Thomas Monjalon
2 siblings, 2 replies; 10+ messages in thread
From: Stephen Hemminger @ 2020-09-04 22:31 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger
If using lots of queues and ports, and having per port or per queue
metrics it is easy to exceed the upper bound of the metric library.
Move the limit into rte_config where user can change it.
Ideally, there would be no upper bound and a dynamic structure
such as red-black tree or hash table would be used for these.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
config/rte_config.h | 3 +++
lib/librte_metrics/rte_metrics.h | 1 -
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/config/rte_config.h b/config/rte_config.h
index 9bb915347cb6..b880974f5787 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -104,6 +104,9 @@
#define RTE_GRAPH_BURST_SIZE 256
#define RTE_LIBRTE_GRAPH_STATS 1
+/* rte_metrics defines */
+#define RTE_METRICS_MAX_METRICS 256
+
/****** driver defines ********/
/* QuickAssist device */
diff --git a/lib/librte_metrics/rte_metrics.h b/lib/librte_metrics/rte_metrics.h
index fbe64ddf2b47..40f015b8bb93 100644
--- a/lib/librte_metrics/rte_metrics.h
+++ b/lib/librte_metrics/rte_metrics.h
@@ -34,7 +34,6 @@ extern int metrics_initialized;
/** Maximum length of metric name (including null-terminator) */
#define RTE_METRICS_MAX_NAME_LEN 64
-#define RTE_METRICS_MAX_METRICS 256
/**
* Global metric special id.
--
2.27.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH] rte_metrics: move maximum number of metrics into rte_config
2020-09-04 22:31 ` [dpdk-dev] [PATCH] rte_metrics: move maximum number of metrics into rte_config Stephen Hemminger
@ 2020-09-05 3:11 ` Stephen Hemminger
2020-10-20 11:50 ` Thomas Monjalon
1 sibling, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2020-09-05 3:11 UTC (permalink / raw)
To: dev
On Fri, 4 Sep 2020 15:31:19 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:
> If using lots of queues and ports, and having per port or per queue
> metrics it is easy to exceed the upper bound of the metric library.
> Move the limit into rte_config where user can change it.
>
> Ideally, there would be no upper bound and a dynamic structure
> such as red-black tree or hash table would be used for these.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Note: this version of the patch assumes meson build, it won't work with make based build.
Ignore the CI failures.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH] rte_metrics: move maximum number of metrics into rte_config
2020-09-04 22:31 ` [dpdk-dev] [PATCH] rte_metrics: move maximum number of metrics into rte_config Stephen Hemminger
2020-09-05 3:11 ` Stephen Hemminger
@ 2020-10-20 11:50 ` Thomas Monjalon
2020-10-20 15:02 ` Stephen Hemminger
1 sibling, 1 reply; 10+ messages in thread
From: Thomas Monjalon @ 2020-10-20 11:50 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, bruce.richardson
05/09/2020 00:31, Stephen Hemminger:
> If using lots of queues and ports, and having per port or per queue
> metrics it is easy to exceed the upper bound of the metric library.
> Move the limit into rte_config where user can change it.
>
> Ideally, there would be no upper bound and a dynamic structure
> such as red-black tree or hash table would be used for these.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> +/* rte_metrics defines */
> +#define RTE_METRICS_MAX_METRICS 256
Not sure we want to go in the direction of adding such tuning
in rte_config.h.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH] rte_metrics: move maximum number of metrics into rte_config
2020-10-20 11:50 ` Thomas Monjalon
@ 2020-10-20 15:02 ` Stephen Hemminger
2020-10-20 15:15 ` Thomas Monjalon
0 siblings, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2020-10-20 15:02 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev, bruce.richardson
On Tue, 20 Oct 2020 13:50:55 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:
> 05/09/2020 00:31, Stephen Hemminger:
> > If using lots of queues and ports, and having per port or per queue
> > metrics it is easy to exceed the upper bound of the metric library.
> > Move the limit into rte_config where user can change it.
> >
> > Ideally, there would be no upper bound and a dynamic structure
> > such as red-black tree or hash table would be used for these.
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > --- a/config/rte_config.h
> > +++ b/config/rte_config.h
> > +/* rte_metrics defines */
> > +#define RTE_METRICS_MAX_METRICS 256
>
> Not sure we want to go in the direction of adding such tuning
> in rte_config.h.
>
>
The only other option is to rewrite rte_metrics to support a
dynamic data structure!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH] rte_metrics: move maximum number of metrics into rte_config
2020-10-20 15:02 ` Stephen Hemminger
@ 2020-10-20 15:15 ` Thomas Monjalon
0 siblings, 0 replies; 10+ messages in thread
From: Thomas Monjalon @ 2020-10-20 15:15 UTC (permalink / raw)
To: Stephen Hemminger, bruce.richardson, ciara.power; +Cc: dev
20/10/2020 17:02, Stephen Hemminger:
> Thomas Monjalon <thomas@monjalon.net> wrote:
> > 05/09/2020 00:31, Stephen Hemminger:
> > > If using lots of queues and ports, and having per port or per queue
> > > metrics it is easy to exceed the upper bound of the metric library.
> > > Move the limit into rte_config where user can change it.
> > >
> > > Ideally, there would be no upper bound and a dynamic structure
> > > such as red-black tree or hash table would be used for these.
> > >
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> > > --- a/config/rte_config.h
> > > +++ b/config/rte_config.h
> > > +/* rte_metrics defines */
> > > +#define RTE_METRICS_MAX_METRICS 256
> >
> > Not sure we want to go in the direction of adding such tuning
> > in rte_config.h.
>
> The only other option is to rewrite rte_metrics to support a
> dynamic data structure!
Yes it seems a better solution.
How rte_metrics will fit in the picture of the new telemetry?
Maybe we will deprecate rte_metrics in future?
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-10-20 15:15 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-02 17:28 [dpdk-dev] [PATCH] metrics: make number of metrics names configurable Stephen Hemminger
2020-07-28 5:14 ` Stephen Hemminger
2020-07-28 10:24 ` Bruce Richardson
2020-07-28 19:59 ` Stephen Hemminger
2020-07-29 9:47 ` Bruce Richardson
2020-09-04 22:31 ` [dpdk-dev] [PATCH] rte_metrics: move maximum number of metrics into rte_config Stephen Hemminger
2020-09-05 3:11 ` Stephen Hemminger
2020-10-20 11:50 ` Thomas Monjalon
2020-10-20 15:02 ` Stephen Hemminger
2020-10-20 15:15 ` 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).