On Mon, Jan 16, 2017 at 03:40:42PM +0000, Harry van Haaren wrote:
> From: Bruce Richardson <bruce.richard...@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.richard...@intel.com>
> Signed-off-by: Harry van Haaren <harry.van.haa...@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);
