[dpdk-dev,v9,1/7] lib: add information metrics library
Checks
Commit Message
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 <remy.horton@intel.com>
---
MAINTAINERS | 4 +
config/common_base | 5 +
doc/api/doxy-api-index.md | 1 +
doc/api/doxy-api.conf | 1 +
doc/guides/rel_notes/release_17_02.rst | 8 +
lib/Makefile | 1 +
lib/librte_metrics/Makefile | 51 +++++
lib/librte_metrics/rte_metrics.c | 308 +++++++++++++++++++++++++++++
lib/librte_metrics/rte_metrics.h | 231 ++++++++++++++++++++++
lib/librte_metrics/rte_metrics_version.map | 13 ++
mk/rte.app.mk | 2 +
11 files changed, 625 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
Comments
Hi Remy,
> 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.
I'm still not convinced by this library, and this introduction does
not help a lot.
I would like to thanks Harry for the review of this series.
If we had more opinions or enthousiasm about this patch, it would
be easier to accept this new library and assert it is the way to go.
It could be a matter of technical board discussion if we had a clear
explanation of the needs, the pros and cons of this design.
The overview for using this library should be given in the prog guide.
2017-01-18 15:05, Remy Horton:
> --- a/config/common_base
> +++ b/config/common_base
> @@ -593,3 +593,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
I know the config file is not so well sorted.
However it would be a bit more logical below CONFIG_RTE_LIBRTE_JOBSTATS.
> diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
> index 72d59b2..94f0f69 100644
> --- a/doc/api/doxy-api-index.md
> +++ b/doc/api/doxy-api-index.md
> @@ -150,4 +150,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),
No first letter uppercase in this list.
> --- a/doc/guides/rel_notes/release_17_02.rst
> +++ b/doc/guides/rel_notes/release_17_02.rst
> @@ -34,6 +34,12 @@ New Features
>
> Refer to the previous release notes for examples.
>
> + * **Added information metric library.**
> +
> + A library that allows information metrics to be added and update. It is
update -> updated
added and updated by who?
> + intended to provide a reporting mechanism that is independent of the
> + ethdev library.
and independent of the cryptodev library?
Does it apply to other types of devices (cryptodev/eventdev)?
> +
> This section is a comment. do not overwrite or remove it.
> Also, make sure to start the actual text at the margin.
> =========================================================
Your text should start below this line, and indented at the margin.
> @@ -205,6 +211,7 @@ The libraries prepended with a plus sign were incremented in this version.
> .. code-block:: diff
>
> librte_acl.so.2
> + + librte_bitratestats.so.1
not part of this patch
> --- 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
insert it below librte_jobstats is a better choice
> --- /dev/null
> +++ b/lib/librte_metrics/rte_metrics.c
> +/**
> + * Internal stats metadata and value entry.
> + *
> + * @internal
> + * @param name
> + * Name of metric
> + * @param value
> + * Current value for metric
> + * @param idx_next_set
> + * Index of next root element (zero for none)
> + * @param idx_next_metric
> + * Index of next metric in set (zero for none)
> + *
> + * Only the root of each set needs idx_next_set but since it has to be
> + * assumed that number of sets could equal total number of metrics,
> + * having a separate set metadata table doesn't save any memory.
> + */
> +struct rte_metrics_meta_s {
> + char name[RTE_METRICS_MAX_NAME_LEN];
> + uint64_t value[RTE_MAX_ETHPORTS];
> + uint64_t nonport_value;
> + uint16_t idx_next_set;
> + uint16_t idx_next_stat;
> +};
It would be a lot easier to read with comments near each field.
It would avoid to forget some fields like nonport_value in this struct.
You do not need to use a doxygen syntax in a .c file.
> --- /dev/null
> +++ b/lib/librte_metrics/rte_metrics.h
> +/**
> + * @file
> + *
> + * RTE Metrics module
RTE is not meaningful here.
Please prefer DPDK.
> + *
> + * Metric information is populated using a push model, where the
> + * information provider calls an update function on the relevant
> + * metrics. Currently only bulk querying of metrics is supported.
> + */
This description should explain who is a provider (drivers?) and who
is the reader (registered thread?).
What do you mean by "push model"? A callback is used?
> +/**
> + * Global (rather than port-specific) metric.
It does not say what kind of constant it is. A special metric id?
> + *
> + * When used instead of port number by rte_metrics_update_metric()
> + * or rte_metrics_update_metric(), the global metrics, which are
> + * not associated with any specific port, are updated.
> + */
> +#define RTE_METRICS_GLOBAL -1
I thought you agreed that "port" is not really a good wording.
> +/**
> + * A name-key lookup for metrics.
> + *
> + * An array of this structure is returned by rte_metrics_get_names().
> + * The struct rte_eth_stats references these names via their array index.
rte_eth_stats?
> + */
> +struct rte_metric_name {
> + /** String describing metric */
> + char name[RTE_METRICS_MAX_NAME_LEN];
> +};
[...]
> +/**
> + * Metric value structure.
> + *
> + * This structure is used by rte_metrics_get_values() to return metrics,
> + * which are statistics that are not generated by PMDs. It maps a name key,
Here we have a definition of what is a metric:
"statistics that are not generated by PMDs"
It could help in the introduction.
> + * which corresponds to an index in the array returned by
> + * rte_metrics_get_names().
> + */
> +struct rte_metric_value {
> + /** Numeric identifier of metric. */
> + uint16_t key;
> + /** Value for metric */
> + uint64_t value;
> +};
> +
> +
> +/**
> + * Initializes metric module. This function must be called from
> + * a primary process before metrics are used.
Why not integrating it in the global init?
Is there some performance drawbacks?
> + *
> + * @param socket_id
> + * Socket to use for shared memory allocation.
> + */
> +void rte_metrics_init(int socket_id);
> +
> +/**
> + * Register a metric, making it available as a reporting parameter.
> + *
> + * Registering a metric is the way third-parties declare a parameter
third-party? You mean the provider?
> + * that they wish to be reported. Once registered, the associated
> + * numeric key can be obtained via rte_metrics_get_names(), which
> + * is required for updating said metric's value.
> + *
> + * @param name
> + * Metric name
> + *
> + * @return
> + * - Zero or positive: Success (index key of new metric)
> + * - \b -EIO: Error, unable to access metrics shared memory
> + * (rte_metrics_init() not called)
> + * - \b -EINVAL: Error, invalid parameters
> + * - \b -ENOMEM: Error, maximum metrics reached
Please, no extra formatting in doxygen.
> + */
> +int rte_metrics_reg_name(const char *name);
> +
Some points addressed. Will cover other ones later.
On 30/01/2017 15:50, Thomas Monjalon wrote:
[..]
>> +CONFIG_RTE_LIBRTE_METRICS=y
> I know the config file is not so well sorted.
> However it would be a bit more logical below CONFIG_RTE_LIBRTE_JOBSTATS.
Done. Rebase merging doesn't help with sorting here.
>> + [Device Metrics] (@ref rte_metrics.h),
> No first letter uppercase in this list.
Fixed.
>> + A library that allows information metrics to be added and update. It is
> update -> updated
Fixed.
> added and updated by who?
Elaborated.
>> + intended to provide a reporting mechanism that is independent of the
>> + ethdev library.
> and independent of the cryptodev library?
Yes. The aim is to have no sub-dependencies.
My original plan was to introduce some form of parameter registration
scheme into xstats to replace the current hard-coded tables, since I
suspected libbitrate/liblatency would not be the last additions. I
decided to spin it out into a library in its own right, as it seemed
cleaner than shoving a load of non-driver stuff into xstats.
> Does it apply to other types of devices (cryptodev/eventdev)?
I've not been following cryptodev/eventdev, but short answer yes.
>> This section is a comment. do not overwrite or remove it.
>> Also, make sure to start the actual text at the margin.
>> =========================================================
> Your text should start below this line, and indented at the margin.
Fixed.
>> + + librte_bitratestats.so.1
> not part of this patch
Fixed. Artefact of sorting out a merge mess-up.
>> +DIRS-$(CONFIG_RTE_LIBRTE_METRICS) += librte_metrics
> insert it below librte_jobstats is a better choice
Done.
>> +struct rte_metrics_meta_s {
>> + char name[RTE_METRICS_MAX_NAME_LEN];
>> + uint64_t value[RTE_MAX_ETHPORTS];
>> + uint64_t nonport_value;
>> + uint16_t idx_next_set;
>> + uint16_t idx_next_stat;
>> +};
> It would be a lot easier to read with comments near each field.
> It would avoid to forget some fields like nonport_value in this struct.
> You do not need to use a doxygen syntax in a .c file.
Noted. Even though Doxygen isn't required, I think it preferable to use
a consistent style in both .c and .h files.
>> + * RTE Metrics module
> RTE is not meaningful here.
> Please prefer DPDK.
Fixed.
>> + * Metric information is populated using a push model, where the
>> + * information provider calls an update function on the relevant
>> + * metrics. Currently only bulk querying of metrics is supported.
>> + */
> This description should explain who is a provider (drivers?) and who
> is the reader (registered thread?).
Noted. Will elaborate.
> What do you mean by "push model"? A callback is used?
Updating is done in response to producers having new information. In
contrast in a pull model an update would happen in response to a polling
by a consumer.
Originally (back in August I think) I used a pull model where producers
would register callbacks that were called in response to
rte_metrics_get() by a consumer, but that assumed that producers and
consumers were within the same process. Using shared memory and making
it ASLR-safe means not using pointers.
Aside: In this former pull model, port_id was passed verbatim to the
producers' callbacks to interpret in whatever way they saw fit, so there
was no inherant need to stop "magic" values outside the usual 0-255 used
for port ids. Hence where the next thing originally came from...
>> +/**
>> + * Global (rather than port-specific) metric.
> It does not say what kind of constant it is. A special metric id?
Yes. Elaborated.
>> + *
>> + * When used instead of port number by rte_metrics_update_metric()
>> + * or rte_metrics_update_metric(), the global metrics, which are
>> + * not associated with any specific port, are updated.
>> + */
>> +#define RTE_METRICS_GLOBAL -1
>
> I thought you agreed that "port" is not really a good wording.
Certainly within the constant name. Don't see what's wrong with
referring to it in description though.
>> + * An array of this structure is returned by rte_metrics_get_names().
>> + * The struct rte_eth_stats references these names via their array index.
> rte_eth_stats?
Good question - was going to put it down to cut'n'paste while baseing
the descriptions on Olivier Matz's rewording, but that was for xstats..
>> + * Initializes metric module. This function must be called from
>> + * a primary process before metrics are used.
>
> Why not integrating it in the global init?
> Is there some performance drawbacks?
There shouldn't be any significant performance penalities, but I am not
particularly fond in principle of initalising component libraries
regardless of whether they are used. (Actually it was previously
initalised on first use, but that had a race condition.)
>> + * Registering a metric is the way third-parties declare a parameter
> third-party? You mean the provider?
Yes.
>> + * - \b -EINVAL: Error, invalid parameters
>> + * - \b -ENOMEM: Error, maximum metrics reached
> Please, no extra formatting in doxygen.
Fixed.
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Monday, January 30, 2017 3:50 PM
> To: Horton, Remy <remy.horton@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v9 1/7] lib: add information metrics
> library
>
> Hi Remy,
>
> > 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.
>
> I'm still not convinced by this library, and this introduction does not
> help a lot.
>
> I would like to thanks Harry for the review of this series.
> If we had more opinions or enthousiasm about this patch, it would be
> easier to accept this new library and assert it is the way to go.
Hi,
The RFCs for this library (initially two, merged into one) have been up since October, during the 16.11 timeframe. Comments were made and applied.
http://dpdk.org/ml/archives/dev/2016-October/049571.html
http://dpdk.org/ml/archives/dev/2016-October/048390.html
I'm concerned that these new comments/reservations are coming in very, very late in the 17.02 release cycle.
If there hasn't been a lot of opinions or enthusiasm then equally there hasn't been other reservations. If there had been we would have addressed them.
> It could be a matter of technical board discussion if we had a clear
> explanation of the needs, the pros and cons of this design.
We are happy to have the design discussed at the Technical Board. We would also like the inclusion of this library in RC3 to be discussed since that is still our desired outcome.
We have, as any other company would have, customers awaiting features, developers committed to timelines, and testing and integration roadmaps. Blocking or delaying features at the last moment isn't an effective model that we, and I'm sure other companies, can work with.
As such, it is probably best, that all current and future RFCs are reviewed at the Technical Board and that the board gives an indication on whether the proposal is acceptable for upstreaming or not.
> The overview for using this library should be given in the prog guide.
We will add a section to the Programmers Guide.
John
On Tue, Jan 31, 2017 at 01:13:11PM +0000, Mcnamara, John wrote:
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > Sent: Monday, January 30, 2017 3:50 PM
> > To: Horton, Remy <remy.horton@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v9 1/7] lib: add information metrics
> > library
> >
> > Hi Remy,
> >
> > > 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.
> >
> > I'm still not convinced by this library, and this introduction does not
> > help a lot.
> >
> > I would like to thanks Harry for the review of this series.
> > If we had more opinions or enthousiasm about this patch, it would be
> > easier to accept this new library and assert it is the way to go.
>
> Hi,
>
> The RFCs for this library (initially two, merged into one) have been up since October, during the 16.11 timeframe. Comments were made and applied.
>
> http://dpdk.org/ml/archives/dev/2016-October/049571.html
> http://dpdk.org/ml/archives/dev/2016-October/048390.html
>
> I'm concerned that these new comments/reservations are coming in very, very late in the 17.02 release cycle.
>
> If there hasn't been a lot of opinions or enthusiasm then equally there hasn't been other reservations. If there had been we would have addressed them.
>
>
> > It could be a matter of technical board discussion if we had a clear
> > explanation of the needs, the pros and cons of this design.
>
> We are happy to have the design discussed at the Technical Board. We would also like the inclusion of this library in RC3 to be discussed since that is still our desired outcome.
>
> We have, as any other company would have, customers awaiting features, developers committed to timelines, and testing and integration roadmaps. Blocking or delaying features at the last moment isn't an effective model that we, and I'm sure other companies, can work with.
>
> As such, it is probably best, that all current and future RFCs are reviewed at the Technical Board and that the board gives an indication on whether the proposal is acceptable for upstreaming or not.
>
I would tend to agree with this. The tech board should indeed look to
insure that all RFCs and V1s have had some feedback on them well before
the merge deadline.
I don't believe it's fair on developers to suddenly give feedback at
merge-time and thereby prevent the patch making it into a release,
without giving time to do any rework. This is especially true if the
patch had already been reviewed and acked, and so could be considered
"ready for merge".
The tech board should also discuss some reasonable guidelines
for this area. My opinion is that by the time RC1 is released, any
patches that have been reviewed, acked and have no outstanding
comments on them for e.g. 1 week, must be merged in for the release. Any
additional feedback thereafter should be considered "too late", and
should be addressed in the following release. This will help to
incentivize reviewers to review early, and also give developers some
degree of confidence that their patches will be merged in. We have
deadlines for submitters to get patches in, we should also have
deadlines for reviewers to object to those patches.
Regards,
/Bruce
2017-01-31 13:28, Bruce Richardson:
> On Tue, Jan 31, 2017 at 01:13:11PM +0000, Mcnamara, John wrote:
> > From: Thomas Monjalon
> > > Hi Remy,
> > >
> > > > 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.
> > >
> > > I'm still not convinced by this library, and this introduction does not
> > > help a lot.
> > >
> > > I would like to thanks Harry for the review of this series.
> > > If we had more opinions or enthousiasm about this patch, it would be
> > > easier to accept this new library and assert it is the way to go.
> >
> > Hi,
> >
> > The RFCs for this library (initially two, merged into one) have been up since October, during the 16.11 timeframe. Comments were made and applied.
> >
> > http://dpdk.org/ml/archives/dev/2016-October/049571.html
> > http://dpdk.org/ml/archives/dev/2016-October/048390.html
> >
> > I'm concerned that these new comments/reservations are coming in very, very late in the 17.02 release cycle.
> >
> > If there hasn't been a lot of opinions or enthusiasm then equally there hasn't been other reservations. If there had been we would have addressed them.
> >
> >
> > > It could be a matter of technical board discussion if we had a clear
> > > explanation of the needs, the pros and cons of this design.
> >
> > We are happy to have the design discussed at the Technical Board. We would also like the inclusion of this library in RC3 to be discussed since that is still our desired outcome.
> >
> > We have, as any other company would have, customers awaiting features, developers committed to timelines, and testing and integration roadmaps. Blocking or delaying features at the last moment isn't an effective model that we, and I'm sure other companies, can work with.
> >
> > As such, it is probably best, that all current and future RFCs are reviewed at the Technical Board and that the board gives an indication on whether the proposal is acceptable for upstreaming or not.
> >
>
> I would tend to agree with this. The tech board should indeed look to
> insure that all RFCs and V1s have had some feedback on them well before
> the merge deadline.
>
> I don't believe it's fair on developers to suddenly give feedback at
> merge-time and thereby prevent the patch making it into a release,
> without giving time to do any rework. This is especially true if the
> patch had already been reviewed and acked, and so could be considered
> "ready for merge".
>
> The tech board should also discuss some reasonable guidelines
> for this area. My opinion is that by the time RC1 is released, any
> patches that have been reviewed, acked and have no outstanding
> comments on them for e.g. 1 week, must be merged in for the release. Any
> additional feedback thereafter should be considered "too late", and
> should be addressed in the following release. This will help to
> incentivize reviewers to review early, and also give developers some
> degree of confidence that their patches will be merged in. We have
> deadlines for submitters to get patches in, we should also have
> deadlines for reviewers to object to those patches.
We are talking about adding some new libraries in DPDK.
I think it is a special case where the submitter should make sure other
contributors agree to add such new capabilities in DPDK.
Saying there are some customers waiting for this feature to be upstreamed is
not a good argument. But it could help to explain what the goal of this series.
I agree this kind of comment should happen earlier and I'm sorry to not have
explicit them in earlier stages. That's why I suggest the Technical Board
could monitor this kind of proposal and make sure the discussion is progressing.
@@ -596,6 +596,10 @@ F: lib/librte_jobstats/
F: examples/l2fwd-jobstats/
F: doc/guides/sample_app_ug/l2_forward_job_stats.rst
+Metrics
+M: Remy Horton <remy.horton@intel.com>
+F: lib/librte_metrics/
+
Test Applications
-----------------
@@ -593,3 +593,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
@@ -150,4 +150,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)
@@ -50,6 +50,7 @@ INPUT = doc/api/doxy-api-index.md \
lib/librte_mbuf \
lib/librte_mempool \
lib/librte_meter \
+ lib/librte_metrics \
lib/librte_net \
lib/librte_pdump \
lib/librte_pipeline \
@@ -34,6 +34,12 @@ New Features
Refer to the previous release notes for examples.
+ * **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.
+
This section is a comment. do not overwrite or remove it.
Also, make sure to start the actual text at the margin.
=========================================================
@@ -205,6 +211,7 @@ The libraries prepended with a plus sign were incremented in this version.
.. code-block:: diff
librte_acl.so.2
+ + librte_bitratestats.so.1
librte_cfgfile.so.2
librte_cmdline.so.2
librte_cryptodev.so.2
@@ -220,6 +227,7 @@ The libraries prepended with a plus sign were incremented in this version.
librte_mbuf.so.2
librte_mempool.so.2
librte_meter.so.1
+ + librte_metrics.so.1
librte_net.so.1
librte_pdump.so.1
librte_pipeline.so.3
@@ -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
new file mode 100644
@@ -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 CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+# library name
+LIB = librte_metrics.a
+
+CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR)
+
+EXPORT_MAP := rte_metrics_version.map
+
+LIBABIVER := 1
+
+# all source are stored in SRCS-y
+SRCS-$(CONFIG_RTE_LIBRTE_METRICS) := rte_metrics.c
+
+# Install header file
+SYMLINK-$(CONFIG_RTE_LIBRTE_METRICS)-include += rte_metrics.h
+
+DEPDIRS-$(CONFIG_RTE_LIBRTE_METRICS) += lib/librte_eal
+
+include $(RTE_SDK)/mk/rte.lib.mk
new file mode 100644
@@ -0,0 +1,308 @@
+/*-
+ * BSD LICENSE
+ *
+ * Copyright(c) 2016-2017 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 CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <string.h>
+#include <sys/queue.h>
+
+#include <rte_common.h>
+#include <rte_malloc.h>
+#include <rte_metrics.h>
+#include <rte_lcore.h>
+#include <rte_memzone.h>
+#include <rte_spinlock.h>
+
+#define RTE_METRICS_MAX_METRICS 256
+#define RTE_METRICS_MEMZONE_NAME "RTE_METRICS"
+
+/**
+ * Internal stats metadata and value entry.
+ *
+ * @internal
+ * @param name
+ * Name of metric
+ * @param value
+ * Current value for metric
+ * @param idx_next_set
+ * Index of next root element (zero for none)
+ * @param idx_next_metric
+ * Index of next metric in set (zero for none)
+ *
+ * Only the root of each set needs idx_next_set but since it has to be
+ * assumed that number of sets could equal total number of metrics,
+ * having a separate set metadata table doesn't save any memory.
+ */
+struct rte_metrics_meta_s {
+ char name[RTE_METRICS_MAX_NAME_LEN];
+ uint64_t value[RTE_MAX_ETHPORTS];
+ uint64_t nonport_value;
+ uint16_t idx_next_set;
+ uint16_t idx_next_stat;
+};
+
+/**
+ * Internal stats info structure.
+ *
+ * @internal
+ * @param idx_last_set
+ * Index of last metadata entry with valid data. This value is
+ * not valid if cnt_stats is zero.
+ * @param cnt_stats
+ * Number of metrics.
+ * @param metadata
+ * Stat data memory block.
+ *
+ * Offsets into metadata are used instead of pointers because ASLR
+ * means that having the same physical addresses in different
+ * processes is not guaranteed.
+ */
+struct rte_metrics_data_s {
+ uint16_t idx_last_set;
+ uint16_t cnt_stats;
+ struct rte_metrics_meta_s metadata[RTE_METRICS_MAX_METRICS];
+ rte_spinlock_t lock;
+};
+
+void
+rte_metrics_init(int socket_id)
+{
+ struct rte_metrics_data_s *stats;
+ const struct rte_memzone *memzone;
+
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+ return;
+
+ memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
+ if (memzone != NULL)
+ return;
+ memzone = rte_memzone_reserve(RTE_METRICS_MEMZONE_NAME,
+ sizeof(struct rte_metrics_data_s), socket_id, 0);
+ if (memzone == NULL)
+ rte_exit(EXIT_FAILURE, "Unable to allocate stats memzone\n");
+ stats = memzone->addr;
+ memset(stats, 0, sizeof(struct rte_metrics_data_s));
+ rte_spinlock_init(&stats->lock);
+}
+
+int
+rte_metrics_reg_name(const char *name)
+{
+ const char * const list_names[] = {name};
+
+ return rte_metrics_reg_names(list_names, 1);
+}
+
+int
+rte_metrics_reg_names(const char * const *names, uint16_t cnt_names)
+{
+ struct rte_metrics_meta_s *entry;
+ struct rte_metrics_data_s *stats;
+ const struct rte_memzone *memzone;
+ uint16_t idx_name;
+ uint16_t idx_base;
+
+ /* Some sanity checks */
+ if (cnt_names < 1 || names == NULL)
+ return -EINVAL;
+
+ memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
+ if (memzone == NULL)
+ return -EIO;
+ stats = memzone->addr;
+
+ if (stats->cnt_stats + cnt_names >= RTE_METRICS_MAX_METRICS)
+ return -ENOMEM;
+
+ rte_spinlock_lock(&stats->lock);
+
+ /* Overwritten later if this is actually first set.. */
+ stats->metadata[stats->idx_last_set].idx_next_set = stats->cnt_stats;
+
+ stats->idx_last_set = idx_base = stats->cnt_stats;
+
+ for (idx_name = 0; idx_name < cnt_names; idx_name++) {
+ entry = &stats->metadata[idx_name + stats->cnt_stats];
+ strncpy(entry->name, names[idx_name],
+ RTE_METRICS_MAX_NAME_LEN);
+ memset(entry->value, 0, sizeof(entry->value));
+ entry->idx_next_stat = idx_name + stats->cnt_stats + 1;
+ }
+ entry->idx_next_stat = 0;
+ entry->idx_next_set = 0;
+ stats->cnt_stats += cnt_names;
+
+ rte_spinlock_unlock(&stats->lock);
+
+ return idx_base;
+}
+
+int
+rte_metrics_update_value(int port_id, uint16_t key, const uint64_t value)
+{
+ return rte_metrics_update_values(port_id, key, &value, 1);
+}
+
+int
+rte_metrics_update_values(int port_id,
+ uint16_t key,
+ const uint64_t *values,
+ uint32_t count)
+{
+ struct rte_metrics_meta_s *entry;
+ struct rte_metrics_data_s *stats;
+ const struct rte_memzone *memzone;
+ uint16_t idx_metric;
+ uint16_t idx_value;
+ uint16_t cnt_setsize;
+
+ if (port_id != RTE_METRICS_GLOBAL &&
+ (port_id < 0 || port_id > RTE_MAX_ETHPORTS))
+ return -EINVAL;
+
+ memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
+ if (memzone == NULL)
+ return -EIO;
+ stats = memzone->addr;
+
+ rte_spinlock_lock(&stats->lock);
+ idx_metric = key;
+ cnt_setsize = 1;
+ while (idx_metric < stats->cnt_stats) {
+ entry = &stats->metadata[idx_metric];
+ if (entry->idx_next_stat == 0)
+ break;
+ cnt_setsize++;
+ idx_metric++;
+ }
+ /* Check update does not cross set border */
+ if (count > cnt_setsize) {
+ rte_spinlock_unlock(&stats->lock);
+ return -ERANGE;
+ }
+
+ if (port_id == RTE_METRICS_GLOBAL)
+ for (idx_value = 0; idx_value < count; idx_value++) {
+ idx_metric = key + idx_value;
+ stats->metadata[idx_metric].nonport_value =
+ values[idx_value];
+ }
+ else
+ for (idx_value = 0; idx_value < count; idx_value++) {
+ idx_metric = key + idx_value;
+ stats->metadata[idx_metric].value[port_id] =
+ values[idx_value];
+ }
+ rte_spinlock_unlock(&stats->lock);
+ return 0;
+}
+
+int
+rte_metrics_get_names(struct rte_metric_name *names,
+ uint16_t capacity)
+{
+ 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(&stats->lock);
+ if (names != NULL) {
+ if (capacity < stats->cnt_stats) {
+ return_value = stats->cnt_stats;
+ rte_spinlock_unlock(&stats->lock);
+ return return_value;
+ }
+ for (idx_name = 0; idx_name < stats->cnt_stats; idx_name++)
+ strncpy(names[idx_name].name,
+ stats->metadata[idx_name].name,
+ RTE_METRICS_MAX_NAME_LEN);
+ }
+ return_value = stats->cnt_stats;
+ rte_spinlock_unlock(&stats->lock);
+ return return_value;
+}
+
+int
+rte_metrics_get_values(int port_id,
+ struct rte_metric_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;
+
+ if (port_id != RTE_METRICS_GLOBAL &&
+ (port_id < 0 || port_id > RTE_MAX_ETHPORTS))
+ return -EINVAL;
+
+ memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
+ /* If not allocated, fail silently */
+ if (memzone == NULL)
+ return 0;
+ stats = memzone->addr;
+ rte_spinlock_lock(&stats->lock);
+
+ if (values != NULL) {
+ if (capacity < stats->cnt_stats) {
+ return_value = stats->cnt_stats;
+ rte_spinlock_unlock(&stats->lock);
+ return return_value;
+ }
+ if (port_id == RTE_METRICS_GLOBAL)
+ for (idx_name = 0;
+ idx_name < stats->cnt_stats;
+ idx_name++) {
+ entry = &stats->metadata[idx_name];
+ values[idx_name].key = idx_name;
+ values[idx_name].value = entry->nonport_value;
+ }
+ else
+ for (idx_name = 0;
+ idx_name < stats->cnt_stats;
+ idx_name++) {
+ entry = &stats->metadata[idx_name];
+ values[idx_name].key = idx_name;
+ values[idx_name].value = entry->value[port_id];
+ }
+ }
+ return_value = stats->cnt_stats;
+ rte_spinlock_unlock(&stats->lock);
+ return return_value;
+}
new file mode 100644
@@ -0,0 +1,231 @@
+/*-
+ * BSD LICENSE
+ *
+ * Copyright(c) 2016-2017 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 CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/**
+ * @file
+ *
+ * RTE Metrics module
+ *
+ * Metric information is populated using a push model, where the
+ * information provider calls an update function on the relevant
+ * metrics. Currently only bulk querying of metrics is supported.
+ */
+
+#ifndef _RTE_METRICS_H_
+#define _RTE_METRICS_H_
+
+/** Maximum length of metric name (including null-terminator) */
+#define RTE_METRICS_MAX_NAME_LEN 64
+
+/**
+ * Global (rather than port-specific) metric.
+ *
+ * When used instead of port number by rte_metrics_update_metric()
+ * or rte_metrics_update_metric(), the global metrics, which are
+ * not associated with any specific port, are updated.
+ */
+#define RTE_METRICS_GLOBAL -1
+
+
+/**
+ * A name-key lookup for metrics.
+ *
+ * An array of this structure is returned by rte_metrics_get_names().
+ * The struct rte_eth_stats references these names via their array index.
+ */
+struct rte_metric_name {
+ /** String describing metric */
+ char name[RTE_METRICS_MAX_NAME_LEN];
+};
+
+
+/**
+ * Metric value structure.
+ *
+ * This structure is used by rte_metrics_get_values() to return metrics,
+ * which are statistics that are not generated by PMDs. It maps a name key,
+ * which corresponds to an index in the array returned by
+ * rte_metrics_get_names().
+ */
+struct rte_metric_value {
+ /** Numeric identifier of metric. */
+ uint16_t key;
+ /** Value for metric */
+ uint64_t value;
+};
+
+
+/**
+ * Initializes metric module. This function must be called from
+ * a primary process before metrics are used.
+ *
+ * @param socket_id
+ * Socket to use for shared memory allocation.
+ */
+void rte_metrics_init(int socket_id);
+
+/**
+ * Register a metric, making it available as a reporting parameter.
+ *
+ * Registering a metric is the way third-parties declare a parameter
+ * that they wish to be reported. Once registered, the associated
+ * numeric key can be obtained via rte_metrics_get_names(), which
+ * is required for updating said metric's value.
+ *
+ * @param name
+ * Metric name
+ *
+ * @return
+ * - Zero or positive: Success (index key of new metric)
+ * - \b -EIO: Error, unable to access metrics shared memory
+ * (rte_metrics_init() not called)
+ * - \b -EINVAL: Error, invalid parameters
+ * - \b -ENOMEM: Error, maximum metrics reached
+ */
+int rte_metrics_reg_name(const char *name);
+
+/**
+ * Register a set of metrics.
+ *
+ * This is a bulk version of rte_metrics_reg_metrics() and aside from
+ * handling multiple keys at once is functionally identical.
+ *
+ * @param names
+ * List of metric names
+ *
+ * @param cnt_names
+ * Number of metrics in set
+ *
+ * @return
+ * - Zero or positive: Success (index key of start of set)
+ * - \b -EIO: Error, unable to access metrics shared memory
+ * (rte_metrics_init() not called)
+ * - \b -EINVAL: Error, invalid parameters
+ * - \b -ENOMEM: Error, maximum metrics reached
+ */
+int rte_metrics_reg_names(const char * const *names, uint16_t cnt_names);
+
+/**
+ * Get metric name-key lookup table.
+ *
+ * @param names
+ * A struct rte_metric_name array of at least *capacity* in size to
+ * receive key names. If this is NULL, function returns the required
+ * number of elements for this array.
+ *
+ * @param capacity
+ * Size (number of elements) of struct rte_metric_name array.
+ * Disregarded if names is NULL.
+ *
+ * @return
+ * - Positive value above capacity: error, *names* is too small.
+ * Return value is required size.
+ * - Positive value equal or less than capacity: Success. Return
+ * value is number of elements filled in.
+ * - Negative value: error.
+ */
+int rte_metrics_get_names(
+ struct rte_metric_name *names,
+ uint16_t capacity);
+
+/**
+ * Get metric value table.
+ *
+ * @param port_id
+ * Port id to query
+ *
+ * @param values
+ * A struct rte_metric_value array of at least *capacity* in size to
+ * receive metric ids and values. If this is NULL, function returns
+ * the required number of elements for this array.
+ *
+ * @param capacity
+ * Size (number of elements) of struct rte_metric_value array.
+ * Disregarded if names is NULL.
+ *
+ * @return
+ * - Positive value above capacity: error, *values* is too small.
+ * Return value is required size.
+ * - Positive value equal or less than capacity: Success. Return
+ * value is number of elements filled in.
+ * - Negative value: error.
+ */
+int rte_metrics_get_values(
+ int port_id,
+ struct rte_metric_value *values,
+ uint16_t capacity);
+
+/**
+ * Updates a metric
+ *
+ * @param port_id
+ * Port to update metrics for
+ * @param key
+ * Id of metric to update
+ * @param value
+ * New value
+ *
+ * @return
+ * - -EIO if unable to access shared metrics memory
+ * - Zero on success
+ */
+int rte_metrics_update_value(
+ int port_id,
+ uint16_t key,
+ const uint64_t value);
+
+/**
+ * Updates a metric set. Note that it is an error to try to
+ * update across a set boundary.
+ *
+ * @param port_id
+ * Port to update metrics for
+ * @param key
+ * Base id of metrics set to update
+ * @param values
+ * Set of new values
+ * @param count
+ * Number of new values
+ *
+ * @return
+ * - -ERANGE if count exceeds metric set size
+ * - -EIO if upable to access shared metrics memory
+ * - Zero on success
+ */
+int rte_metrics_update_values(
+ int port_id,
+ uint16_t key,
+ const uint64_t *values,
+ uint32_t count);
+
+#endif
new file mode 100644
@@ -0,0 +1,13 @@
+DPDK_17.02 {
+ global:
+
+ rte_metrics_get_names;
+ rte_metrics_get_values;
+ rte_metrics_init;
+ rte_metrics_reg_name;
+ rte_metrics_reg_names;
+ rte_metrics_update_value;
+ rte_metrics_update_values;
+
+ local: *;
+};
@@ -98,6 +98,8 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_RING) += -lrte_ring
_LDLIBS-$(CONFIG_RTE_LIBRTE_EAL) += -lrte_eal
_LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE) += -lrte_cmdline
_LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE) += -lrte_cfgfile
+_LDLIBS-$(CONFIG_RTE_LIBRTE_METRICS) += -lrte_metrics
+
_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += -lrte_pmd_bond
_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lrte_pmd_xenvirt -lxenstore