Re: [PATCH v3] dmadev: add telemetry support

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Apr 01, 2022 at 10:24:02AM +0000, Sean Morrissey wrote:
> Telemetry commands are now registered through the dmadev library
> for the gathering of DSA stats. The corresponding callback
> functions for listing dmadevs and providing info and stats for a
> specific dmadev are implemented in the dmadev library.
> 
> An example usage can be seen below:
> 
> Connecting to /var/run/dpdk/rte/dpdk_telemetry.v2
> {"version": "DPDK 22.03.0-rc2", "pid": 2956551, "max_output_len": 16384}
> Connected to application: "dpdk-dma"
> --> /
> {"/": ["/", "/dmadev/info", "/dmadev/list", "/dmadev/stats", ...]}
> --> /dmadev/list
> {"/dmadev/list": [0, 1]}
> --> /dmadev/info,0
> {"/dmadev/info": {"name": "0000:00:01.0", "nb_vchans": 1, "numa_node": 0,
> "max_vchans": 1, "max_desc": 4096, "min_desc": 32, "max_sges": 0,
> "capabilities": {"fill": 1, "sva": 0, "silent": 0, ...}}}
> --> /dmadev/stats,0,0
> {"/dmadev/stats": {"submitted": 0, "completed": 0, "errors": 0}}
> 
> Signed-off-by: Sean Morrissey <sean.morrissey@intel.com>
> Tested-by: Sunil Pai G <sunil.pai.g@intel.com>

Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>

One comment inline below, which I'd like feedback from others on.
> ---
> V3:
> * update docs with correct examples
> * code cleanup and added comments
<snip>

> +
> +#define ADD_CAPA(c, s) rte_tel_data_add_dict_int(dma_caps, #c, !!(dev_capa & RTE_DMA_CAPA_ ## s))
> +
> +static int
> +dmadev_handle_dev_info(const char *cmd __rte_unused,
> +		const char *params, struct rte_tel_data *d)
> +{
> +	struct rte_dma_info dma_info;
> +	struct rte_tel_data *dma_caps;
<snip>
> +	dma_caps = rte_tel_data_alloc();
> +	if (!dma_caps)
> +		return -ENOMEM;
> +
> +	rte_tel_data_start_dict(dma_caps);
> +	ADD_CAPA(fill, OPS_FILL);
> +	ADD_CAPA(sva, SVA);
> +	ADD_CAPA(silent, SILENT);
> +	ADD_CAPA(copy, OPS_COPY);
> +	ADD_CAPA(mem2mem, MEM_TO_MEM);

I'm not 100% sure about this approach of having slightly different names
compared to the flags, just to have things in lower-case. Looking to have
some more input here - I'd tend to have the capabilities in upper case to
avoid duplicating parameters, but I'm not massively concerned either way.

> +	ADD_CAPA(mem2dev, MEM_TO_DEV);
> +	ADD_CAPA(dev2mem, DEV_TO_MEM);
> +	ADD_CAPA(dev2dev, DEV_TO_DEV);
> +	ADD_CAPA(copy_sg, OPS_COPY_SG);
> +	ADD_CAPA(handles_errors, HANDLES_ERRORS);
> +	rte_tel_data_add_dict_container(d, "capabilities", dma_caps, 0);
> +
> +	return 0;
> +}