From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8CD46A00BE; Fri, 12 Jun 2020 15:10:28 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 56DA31BE88; Fri, 12 Jun 2020 15:10:28 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 544314F9C for ; Fri, 12 Jun 2020 15:10:26 +0200 (CEST) IronPort-SDR: GuxEb2iDjsIODMNmOEdLyyldYYYbVl9SSqRJM4oOQtG7yeA53JZ4awyFWfPIXjpEga47VVTU7o yTvmQaS2Rxfg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Jun 2020 06:10:25 -0700 IronPort-SDR: QEzp/+WycYyq/y/s5pBhZTUbwphqDox2qHczp6mz1cCKShimqKrj24hCKDSIL381zJ+ph1+TZm wqmRkj8ckiJw== X-IronPort-AV: E=Sophos;i="5.73,503,1583222400"; d="scan'208";a="474205603" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.13.196]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 12 Jun 2020 06:10:23 -0700 Date: Fri, 12 Jun 2020 14:10:20 +0100 From: Bruce Richardson To: Ciara Power Cc: kevin.laatz@intel.com, thomas@monjalon.net, ferruh.yigit@intel.com, arybchenko@solarflare.com, keith.wiles@intel.com, dev@dpdk.org Message-ID: <20200612131020.GC1607@bricha3-MOBL.ger.corp.intel.com> References: <20200612105344.15383-1-ciara.power@intel.com> <20200612105344.15383-3-ciara.power@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200612105344.15383-3-ciara.power@intel.com> Subject: Re: [dpdk-dev] [RFC 2/2] ethdev: add basic stats for telemetry X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Fri, Jun 12, 2020 at 11:53:44AM +0100, Ciara Power wrote: > The ethdev library now registers a telemetry command for basic ethdev > statistics. > > An example usage is shown below: > > Connecting to /var/run/dpdk/rte/dpdk_telemetry.v2 > {"version": "DPDK 20.05.0-rc3", "pid": 14119, "max_output_len": 16384} > --> /ethdev/stats,0 > {"/ethdev/stats": {"ipackets": 0, "opackets": 0, "ibytes": 0, "obytes": \ > 0, "imissed": 0, "ierrors": 0, "oerrors": 0, "rx_nombuf": 0, \ > "q_ipackets": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], \ > "q_opackets": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], \ > "q_ibytes": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], \ > "q_obytes": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], \ > "q_errors": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]}} > > Signed-off-by: Ciara Power > --- > lib/librte_ethdev/rte_ethdev.c | 54 ++++++++++++++++++++++++++++++++++ > 1 file changed, 54 insertions(+) > > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c > index 8e10a6fc3..1a33e41d3 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -5215,6 +5215,58 @@ handle_port_list(const char *cmd __rte_unused, > return 0; > } > > +static void > +add_port_queue_stats(struct rte_tel_data *d, uint64_t *q_stats, > + const char *stat_name) > +{ > + int q; > + struct rte_tel_data *q_data = malloc(rte_tel_get_data_size()); > + rte_tel_data_start_array(q_data, RTE_TEL_U64_VAL); > + for (q = 0; q < RTE_ETHDEV_QUEUE_STAT_CNTRS; q++) > + rte_tel_data_add_array_u64(q_data, q_stats[q]); > + rte_tel_data_add_dict_data(d, stat_name, q_data); > +} It might be worthwhile adding an function to create an array based off existing numbers. Save a function call per element. > + > +static int > +handle_port_stats(const char *cmd __rte_unused, > + const char *params, > + struct rte_tel_data *d) > +{ > + struct rte_eth_stats stats; > + int port_id, ret; > + > +#define ADD_DICT_STAT(s) rte_tel_data_add_dict_u64(d, #s, stats.s) I think the macro would be better defined just above the function, rather than inside it. Perhaps just after the opening brace might also work, but I think it looks wrong being in the middle of the code itself. > + > + 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_stats_get(port_id, &stats); > + if (ret < 0) > + return -1; > + > + rte_tel_data_start_dict(d); > + ADD_DICT_STAT(ipackets); > + ADD_DICT_STAT(opackets); > + ADD_DICT_STAT(ibytes); > + ADD_DICT_STAT(obytes); > + ADD_DICT_STAT(imissed); > + ADD_DICT_STAT(ierrors); > + ADD_DICT_STAT(oerrors); > + ADD_DICT_STAT(rx_nombuf); > + > + add_port_queue_stats(d, stats.q_ipackets, "q_ipackets"); > + add_port_queue_stats(d, stats.q_opackets, "q_opackets"); > + add_port_queue_stats(d, stats.q_ibytes, "q_ibytes"); > + add_port_queue_stats(d, stats.q_obytes, "q_obytes"); > + add_port_queue_stats(d, stats.q_errors, "q_errors"); > + > + return 0; > +} > + > static int > handle_port_xstats(const char *cmd __rte_unused, > const char *params, > @@ -5302,6 +5354,8 @@ RTE_INIT(ethdev_init_log) > rte_log_set_level(rte_eth_dev_logtype, RTE_LOG_INFO); > rte_telemetry_register_cmd("/ethdev/list", handle_port_list, > "Returns list of available ethdev ports. Takes no parameters"); > + rte_telemetry_register_cmd("/ethdev/stats", handle_port_stats, > + "Returns the basic stats for a port. Parameters: int port_id"); > rte_telemetry_register_cmd("/ethdev/xstats", handle_port_xstats, > "Returns the extended stats for a port. Parameters: int port_id"); > rte_telemetry_register_cmd("/ethdev/link_status", > -- > 2.17.1 >