Re: [dpdk-dev] [PATCH v2 08/16] ethdev: add callback support for telemetry

["Wiles, Keith" <keith.wiles@intel.com>]

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