diff mbox series

metrics: make number of metrics names configurable

Message ID 20200702172852.6201-1-stephen@networkplumber.org (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series metrics: make number of metrics names configurable |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/Intel-compilation fail apply issues

Commit Message

Stephen Hemminger July 2, 2020, 5:28 p.m. UTC 
  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(-)

Comments

Stephen Hemminger July 28, 2020, 5:14 a.m. UTC | #1 
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.
Bruce Richardson July 28, 2020, 10:24 a.m. UTC | #2 
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
Stephen Hemminger July 28, 2020, 7:59 p.m. UTC | #3 
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?
Bruce Richardson July 29, 2020, 9:47 a.m. UTC | #4 
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
diff mbox series

Patch

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.