From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3D115A0508; Tue, 29 Mar 2022 15:10:23 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 15CEB40691; Tue, 29 Mar 2022 15:10:22 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id 2F8A040141 for ; Tue, 29 Mar 2022 15:10:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1648559420; x=1680095420; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=UmOOGmRKsTuY+s451Sc4SRKPQC9La5pY1s9kJZ0nEgI=; b=VmVdCaCJZUCEPTjEg99KWpK5QXghAxDQ3mV8euNQhSYnaOJkcNfkV1Us mvfLAXHhlSxD7hOe9Q0ykWxvi1H8hteSuNcKtYfAYIQ8JKvDTtvxX4Wdz 6MrR9ixCUps6mSjUS48/IpYjEigPqztbEkyOwtMEsQwsCkgp2Vji6eWeb E4BA31sUNizbAqXSZpL68XN7fG6BEBFT1M8s4D6NnCBhxJQ+ppgQhIgGU ZRDxTHJc0NicbIkOvXiUZP2mOlA5apVdGnJfYB5WUfxPdNu0Rnnq2DAh/ jz88Hse7P30vCPFt6/KhZscthrZPEYvE6zu6gSlkaeEFuCuALcQJAcJfX g==; X-IronPort-AV: E=McAfee;i="6200,9189,10300"; a="322430403" X-IronPort-AV: E=Sophos;i="5.90,220,1643702400"; d="scan'208";a="322430403" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Mar 2022 06:10:19 -0700 X-IronPort-AV: E=Sophos;i="5.90,220,1643702400"; d="scan'208";a="521433690" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.12.222]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 29 Mar 2022 06:10:17 -0700 Date: Tue, 29 Mar 2022 14:10:14 +0100 From: Bruce Richardson To: Sean Morrissey Cc: Chengwen Feng , Kevin Laatz , dev@dpdk.org Subject: Re: [PATCH v1] dmadev: add telemetry support Message-ID: References: <20220323164907.548318-1-sean.morrissey@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220323164907.548318-1-sean.morrissey@intel.com> X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, Mar 23, 2022 at 04:49:07PM +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}} > --> /dmadev/stats,0,0 > {"/dmadev/stats": {"submitted": 0, "completed": 0, "errors": 0}} I am wondering if for simplicity we should allow the vchan id to be omitted if only one vchan is present. For automated use by tools it probably won't matter, but for interactive use it might be nice. > > Signed-off-by: Sean Morrissey > --- > doc/guides/prog_guide/dmadev.rst | 24 ++++++ > doc/guides/rel_notes/release_22_07.rst | 4 + > lib/dmadev/meson.build | 2 + > lib/dmadev/rte_dmadev.c | 105 +++++++++++++++++++++++++ > 4 files changed, 135 insertions(+) > > diff --git a/doc/guides/prog_guide/dmadev.rst b/doc/guides/prog_guide/dmadev.rst > index 77863f8028..1ff66dfe2a 100644 > --- a/doc/guides/prog_guide/dmadev.rst > +++ b/doc/guides/prog_guide/dmadev.rst > @@ -118,3 +118,27 @@ 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:: > + Explain here that the device id is passed as parameter. > + --> /dmadev/info,0 > + {"/dmadev/info": {"name": "0000:00:01.0", "nb_vchans": 1, > + "numa_node": 0}} > + > +#. Get the statistics for a particular DMA device and virtual DMA channel:: > + > + --> /dmadev/stats,0,0 Again, explain the parameters. > + {"/dmadev/stats": {"submitted": 0, "completed": 0, > + "errors": 0}} > + > +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. > > 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'] This is actually unnecessary as EAL depends on telemetry so it's an implicit dependency of every other lib. While functionally it doesn't hurt to list it, I'd tend towards not doing so, as in the past I've seen meson configuration runs slow down if we have unnecessary dependencies called out that then need to be deduplicated in python. > diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c > index d4b32b2971..2f068cb9d8 100644 > --- a/lib/dmadev/rte_dmadev.c > +++ b/lib/dmadev/rte_dmadev.c > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > > #include "rte_dmadev.h" > #include "rte_dmadev_pmd.h" > @@ -864,3 +865,107 @@ 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; > + > + if (rte_dma_count_avail() == 0) > + return -EINVAL; > + Rather than error, we should just return an empty list. Just removing this check should do the trick. > + 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; > + 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(ERR, > + "Extra parameters passed to dmadev telemetry command, ignoring"); > + > + if (!rte_dma_is_valid(dev_id)) > + return -EINVAL; > + > + ret = rte_dma_info_get(dev_id, &dma_info); > + 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); > + > + return 0; > +} There is more information returned by info_get function than called out here, so I think the function should be expanded. For the capabilities, for example, I think each one should be called out by name with a 0 or 1 value, so things are nice and clear. > + > +#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_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); > + 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(ERR, > + "Extra parameters passed to dmadev telemetry command, ignoring"); You don't need to split these lines like this. Lines up to 100 chars are allowed, and any strings are allowed to continue on past that, so it's only after a long string you need to split the line. Also, according to our coding guidelines, line continuations should be double-indented or alternatively aligned to the opening brace on the previous line. Check what is done already the file and keep it consistent with that. > + > + if (!rte_dma_is_valid(dev_id)) > + return -EINVAL; > + > + 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."); This can be shrunk to 2 lines I think. > + 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. Parameters: int dev_id, vchan_id"); > +} > -- > 2.25.1 >