[v2] dmadev: add telemetry support

  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}}
--> /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>
---
V2:
* add device capabilities to info command
* no requirement to pass vchan id
  if the device only has one vchan
* minor code cleanup
---
 doc/guides/prog_guide/dmadev.rst       |  27 ++++++
 doc/guides/rel_notes/release_22_07.rst |   4 +
 lib/dmadev/meson.build                 |   2 +
 lib/dmadev/rte_dmadev.c                | 127 +++++++++++++++++++++++++
 4 files changed, 160 insertions(+)
  

On Thu, Mar 31, 2022 at 06:39:46PM +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}}

This needs an update now that more info is being output. It probably all
does not need to be displayed here, so adding "..." may be enough if you
like.

> --> /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>
> ---
> V2:
> * add device capabilities to info command
> * no requirement to pass vchan id
>   if the device only has one vchan
> * minor code cleanup
> ---
>  doc/guides/prog_guide/dmadev.rst       |  27 ++++++
>  doc/guides/rel_notes/release_22_07.rst |   4 +
>  lib/dmadev/meson.build                 |   2 +
>  lib/dmadev/rte_dmadev.c                | 127 +++++++++++++++++++++++++
>  4 files changed, 160 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/dmadev.rst b/doc/guides/prog_guide/dmadev.rst
> index 77863f8028..a2e4617091 100644
> --- a/doc/guides/prog_guide/dmadev.rst
> +++ b/doc/guides/prog_guide/dmadev.rst
> @@ -118,3 +118,30 @@ i.e. ``rte_dma_stats_get()``. The statistics returned for each device instance a
>  * ``submitted``: The number of operations submitted to the device.
>  * ``completed``: The number of operations which have completed (successful and failed).
>  * ``errors``: The number of operations that completed with error.
> +
> +The dmadev library has support for displaying DMA device information
> +through the Telemetry interface. Telemetry commands that can be used
> +are shown below.
> +
> +#. Get the list of available DMA devices by ID::
> +
> +     --> /dmadev/list
> +     {"/dmadev/list": [0, 1]}
> +
> +#. Get general information from a DMA device by passing the device id as a parameter::
> +
> +     --> /dmadev/info,0
> +     {"/dmadev/info": {"name": "0000:00:01.0", "nb_vchans": 1, "numa_node": 0}}
> +
As above

> +#. Get the statistics for a particular DMA device and virtual DMA channel by passing the device id and vchan id as parameters::
> +
> +     --> /dmadev/stats,0,0
> +     {"/dmadev/stats": {"submitted": 0, "completed": 0, "errors": 0}}
> +
> +#. If a DMA device only has one virtual DMA channel you only need to pass the device id to get its statistics::
> +
> +     --> /dmadev/stats,0
> +     {"/dmadev/stats": {"submitted": 0, "completed": 0, "errors": 0}}
> +

Since this is only a convenience, I think you can omit the example for it,
and just include the info about it in brackets at the end of the bullet
point above - it doesn't need its own bullet.

> +For more information on how to use the Telemetry interface, see
> +the :doc:`../howto/telemetry`.
> diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
> index 42a5f2d990..5a236b45ae 100644
> --- a/doc/guides/rel_notes/release_22_07.rst
> +++ b/doc/guides/rel_notes/release_22_07.rst
> @@ -55,6 +55,10 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =======================================================
>  
> +* **Added telemetry callbacks to dmadev library.**
> +
> +  Added telemetry callback functions which allow for a list of DMA devices,
> +  stats for a DMA device, and other DMA device information to be queried.
>  

For correct formatting of the doc, I think you need to add a new blank line
here, since sections end with two empty lines.

>  Removed Items
>  -------------
> diff --git a/lib/dmadev/meson.build b/lib/dmadev/meson.build
> index d2fc85e8c7..2f17587b75 100644
> --- a/lib/dmadev/meson.build
> +++ b/lib/dmadev/meson.build
> @@ -5,3 +5,5 @@ sources = files('rte_dmadev.c')
>  headers = files('rte_dmadev.h')
>  indirect_headers += files('rte_dmadev_core.h')
>  driver_sdk_headers += files('rte_dmadev_pmd.h')
> +
> +deps += ['telemetry']
> diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c
> index d4b32b2971..6cd6a8db46 100644
> --- a/lib/dmadev/rte_dmadev.c
> +++ b/lib/dmadev/rte_dmadev.c
> @@ -11,6 +11,7 @@
>  #include <rte_malloc.h>
>  #include <rte_memzone.h>
>  #include <rte_string_fns.h>
> +#include <rte_telemetry.h>
>  
>  #include "rte_dmadev.h"
>  #include "rte_dmadev_pmd.h"
> @@ -864,3 +865,129 @@ dma_fp_object_dummy(struct rte_dma_fp_object *obj)
>  	obj->completed_status = dummy_completed_status;
>  	obj->burst_capacity   = dummy_burst_capacity;
>  }
> +
> +static int
> +dmadev_handle_dev_list(const char *cmd __rte_unused,
> +		const char *params __rte_unused,
> +		struct rte_tel_data *d)
> +{
> +	int dev_id;
> +
> +	rte_tel_data_start_array(d, RTE_TEL_INT_VAL);
> +	for (dev_id = 0; dev_id < dma_devices_max; dev_id++)
> +		if (rte_dma_is_valid(dev_id))
> +			rte_tel_data_add_array_int(d, dev_id);
> +
> +	return 0;
> +}
> +
> +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;
> +	int dev_id, ret;
> +	char *end_param;
> +
> +	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
> +		return -EINVAL;
> +
> +	dev_id = strtoul(params, &end_param, 0);
> +	if (*end_param != '\0')
> +		RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
> +
> +	ret = rte_dma_info_get(dev_id, &dma_info);

For clarity, I think it might be worthwhile adding a one-line comment here
that info_get does parameter checking of dev_id so we don't need to do so.
Same comment would also apply to stats function below.

> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	rte_tel_data_start_dict(d);
> +	rte_tel_data_add_dict_string(d, "name", dma_info.dev_name);
> +	rte_tel_data_add_dict_int(d, "nb_vchans", dma_info.nb_vchans);
> +	rte_tel_data_add_dict_int(d, "numa_node", dma_info.numa_node);
> +	rte_tel_data_add_dict_int(d, "max_vchans", dma_info.max_vchans);
> +	rte_tel_data_add_dict_int(d, "max_desc", dma_info.max_desc);
> +	rte_tel_data_add_dict_int(d, "min_desc", dma_info.min_desc);
> +	rte_tel_data_add_dict_int(d, "max_sges", dma_info.max_sges);
> +
> +	dma_caps = rte_tel_data_alloc();
> +	if (!dma_caps)
> +		return -ENOMEM;
> +
> +	rte_tel_data_start_dict(dma_caps);
> +	rte_tel_data_add_dict_int(dma_caps, "fill", !!(dma_info.dev_capa & RTE_DMA_CAPA_OPS_FILL));
> +	rte_tel_data_add_dict_int(dma_caps, "sva", !!(dma_info.dev_capa & RTE_DMA_CAPA_SVA));
> +	rte_tel_data_add_dict_int(dma_caps, "silent", !!(dma_info.dev_capa & RTE_DMA_CAPA_SILENT));
> +	rte_tel_data_add_dict_int(dma_caps, "copy", !!(dma_info.dev_capa & RTE_DMA_CAPA_OPS_COPY));
> +	rte_tel_data_add_dict_int(dma_caps, "mem2mem",
> +					!!(dma_info.dev_capa & RTE_DMA_CAPA_MEM_TO_MEM));

Two ideas here in case you wan to consider them for shortening these lines:
* assign "dma_info.dev_capa" to a local var with a short name
* define a macro for adding the capabilities based on the name, something
  like (completely untested!):

#define ADD_CAPA (td, c, name) \
	rte_tel_data_add_dict_int(td, #name, !!(RTE_DMA_CAPA_ ## name & c))

...
ADD_CAPA(dma_caps, dev_capa, MEM_TO_MEM);
ADD_CAPA(dma_caps, dev_capa, MEM_TO_DEV);

The small downside of the macro scheme is that you'd have to do extra work
to avoid having the capabilities in caps, but it does guarantee consistency
of naming.

> +	rte_tel_data_add_dict_int(dma_caps, "mem2dev",
> +					!!(dma_info.dev_capa & RTE_DMA_CAPA_MEM_TO_DEV));
> +	rte_tel_data_add_dict_int(dma_caps, "dev2mem",
> +					!!(dma_info.dev_capa & RTE_DMA_CAPA_DEV_TO_MEM));
> +	rte_tel_data_add_dict_int(dma_caps, "dev2dev",
> +					!!(dma_info.dev_capa & RTE_DMA_CAPA_DEV_TO_DEV));
> +	rte_tel_data_add_dict_int(dma_caps, "copy_sg",
> +					!!(dma_info.dev_capa & RTE_DMA_CAPA_OPS_COPY_SG));
> +	rte_tel_data_add_dict_int(dma_caps, "handles_errors",
> +					!!(dma_info.dev_capa & RTE_DMA_CAPA_HANDLES_ERRORS));
> +	rte_tel_data_add_dict_container(d, "capabilities", dma_caps, 0);
> +
> +	return 0;
> +}
> +
> +#define ADD_DICT_STAT(s) rte_tel_data_add_dict_u64(d, #s, dma_stats.s)
> +
> +static int
> +dmadev_handle_dev_stats(const char *cmd __rte_unused,
> +		const char *params,
> +		struct rte_tel_data *d)
> +{
> +	struct rte_dma_info dma_info;
> +	struct rte_dma_stats dma_stats;
> +	int dev_id, vchan_id, ret;
> +	char *end_param;
> +	const char *vchan_param;
> +
> +	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
> +		return -EINVAL;
> +
> +	dev_id = strtoul(params, &end_param, 0);
> +
> +	ret = rte_dma_info_get(dev_id, &dma_info);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	if (dma_info.nb_vchans == 1 && *end_param == '\0') {

Needs a comment explaining what is happening.
Also for single-line blocks in DPDK, we allow omitting the braces (even if
the else leg needs them).

If you want to remove this block, you can also just set vchan_id = 0 when
defining it, and then change the else to be and if with the inverse of the
condition above

> +		vchan_id = 0;
> +	} else {
> +		vchan_param = strtok(end_param, ",");
> +		if (!vchan_param || strlen(vchan_param) == 0 || !isdigit(*vchan_param))
> +			return -EINVAL;
> +
> +		vchan_id = strtoul(vchan_param, &end_param, 0);
> +	}
> +	if (*end_param != '\0')
> +		RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
> +
> +	ret = rte_dma_stats_get(dev_id, vchan_id, &dma_stats);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	rte_tel_data_start_dict(d);
> +	ADD_DICT_STAT(submitted);
> +	ADD_DICT_STAT(completed);
> +	ADD_DICT_STAT(errors);
> +
> +	return 0;
> +}
> +
> +RTE_INIT(dmadev_init_telemetry)
> +{
> +	rte_telemetry_register_cmd("/dmadev/list", dmadev_handle_dev_list,
> +			"Returns list of available dmadev devices by IDs. No parameters.");
> +	rte_telemetry_register_cmd("/dmadev/info", dmadev_handle_dev_info,
> +			"Returns information for a dmadev. Parameters: int dev_id");
> +	rte_telemetry_register_cmd("/dmadev/stats", dmadev_handle_dev_stats,
> +			"Returns the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (If only one vchannel, vchan_id param is optional)");

Can shorten text in brackets to "(Optional if only one vchannel)", since it
immediately follows the parameter.

Regards,
/Bruce