[dpdk-dev] [PATCH v3 1/3] lib: add information metrics library
On 07/11/2016 23:25, Pattan, Reshma wrote: [..] >>> + * Initialises statistic module. This only has to be explicitly >>> +called >> >> Typo < Initialises> > > To avoid confusion, here I mean to say "Initializes" should be used (i.e. US > English) . Sorta guessed that.. :) > Also another non-related comment, you may need to add a comment about EMWA > near the mean bit rate calculation code. Quite a few comments needed updating. Going to hold off the v5 until 16.11 is out as I'll also need to change all the references to 17.02.. ..Remy
[dpdk-dev] [PATCH v3 1/3] lib: add information metrics library
Hi,
> -Original Message-
> From: Pattan, Reshma
> Sent: Friday, November 4, 2016 4:43 PM
> To: Remy Horton
> Cc: dev at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3 1/3] lib: add information metrics library
>
> Hi,
>
> Few comments below.
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton
> > Sent: Friday, November 4, 2016 3:36 AM
> > To: dev at dpdk.org
> > Subject: [dpdk-dev] [PATCH v3 1/3] lib: add information metrics
> > library
> >
> > This patch adds a new information metric library that allows other
> > modules to register named metrics and update their values. It is
> > intended to be independent of ethdev, rather than mixing ethdev and
> > non-ethdev information in xstats.
> >
> > Signed-off-by: Remy Horton
> > ---
> > +int
> > +
> > +int
> > +rte_metrics_get_values(int port_id,
> > + struct rte_stat_value *values,
> > + uint16_t capacity)
> > +{
> > + struct rte_metrics_meta_s *entry;
> > + struct rte_metrics_data_s *stats;
> > + const struct rte_memzone *memzone;
> > + uint16_t idx_name;
> > + int return_value;
> > +
> > + memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
> > + /* If not allocated, fail silently */
> > + if (memzone == NULL)
> > + return 0;
> > + stats = memzone->addr;
> > + rte_spinlock_lock(>lock);
> > +
> > + if (values != NULL) {
> > + if (capacity < stats->cnt_stats) {
> > + rte_spinlock_unlock(>lock);
> > + return -ERANGE;
> > + }
> > + for (idx_name = 0; idx_name < stats->cnt_stats; idx_name++)
> > {
> > + entry = >metadata[idx_name];
> > + values[idx_name].key = idx_name;
> > + values[idx_name].value = entry->value[port_id];
> > + }
>
> Here you also need to include logic to return values for port independent
> metrics.
>
> > diff --git a/lib/librte_metrics/rte_metrics.h
> > b/lib/librte_metrics/rte_metrics.h
> > new file mode 100644
> > index 000..6b75404
> > --- /dev/null
> > +++ b/lib/librte_metrics/rte_metrics.h
> > @@ -0,0 +1,204 @@
> > +/**
> > + * Statistic name
> > + */
> > +struct rte_metric_name {
> > + /** String describing statistic */
> > + char name[RTE_METRICS_MAX_NAME_LEN]; };
> > +
> > +
> > +/**
> > + * Statistic name.
> > + */
>
> Need to correct the description to "stats values" or "metric values."
>
> > +struct rte_stat_value {
>
> Need to change the name to rte_metric_value.
>
> > + /** Numeric identifier of statistic */
> > + uint16_t key;
> > + /** Value for statistic */
> > + uint64_t value;
> > +};
> > +
> > +
> > +/**
> > + * Initialises statistic module. This only has to be explicitly
> > +called
>
> Typo < Initialises>
To avoid confusion, here I mean to say "Initializes" should be used (i.e. US
English) .
Also another non-related comment, you may need to add a comment about EMWA
near the mean bit rate calculation code.
>
> Thanks,
> Reshma
[dpdk-dev] [PATCH v3 1/3] lib: add information metrics library
Hi,
Few comments below.
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton
> Sent: Friday, November 4, 2016 3:36 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v3 1/3] lib: add information metrics library
>
> This patch adds a new information metric library that allows other modules
> to register named metrics and update their values. It is intended to be
> independent of ethdev, rather than mixing ethdev and non-ethdev
> information in xstats.
>
> Signed-off-by: Remy Horton
> ---
> +int
> +
> +int
> +rte_metrics_get_values(int port_id,
> + struct rte_stat_value *values,
> + uint16_t capacity)
> +{
> + struct rte_metrics_meta_s *entry;
> + struct rte_metrics_data_s *stats;
> + const struct rte_memzone *memzone;
> + uint16_t idx_name;
> + int return_value;
> +
> + memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
> + /* If not allocated, fail silently */
> + if (memzone == NULL)
> + return 0;
> + stats = memzone->addr;
> + rte_spinlock_lock(>lock);
> +
> + if (values != NULL) {
> + if (capacity < stats->cnt_stats) {
> + rte_spinlock_unlock(>lock);
> + return -ERANGE;
> + }
> + for (idx_name = 0; idx_name < stats->cnt_stats; idx_name++)
> {
> + entry = >metadata[idx_name];
> + values[idx_name].key = idx_name;
> + values[idx_name].value = entry->value[port_id];
> + }
Here you also need to include logic to return values for port independent
metrics.
> diff --git a/lib/librte_metrics/rte_metrics.h
> b/lib/librte_metrics/rte_metrics.h
> new file mode 100644
> index 000..6b75404
> --- /dev/null
> +++ b/lib/librte_metrics/rte_metrics.h
> @@ -0,0 +1,204 @@
> +/**
> + * Statistic name
> + */
> +struct rte_metric_name {
> + /** String describing statistic */
> + char name[RTE_METRICS_MAX_NAME_LEN];
> +};
> +
> +
> +/**
> + * Statistic name.
> + */
Need to correct the description to "stats values" or "metric values."
> +struct rte_stat_value {
Need to change the name to rte_metric_value.
> + /** Numeric identifier of statistic */
> + uint16_t key;
> + /** Value for statistic */
> + uint64_t value;
> +};
> +
> +
> +/**
> + * Initialises statistic module. This only has to be explicitly called
Typo < Initialises>
Thanks,
Reshma
[dpdk-dev] [PATCH v3 1/3] lib: add information metrics library
This patch adds a new information metric library that allows other modules to register named metrics and update their values. It is intended to be independent of ethdev, rather than mixing ethdev and non-ethdev information in xstats. Signed-off-by: Remy Horton --- config/common_base | 5 + doc/api/doxy-api-index.md | 1 + doc/api/doxy-api.conf | 1 + doc/guides/rel_notes/release_16_11.rst | 6 + lib/Makefile | 1 + lib/librte_metrics/Makefile| 51 + lib/librte_metrics/rte_metrics.c | 300 + lib/librte_metrics/rte_metrics.h | 204 lib/librte_metrics/rte_metrics_version.map | 13 ++ mk/rte.app.mk | 2 + 10 files changed, 584 insertions(+) create mode 100644 lib/librte_metrics/Makefile create mode 100644 lib/librte_metrics/rte_metrics.c create mode 100644 lib/librte_metrics/rte_metrics.h create mode 100644 lib/librte_metrics/rte_metrics_version.map diff --git a/config/common_base b/config/common_base index 21d18f8..2277727 100644 --- a/config/common_base +++ b/config/common_base @@ -589,3 +589,8 @@ CONFIG_RTE_APP_TEST_RESOURCE_TAR=n CONFIG_RTE_TEST_PMD=y CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES=n CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS=n + +# +# Compile the device metrics library +# +CONFIG_RTE_LIBRTE_METRICS=y diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md index 6675f96..ca50fa6 100644 --- a/doc/api/doxy-api-index.md +++ b/doc/api/doxy-api-index.md @@ -147,4 +147,5 @@ There are many libraries, so their headers may be grouped by topics: [common] (@ref rte_common.h), [ABI compat] (@ref rte_compat.h), [keepalive] (@ref rte_keepalive.h), + [Device Metrics] (@ref rte_metrics.h), [version](@ref rte_version.h) diff --git a/doc/api/doxy-api.conf b/doc/api/doxy-api.conf index 9dc7ae5..fe830eb 100644 --- a/doc/api/doxy-api.conf +++ b/doc/api/doxy-api.conf @@ -57,6 +57,7 @@ INPUT = doc/api/doxy-api-index.md \ lib/librte_reorder \ lib/librte_ring \ lib/librte_sched \ + lib/librte_metrics \ lib/librte_table \ lib/librte_timer \ lib/librte_vhost diff --git a/doc/guides/rel_notes/release_16_11.rst b/doc/guides/rel_notes/release_16_11.rst index aa0c09a..507f715 100644 --- a/doc/guides/rel_notes/release_16_11.rst +++ b/doc/guides/rel_notes/release_16_11.rst @@ -131,6 +131,12 @@ New Features The GCC 4.9 ``-march`` option supports the Intel processor code names. The config option ``RTE_MACHINE`` can be used to pass code names to the compiler as ``-march`` flag. +* **Added information metric library.** + + A library that allows information metrics to be added and update. It is + intended to provide a reporting mechanism that is independent of the + ethdev library. + Resolved Issues --- diff --git a/lib/Makefile b/lib/Makefile index 990f23a..5d85dcf 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -58,6 +58,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_TABLE) += librte_table DIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += librte_pipeline DIRS-$(CONFIG_RTE_LIBRTE_REORDER) += librte_reorder DIRS-$(CONFIG_RTE_LIBRTE_PDUMP) += librte_pdump +DIRS-$(CONFIG_RTE_LIBRTE_METRICS) += librte_metrics ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni diff --git a/lib/librte_metrics/Makefile b/lib/librte_metrics/Makefile new file mode 100644 index 000..8d6e23a --- /dev/null +++ b/lib/librte_metrics/Makefile @@ -0,0 +1,51 @@ +# BSD LICENSE +# +# Copyright(c) 2016 Intel Corporation. All rights reserved. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in +# the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Intel Corporation nor the names of its +# contributors may be used to endorse or promote products derived +# from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR
