[01/11] telemetry: initial telemetry infrastructure
diff mbox series

Message ID 1535026093-101872-2-git-send-email-ciara.power@intel.com
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show
Series
  • introduce telemetry library
Related show

Checks

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

Commit Message

Ciara Power Aug. 23, 2018, 12:08 p.m. UTC
This patch adds the infrastructure and initial code for the
telemetry library.

A virtual device is used for telemetry, which is included in
this patch. To use telemetry, a command-line argument must be
used - "--vdev=telemetry".

Control threads are used to get CPU cycles for telemetry, which
are configured in this patch also.

Signed-off-by: Ciara Power <ciara.power@intel.com>
Signed-off-by: Brian Archbold <brian.archbold@intel.com>
---
 config/common_base                                 |   5 +
 drivers/Makefile                                   |   3 +
 drivers/meson.build                                |   3 +-
 drivers/telemetry/Makefile                         |   8 ++
 drivers/telemetry/meson.build                      |   6 ++
 drivers/telemetry/telemetry/Makefile               |  26 +++++
 drivers/telemetry/telemetry/meson.build            |   5 +
 .../telemetry/rte_pmd_telemetry_version.map        |   9 ++
 drivers/telemetry/telemetry/telemetry_driver.c     |  40 ++++++++
 lib/Makefile                                       |   2 +
 lib/librte_telemetry/Makefile                      |  26 +++++
 lib/librte_telemetry/meson.build                   |   7 ++
 lib/librte_telemetry/rte_telemetry.c               | 108 +++++++++++++++++++++
 lib/librte_telemetry/rte_telemetry.h               |  40 ++++++++
 lib/librte_telemetry/rte_telemetry_internal.h      |  32 ++++++
 lib/librte_telemetry/rte_telemetry_version.map     |   6 ++
 lib/meson.build                                    |   2 +-
 mk/rte.app.mk                                      |   2 +
 18 files changed, 328 insertions(+), 2 deletions(-)
 create mode 100644 drivers/telemetry/Makefile
 create mode 100644 drivers/telemetry/meson.build
 create mode 100644 drivers/telemetry/telemetry/Makefile
 create mode 100644 drivers/telemetry/telemetry/meson.build
 create mode 100644 drivers/telemetry/telemetry/rte_pmd_telemetry_version.map
 create mode 100644 drivers/telemetry/telemetry/telemetry_driver.c
 create mode 100644 lib/librte_telemetry/Makefile
 create mode 100644 lib/librte_telemetry/meson.build
 create mode 100644 lib/librte_telemetry/rte_telemetry.c
 create mode 100644 lib/librte_telemetry/rte_telemetry.h
 create mode 100644 lib/librte_telemetry/rte_telemetry_internal.h
 create mode 100644 lib/librte_telemetry/rte_telemetry_version.map

Comments

Stephen Hemminger Aug. 23, 2018, 11:17 p.m. UTC | #1
On Thu, 23 Aug 2018 13:08:03 +0100
Ciara Power <ciara.power@intel.com> wrote:

> +
> +static int
> +telemetry_probe(struct rte_vdev_device *vdev)
> +{
> +	int ret;
> +
> +	RTE_SET_USED(vdev);
> +	ret = rte_telemetry_init(rte_socket_id());
> +	if (ret < 0) {
> +		printf("Error - Telemetry initialisation failed\n");
> +		return -1;
> +	}
> +	return 0;
> +}
> +
> +static int
> +telemetry_remove(struct rte_vdev_device *vdev)
> +{
> +	const char *name;
> +	name = rte_vdev_device_name(vdev);
> +	printf("Uninitialising the device: %s\n", name);

Please use DPDK (rte) logging for all messages.
Stephen Hemminger Aug. 23, 2018, 11:17 p.m. UTC | #2
On Thu, 23 Aug 2018 13:08:03 +0100
Ciara Power <ciara.power@intel.com> wrote:

> +static int32_t
> +rte_telemetry_run(void *userdata)
> +{
> +	struct telemetry_impl *telemetry = (struct telemetry_impl *)userdata;

Casting a void * pointer is unnecessary in C.
Assignment has an implicit cast.
Stephen Hemminger Aug. 23, 2018, 11:18 p.m. UTC | #3
On Thu, 23 Aug 2018 13:08:03 +0100
Ciara Power <ciara.power@intel.com> wrote:

> +			TELEMETRY_LOG_ERR("Error - Calling thread could not be"
> +				" put to sleep\n");

Don't break strings across lines. It makes it harder for developers who want
to use tools to look for log message in source.  Checkpatch and friends allow for long
line exception for string literals.
Stephen Hemminger Aug. 23, 2018, 11:19 p.m. UTC | #4
On Thu, 23 Aug 2018 13:08:03 +0100
Ciara Power <ciara.power@intel.com> wrote:

> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#include <stdint.h>
> +
> +#ifndef _RTE_TELEMETRY_H_
> +#define _RTE_TELEMETRY_H_
> +
> +/**
> + * Get the telemetry_impl structure device pointer initialised.
> + *
> + * @param socket_id
> + *  Unsigned integer representing the socket id to be used
> + *  for the telemetry structure.
> + *
> + * @return
> + *  0 on successful initialisation.
> + * @return
> + *  -ENOMEM on memory allocation error
> + * @return
> + *  -EPERM on unknown error failure
> + * @return
> + *  -EALREADY if Telemetry is already initialised.
> + */
> +int32_t
> +rte_telemetry_init(uint32_t socket_id);
> +
> +/**
> + * Clean up and free memory.
> + *
> + * @return
> + *  0 on success
> + * @return
> + *  -EPERM on failure
> + */
> +int32_t
> +rte_telemetry_cleanup(void);
> +

Can this be done with RTE_INIT (i.e automatic constructor).
Stephen Hemminger Aug. 23, 2018, 11:22 p.m. UTC | #5
On Thu, 23 Aug 2018 13:08:03 +0100
Ciara Power <ciara.power@intel.com> wrote:

> +/* Logging Macros */
> +extern int telemetry_log_level;
> +
> +#define TELEMETRY_LOG(level, fmt, args...) \
> +	rte_log(RTE_LOG_ ##level, telemetry_log_level, "%s(): "fmt "\n", \
> +		__func__, ##args)
> +
> +#define TELEMETRY_LOG_ERR(fmt, args...) \
> +	TELEMETRY_LOG(ERR, fmt, ## args)
> +
> +#define TELEMETRY_LOG_WARN(fmt, args...) \
> +	TELEMETRY_LOG(WARNING, fmt, ## args)
> +
> +#define TELEMETRY_LOG_INFO(fmt, args...) \
> +	TELEMETRY_LOG(INFO, fmt, ## args)
> +
> +typedef struct telemetry_impl {
> +	pthread_t thread_id;
> +	int thread_status;
> +	uint32_t socket_id;
> +} telemetry_impl;
> +

Your logging macros follow the standard DPDK style. Including automatically
adding a new line. But as I look at the code, many of the TELEMETRY_LOG calls
have a newline in the format. Therefore your log messages will be double spaced.
Shreyansh Jain Aug. 24, 2018, 1:03 p.m. UTC | #6
On Thursday 23 August 2018 05:38 PM, Ciara Power wrote:
> This patch adds the infrastructure and initial code for the
> telemetry library.
> 
> A virtual device is used for telemetry, which is included in
> this patch. To use telemetry, a command-line argument must be
> used - "--vdev=telemetry".
> 
> Control threads are used to get CPU cycles for telemetry, which
> are configured in this patch also.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> Signed-off-by: Brian Archbold <brian.archbold@intel.com>
> ---

[...]

/rte_pmd_telemetry_version.map
> new file mode 100644
> index 0000000..a73e0f2
> --- /dev/null
> +++ b/drivers/telemetry/telemetry/rte_pmd_telemetry_version.map
> @@ -0,0 +1,9 @@
> +DPDK_18.05 {

         ^^^^^
I think you want 18.11 here.

> +	global:
> +
> +	telemetry_probe;
> +	telemetry_remove;
> +
> +	local: *;
> +
> +};


[...]

> diff --git a/lib/Makefile b/lib/Makefile
> index afa604e..8cbd035 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -105,6 +105,8 @@ DEPDIRS-librte_gso := librte_eal librte_mbuf librte_ethdev librte_net
>   DEPDIRS-librte_gso += librte_mempool
>   DIRS-$(CONFIG_RTE_LIBRTE_BPF) += librte_bpf
>   DEPDIRS-librte_bpf := librte_eal librte_mempool librte_mbuf librte_ethdev
> +DIRS-$(CONFIG_RTE_LIBRTE_TELEMETRY) += librte_telemetry
> +DEPDIRS-librte_telemetry := librte_eal librte_metrics librte_ethdev
>   
>   ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
>   DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
> diff --git a/lib/librte_telemetry/Makefile b/lib/librte_telemetry/Makefile
> new file mode 100644
> index 0000000..bda3788
> --- /dev/null
> +++ b/lib/librte_telemetry/Makefile
> @@ -0,0 +1,26 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2018 Intel Corporation
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +# library name
> +LIB = librte_telemetry.a
> +
> +CFLAGS += -O3
> +CFLAGS += -I$(SRCDIR)
> +CFLAGS += -DALLOW_EXPERIMENTAL_API
> +
> +LDLIBS += -lrte_eal -lrte_ethdev
> +LDLIBS += -lrte_metrics
> +
> +EXPORT_MAP := rte_telemetry_version.map
> +
> +LIBABIVER := 1
> +
> +# library source files
> +SRCS-$(CONFIG_RTE_LIBRTE_TELEMETRY) := rte_telemetry.c
> +
> +# export include files
> +SYMLINK-$(CONFIG_RTE_LIBRTE_TELEMETRY)-include := rte_telemetry.h
> +
> +include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_telemetry/meson.build b/lib/librte_telemetry/meson.build
> new file mode 100644
> index 0000000..7716076
> --- /dev/null
> +++ b/lib/librte_telemetry/meson.build
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2018 Intel Corporation
> +
> +sources = files('rte_telemetry.c')
> +headers = files('rte_telemetry.h', 'rte_telemetry_internal.h')
> +deps += ['metrics', 'ethdev']
> +cflags += '-DALLOW_EXPERIMENTAL_API'
> diff --git a/lib/librte_telemetry/rte_telemetry.c b/lib/librte_telemetry/rte_telemetry.c
> new file mode 100644
> index 0000000..8d7b0e3
> --- /dev/null
> +++ b/lib/librte_telemetry/rte_telemetry.c
> @@ -0,0 +1,108 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#include <unistd.h>
> +
> +#include <rte_eal.h>
> +#include <rte_ethdev.h>
> +#include <rte_metrics.h>
> +
> +#include "rte_telemetry.h"
> +#include "rte_telemetry_internal.h"
> +
> +#define SLEEP_TIME 10
> +
> +static telemetry_impl *static_telemetry;
> +
> +static int32_t
> +rte_telemetry_run(void *userdata)
> +{
> +	struct telemetry_impl *telemetry = (struct telemetry_impl *)userdata;
> +	if (!telemetry) {
> +		TELEMETRY_LOG_WARN("Warning - TELEMETRY could not be "
> +			"initialised\n");

Your 'TELEMETRY_LOG_WARNING' already includes a '\n' in its definition. 
This would add another one. Can you re-check?

> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static void
> +*rte_telemetry_run_thread_func(void *userdata)
> +{
> +	int ret;
> +	struct telemetry_impl *telemetry = (struct telemetry_impl *)userdata;
> +
> +	if (!telemetry) {
> +		TELEMETRY_LOG_ERR("Error - %s passed a NULL instance\n",
> +			__func__);

I might be picky - but this is an internal function spawned using 
rte_ctrl_thread_create which already has a check whether the argument 
(static_telemetry) is NULL or not. So, this is like duplicating that work.

> +		pthread_exit(0);
> +	}
> +
> +	while (telemetry->thread_status) {
> +		rte_telemetry_run(telemetry);
> +		ret = usleep(SLEEP_TIME);
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - Calling thread could not be"
> +				" put to sleep\n");

If the calling thread couldn't be put to sleep, you would continue 
looping without sleeping? Wouldn't that simply hog your CPU? Or, is that 
expected design?

> +	}
> +	pthread_exit(0);
> +}
> +
> +int32_t
> +rte_telemetry_init(uint32_t socket_id)
> +{
> +	int ret;
> +	pthread_attr_t attr;
> +	const char *telemetry_ctrl_thread = "telemetry";
> +
> +	if (static_telemetry) {
> +		TELEMETRY_LOG_WARN("Warning - TELEMETRY structure already "
> +			"initialised\n");
> +		return -EALREADY;
> +	}
> +
> +	static_telemetry = calloc(1, sizeof(struct telemetry_impl));
> +	if (!static_telemetry) {
> +		TELEMETRY_LOG_ERR("Error - Memory could not be allocated\n");
> +		return -ENOMEM;
> +	}
> +
> +	static_telemetry->socket_id = socket_id;
> +	rte_metrics_init(static_telemetry->socket_id);
> +	pthread_attr_init(&attr);
> +	ret = rte_ctrl_thread_create(&static_telemetry->thread_id,
> +		telemetry_ctrl_thread, &attr, rte_telemetry_run_thread_func,
> +		(void *)static_telemetry);
> +	static_telemetry->thread_status = 1;
> +	if (ret < 0) {
> +		ret = rte_telemetry_cleanup();
> +		if (ret < 0)
> +			TELEMETRY_LOG_ERR("Error - TELEMETRY cleanup failed\n");
> +		return -EPERM;
> +	}
> +	return 0;
> +}
> +
> +int32_t
> +rte_telemetry_cleanup(void)
> +{
> +	struct telemetry_impl *telemetry = static_telemetry;
> +	telemetry->thread_status = 0;
> +	pthread_join(telemetry->thread_id, NULL);
> +	free(telemetry);
> +	static_telemetry = NULL;
> +	return 0;

Maybe if you could use be a little more liberal with new lines, it would 
be slightly easier to read.
But again, that is my personal opinion.

> +}
> +
> +int telemetry_log_level;
> +RTE_INIT(rte_telemetry_log_init);
> +
> +static void
> +rte_telemetry_log_init(void)
> +{
> +	telemetry_log_level = rte_log_register("lib.telemetry");
> +	if (telemetry_log_level >= 0)
> +		rte_log_set_level(telemetry_log_level, RTE_LOG_ERR);
> +}

[...]

> +#endif
> diff --git a/lib/librte_telemetry/rte_telemetry_version.map b/lib/librte_telemetry/rte_telemetry_version.map
> new file mode 100644
> index 0000000..efd437d
> --- /dev/null
> +++ b/lib/librte_telemetry/rte_telemetry_version.map
> @@ -0,0 +1,6 @@
> +DPDK_18.05 {

         ^^^^^^
I think you want it to be 18.11 here.

> +	global:
> +
> +	rte_telemetry_init;
> +	local: *;
> +};

[...]
Gaëtan Rivet Aug. 28, 2018, 11:46 a.m. UTC | #7
Hi Ciara,

On Thu, Aug 23, 2018 at 01:08:03PM +0100, Ciara Power wrote:
> This patch adds the infrastructure and initial code for the
> telemetry library.
> 
> A virtual device is used for telemetry, which is included in
> this patch. To use telemetry, a command-line argument must be
> used - "--vdev=telemetry".
> 

Why use a virtual device?

It seems that you are only using the device semantic as a way to carry
around a tag telling the DPDK framework to init your library once it has
finished its initialization.

I guess you wanted to avoid having to add the call to rte_telemetry_init
to all applications. In the absence of a proper EAL option framework,
you can workaround by adding a --telemetry EAL parameter, setting a flag
on, and checking this flag from librte_telemetry, within a routine
declared with RTE_INIT_PRIO.

I only see afterward the selftest being triggered via kvargs. I haven't
yet looked at the testing done, but if it is only unit test, the "test"
app would be better suited. If it is integration testing to verify the
behavior of the library with other PMDs, you probably need specific
context, thus selftest being insufficient on its own and useless for
other users.

> Control threads are used to get CPU cycles for telemetry, which
> are configured in this patch also.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> Signed-off-by: Brian Archbold <brian.archbold@intel.com>

Regards,
Van Haaren, Harry Aug. 28, 2018, 4:50 p.m. UTC | #8
> From: Shreyansh Jain [mailto:shreyansh.jain@nxp.com]
> Sent: Friday, August 24, 2018 2:04 PM
> To: Power, Ciara <ciara.power@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Archbold, Brian <brian.archbold@intel.com>;
> Kenny, Emma <emma.kenny@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 01/11] telemetry: initial telemetry
> infrastructure
> 
> On Thursday 23 August 2018 05:38 PM, Ciara Power wrote:
> > This patch adds the infrastructure and initial code for the
> > telemetry library.
> >
> > A virtual device is used for telemetry, which is included in
> > this patch. To use telemetry, a command-line argument must be
> > used - "--vdev=telemetry".
> >
> > Control threads are used to get CPU cycles for telemetry, which
> > are configured in this patch also.
> >
> > Signed-off-by: Ciara Power <ciara.power@intel.com>
> > Signed-off-by: Brian Archbold <brian.archbold@intel.com>
> > ---
> 
> [...]
> 
> /rte_pmd_telemetry_version.map
> > new file mode 100644
> > index 0000000..a73e0f2
> > --- /dev/null
> > +++ b/drivers/telemetry/telemetry/rte_pmd_telemetry_version.map
> > @@ -0,0 +1,9 @@
> > +DPDK_18.05 {
> 
>          ^^^^^
> I think you want 18.11 here.

Yes indeed.


> > +	global:
> > +
> > +	telemetry_probe;
> > +	telemetry_remove;
> > +
> > +	local: *;
> > +
> > +};
> 
> 
> [...]
> 
> > diff --git a/lib/Makefile b/lib/Makefile
> > index afa604e..8cbd035 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -105,6 +105,8 @@ DEPDIRS-librte_gso := librte_eal librte_mbuf
> librte_ethdev librte_net
> >   DEPDIRS-librte_gso += librte_mempool
> >   DIRS-$(CONFIG_RTE_LIBRTE_BPF) += librte_bpf
> >   DEPDIRS-librte_bpf := librte_eal librte_mempool librte_mbuf
> librte_ethdev
> > +DIRS-$(CONFIG_RTE_LIBRTE_TELEMETRY) += librte_telemetry
> > +DEPDIRS-librte_telemetry := librte_eal librte_metrics librte_ethdev
> >
> >   ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
> >   DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
> > diff --git a/lib/librte_telemetry/Makefile b/lib/librte_telemetry/Makefile
> > new file mode 100644
> > index 0000000..bda3788
> > --- /dev/null
> > +++ b/lib/librte_telemetry/Makefile
> > @@ -0,0 +1,26 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2018 Intel Corporation
> > +
> > +include $(RTE_SDK)/mk/rte.vars.mk
> > +
> > +# library name
> > +LIB = librte_telemetry.a
> > +
> > +CFLAGS += -O3
> > +CFLAGS += -I$(SRCDIR)
> > +CFLAGS += -DALLOW_EXPERIMENTAL_API
> > +
> > +LDLIBS += -lrte_eal -lrte_ethdev
> > +LDLIBS += -lrte_metrics
> > +
> > +EXPORT_MAP := rte_telemetry_version.map
> > +
> > +LIBABIVER := 1
> > +
> > +# library source files
> > +SRCS-$(CONFIG_RTE_LIBRTE_TELEMETRY) := rte_telemetry.c
> > +
> > +# export include files
> > +SYMLINK-$(CONFIG_RTE_LIBRTE_TELEMETRY)-include := rte_telemetry.h
> > +
> > +include $(RTE_SDK)/mk/rte.lib.mk
> > diff --git a/lib/librte_telemetry/meson.build
> b/lib/librte_telemetry/meson.build
> > new file mode 100644
> > index 0000000..7716076
> > --- /dev/null
> > +++ b/lib/librte_telemetry/meson.build
> > @@ -0,0 +1,7 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2018 Intel Corporation
> > +
> > +sources = files('rte_telemetry.c')
> > +headers = files('rte_telemetry.h', 'rte_telemetry_internal.h')
> > +deps += ['metrics', 'ethdev']
> > +cflags += '-DALLOW_EXPERIMENTAL_API'
> > diff --git a/lib/librte_telemetry/rte_telemetry.c
> b/lib/librte_telemetry/rte_telemetry.c
> > new file mode 100644
> > index 0000000..8d7b0e3
> > --- /dev/null
> > +++ b/lib/librte_telemetry/rte_telemetry.c
> > @@ -0,0 +1,108 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2018 Intel Corporation
> > + */
> > +
> > +#include <unistd.h>
> > +
> > +#include <rte_eal.h>
> > +#include <rte_ethdev.h>
> > +#include <rte_metrics.h>
> > +
> > +#include "rte_telemetry.h"
> > +#include "rte_telemetry_internal.h"
> > +
> > +#define SLEEP_TIME 10
> > +
> > +static telemetry_impl *static_telemetry;
> > +
> > +static int32_t
> > +rte_telemetry_run(void *userdata)
> > +{
> > +	struct telemetry_impl *telemetry = (struct telemetry_impl *)userdata;
> > +	if (!telemetry) {
> > +		TELEMETRY_LOG_WARN("Warning - TELEMETRY could not be "
> > +			"initialised\n");
> 
> Your 'TELEMETRY_LOG_WARNING' already includes a '\n' in its definition.
> This would add another one. Can you re-check?

Yes, as Stephen noted too. Will fix!

> 
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void
> > +*rte_telemetry_run_thread_func(void *userdata)
> > +{
> > +	int ret;
> > +	struct telemetry_impl *telemetry = (struct telemetry_impl *)userdata;
> > +
> > +	if (!telemetry) {
> > +		TELEMETRY_LOG_ERR("Error - %s passed a NULL instance\n",
> > +			__func__);
> 
> I might be picky - but this is an internal function spawned using
> rte_ctrl_thread_create which already has a check whether the argument
> (static_telemetry) is NULL or not. So, this is like duplicating that work.
> 
> > +		pthread_exit(0);
> > +	}
> > +
> > +	while (telemetry->thread_status) {
> > +		rte_telemetry_run(telemetry);
> > +		ret = usleep(SLEEP_TIME);
> > +		if (ret < 0)
> > +			TELEMETRY_LOG_ERR("Error - Calling thread could not be"
> > +				" put to sleep\n");
> 
> If the calling thread couldn't be put to sleep, you would continue
> looping without sleeping? Wouldn't that simply hog your CPU? Or, is that
> expected design?

Will look into this.


> > +	}
> > +	pthread_exit(0);
> > +}
> > +
> > +int32_t
> > +rte_telemetry_init(uint32_t socket_id)
> > +{
> > +	int ret;
> > +	pthread_attr_t attr;
> > +	const char *telemetry_ctrl_thread = "telemetry";
> > +
> > +	if (static_telemetry) {
> > +		TELEMETRY_LOG_WARN("Warning - TELEMETRY structure already "
> > +			"initialised\n");
> > +		return -EALREADY;
> > +	}
> > +
> > +	static_telemetry = calloc(1, sizeof(struct telemetry_impl));
> > +	if (!static_telemetry) {
> > +		TELEMETRY_LOG_ERR("Error - Memory could not be allocated\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	static_telemetry->socket_id = socket_id;
> > +	rte_metrics_init(static_telemetry->socket_id);
> > +	pthread_attr_init(&attr);
> > +	ret = rte_ctrl_thread_create(&static_telemetry->thread_id,
> > +		telemetry_ctrl_thread, &attr, rte_telemetry_run_thread_func,
> > +		(void *)static_telemetry);
> > +	static_telemetry->thread_status = 1;
> > +	if (ret < 0) {
> > +		ret = rte_telemetry_cleanup();
> > +		if (ret < 0)
> > +			TELEMETRY_LOG_ERR("Error - TELEMETRY cleanup failed\n");
> > +		return -EPERM;
> > +	}
> > +	return 0;
> > +}
> > +
> > +int32_t
> > +rte_telemetry_cleanup(void)
> > +{
> > +	struct telemetry_impl *telemetry = static_telemetry;
> > +	telemetry->thread_status = 0;
> > +	pthread_join(telemetry->thread_id, NULL);
> > +	free(telemetry);
> > +	static_telemetry = NULL;
> > +	return 0;
> 
> Maybe if you could use be a little more liberal with new lines, it would
> be slightly easier to read.
> But again, that is my personal opinion.

Thanks for the input, will be kept in mind.

> > +}
> > +
> > +int telemetry_log_level;
> > +RTE_INIT(rte_telemetry_log_init);
> > +
> > +static void
> > +rte_telemetry_log_init(void)
> > +{
> > +	telemetry_log_level = rte_log_register("lib.telemetry");
> > +	if (telemetry_log_level >= 0)
> > +		rte_log_set_level(telemetry_log_level, RTE_LOG_ERR);
> > +}
> 
> [...]
> 
> > +#endif
> > diff --git a/lib/librte_telemetry/rte_telemetry_version.map
> b/lib/librte_telemetry/rte_telemetry_version.map
> > new file mode 100644
> > index 0000000..efd437d
> > --- /dev/null
> > +++ b/lib/librte_telemetry/rte_telemetry_version.map
> > @@ -0,0 +1,6 @@
> > +DPDK_18.05 {
> 
>          ^^^^^^
> I think you want it to be 18.11 here.

Correct.

Thanks for review, hopefully see you at Userspace where
there will be a lightning talk about this patchset.

Cheers, -Harry
Van Haaren, Harry Aug. 28, 2018, 4:54 p.m. UTC | #9
Hi Gaetan,

> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Tuesday, August 28, 2018 12:47 PM
> To: Power, Ciara <ciara.power@intel.com>
> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Archbold, Brian
> <brian.archbold@intel.com>; Kenny, Emma <emma.kenny@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 01/11] telemetry: initial telemetry
> infrastructure
> 
> Hi Ciara,
> 
> On Thu, Aug 23, 2018 at 01:08:03PM +0100, Ciara Power wrote:
> > This patch adds the infrastructure and initial code for the
> > telemetry library.
> >
> > A virtual device is used for telemetry, which is included in
> > this patch. To use telemetry, a command-line argument must be
> > used - "--vdev=telemetry".
> >
> 
> Why use a virtual device?
> 
> It seems that you are only using the device semantic as a way to carry
> around a tag telling the DPDK framework to init your library once it has
> finished its initialization.
> 
> I guess you wanted to avoid having to add the call to rte_telemetry_init
> to all applications. In the absence of a proper EAL option framework,
> you can workaround by adding a --telemetry EAL parameter, setting a flag
> on, and checking this flag from librte_telemetry, within a routine
> declared with RTE_INIT_PRIO.

I suppose that an EAL --flag could work too, it would mean that EAL would
depend on this library. The --vdev trick keeps the library standalone.

I don't have a strong opinion either way. :)


> I only see afterward the selftest being triggered via kvargs. I haven't
> yet looked at the testing done, but if it is only unit test, the "test"
> app would be better suited. If it is integration testing to verify the
> behavior of the library with other PMDs, you probably need specific
> context, thus selftest being insufficient on its own and useless for
> other users.

Correct, self tests are triggered by kvargs. This same model is used
in eg: eventdev PMDs to run selftests, where the tests are pretty complex
and specific to the device under test.

Again, I don't have a strong opinion but I don't see any issue with it
being included in the vdev / telemetry library. We could write a shim
test that the "official" test binary runs the telemetry tests if that is
your concern?


> > Control threads are used to get CPU cycles for telemetry, which
> > are configured in this patch also.
> >
> > Signed-off-by: Ciara Power <ciara.power@intel.com>
> > Signed-off-by: Brian Archbold <brian.archbold@intel.com>
> 
> Regards,
> --
> Gaëtan Rivet
> 6WIND

Thanks for review, and there's a lightning talk at Userspace so please
do provide input there too :) -Harry
Van Haaren, Harry Aug. 28, 2018, 5:12 p.m. UTC | #10
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, August 24, 2018 12:22 AM
> To: Power, Ciara <ciara.power@intel.com>
> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Archbold, Brian
> <brian.archbold@intel.com>; Kenny, Emma <emma.kenny@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 01/11] telemetry: initial telemetry
> infrastructure
> 
> On Thu, 23 Aug 2018 13:08:03 +0100
> Ciara Power <ciara.power@intel.com> wrote:
> 
> > +/* Logging Macros */
> > +extern int telemetry_log_level;
> > +
> > +#define TELEMETRY_LOG(level, fmt, args...) \
> > +	rte_log(RTE_LOG_ ##level, telemetry_log_level, "%s(): "fmt "\n", \
> > +		__func__, ##args)
> > +
> > +#define TELEMETRY_LOG_ERR(fmt, args...) \
> > +	TELEMETRY_LOG(ERR, fmt, ## args)
> > +
> > +#define TELEMETRY_LOG_WARN(fmt, args...) \
> > +	TELEMETRY_LOG(WARNING, fmt, ## args)
> > +
> > +#define TELEMETRY_LOG_INFO(fmt, args...) \
> > +	TELEMETRY_LOG(INFO, fmt, ## args)
> > +
> > +typedef struct telemetry_impl {
> > +	pthread_t thread_id;
> > +	int thread_status;
> > +	uint32_t socket_id;
> > +} telemetry_impl;
> > +
> 
> Your logging macros follow the standard DPDK style. Including automatically
> adding a new line. But as I look at the code, many of the TELEMETRY_LOG
> calls
> have a newline in the format. Therefore your log messages will be double
> spaced.

Correct, will get that fixed.

Thanks for reviewing! Looking forward to Userspace, curious to hear of
your use-cases for Telemetry lib, assuming you have some in mind.
Gaëtan Rivet Aug. 29, 2018, 8:23 a.m. UTC | #11
On Tue, Aug 28, 2018 at 04:54:33PM +0000, Van Haaren, Harry wrote:
> Hi Gaetan,
> 
> > -----Original Message-----
> > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> > Sent: Tuesday, August 28, 2018 12:47 PM
> > To: Power, Ciara <ciara.power@intel.com>
> > Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Archbold, Brian
> > <brian.archbold@intel.com>; Kenny, Emma <emma.kenny@intel.com>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 01/11] telemetry: initial telemetry
> > infrastructure
> > 
> > Hi Ciara,
> > 
> > On Thu, Aug 23, 2018 at 01:08:03PM +0100, Ciara Power wrote:
> > > This patch adds the infrastructure and initial code for the
> > > telemetry library.
> > >
> > > A virtual device is used for telemetry, which is included in
> > > this patch. To use telemetry, a command-line argument must be
> > > used - "--vdev=telemetry".
> > >
> > 
> > Why use a virtual device?
> > 
> > It seems that you are only using the device semantic as a way to carry
> > around a tag telling the DPDK framework to init your library once it has
> > finished its initialization.
> > 
> > I guess you wanted to avoid having to add the call to rte_telemetry_init
> > to all applications. In the absence of a proper EAL option framework,
> > you can workaround by adding a --telemetry EAL parameter, setting a flag
> > on, and checking this flag from librte_telemetry, within a routine
> > declared with RTE_INIT_PRIO.
> 
> I suppose that an EAL --flag could work too, it would mean that EAL would
> depend on this library. The --vdev trick keeps the library standalone.
> 
> I don't have a strong opinion either way. :)
> 

This was done already for specific EAL configuration items such as
vfio intr_mode or PCI uio configuration.

Of course this is ugly, but the --telemetry parameter can exist without
compiling the lib. You can add a warning if the TELEMETRY Mconfig
item is not set to mitigate. The main issue is that you need to add
getters because you cannot declare an external *struct internal_config*
reference.

I agree this is awkward, and this is exactly the reason we need a
way for libraries to register options in the EAL, but this is not
yet done.

The virtual device solution however is a crutch used to emulate this
absent framework. This will complicate developping the proper solution
and its adoption once done. I would not be clear then to the dev that they
can translate the telemetry shim parameter to the new framework, without
having to rework the whole infrastructure of the lib (and this is without
talking about reworking the build system to remove the telemetry driver).

Even having to add a new driver subsection only for telemetry is awkward.

So we might certainly wait for second or third opinions, but I am firmly
convinced it would be easier in order to maintain the project (both from EAL
and systems standpoint and library standpoint) without the vdev trick.

> 
> > I only see afterward the selftest being triggered via kvargs. I haven't
> > yet looked at the testing done, but if it is only unit test, the "test"
> > app would be better suited. If it is integration testing to verify the
> > behavior of the library with other PMDs, you probably need specific
> > context, thus selftest being insufficient on its own and useless for
> > other users.
> 
> Correct, self tests are triggered by kvargs. This same model is used
> in eg: eventdev PMDs to run selftests, where the tests are pretty complex
> and specific to the device under test.
> 
> Again, I don't have a strong opinion but I don't see any issue with it
> being included in the vdev / telemetry library. We could write a shim
> test that the "official" test binary runs the telemetry tests if that is
> your concern?
> 
> 

Okay, I have no strong opinion about this (actually I prefer having the
test code close to the code-under-test), but eventdev can spawn device
objects to drive the test and provide configuration.

It would be more complicated using the same logic with a pure library,
without the vdev.

> > > Control threads are used to get CPU cycles for telemetry, which
> > > are configured in this patch also.
> > >
> > > Signed-off-by: Ciara Power <ciara.power@intel.com>
> > > Signed-off-by: Brian Archbold <brian.archbold@intel.com>
> > 
> > Regards,
> > --
> > Gaëtan Rivet
> > 6WIND
> 
> Thanks for review, and there's a lightning talk at Userspace so please
> do provide input there too :) -Harry

Patch
diff mbox series

diff --git a/config/common_base b/config/common_base
index 4bcbaf9..682f8bf 100644
--- a/config/common_base
+++ b/config/common_base
@@ -716,6 +716,11 @@  CONFIG_RTE_LIBRTE_HASH=y
 CONFIG_RTE_LIBRTE_HASH_DEBUG=n
 
 #
+# Compile librte_telemetry
+#
+CONFIG_RTE_LIBRTE_TELEMETRY=y
+
+#
 # Compile librte_efd
 #
 CONFIG_RTE_LIBRTE_EFD=y
diff --git a/drivers/Makefile b/drivers/Makefile
index 7566076..2238290 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -21,5 +21,8 @@  DIRS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += event
 DEPDIRS-event := common bus mempool net
 DIRS-$(CONFIG_RTE_LIBRTE_RAWDEV) += raw
 DEPDIRS-raw := common bus mempool net event
+DIRS-$(CONFIG_RTE_LIBRTE_TELEMETRY) += telemetry
+DEPDIRS-telemetry := common bus mempool
+
 
 include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/drivers/meson.build b/drivers/meson.build
index f94e2fe..d38f22f 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -9,7 +9,8 @@  driver_classes = ['common',
 	       'crypto',  # depends on common, bus and mempool (net in future).
 	       'compress', # depends on common, bus, mempool.
 	       'event',   # depends on common, bus, mempool and net.
-	       'raw']     # depends on common, bus, mempool, net and event.
+	       'raw',     # depends on common, bus, mempool, net and event.
+	       'telemetry']   # depends on common, bus, and mempool.
 
 default_cflags = machine_args
 if cc.has_argument('-Wno-format-truncation')
diff --git a/drivers/telemetry/Makefile b/drivers/telemetry/Makefile
new file mode 100644
index 0000000..86b0342
--- /dev/null
+++ b/drivers/telemetry/Makefile
@@ -0,0 +1,8 @@ 
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Intel Corporation
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+DIRS-$(CONFIG_RTE_LIBRTE_TELEMETRY) += telemetry
+
+include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/drivers/telemetry/meson.build b/drivers/telemetry/meson.build
new file mode 100644
index 0000000..b7eb791
--- /dev/null
+++ b/drivers/telemetry/meson.build
@@ -0,0 +1,6 @@ 
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Intel Corporation
+
+drivers = ['telemetry']
+config_flag_fmt = 'RTE_LIBRTE_@0@_PMD'
+driver_name_fmt = 'rte_pmd_@0@'
diff --git a/drivers/telemetry/telemetry/Makefile b/drivers/telemetry/telemetry/Makefile
new file mode 100644
index 0000000..e9d98f1
--- /dev/null
+++ b/drivers/telemetry/telemetry/Makefile
@@ -0,0 +1,26 @@ 
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Intel Corporation
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+# library name
+LIB = librte_pmd_telemetry.a
+
+# build flags
+CFLAGS += -O3
+CFLAGS += $(WERROR_FLAGS)
+
+LDLIBS += -lrte_eal -lrte_mbuf
+LDLIBS += -lrte_telemetry
+LDLIBS += -lrte_bus_vdev
+
+# versioning export map
+EXPORT_MAP := rte_pmd_telemetry_version.map
+
+# library version
+LIBABIVER := 1
+
+# library source files
+SRCS-$(CONFIG_RTE_LIBRTE_TELEMETRY) := telemetry_driver.c
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/telemetry/telemetry/meson.build b/drivers/telemetry/telemetry/meson.build
new file mode 100644
index 0000000..baebcab
--- /dev/null
+++ b/drivers/telemetry/telemetry/meson.build
@@ -0,0 +1,5 @@ 
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Intel Corporation
+
+sources = files('telemetry_driver.c')
+deps += ['telemetry', 'bus_pci', 'bus_vdev']
diff --git a/drivers/telemetry/telemetry/rte_pmd_telemetry_version.map b/drivers/telemetry/telemetry/rte_pmd_telemetry_version.map
new file mode 100644
index 0000000..a73e0f2
--- /dev/null
+++ b/drivers/telemetry/telemetry/rte_pmd_telemetry_version.map
@@ -0,0 +1,9 @@ 
+DPDK_18.05 {
+	global:
+
+	telemetry_probe;
+	telemetry_remove;
+
+	local: *;
+
+};
diff --git a/drivers/telemetry/telemetry/telemetry_driver.c b/drivers/telemetry/telemetry/telemetry_driver.c
new file mode 100644
index 0000000..c56f60c
--- /dev/null
+++ b/drivers/telemetry/telemetry/telemetry_driver.c
@@ -0,0 +1,40 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#include <stdio.h>
+
+#include <rte_telemetry.h>
+#include <rte_malloc.h>
+#include <rte_bus_vdev.h>
+#include <rte_lcore.h>
+
+static int
+telemetry_probe(struct rte_vdev_device *vdev)
+{
+	int ret;
+
+	RTE_SET_USED(vdev);
+	ret = rte_telemetry_init(rte_socket_id());
+	if (ret < 0) {
+		printf("Error - Telemetry initialisation failed\n");
+		return -1;
+	}
+	return 0;
+}
+
+static int
+telemetry_remove(struct rte_vdev_device *vdev)
+{
+	const char *name;
+	name = rte_vdev_device_name(vdev);
+	printf("Uninitialising the device: %s\n", name);
+	return 0;
+}
+
+static struct rte_vdev_driver pmd_telemetry_drv = {
+	.probe = telemetry_probe,
+	.remove = telemetry_remove
+};
+
+RTE_PMD_REGISTER_VDEV(telemetry, pmd_telemetry_drv);
diff --git a/lib/Makefile b/lib/Makefile
index afa604e..8cbd035 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -105,6 +105,8 @@  DEPDIRS-librte_gso := librte_eal librte_mbuf librte_ethdev librte_net
 DEPDIRS-librte_gso += librte_mempool
 DIRS-$(CONFIG_RTE_LIBRTE_BPF) += librte_bpf
 DEPDIRS-librte_bpf := librte_eal librte_mempool librte_mbuf librte_ethdev
+DIRS-$(CONFIG_RTE_LIBRTE_TELEMETRY) += librte_telemetry
+DEPDIRS-librte_telemetry := librte_eal librte_metrics librte_ethdev
 
 ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
 DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
diff --git a/lib/librte_telemetry/Makefile b/lib/librte_telemetry/Makefile
new file mode 100644
index 0000000..bda3788
--- /dev/null
+++ b/lib/librte_telemetry/Makefile
@@ -0,0 +1,26 @@ 
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Intel Corporation
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+# library name
+LIB = librte_telemetry.a
+
+CFLAGS += -O3
+CFLAGS += -I$(SRCDIR)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
+LDLIBS += -lrte_eal -lrte_ethdev
+LDLIBS += -lrte_metrics
+
+EXPORT_MAP := rte_telemetry_version.map
+
+LIBABIVER := 1
+
+# library source files
+SRCS-$(CONFIG_RTE_LIBRTE_TELEMETRY) := rte_telemetry.c
+
+# export include files
+SYMLINK-$(CONFIG_RTE_LIBRTE_TELEMETRY)-include := rte_telemetry.h
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_telemetry/meson.build b/lib/librte_telemetry/meson.build
new file mode 100644
index 0000000..7716076
--- /dev/null
+++ b/lib/librte_telemetry/meson.build
@@ -0,0 +1,7 @@ 
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Intel Corporation
+
+sources = files('rte_telemetry.c')
+headers = files('rte_telemetry.h', 'rte_telemetry_internal.h')
+deps += ['metrics', 'ethdev']
+cflags += '-DALLOW_EXPERIMENTAL_API'
diff --git a/lib/librte_telemetry/rte_telemetry.c b/lib/librte_telemetry/rte_telemetry.c
new file mode 100644
index 0000000..8d7b0e3
--- /dev/null
+++ b/lib/librte_telemetry/rte_telemetry.c
@@ -0,0 +1,108 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#include <unistd.h>
+
+#include <rte_eal.h>
+#include <rte_ethdev.h>
+#include <rte_metrics.h>
+
+#include "rte_telemetry.h"
+#include "rte_telemetry_internal.h"
+
+#define SLEEP_TIME 10
+
+static telemetry_impl *static_telemetry;
+
+static int32_t
+rte_telemetry_run(void *userdata)
+{
+	struct telemetry_impl *telemetry = (struct telemetry_impl *)userdata;
+	if (!telemetry) {
+		TELEMETRY_LOG_WARN("Warning - TELEMETRY could not be "
+			"initialised\n");
+		return -1;
+	}
+
+	return 0;
+}
+
+static void
+*rte_telemetry_run_thread_func(void *userdata)
+{
+	int ret;
+	struct telemetry_impl *telemetry = (struct telemetry_impl *)userdata;
+
+	if (!telemetry) {
+		TELEMETRY_LOG_ERR("Error - %s passed a NULL instance\n",
+			__func__);
+		pthread_exit(0);
+	}
+
+	while (telemetry->thread_status) {
+		rte_telemetry_run(telemetry);
+		ret = usleep(SLEEP_TIME);
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - Calling thread could not be"
+				" put to sleep\n");
+	}
+	pthread_exit(0);
+}
+
+int32_t
+rte_telemetry_init(uint32_t socket_id)
+{
+	int ret;
+	pthread_attr_t attr;
+	const char *telemetry_ctrl_thread = "telemetry";
+
+	if (static_telemetry) {
+		TELEMETRY_LOG_WARN("Warning - TELEMETRY structure already "
+			"initialised\n");
+		return -EALREADY;
+	}
+
+	static_telemetry = calloc(1, sizeof(struct telemetry_impl));
+	if (!static_telemetry) {
+		TELEMETRY_LOG_ERR("Error - Memory could not be allocated\n");
+		return -ENOMEM;
+	}
+
+	static_telemetry->socket_id = socket_id;
+	rte_metrics_init(static_telemetry->socket_id);
+	pthread_attr_init(&attr);
+	ret = rte_ctrl_thread_create(&static_telemetry->thread_id,
+		telemetry_ctrl_thread, &attr, rte_telemetry_run_thread_func,
+		(void *)static_telemetry);
+	static_telemetry->thread_status = 1;
+	if (ret < 0) {
+		ret = rte_telemetry_cleanup();
+		if (ret < 0)
+			TELEMETRY_LOG_ERR("Error - TELEMETRY cleanup failed\n");
+		return -EPERM;
+	}
+	return 0;
+}
+
+int32_t
+rte_telemetry_cleanup(void)
+{
+	struct telemetry_impl *telemetry = static_telemetry;
+	telemetry->thread_status = 0;
+	pthread_join(telemetry->thread_id, NULL);
+	free(telemetry);
+	static_telemetry = NULL;
+	return 0;
+}
+
+int telemetry_log_level;
+RTE_INIT(rte_telemetry_log_init);
+
+static void
+rte_telemetry_log_init(void)
+{
+	telemetry_log_level = rte_log_register("lib.telemetry");
+	if (telemetry_log_level >= 0)
+		rte_log_set_level(telemetry_log_level, RTE_LOG_ERR);
+}
diff --git a/lib/librte_telemetry/rte_telemetry.h b/lib/librte_telemetry/rte_telemetry.h
new file mode 100644
index 0000000..b691845
--- /dev/null
+++ b/lib/librte_telemetry/rte_telemetry.h
@@ -0,0 +1,40 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#include <stdint.h>
+
+#ifndef _RTE_TELEMETRY_H_
+#define _RTE_TELEMETRY_H_
+
+/**
+ * Get the telemetry_impl structure device pointer initialised.
+ *
+ * @param socket_id
+ *  Unsigned integer representing the socket id to be used
+ *  for the telemetry structure.
+ *
+ * @return
+ *  0 on successful initialisation.
+ * @return
+ *  -ENOMEM on memory allocation error
+ * @return
+ *  -EPERM on unknown error failure
+ * @return
+ *  -EALREADY if Telemetry is already initialised.
+ */
+int32_t
+rte_telemetry_init(uint32_t socket_id);
+
+/**
+ * Clean up and free memory.
+ *
+ * @return
+ *  0 on success
+ * @return
+ *  -EPERM on failure
+ */
+int32_t
+rte_telemetry_cleanup(void);
+
+#endif
diff --git a/lib/librte_telemetry/rte_telemetry_internal.h b/lib/librte_telemetry/rte_telemetry_internal.h
new file mode 100644
index 0000000..4e810a8
--- /dev/null
+++ b/lib/librte_telemetry/rte_telemetry_internal.h
@@ -0,0 +1,32 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#include <rte_log.h>
+
+#ifndef _RTE_TELEMETRY_INTERNAL_H_
+#define _RTE_TELEMETRY_INTERNAL_H_
+
+/* Logging Macros */
+extern int telemetry_log_level;
+
+#define TELEMETRY_LOG(level, fmt, args...) \
+	rte_log(RTE_LOG_ ##level, telemetry_log_level, "%s(): "fmt "\n", \
+		__func__, ##args)
+
+#define TELEMETRY_LOG_ERR(fmt, args...) \
+	TELEMETRY_LOG(ERR, fmt, ## args)
+
+#define TELEMETRY_LOG_WARN(fmt, args...) \
+	TELEMETRY_LOG(WARNING, fmt, ## args)
+
+#define TELEMETRY_LOG_INFO(fmt, args...) \
+	TELEMETRY_LOG(INFO, fmt, ## args)
+
+typedef struct telemetry_impl {
+	pthread_t thread_id;
+	int thread_status;
+	uint32_t socket_id;
+} telemetry_impl;
+
+#endif
diff --git a/lib/librte_telemetry/rte_telemetry_version.map b/lib/librte_telemetry/rte_telemetry_version.map
new file mode 100644
index 0000000..efd437d
--- /dev/null
+++ b/lib/librte_telemetry/rte_telemetry_version.map
@@ -0,0 +1,6 @@ 
+DPDK_18.05 {
+	global:
+
+	rte_telemetry_init;
+	local: *;
+};
diff --git a/lib/meson.build b/lib/meson.build
index eb91f10..fc84b2f 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -24,7 +24,7 @@  libraries = [ 'compat', # just a header, used for versioning
 	# add pkt framework libs which use other libs from above
 	'port', 'table', 'pipeline',
 	# flow_classify lib depends on pkt framework table lib
-	'flow_classify', 'bpf']
+	'flow_classify', 'bpf', 'telemetry']
 
 default_cflags = machine_args
 if cc.has_argument('-Wno-format-truncation')
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index de33883..8551191 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -80,6 +80,7 @@  _LDLIBS-$(CONFIG_RTE_LIBRTE_SECURITY)       += -lrte_security
 _LDLIBS-$(CONFIG_RTE_LIBRTE_COMPRESSDEV)    += -lrte_compressdev
 _LDLIBS-$(CONFIG_RTE_LIBRTE_EVENTDEV)       += -lrte_eventdev
 _LDLIBS-$(CONFIG_RTE_LIBRTE_RAWDEV)         += -lrte_rawdev
+_LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += -lrte_metrics -lrte_telemetry
 _LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER)          += -lrte_timer
 _LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL)        += -lrte_mempool
 _LDLIBS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING)   += -lrte_mempool_ring
@@ -164,6 +165,7 @@  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SOFTNIC)      += -lrte_pmd_softnic
 endif
 _LDLIBS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD)    += -lrte_pmd_sfc_efx
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SZEDATA2)   += -lrte_pmd_szedata2 -lsze2
+_LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += -lrte_pmd_telemetry
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_TAP)        += -lrte_pmd_tap
 _LDLIBS-$(CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD) += -lrte_pmd_thunderx_nicvf
 _LDLIBS-$(CONFIG_RTE_LIBRTE_VDEV_NETVSC_PMD) += -lrte_pmd_vdev_netvsc