diff mbox series

[v2,08/16] ethdev: add callback support for telemetry

Message ID 20200408164956.47864-9-ciara.power@intel.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series update and simplify telemetry library. |

Checks

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

Commit Message

Power, Ciara April 8, 2020, 4:49 p.m. UTC 
  From: Bruce Richardson <bruce.richardson@intel.com>

The ethdev library now registers commands with telemetry, and
implements the callback functions. These commands allow the list of
ethdev ports and the stats and link status for a port to be queried.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Signed-off-by: Ciara Power <ciara.power@intel.com>

---
v2:
  - Renamed stats to xstats for device specific stats.
  - Added link status command for ethdev ports.
---
 lib/librte_ethdev/Makefile     |   4 ++
 lib/librte_ethdev/meson.build  |   4 ++
 lib/librte_ethdev/rte_ethdev.c | 106 +++++++++++++++++++++++++++++++++
 3 files changed, 114 insertions(+)

Comments

Wiles, Keith April 8, 2020, 6:16 p.m. UTC | #1 
> On Apr 8, 2020, at 11:49 AM, Power, Ciara <ciara.power@intel.com> wrote:
> 
> From: Bruce Richardson <bruce.richardson@intel.com>
> 
> The ethdev library now registers commands with telemetry, and
> implements the callback functions. These commands allow the list of
> ethdev ports and the stats and link status for a port to be queried.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> 
> ---
> v2:
>  - Renamed stats to xstats for device specific stats.
>  - Added link status command for ethdev ports.
> ---
> lib/librte_ethdev/Makefile     |   4 ++
> lib/librte_ethdev/meson.build  |   4 ++
> lib/librte_ethdev/rte_ethdev.c | 106 +++++++++++++++++++++++++++++++++
> 3 files changed, 114 insertions(+)
> 
> diff --git a/lib/librte_ethdev/Makefile b/lib/librte_ethdev/Makefile
> index b627e4e23b..4ba9cb8f47 100644
> --- a/lib/librte_ethdev/Makefile
> +++ b/lib/librte_ethdev/Makefile
> @@ -24,6 +24,10 @@ SRCS-y += rte_tm.c
> SRCS-y += rte_mtr.c
> SRCS-y += ethdev_profile.c
> 
> +ifeq ($(CONFIG_RTE_LIBRTE_TELEMETRY),y)
> +LDLIBS += -lrte_telemetry
> +endif
> +
> #
> # Export include files
> #
> diff --git a/lib/librte_ethdev/meson.build b/lib/librte_ethdev/meson.build
> index 3537f22f59..072805cf18 100644
> --- a/lib/librte_ethdev/meson.build
> +++ b/lib/librte_ethdev/meson.build
> @@ -26,3 +26,7 @@ headers = files('rte_ethdev.h',
> 	'rte_tm_driver.h')
> 
> deps += ['net', 'kvargs', 'meter']
> +
> +if dpdk_conf.has('RTE_LIBRTE_TELEMETRY')
> +	deps += ['telemetry']
> +endif
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 0854ef8832..f033d60152 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -38,6 +38,9 @@
> #include <rte_kvargs.h>
> #include <rte_class.h>
> #include <rte_ether.h>
> +#ifdef RTE_LIBRTE_TELEMETRY
> +#include <rte_telemetry.h>
> +#endif
> 
> #include "rte_ethdev.h"
> #include "rte_ethdev_driver.h"
> @@ -5189,9 +5192,112 @@ rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
> 	return result;
> }
> 
> +#ifdef RTE_LIBRTE_TELEMETRY
> +static int
> +handle_port_list(const char *cmd __rte_unused,
> +		const char *params __rte_unused,
> +		char *buffer, int buf_len)
> +{
> +	int port_id, used = 0;
> +
> +	used = rte_tel_json_empty_array(buffer, buf_len, used);
> +	RTE_ETH_FOREACH_DEV(port_id)
> +		used = rte_tel_json_add_array_int(buffer, buf_len, used,
> +				port_id);
> +	return used;
> +}
> +
> +static int
> +handle_port_xstats(const char *cmd __rte_unused,
> +		const char *params,
> +		char *buffer, int buf_len)
> +{
> +	struct rte_eth_xstat *eth_xstats;
> +	struct rte_eth_xstat_name *xstat_names;
> +	int port_id, num_xstats;
> +	int i, ret;
> +	int used = 0;
> +
> +	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
> +		return -1;
> +
> +	port_id = atoi(params);
> +	if (!rte_eth_dev_is_valid_port(port_id))
> +		return -1;
> +
> +	num_xstats = rte_eth_xstats_get(port_id, NULL, 0);
> +	if (num_xstats < 0)
> +		return -1;
> +
> +	/* use one malloc for both names and stats */
> +	eth_xstats = malloc((sizeof(struct rte_eth_xstat) +
> +			sizeof(struct rte_eth_xstat_name)) * num_xstats);
> +	if (eth_xstats == NULL)
> +		return -1;
> +	xstat_names = (void *)&eth_xstats[num_xstats];
> +
> +	ret = rte_eth_xstats_get_names(port_id, xstat_names, num_xstats);
> +	if (ret < 0 || ret > num_xstats) {
> +		free(eth_xstats);
> +		return -1;
> +	}
> +
> +	ret = rte_eth_xstats_get(port_id, eth_xstats, num_xstats);
> +	if (ret < 0 || ret > num_xstats) {
> +		free(eth_xstats);
> +		return -1;
> +	}
> +
> +	used = rte_tel_json_empty_obj(buffer, buf_len, used);
> +	for (i = 0; i < num_xstats; i++)
> +		used = rte_tel_json_add_obj_u64(buffer, buf_len, used,
> +				xstat_names[i].name, eth_xstats[i].value);
> +
> +	return used;
> +}
> +
> +static int
> +handle_port_link_status(const char *cmd __rte_unused,
> +		const char *params,
> +		char *buffer, int buf_len)
> +{
> +	int ret, port_id;
> +	struct rte_eth_link link;
> +
> +	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
> +		return -1;
> +
> +	port_id = atoi(params);
> +	if (!rte_eth_dev_is_valid_port(port_id))
> +		return -1;
> +
> +	ret = rte_eth_link_get(port_id, &link);
> +	if (ret < 0)
> +		return -1;
> +
> +	if (link.link_status)
> +		ret = snprintf(buffer, buf_len,
> +				"{\"status\":\"%s\", \"speed\":%u, \"duplex\":"
> +				"\"%s\"}", link.link_status ? "UP" : "DOWN:",
> +				link.link_speed,
> +				(link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
> +				("full-duplex") : ("half-duplex"));

Adding link status nice. Please remove the spaces in the output string.

> +	else
> +		ret = snprintf(buffer, buf_len, "{\"status\":\"DOWN\"}");
> +
> +	return ret >= buf_len ? -1 : ret;
> +}
> +#endif
> +
> RTE_INIT(ethdev_init_log)
> {
> 	rte_eth_dev_logtype = rte_log_register("lib.ethdev");
> 	if (rte_eth_dev_logtype >= 0)
> 		rte_log_set_level(rte_eth_dev_logtype, RTE_LOG_INFO);
> +#ifdef RTE_LIBRTE_TELEMETRY
> +	rte_telemetry_register_cmd("/ethdev/list", handle_port_list);
> +	rte_telemetry_register_cmd("/ethdev/xstats", handle_port_xstats);
> +	rte_telemetry_register_cmd("/ethdev/link_status",
> +			handle_port_link_status);
> +#endif
> }
> -- 
> 2.17.1
>
Bruce Richardson April 9, 2020, 8:20 a.m. UTC | #2 
On Wed, Apr 08, 2020 at 07:16:10PM +0100, Wiles, Keith wrote:
> 
> 
> > On Apr 8, 2020, at 11:49 AM, Power, Ciara <ciara.power@intel.com> wrote:
> >
> > From: Bruce Richardson <bruce.richardson@intel.com>
> >
> > The ethdev library now registers commands with telemetry, and
> > implements the callback functions. These commands allow the list of
> > ethdev ports and the stats and link status for a port to be queried.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > Signed-off-by: Ciara Power <ciara.power@intel.com>
> >
> > ---
> > v2:
> >  - Renamed stats to xstats for device specific stats.
> >  - Added link status command for ethdev ports.
> > ---
> > lib/librte_ethdev/Makefile     |   4 ++
> > lib/librte_ethdev/meson.build  |   4 ++
> > lib/librte_ethdev/rte_ethdev.c | 106 +++++++++++++++++++++++++++++++++
> > 3 files changed, 114 insertions(+)
> >
<snip>
> +
> > +if (link.link_status)
> > +ret = snprintf(buffer, buf_len,
> > +"{\"status\":\"%s\", \"speed\":%u, \"duplex\":"
> > +"\"%s\"}", link.link_status ? "UP" : "DOWN:",
> > +link.link_speed,
> > +(link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
> > +("full-duplex") : ("half-duplex"));
> 
> Adding link status nice. Please remove the spaces in the output string.
> 
Good catch, thanks.
Morten Brørup April 10, 2020, 9:57 a.m. UTC | #3 
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Thursday, April 9, 2020 10:21 AM
> 
> On Wed, Apr 08, 2020 at 07:16:10PM +0100, Wiles, Keith wrote:
> >
> >
> > > On Apr 8, 2020, at 11:49 AM, Power, Ciara <ciara.power@intel.com>
> wrote:
> > >
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > >
> > > The ethdev library now registers commands with telemetry, and
> > > implements the callback functions. These commands allow the list of
> > > ethdev ports and the stats and link status for a port to be
> queried.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > Signed-off-by: Ciara Power <ciara.power@intel.com>
> > >
> > > ---
> > > v2:
> > >  - Renamed stats to xstats for device specific stats.
> > >  - Added link status command for ethdev ports.
> > > ---
> > > lib/librte_ethdev/Makefile     |   4 ++
> > > lib/librte_ethdev/meson.build  |   4 ++
> > > lib/librte_ethdev/rte_ethdev.c | 106
> +++++++++++++++++++++++++++++++++
> > > 3 files changed, 114 insertions(+)
> > >
> <snip>
> > +
> > > +if (link.link_status)
> > > +ret = snprintf(buffer, buf_len,
> > > +"{\"status\":\"%s\", \"speed\":%u, \"duplex\":"
> > > +"\"%s\"}", link.link_status ? "UP" : "DOWN:",

Also remove the colon after DOWN.

> > > +link.link_speed,
> > > +(link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
> > > +("full-duplex") : ("half-duplex"));
> >
> > Adding link status nice. Please remove the spaces in the output
> string.
> >
> Good catch, thanks.
diff mbox series

Patch

diff --git a/lib/librte_ethdev/Makefile b/lib/librte_ethdev/Makefile
index b627e4e23b..4ba9cb8f47 100644
--- a/lib/librte_ethdev/Makefile
+++ b/lib/librte_ethdev/Makefile
@@ -24,6 +24,10 @@  SRCS-y += rte_tm.c
 SRCS-y += rte_mtr.c
 SRCS-y += ethdev_profile.c
 
+ifeq ($(CONFIG_RTE_LIBRTE_TELEMETRY),y)
+LDLIBS += -lrte_telemetry
+endif
+
 #
 # Export include files
 #
diff --git a/lib/librte_ethdev/meson.build b/lib/librte_ethdev/meson.build
index 3537f22f59..072805cf18 100644
--- a/lib/librte_ethdev/meson.build
+++ b/lib/librte_ethdev/meson.build
@@ -26,3 +26,7 @@  headers = files('rte_ethdev.h',
 	'rte_tm_driver.h')
 
 deps += ['net', 'kvargs', 'meter']
+
+if dpdk_conf.has('RTE_LIBRTE_TELEMETRY')
+	deps += ['telemetry']
+endif
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 0854ef8832..f033d60152 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -38,6 +38,9 @@ 
 #include <rte_kvargs.h>
 #include <rte_class.h>
 #include <rte_ether.h>
+#ifdef RTE_LIBRTE_TELEMETRY
+#include <rte_telemetry.h>
+#endif
 
 #include "rte_ethdev.h"
 #include "rte_ethdev_driver.h"
@@ -5189,9 +5192,112 @@  rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
 	return result;
 }
 
+#ifdef RTE_LIBRTE_TELEMETRY
+static int
+handle_port_list(const char *cmd __rte_unused,
+		const char *params __rte_unused,
+		char *buffer, int buf_len)
+{
+	int port_id, used = 0;
+
+	used = rte_tel_json_empty_array(buffer, buf_len, used);
+	RTE_ETH_FOREACH_DEV(port_id)
+		used = rte_tel_json_add_array_int(buffer, buf_len, used,
+				port_id);
+	return used;
+}
+
+static int
+handle_port_xstats(const char *cmd __rte_unused,
+		const char *params,
+		char *buffer, int buf_len)
+{
+	struct rte_eth_xstat *eth_xstats;
+	struct rte_eth_xstat_name *xstat_names;
+	int port_id, num_xstats;
+	int i, ret;
+	int used = 0;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -1;
+
+	port_id = atoi(params);
+	if (!rte_eth_dev_is_valid_port(port_id))
+		return -1;
+
+	num_xstats = rte_eth_xstats_get(port_id, NULL, 0);
+	if (num_xstats < 0)
+		return -1;
+
+	/* use one malloc for both names and stats */
+	eth_xstats = malloc((sizeof(struct rte_eth_xstat) +
+			sizeof(struct rte_eth_xstat_name)) * num_xstats);
+	if (eth_xstats == NULL)
+		return -1;
+	xstat_names = (void *)&eth_xstats[num_xstats];
+
+	ret = rte_eth_xstats_get_names(port_id, xstat_names, num_xstats);
+	if (ret < 0 || ret > num_xstats) {
+		free(eth_xstats);
+		return -1;
+	}
+
+	ret = rte_eth_xstats_get(port_id, eth_xstats, num_xstats);
+	if (ret < 0 || ret > num_xstats) {
+		free(eth_xstats);
+		return -1;
+	}
+
+	used = rte_tel_json_empty_obj(buffer, buf_len, used);
+	for (i = 0; i < num_xstats; i++)
+		used = rte_tel_json_add_obj_u64(buffer, buf_len, used,
+				xstat_names[i].name, eth_xstats[i].value);
+
+	return used;
+}
+
+static int
+handle_port_link_status(const char *cmd __rte_unused,
+		const char *params,
+		char *buffer, int buf_len)
+{
+	int ret, port_id;
+	struct rte_eth_link link;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -1;
+
+	port_id = atoi(params);
+	if (!rte_eth_dev_is_valid_port(port_id))
+		return -1;
+
+	ret = rte_eth_link_get(port_id, &link);
+	if (ret < 0)
+		return -1;
+
+	if (link.link_status)
+		ret = snprintf(buffer, buf_len,
+				"{\"status\":\"%s\", \"speed\":%u, \"duplex\":"
+				"\"%s\"}", link.link_status ? "UP" : "DOWN:",
+				link.link_speed,
+				(link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
+				("full-duplex") : ("half-duplex"));
+	else
+		ret = snprintf(buffer, buf_len, "{\"status\":\"DOWN\"}");
+
+	return ret >= buf_len ? -1 : ret;
+}
+#endif
+
 RTE_INIT(ethdev_init_log)
 {
 	rte_eth_dev_logtype = rte_log_register("lib.ethdev");
 	if (rte_eth_dev_logtype >= 0)
 		rte_log_set_level(rte_eth_dev_logtype, RTE_LOG_INFO);
+#ifdef RTE_LIBRTE_TELEMETRY
+	rte_telemetry_register_cmd("/ethdev/list", handle_port_list);
+	rte_telemetry_register_cmd("/ethdev/xstats", handle_port_xstats);
+	rte_telemetry_register_cmd("/ethdev/link_status",
+			handle_port_link_status);
+#endif
 }