On 01/04/2022 12:00, Walsh, Conor wrote:
From: Bruce Richardson <bruce.richard...@intel.com>
Sent: Friday 1 April 2022 11:50
To: Morrissey, Sean <sean.morris...@intel.com>
Cc: Chengwen Feng <fengcheng...@huawei.com>; Laatz, Kevin
<kevin.la...@intel.com>; dev@dpdk.org; Pai G, Sunil
<sunil.pa...@intel.com>
Subject: Re: [PATCH v3] dmadev: add telemetry support
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.morris...@intel.com>
Tested-by: Sunil Pai G <sunil.pa...@intel.com>
Reviewed-by: Bruce Richardson <bruce.richard...@intel.com>
Hi Sean,
I'd agree with Bruce's comment below about trying to keep the names the same.
Looks good to me though and I've tested it with IOAT and dmafwd.
Thanks,
Reviewed-by: Conor Walsh <conor.wa...@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.
Hi all,
If that is the preferred approach then I will send another version. I
got the lower case
names from the capa_names struct in the dma_capability_name() function
and these
naming conventions are also used in the logs i.e. "Device %d don't
support mem2mem transfer".
For this reason, I thought this was the preferred approach to naming the
capabilities, however
I will keep the names consistent with the flags as suggested.
+ 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;
+}
