From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rcdn-iport-1.cisco.com (rcdn-iport-1.cisco.com [173.37.86.72]) by dpdk.org (Postfix) with ESMTP id 78F5E5598 for ; Fri, 29 Apr 2016 15:17:38 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=8475; q=dns/txt; s=iport; t=1461935858; x=1463145458; h=from:to:subject:date:message-id:references:in-reply-to: content-transfer-encoding:mime-version; bh=SZXKKMq/ZduFvI1xKTfsoCnTJABH7GZsEX1IARWHKMY=; b=ahUWDPtjm2WU4BMoD4lwPo8ZdCfn/JqnzKiJWtPoOTL78rnhB+bYfhib T2xBx1G5TtYAynfrfGigaIrYLSxJPqxw2704M96bUxfPEnZFTwxjCrF0R FwNa7QK9zgmbnrlqvyf2TJgEPI3IvCmlnRoTTh+xxu9bN1GOEB4oayIMW w=; X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0ANAgDsXSNX/5pdJa1dgziBUAa3XIIPA?= =?us-ascii?q?Q2BdoYPAoEqOBQBAQEBAQEBZSeEQQEBAQMBJxNEBwQCAQgRBAEBHwkHMhQJCAI?= =?us-ascii?q?EARIIiBoIxB0BAQEBAQEBAQEBAQEBAQEBAQEBAQEVhiGETIQahXkFjVaFTIRxA?= =?us-ascii?q?YhyhR2PGI8vAR4BAUKCBRuBS2yHRD5/AQEB?= X-IronPort-AV: E=Sophos;i="5.24,551,1454976000"; d="scan'208";a="102862403" Received: from rcdn-core-3.cisco.com ([173.37.93.154]) by rcdn-iport-1.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Apr 2016 13:17:37 +0000 Received: from XCH-ALN-017.cisco.com (xch-aln-017.cisco.com [173.36.7.27]) by rcdn-core-3.cisco.com (8.14.5/8.14.5) with ESMTP id u3TDHbBZ028320 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Fri, 29 Apr 2016 13:17:37 GMT Received: from xch-rcd-016.cisco.com (173.37.102.26) by XCH-ALN-017.cisco.com (173.36.7.27) with Microsoft SMTP Server (TLS) id 15.0.1104.5; Fri, 29 Apr 2016 08:17:36 -0500 Received: from xch-rcd-016.cisco.com ([173.37.102.26]) by XCH-RCD-016.cisco.com ([173.37.102.26]) with mapi id 15.00.1104.009; Fri, 29 Apr 2016 08:17:36 -0500 From: "David Harton (dharton)" To: Remy Horton , "dev@dpdk.org" , "Thomas Monjalon" Thread-Topic: [dpdk-dev] [RFC PATCH v1 1/3] rte: change xstats to use integer keys Thread-Index: AQHRlyVWz6GVPymz2UGtMOvW7/xN6J+g/qPQ Date: Fri, 29 Apr 2016 13:17:35 +0000 Message-ID: References: <1460731462-21229-1-git-send-email-remy.horton@intel.com> <1460731462-21229-2-git-send-email-remy.horton@intel.com> In-Reply-To: <1460731462-21229-2-git-send-email-remy.horton@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.82.212.42] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC PATCH v1 1/3] rte: change xstats to use integer keys X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Apr 2016 13:17:38 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Remy Horton > Sent: Friday, April 15, 2016 10:44 AM > To: dev@dpdk.org; Thomas Monjalon > Subject: [dpdk-dev] [RFC PATCH v1 1/3] rte: change xstats to use integer > keys >=20 > Signed-off-by: Remy Horton > --- > lib/librte_ether/rte_ethdev.c | 87 > +++++++++++++++++++++++++++++++++++++++---- > lib/librte_ether/rte_ethdev.h | 38 +++++++++++++++++++ > 2 files changed, 117 insertions(+), 8 deletions(-) >=20 > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.= c > index a31018e..cdd0685 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -1507,6 +1507,82 @@ rte_eth_stats_reset(uint8_t port_id) > dev->data->rx_mbuf_alloc_failed =3D 0; > } >=20 > +static int > +rte_eth_xstats_count(uint8_t port_id) Thanks for adding this. I believe an overt API is much more clear. > +{ > + struct rte_eth_dev *dev; > + int count; > + > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL); > + dev =3D &rte_eth_devices[port_id]; > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->xstats_names, -ENOTSUP); > + count =3D (*dev->dev_ops->xstats_names)(dev, NULL, 0); > + if (count >=3D 0) { > + count +=3D RTE_NB_STATS; > + count +=3D dev->data->nb_rx_queues * RTE_NB_RXQ_STATS; > + count +=3D dev->data->nb_tx_queues * RTE_NB_TXQ_STATS; > + } > + return count; > +} > + > +int > +rte_eth_xstats_names(uint8_t port_id, struct rte_eth_xstats_name > *ptr_names, > + unsigned limit) > +{ > + struct rte_eth_dev *dev; > + int cnt_used_entries; > + int cnt_expected_entries; > + uint32_t idx, id_queue; > + int offset; > + > + cnt_expected_entries =3D rte_eth_xstats_count(port_id); > + if (cnt_expected_entries < 0 || ptr_names =3D=3D NULL) > + return cnt_expected_entries; I suggest we don't provide two ways to get the number of stats and=20 that users always call rte_eth_xstats_count(). Recommend returning -EINVAL if ptr_names is NULL. > + > + if ((int)limit < cnt_expected_entries) > + return -ERANGE; > + > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL); > + dev =3D &rte_eth_devices[port_id]; > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->xstats_names, -ENOTSUP); I think this check is too restrictive. There are drivers that do not=20 provide device specific xstats today but the xstats API will still=20 return the per q stats. Recommend skipping the device specific steps that follow. > + cnt_used_entries =3D (*dev->dev_ops->xstats_names)( > + dev, ptr_names, limit); > + > + if (cnt_used_entries < 0) > + return cnt_used_entries; > + > + offset =3D cnt_used_entries * RTE_ETH_XSTATS_NAME_SIZE; > + for (idx =3D 0; idx < RTE_NB_STATS; idx++) { > + snprintf(ptr_names[cnt_used_entries].name, > + sizeof(ptr_names[0].name), > + "%s", rte_stats_strings[idx].name); > + offset +=3D RTE_ETH_XSTATS_NAME_SIZE; > + cnt_used_entries++; > + } > + for (id_queue =3D 0; id_queue < dev->data->nb_rx_queues; id_queue++) { > + for (idx =3D 0; idx < RTE_NB_RXQ_STATS; idx++) { > + snprintf(ptr_names[cnt_used_entries].name, > + sizeof(ptr_names[0].name), > + "rx_q%u%s", > + id_queue, rte_rxq_stats_strings[idx].name); > + offset +=3D RTE_ETH_XSTATS_NAME_SIZE; > + cnt_used_entries++; > + } > + > + } > + for (id_queue =3D 0; id_queue < dev->data->nb_tx_queues; id_queue++) { > + for (idx =3D 0; idx < RTE_NB_TXQ_STATS; idx++) { > + snprintf(ptr_names[cnt_used_entries].name, > + sizeof(ptr_names[0].name), > + "tx_q%u%s", > + id_queue, rte_txq_stats_strings[idx].name); > + offset +=3D RTE_ETH_XSTATS_NAME_SIZE; > + cnt_used_entries++; > + } > + } > + return cnt_used_entries; > +} > + > /* retrieve ethdev extended statistics */ > int > rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstats *xstats, > @@ -1551,8 +1627,7 @@ rte_eth_xstats_get(uint8_t port_id, struct > rte_eth_xstats *xstats, > stats_ptr =3D RTE_PTR_ADD(ð_stats, > rte_stats_strings[i].offset); > val =3D *stats_ptr; > - snprintf(xstats[count].name, sizeof(xstats[count].name), > - "%s", rte_stats_strings[i].name); > + xstats[count].key =3D count + xcount; Suggest setting adding: xstats[count].name[0] =3D '\0' until name is removed. > xstats[count++].value =3D val; > } >=20 > @@ -1563,9 +1638,7 @@ rte_eth_xstats_get(uint8_t port_id, struct > rte_eth_xstats *xstats, > rte_rxq_stats_strings[i].offset + > q * sizeof(uint64_t)); > val =3D *stats_ptr; > - snprintf(xstats[count].name, sizeof(xstats[count].name), > - "rx_q%u_%s", q, > - rte_rxq_stats_strings[i].name); > + xstats[count].key =3D count + xcount; Same name comment. > xstats[count++].value =3D val; > } > } > @@ -1577,9 +1650,7 @@ rte_eth_xstats_get(uint8_t port_id, struct > rte_eth_xstats *xstats, > rte_txq_stats_strings[i].offset + > q * sizeof(uint64_t)); > val =3D *stats_ptr; > - snprintf(xstats[count].name, sizeof(xstats[count].name), > - "tx_q%u_%s", q, > - rte_txq_stats_strings[i].name); Same name comment. > + xstats[count].key =3D count + xcount; > xstats[count++].value =3D val; > } > } > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.= h > index 022733e..4b81c59 100644 > --- a/lib/librte_ether/rte_ethdev.h > +++ b/lib/librte_ether/rte_ethdev.h > @@ -941,10 +941,23 @@ struct rte_eth_txq_info { > * structure. > */ > struct rte_eth_xstats { > + /* FIXME: Remove name[] once remaining drivers converted */ > char name[RTE_ETH_XSTATS_NAME_SIZE]; > + uint64_t key; > uint64_t value; > }; >=20 > +/** > + * A name-key lookup element for extended statistics. > + * > + * This structure is used to map between names and ID numbers > + * for extended ethernet statistics. > + */ > +struct rte_eth_xstats_name { > + char name[RTE_ETH_XSTATS_NAME_SIZE]; > + uint64_t id; Suggest naming this key as well or modifying rte_eth_xstats to use id. > +}; > + > #define ETH_DCB_NUM_TCS 8 > #define ETH_MAX_VMDQ_POOL 64 >=20 > @@ -1080,6 +1093,10 @@ typedef int (*eth_xstats_get_t)(struct rte_eth_dev > *dev, > typedef void (*eth_xstats_reset_t)(struct rte_eth_dev *dev); > /**< @internal Reset extended stats of an Ethernet device. */ >=20 > +typedef int (*eth_xstats_names_t)(struct rte_eth_dev *dev, > + struct rte_eth_xstats_name *ptr_names, unsigned limit); > +/**< @internal Get names of extended stats of an Ethernet device. */ > + > typedef int (*eth_queue_stats_mapping_set_t)(struct rte_eth_dev *dev, > uint16_t queue_id, > uint8_t stat_idx, > @@ -1427,6 +1444,8 @@ struct eth_dev_ops { > eth_stats_reset_t stats_reset; /**< Reset generic device > statistics. */ > eth_xstats_get_t xstats_get; /**< Get extended device > statistics. */ > eth_xstats_reset_t xstats_reset; /**< Reset extended device > statistics. */ > + eth_xstats_names_t xstats_names; > + /**< Get names of extended statistics. */ > eth_queue_stats_mapping_set_t queue_stats_mapping_set; > /**< Configure per queue stat counter mapping. */ > eth_dev_infos_get_t dev_infos_get; /**< Get device info. */ > @@ -2279,6 +2298,25 @@ int rte_eth_stats_get(uint8_t port_id, struct > rte_eth_stats *stats); > void rte_eth_stats_reset(uint8_t port_id); >=20 > /** > + * Retrieve names of extended statistics of an Ethernet device. > + * > + * Names within ptr_strings will be aligned to RTE_ETH_XSTATS_NAME_SIZE > and > + * will be listed in ascending mapping order. > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param ptr_names > + * Block of memory to insert names into. Must be at least limit in size= . > + * If NULL, function returns how many statistics are available. > + * @param limit > + * Capacity of ptr_strings (number of names). Ignored if ptr_string is > NULL. > + * @return > + * If successful, number of statistics; negative on error. > + */ > +int rte_eth_xstats_names(uint8_t port_id, struct rte_eth_xstats_name > *ptr_names, > + unsigned limit); > + > +/** > * Retrieve extended statistics of an Ethernet device. > * > * @param port_id > -- > 2.5.5 Nice work. Regards, Dave