From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH 02/15] eventdev: add APIs for extended stats Date: Sun, 22 Jan 2017 00:29:53 +0530 Message-ID: <20170121185951.GB13156@localhost.localdomain> References: <1484581255-148720-1-git-send-email-harry.van.haaren@intel.com> <1484581255-148720-3-git-send-email-harry.van.haaren@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Cc: , Bruce Richardson To: Harry van Haaren Return-path: Received: from NAM01-BY2-obe.outbound.protection.outlook.com (mail-by2nam01on0076.outbound.protection.outlook.com [104.47.34.76]) by dpdk.org (Postfix) with ESMTP id A2D65106A for ; Sat, 21 Jan 2017 20:00:16 +0100 (CET) Content-Disposition: inline In-Reply-To: <1484581255-148720-3-git-send-email-harry.van.haaren@intel.com> 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 Mon, Jan 16, 2017 at 03:40:42PM +0000, Harry van Haaren wrote: > From: Bruce Richardson > > Add in APIs for extended stats so that eventdev implementations can report > out information on their internal state. The APIs are based on, but not > identical to, the equivalent ethdev functions. Overall the xstat API looks good. I think, we need to extend the API to support event port and event queue specific xstats too. > > Signed-off-by: Bruce Richardson > Signed-off-by: Harry van Haaren > --- > lib/librte_eventdev/rte_eventdev.c | 64 ++++++++++++++++++++++++ > lib/librte_eventdev/rte_eventdev.h | 75 ++++++++++++++++++++++++++++ > lib/librte_eventdev/rte_eventdev_pmd.h | 58 +++++++++++++++++++++ > lib/librte_eventdev/rte_eventdev_version.map | 3 ++ > 4 files changed, 200 insertions(+) > > + > +uint64_t > +rte_event_dev_get_xstat_by_name(uint8_t dev_id, const char *name, > + unsigned int *id) > +{ > + const struct rte_eventdev *dev = &rte_eventdevs[dev_id]; > + unsigned int temp = -1; > + > + if (id != NULL) > + *id = (unsigned int)-1; > + else > + id = &temp; /* ensure driver never gets a NULL value */ > + > + RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, 0); > + > + /* implemented by driver */ > + if (dev->dev_ops->get_xstat_by_name != NULL) > + return (*dev->dev_ops->get_xstat_by_name)(dev, name, id); > + return 0; Shouldn't we return -1 as per API specification? > +} > + > int > rte_event_dev_start(uint8_t dev_id) > { > diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h > index c2f9310..681cbfa 100644 > --- a/lib/librte_eventdev/rte_eventdev.h > +++ b/lib/librte_eventdev/rte_eventdev.h > @@ -1401,6 +1401,81 @@ rte_event_port_links_get(uint8_t dev_id, uint8_t port_id, > int > rte_event_dev_dump(uint8_t dev_id, FILE *f); > > +/** Maximum name length for extended statistics counters */ > +#define RTE_EVENT_DEV_XSTAT_NAME_SIZE 64 > + > +/** > + * A name-key lookup element for extended statistics. > + * > + * This structure is used to map between names and ID numbers > + * for extended ethdev statistics. > + */ > +struct rte_event_dev_xstat_name { > + char name[RTE_EVENT_DEV_XSTAT_NAME_SIZE]; > +}; > + > +/** > + * Retrieve names of extended statistics of an event device. > + * > + * @param dev_id > + * The identifier of the event device. > + * @param xstat_names > + * Block of memory to insert names into. Must be at least size in capacity. > + * If set to NULL, function returns required capacity. > + * @param size > + * Capacity of xstat_names (number of names). > + * @return > + * - positive value lower or equal to size: success. The return value > + * is the number of entries filled in the stats table. > + * - positive value higher than size: error, the given statistics table > + * is too small. The return value corresponds to the size that should > + * be given to succeed. The entries in the table are not valid and > + * shall not be used by the caller. > + * - negative value on error (invalid port id) How about updating the rte_errno also in the failure case? It may useful for application developer to have single check for error condition > + */ > +int > +rte_event_dev_get_xstat_names(uint8_t dev_id, > + struct rte_event_dev_xstat_name *xstat_names, > + unsigned int size); > + > +/** > + * Retrieve extended statistics of an event device. > + * > + * @param dev_id > + * The identifier of the device. > + * @param ids > + * The id numbers of the stats to get. The ids can be got from the stat > + * position in the stat list from rte_event_dev_get_xstat_names(), or > + * by using rte_eventdev_get_xstat_by_name() > + * @param values Please add [out] to indicate it is a output parameter example: @param[out] values > + * The values for each stats request by ID. > + * @param n > + * The number of stats requested > + * @return > + * Number of stat entries filled into the values array > + */ > +int > +rte_event_dev_get_xstats(uint8_t dev_id, const unsigned int ids[], > + uint64_t values[], unsigned int n); > + > +/** > + * Retrieve the value of a single stat by requesting it by name. > + * > + * @param dev_id > + * The identifier of the device > + * @param name > + * The stat name to retrieve > + * @param id Please add [out] to indicate it is output parameter > + * If non-NULL, the numerical id of the stat will be returned, so that further > + * requests for the stat can be got using rte_eventdev_xstats_get, which will The function prototype is rte_event_dev_get_xstats. Change the rte_eventdev_xstats_get to rte_event_dev_get_xstats in the above description The rest of the file is following rte_eventdev_xxx_xxx_get syntax for get functions. How about changing rte_eventdev_xxx_xxx_get syntax to maintain the consistency? > + * be faster as it doesn't need to scan a list of names for the stat. > + * If the stat cannot be found, the id returned will be (unsigned)-1. > + * @return > + * The stat value, or -1 if not found. > + */ > +uint64_t > +rte_event_dev_get_xstat_by_name(uint8_t dev_id, const char *name, unsigned int *id);