[dpdk-dev,02/15] eventdev: add APIs for extended stats

Message ID 1484581255-148720-3-git-send-email-harry.van.haaren@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation fail apply patch file failure

Commit Message

Van Haaren, Harry Jan. 16, 2017, 3:40 p.m. UTC
  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.

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(+)
  

Comments

Jerin Jacob Jan. 21, 2017, 6:59 p.m. UTC | #1
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);
  

Patch

diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
index c8f3e94..ca11d54 100644
--- a/lib/librte_eventdev/rte_eventdev.c
+++ b/lib/librte_eventdev/rte_eventdev.c
@@ -920,6 +920,70 @@  rte_event_dev_dump(uint8_t dev_id, FILE *f)
 
 }
 
+static int
+get_xstats_count(uint8_t dev_id)
+{
+	struct rte_eventdev *dev = &rte_eventdevs[dev_id];
+	if (dev->dev_ops->get_xstat_names != NULL)
+		return (*dev->dev_ops->get_xstat_names)(dev, NULL, 0);
+	return 0;
+}
+
+int
+rte_event_dev_get_xstat_names(uint8_t dev_id,
+	struct rte_event_dev_xstat_name *xstats_names,
+	unsigned int size)
+{
+	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
+	const int cnt_expected_entries = get_xstats_count(dev_id);
+	if (xstats_names == NULL || cnt_expected_entries < 0 ||
+			(int)size < cnt_expected_entries)
+		return cnt_expected_entries;
+
+	/* dev_id checked in get_xstats_count() */
+	const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
+
+	if (dev->dev_ops->get_xstat_names != NULL)
+		return (*dev->dev_ops->get_xstat_names)(dev,
+				xstats_names, size);
+
+	return 0;
+}
+
+/* retrieve eventdev extended statistics */
+int
+rte_event_dev_get_xstats(uint8_t dev_id, const unsigned int ids[],
+	uint64_t values[], unsigned int n)
+{
+	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
+	const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
+
+	/* implemented by the driver */
+	if (dev->dev_ops->get_xstats != NULL)
+		return (*dev->dev_ops->get_xstats)(dev, ids, values, n);
+	return 0;
+}
+
+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;
+}
+
 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)
+ */
+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
+ *   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
+ *   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
+ *   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);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
index c84c9a2..7a6229d 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd.h
@@ -418,6 +418,59 @@  typedef void (*eventdev_dequeue_timeout_ticks_t)(struct rte_eventdev *dev,
  */
 typedef void (*eventdev_dump_t)(struct rte_eventdev *dev, FILE *f);
 
+/**
+ * Retrieve a set of statistics from device
+ *
+ * @param dev
+ *   Event device pointer
+ * @param ids
+ *   The stat ids to retrieve
+ * @param values
+ *   The returned stat values
+ * @param n
+ *   The number of id values and entries in the values array
+ * @return
+ *   The number of stat values successfully filled into the values array
+ */
+typedef int (*eventdev_get_xstats_t)(const struct rte_eventdev *dev,
+	const unsigned int ids[], uint64_t values[], unsigned int n);
+
+/**
+ * Get names of extended stats of an event device
+ *
+ * @param dev
+ *   Event device pointer
+ * @param xstat_names
+ *   Array of name values to be filled in
+ * @param size
+ *   Number of values in the xstat_names array
+ * @return
+ *   When size >= the number of stats, return the number of stat values filled
+ *   into the array.
+ *   When size < the number of available stats, return the number of stats
+ *   values, and do not fill in any data into xstat_names.
+ */
+typedef int (*eventdev_get_xstat_names_t)(const struct rte_eventdev *dev,
+	struct rte_event_dev_xstat_name *xstat_names, unsigned int size);
+
+/**
+ * Get value of one stats and optionally return its id
+ *
+ * @param dev
+ *   Event device pointer
+ * @param name
+ *   The name of the stat to retrieve
+ * @param id
+ *   Pointer to an unsigned int where we store the stat-id for future reference.
+ *   This pointer may be null if the id is not required.
+ * @return
+ *   The value of the stat, or (uint64_t)-1 if the stat is not found.
+ *   If the stat is not found, the id value will be returned as (unsigned)-1,
+ *   if id pointer is non-NULL
+ */
+typedef uint64_t (*eventdev_get_xstat_by_name)(const struct rte_eventdev *dev,
+		const char *name, unsigned int *id);
+
 /** Event device operations function pointer table */
 struct rte_eventdev_ops {
 	eventdev_info_get_t dev_infos_get;	/**< Get device info. */
@@ -448,6 +501,11 @@  struct rte_eventdev_ops {
 	/**< Converts ns to *timeout_ticks* value for rte_event_dequeue() */
 	eventdev_dump_t dump;
 	/* Dump internal information */
+
+	eventdev_get_xstats_t get_xstats;    /**< Get extended device statistics. */
+	eventdev_get_xstat_names_t get_xstat_names; /**< Get names of extended stats. */
+	eventdev_get_xstat_by_name get_xstat_by_name; /**< Get one value by name */
+
 };
 
 /**
diff --git a/lib/librte_eventdev/rte_eventdev_version.map b/lib/librte_eventdev/rte_eventdev_version.map
index 68b8c81..b138eb3 100644
--- a/lib/librte_eventdev/rte_eventdev_version.map
+++ b/lib/librte_eventdev/rte_eventdev_version.map
@@ -12,6 +12,9 @@  DPDK_17.02 {
 	rte_event_dev_stop;
 	rte_event_dev_close;
 	rte_event_dev_dump;
+	rte_event_dev_get_xstats;
+	rte_event_dev_get_xstat_names;
+	rte_event_dev_get_xstat_by_name;
 
 	rte_event_port_default_conf_get;
 	rte_event_port_setup;