[dpdk-dev,v7,1/6] lib: add information metrics library

Message ID 1484583573-30163-2-git-send-email-remy.horton@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel compilation success Compilation OK

Commit Message

Remy Horton Jan. 16, 2017, 4:19 p.m. UTC
  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                                |   5 +
 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           | 310 +++++++++++++++++++++++++++++
 lib/librte_metrics/rte_metrics.h           | 223 +++++++++++++++++++++
 lib/librte_metrics/rte_metrics_version.map |  13 ++
 mk/rte.app.mk                              |   2 +
 11 files changed, 620 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

Van Haaren, Harry Jan. 17, 2017, 11:01 a.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Remy Horton

> 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>


Comments inline below.


> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9645c9b..4a19497 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -596,6 +596,11 @@ 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/
> +F: doc/guides/sample_app_ug/keep_alive.rst

Copy paste error, Keep alive sample app guide should not be here. There is no sample app / guide for usage of the metrics library in this patchset as it is.

> +++ b/lib/librte_metrics/rte_metrics.c
> @@ -0,0 +1,310 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2016 Intel Corporation. All rights reserved.
> + *   All rights reserved.

Update to 2016-2017

> +/**
> + * 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;
> +};

Nit: why the _s in rte_metrics_meta_s? Is there a reason why struct rte_metrics_meta {  is not suitable? Same question for rte_metrics_data_s.

> +void
> +rte_metrics_init(void)
> +{
> +	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), rte_socket_id(), 0);

In my opinion, passing socket_id to rte_metrics_init() would be a better API, than relying on the metrics initialization core to be on the correct socket.

> +	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);

Should this spinlock be initialized before the creation of the memzone? Creating the memzone first, and later initializing the locking-structure for it feels racey to me. The lock should probably be taken and unlocked again after init and memset.

> +}
> +
> +int
> +rte_metrics_reg_metric(const char *name)
> +{

Nit: would rte_metrics_reg_name() be a better function name?

> +	const char * const list_names[] = {name};
> +
> +	return rte_metrics_reg_metrics(list_names, 1);
> +}
> +
> +int
> +rte_metrics_reg_metrics(const char * const *names, uint16_t cnt_names)
> +{

Nit: would rte_metrics_reg_names() be a better function name?

> +
> +int
> +rte_metrics_update_metric(int port_id, uint16_t key, const uint64_t value)
> +{

Would rte_metrics_update_single() be a better function name? I presume updating a single metric at a time is exception case, and the (see below) more generic rte_metrics_update() would be more commonly used.

> +	return rte_metrics_update_metrics(port_id, key, &value, 1);
> +}
> +
> +int
> +rte_metrics_update_metrics(int port_id,
> +	uint16_t key,
> +	const uint64_t *values,
> +	uint32_t count)

Would rte_metrics_update() be a better function name?

> +{
> +	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;
> +
> +	rte_metrics_init();

See above comments on rte_metrics_init() taking a socket_id parameter. Here any core could call update_metrics(), and if the library was not yet initialized, the memory for metrics would end up on the socket of this core. This should be avoided by
A) adding socket_id parameter to the metrics_init() function
B) Demanding that metrics_init() is called by the application before any core uses update_metrics() 

> +	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;
> +	}

What is a "set border"? I think this ensures that one set of metrics cannot overwrite the index's of another metrics set - correct?

> +
> +	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;
> +}

<snip>

> diff --git a/lib/librte_metrics/rte_metrics.h b/lib/librte_metrics/rte_metrics.h
> new file mode 100644
> index 0000000..fd82af9
> --- /dev/null
> +++ b/lib/librte_metrics/rte_metrics.h
> @@ -0,0 +1,223 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2016 Intel Corporation. All rights reserved.
> + *   All rights reserved.

Add -2017

> +
> +
> +/**
> + * Initializes metric module. This only has to be explicitly called if you
> + * intend to use rte_metrics_reg_metric() or rte_metrics_reg_metrics() from a
> + * secondary process. This function must be called from a primary process.
> + */
> +void rte_metrics_init(void);

As noted in the C function above:
- accept a socket-id
- demand an application calls it if it will use the metrics library. 

<snip>
  
Remy Horton Jan. 17, 2017, 1:40 p.m. UTC | #2
On 17/01/2017 11:01, Van Haaren, Harry wrote:
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Remy Horton
[..]
> Nit: why the _s in rte_metrics_meta_s? Is there a reason why struct
> rte_metrics_meta {  is not suitable? Same question for
> rte_metrics_data_s.

Think it was originally to avoid a namespace collision, and I left the 
suffix in because this is an internal data-structure.


>> +	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);
>
> Should this spinlock be initialized before the creation of the
> memzone? Creating the memzone first, and later initializing the
> locking-structure for it feels racey to me. The lock should probably
> be taken and unlocked again after init and memset.

Problem is the lock being part of the reserved memzone allocation. It is 
in there so that secondary processes can use the lock.


> Nit: would rte_metrics_reg_name() be a better function name?
[..]
> Nit: would rte_metrics_reg_names() be a better function name?

Agree. Done.


> Would rte_metrics_update_single() be a better function name?
[..]
> Would rte_metrics_update() be a better function name?

Think rte_metrics_update_value & rte_metrics_update_values would make a 
better pair. My preference at the moment is not to nominate a preferred one.


>> +	if (port_id != RTE_METRICS_GLOBAL &&
>> +			(port_id < 0 || port_id > RTE_MAX_ETHPORTS))
>> +		return -EINVAL;
>> +
>> +	rte_metrics_init();
>
> See above comments on rte_metrics_init() taking a socket_id parameter. Here any core could call update_metrics(), and if the library was not yet initialized, the memory for metrics would end up on the socket of this core. This should be avoided by
> A) adding socket_id parameter to the metrics_init() function
> B) Demanding that metrics_init() is called by the application before any core uses update_metrics()

Done. Should also help alleviate the race.


> What is a "set border"? I think this ensures that one set of metrics
> cannot overwrite the index's of another metrics set - correct?

It is intended to stop things like:

  idx1 = rte_metrics_reg_names(some_names, 2);
  idx2 = rte_metrics_reg_names(more_names, 3);
   ...
  rte_metrics_update_values(port_id, idx1, &new_values, 5);

as the above assumes idx2=idx1+2 which is not guaranteed in concurrent 
enviornments

..Remy
  
Van Haaren, Harry Jan. 17, 2017, 2:23 p.m. UTC | #3
> -----Original Message-----
> From: Horton, Remy
> Sent: Tuesday, January 17, 2017 1:40 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> Cc: Thomas Monjalon <thomas.monjalon@6wind.com>
> Subject: Re: [dpdk-dev] [PATCH v7 1/6] lib: add information metrics library
> 
> 
> On 17/01/2017 11:01, Van Haaren, Harry wrote:
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Remy Horton
> [..]
> > Nit: why the _s in rte_metrics_meta_s? Is there a reason why struct
> > rte_metrics_meta {  is not suitable? Same question for
> > rte_metrics_data_s.
> 
> Think it was originally to avoid a namespace collision, and I left the
> suffix in because this is an internal data-structure.

OK.

> >> +	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);
> >
> > Should this spinlock be initialized before the creation of the
> > memzone? Creating the memzone first, and later initializing the
> > locking-structure for it feels racey to me. The lock should probably
> > be taken and unlocked again after init and memset.
> 
> Problem is the lock being part of the reserved memzone allocation. It is
> in there so that secondary processes can use the lock.

Ah ok - apologies I missed that the lock is in the memory itself.

> 
> > Nit: would rte_metrics_reg_name() be a better function name?
> [..]
> > Nit: would rte_metrics_reg_names() be a better function name?
> 
> Agree. Done.

Great

> > Would rte_metrics_update_single() be a better function name?
> [..]
> > Would rte_metrics_update() be a better function name?
> 
> Think rte_metrics_update_value & rte_metrics_update_values would make a
> better pair. My preference at the moment is not to nominate a preferred one.
> 
> 
> >> +	if (port_id != RTE_METRICS_GLOBAL &&
> >> +			(port_id < 0 || port_id > RTE_MAX_ETHPORTS))
> >> +		return -EINVAL;
> >> +
> >> +	rte_metrics_init();
> >
> > See above comments on rte_metrics_init() taking a socket_id parameter. Here any core
> could call update_metrics(), and if the library was not yet initialized, the memory for
> metrics would end up on the socket of this core. This should be avoided by
> > A) adding socket_id parameter to the metrics_init() function
> > B) Demanding that metrics_init() is called by the application before any core uses
> update_metrics()
> 
> Done. Should also help alleviate the race.

Cool.


> > What is a "set border"? I think this ensures that one set of metrics
> > cannot overwrite the index's of another metrics set - correct?
> 
> It is intended to stop things like:
> 
>   idx1 = rte_metrics_reg_names(some_names, 2);
>   idx2 = rte_metrics_reg_names(more_names, 3);
>    ...
>   rte_metrics_update_values(port_id, idx1, &new_values, 5);
> 
> as the above assumes idx2=idx1+2 which is not guaranteed in concurrent
> enviornments


That confirms my understanding - thanks.
  

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 9645c9b..4a19497 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -596,6 +596,11 @@  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/
+F: doc/guides/sample_app_ug/keep_alive.rst
+
 
 Test Applications
 -----------------
diff --git a/config/common_base b/config/common_base
index 8e9dcfa..0eb3866 100644
--- 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
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),
   [version]            (@ref rte_version.h)
diff --git a/doc/api/doxy-api.conf b/doc/api/doxy-api.conf
index b340fcf..194b670 100644
--- a/doc/api/doxy-api.conf
+++ b/doc/api/doxy-api.conf
@@ -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 \
diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst
index d445d64..4fca29b 100644
--- 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
+     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.
      =========================================================
@@ -161,6 +167,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
@@ -176,6 +183,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
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 0000000..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 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
diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
new file mode 100644
index 0000000..5072f4d
--- /dev/null
+++ b/lib/librte_metrics/rte_metrics.c
@@ -0,0 +1,310 @@ 
+/*-
+ *   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 <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(void)
+{
+	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), rte_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_metric(const char *name)
+{
+	const char * const list_names[] = {name};
+
+	return rte_metrics_reg_metrics(list_names, 1);
+}
+
+int
+rte_metrics_reg_metrics(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;
+
+	rte_metrics_init();
+	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_metric(int port_id, uint16_t key, const uint64_t value)
+{
+	return rte_metrics_update_metrics(port_id, key, &value, 1);
+}
+
+int
+rte_metrics_update_metrics(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;
+
+	rte_metrics_init();
+	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;
+}
diff --git a/lib/librte_metrics/rte_metrics.h b/lib/librte_metrics/rte_metrics.h
new file mode 100644
index 0000000..fd82af9
--- /dev/null
+++ b/lib/librte_metrics/rte_metrics.h
@@ -0,0 +1,223 @@ 
+/*-
+ *   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.
+ */
+
+/**
+ * @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 only has to be explicitly called if you
+ * intend to use rte_metrics_reg_metric() or rte_metrics_reg_metrics() from a
+ * secondary process. This function must be called from a primary process.
+ */
+void rte_metrics_init(void);
+
+/**
+ * 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
+ *  - Negative: Failure
+ */
+int rte_metrics_reg_metric(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
+ *  - Negative: Failure
+ */
+int rte_metrics_reg_metrics(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_metric(
+	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_metrics(
+	int port_id,
+	uint16_t key,
+	const uint64_t *values,
+	uint32_t count);
+
+#endif
diff --git a/lib/librte_metrics/rte_metrics_version.map b/lib/librte_metrics/rte_metrics_version.map
new file mode 100644
index 0000000..f904814
--- /dev/null
+++ b/lib/librte_metrics/rte_metrics_version.map
@@ -0,0 +1,13 @@ 
+DPDK_17.02 {
+	global:
+
+	rte_metrics_get_names;
+	rte_metrics_get_values;
+	rte_metrics_init;
+	rte_metrics_reg_metric;
+	rte_metrics_reg_metrics;
+	rte_metrics_update_metric;
+	rte_metrics_update_metrics;
+
+	local: *;
+};
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index f75f0e2..40fcf33 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -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