From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
To: Harry van Haaren <harry.van.haaren@intel.com>
Cc: <dev@dpdk.org>, Bruce Richardson <bruce.richardson@intel.com>
Subject: Re: [dpdk-dev] [PATCH 02/15] eventdev: add APIs for extended stats
Date: Sun, 22 Jan 2017 00:29:53 +0530 [thread overview]
Message-ID: <20170121185951.GB13156@localhost.localdomain> (raw)
In-Reply-To: <1484581255-148720-3-git-send-email-harry.van.haaren@intel.com>
On Mon, Jan 16, 2017 at 03:40:42PM +0000, Harry van Haaren wrote:
> From: Bruce Richardson <bruce.richardson@intel.com>
>
> 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 <bruce.richardson@intel.com>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> ---
> 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);
next prev parent reply other threads:[~2017-01-21 19:00 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-16 15:40 [dpdk-dev] [PATCH 00/15] next-eventdev: event/sw Software Eventdev Harry van Haaren
2017-01-16 15:40 ` [dpdk-dev] [PATCH 01/15] eventdev: remove unneeded dependencies Harry van Haaren
2017-01-17 9:11 ` Jerin Jacob
2017-01-17 9:59 ` Van Haaren, Harry
2017-01-17 10:38 ` Jerin Jacob
2017-01-21 17:34 ` Jerin Jacob
2017-01-16 15:40 ` [dpdk-dev] [PATCH 02/15] eventdev: add APIs for extended stats Harry van Haaren
2017-01-21 18:59 ` Jerin Jacob [this message]
2017-01-16 15:40 ` [dpdk-dev] [PATCH 03/15] event/sw: add new software-only eventdev driver Harry van Haaren
2017-01-16 15:40 ` [dpdk-dev] [PATCH 04/15] event/sw: add function to return device capabilities Harry van Haaren
2017-01-16 15:40 ` [dpdk-dev] [PATCH 05/15] event/sw: add configure function Harry van Haaren
2017-01-16 15:40 ` [dpdk-dev] [PATCH 06/15] event/sw: add fns to return default port/queue config Harry van Haaren
2017-01-16 15:40 ` [dpdk-dev] [PATCH 07/15] event/sw: add support for event queues Harry van Haaren
2017-01-16 15:40 ` [dpdk-dev] [PATCH 08/15] event/sw: add support for event ports Harry van Haaren
2017-01-16 15:40 ` [dpdk-dev] [PATCH 09/15] event/sw: add support for linking queues to ports Harry van Haaren
2017-01-16 15:40 ` [dpdk-dev] [PATCH 10/15] event/sw: add worker core functions Harry van Haaren
2017-01-16 15:40 ` [dpdk-dev] [PATCH 11/15] event/sw: add scheduling logic Harry van Haaren
2017-01-16 15:40 ` [dpdk-dev] [PATCH 12/15] event/sw: add start, stop and close functions Harry van Haaren
2017-01-16 15:40 ` [dpdk-dev] [PATCH 13/15] event/sw: add dump function for easier debugging Harry van Haaren
2017-01-16 15:40 ` [dpdk-dev] [PATCH 14/15] event/sw: add xstats support Harry van Haaren
2017-01-16 15:40 ` [dpdk-dev] [PATCH 15/15] app/test: add unit tests for SW eventdev driver Harry van Haaren
2017-01-21 17:57 ` [dpdk-dev] [PATCH 00/15] next-eventdev: event/sw Software Eventdev Jerin Jacob
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170121185951.GB13156@localhost.localdomain \
--to=jerin.jacob@caviumnetworks.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=harry.van.haaren@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).