Re: [dpdk-dev] [PATCH v2 02/15] eventdev: add APIs for extended stats

Re: [dpdk-dev] [PATCH v2 02/15] eventdev: add APIs for extended stats

[Eads, Gage]

Hi Harry,

Looks like the function names in *_version.map don't quite match those in the eventdev header.

+int
+rte_event_dev_xstats_names_get(uint8_t dev_id,
+       struct rte_event_dev_xstat_name *xstats_names,
+       unsigned int size)



+int

+rte_event_dev_xstats_get(uint8_t dev_id, const unsigned int ids[],

+       uint64_t values[], unsigned int n)



+uint64_t

+rte_event_dev_xstats_by_name_get(uint8_t dev_id, const char *name,

+              unsigned int *id)


+       rte_event_dev_get_xstats;

+       rte_event_dev_get_xstat_names;

+       rte_event_dev_get_xstat_by_name;

Thanks,
Gage

Re: [dpdk-dev] [PATCH v2 02/15] eventdev: add APIs for extended stats

[Van Haaren, Harry]

> From: Eads, Gage 
> Sent: Tuesday, January 31, 2017 8:50 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Subject: Re: [dpdk-dev] [PATCH v2 02/15] eventdev: add APIs for extended stats

> Hi Harry,

> Looks like the function names in *_version.map don't quite match those in the eventdev header.

Doh! Yes you're right - I never updated it - thanks for feedback.

I'll leave a day or two for other comments and roll a v3 then.

-Harry

[dpdk-dev] [PATCH v2 00/15] next-eventdev: event/sw software eventdev

[Harry van Haaren]

The following patchset adds software eventdev implementation
to the next-eventdev tree.

This implementation is based on the previous software eventdev
v1 patchset, now with comments addressed:
1) xstats api return values changed to be consistent
2) xstats api [out] added to appropriate values
3) xstats api now uses xxx_get() for consistency
4) patch names for check-log.sh
5) checkpatch issues resolved (where it makes sense to, there are
   certain places where fixing checkpatch makes the code less readable.
   These checkpatch warnings will still show up - I see no alternative)

In addition, the following improvements have been made to the patchset:
1) Adds test to run automatically with make test
2) Rework the sw implementation event credit scheme

The first two patches make changes to the eventdev API,
then the software implementation is added, and finally
tests are added for the sw eventdev implementation.

This patchset contains the work of multiple developers,
please see signoffs on each patch.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>


Bruce Richardson (15):
  eventdev: remove unneeded dependencies
  eventdev: add APIs for extended stats
  event/sw: add new software-only eventdev driver
  event/sw: add device capabilities function
  event/sw: add configure function
  event/sw: add fns to return default port/queue config
  event/sw: add support for event queues
  event/sw: add support for event ports
  event/sw: add support for linking queues to ports
  event/sw: add worker core functions
  event/sw: add scheduling logic
  event/sw: add start stop and close functions
  event/sw: add dump function for easier debugging
  event/sw: add xstats support
  app/test: add unit tests for SW eventdev driver

 app/test/Makefile                             |    5 +-
 app/test/autotest_data.py                     |   26 +
 app/test/test_sw_eventdev.c                   | 2071 +++++++++++++++++++++++++
 config/common_base                            |    5 +
 drivers/event/Makefile                        |    1 +
 drivers/event/sw/Makefile                     |   69 +
 drivers/event/sw/event_ring.h                 |  179 +++
 drivers/event/sw/iq_ring.h                    |  176 +++
 drivers/event/sw/rte_pmd_evdev_sw_version.map |    3 +
 drivers/event/sw/sw_evdev.c                   |  754 +++++++++
 drivers/event/sw/sw_evdev.h                   |  291 ++++
 drivers/event/sw/sw_evdev_scheduler.c         |  602 +++++++
 drivers/event/sw/sw_evdev_worker.c            |  186 +++
 drivers/event/sw/sw_evdev_xstats.c            |  404 +++++
 lib/librte_eventdev/Makefile                  |    1 -
 lib/librte_eventdev/rte_eventdev.c            |   63 +
 lib/librte_eventdev/rte_eventdev.h            |   85 +-
 lib/librte_eventdev/rte_eventdev_pmd.h        |   60 +
 lib/librte_eventdev/rte_eventdev_version.map  |    3 +
 mk/rte.app.mk                                 |    1 +
 20 files changed, 4981 insertions(+), 4 deletions(-)
 create mode 100644 app/test/test_sw_eventdev.c
 create mode 100644 drivers/event/sw/Makefile
 create mode 100644 drivers/event/sw/event_ring.h
 create mode 100644 drivers/event/sw/iq_ring.h
 create mode 100644 drivers/event/sw/rte_pmd_evdev_sw_version.map
 create mode 100644 drivers/event/sw/sw_evdev.c
 create mode 100644 drivers/event/sw/sw_evdev.h
 create mode 100644 drivers/event/sw/sw_evdev_scheduler.c
 create mode 100644 drivers/event/sw/sw_evdev_worker.c
 create mode 100644 drivers/event/sw/sw_evdev_xstats.c

-- 
2.7.4

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

[Harry van Haaren]

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           | 63 ++++++++++++++++++++++
 lib/librte_eventdev/rte_eventdev.h           | 80 ++++++++++++++++++++++++++++
 lib/librte_eventdev/rte_eventdev_pmd.h       | 60 +++++++++++++++++++++
 lib/librte_eventdev/rte_eventdev_version.map |  3 ++
 4 files changed, 206 insertions(+)

diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
index c8f3e94..95572f4 100644
--- a/lib/librte_eventdev/rte_eventdev.c
+++ b/lib/librte_eventdev/rte_eventdev.c
@@ -920,6 +920,69 @@ 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_xstats_names_get(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 above */
+	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 -ENOTSUP;
+}
+
+/* retrieve eventdev extended statistics */
+int
+rte_event_dev_xstats_get(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 -ENOTSUP;
+}
+
+uint64_t
+rte_event_dev_xstats_by_name_get(uint8_t dev_id, const char *name,
+		unsigned int *id)
+{
+	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, 0);
+	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 */
+
+	/* implemented by driver */
+	if (dev->dev_ops->get_xstat_by_name != NULL)
+		return (*dev->dev_ops->get_xstat_by_name)(dev, name, id);
+	return -ENOTSUP;
+}
+
 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..66fc35a 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -1401,6 +1401,86 @@ 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[out] 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. -EINVAL for invalid dev id, -ENOTSUP if the
+ *     device doesn't support this function.
+ */
+int
+rte_event_dev_xstats_names_get(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[out] values
+ *   The values for each stats request by ID.
+ * @param n
+ *   The number of stats requested
+ * @return
+ *   - positive value: number of stat entries filled into the values array
+ *   - negative value on error. -EINVAL for invalid dev id, -ENOTSUP if the
+ *     device doesn't support this function.
+ */
+int
+rte_event_dev_xstats_get(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[out] 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
+ *   - positive value or zero: the stat value
+ *   - negative value: -EINVAL if stat not found, -ENOTSUP if not supported.
+ */
+uint64_t
+rte_event_dev_xstats_by_name_get(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..15843c6 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,13 @@ 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;
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2 02/15] eventdev: add APIs for extended stats

[Jerin Jacob]

On Tue, Jan 31, 2017 at 04:14:20PM +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.

The APIs Looks good. One minor comment though,

Can you add statistics specific to per event queue and event
port?, To improve the cases like below in the application code(taken from
app/test/test_sw_eventdev.c).

IMO, it is useful because,
- ethdev has similar semantics
- majority of the implementations will have port and queue specific statistics counters

+       for (i = 0; i < MAX_PORTS; i++) {
+               char name[32];
+               snprintf(name, sizeof(name), "port_%u_rx", i);
+               stats->port_rx_pkts[i] = rte_event_dev_xstats_by_name_get(
+                               dev_id, name, &port_rx_pkts_ids[i]);

+       for (i = 0; i < MAX_QIDS; i++) {
+               char name[32];
+               snprintf(name, sizeof(name), "qid_%u_rx", i);
+               stats->qid_rx_pkts[i] = rte_event_dev_xstats_by_name_get(
+                               dev_id, name, &qid_rx_pkts_ids[i]);

Re: [dpdk-dev] [PATCH v2 02/15] eventdev: add APIs for extended stats

[Van Haaren, Harry]

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Monday, February 6, 2017 8:23 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: Re: [PATCH v2 02/15] eventdev: add APIs for extended stats
> 
> On Tue, Jan 31, 2017 at 04:14:20PM +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.
> 
> The APIs Looks good. One minor comment though,
> 
> Can you add statistics specific to per event queue and event
> port?, To improve the cases like below in the application code(taken from
> app/test/test_sw_eventdev.c).
> 
> IMO, it is useful because,
> - ethdev has similar semantics
> - majority of the implementations will have port and queue specific statistics counters

I'm not totally sure what you're asking but if I understand correctly, you're suggesting a struct based stats API like this?

struct rte_event_dev_port_stats {
    uint64_t rx;
    uint64_t tx;
    ...
};

struct rte_event_dev_queue_stats {
    uint64_t rx;
    uint64_t tx;
    ...
};

/** a get function to get a specific port's statistics. The *stats* pointer is filled in */ 
int rte_event_dev_port_stats_get(dev, uint8_t port_id, struct rte_event_dev_port_stats *stats);

/** a get function to get a specific queue's statistics. The *stats* pointer is filled in */ 
int rte_event_dev_queue_stats_get(dev, uint8_t queue_id, struct rte_event_dev_queue_stats *stats);


Is this what you meant, or did I misunderstand?


> 
> +       for (i = 0; i < MAX_PORTS; i++) {
> +               char name[32];
> +               snprintf(name, sizeof(name), "port_%u_rx", i);
> +               stats->port_rx_pkts[i] = rte_event_dev_xstats_by_name_get(
> +                               dev_id, name, &port_rx_pkts_ids[i]);
> 
> +       for (i = 0; i < MAX_QIDS; i++) {
> +               char name[32];
> +               snprintf(name, sizeof(name), "qid_%u_rx", i);
> +               stats->qid_rx_pkts[i] = rte_event_dev_xstats_by_name_get(
> +                               dev_id, name, &qid_rx_pkts_ids[i]);
> 

Re: [dpdk-dev] [PATCH v2 02/15] eventdev: add APIs for extended stats

[Jerin Jacob]

On Mon, Feb 06, 2017 at 10:37:31AM +0000, Van Haaren, Harry wrote:
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Monday, February 6, 2017 8:23 AM
> > To: Van Haaren, Harry <harry.van.haaren@intel.com>
> > Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>
> > Subject: Re: [PATCH v2 02/15] eventdev: add APIs for extended stats
> > 
> > On Tue, Jan 31, 2017 at 04:14:20PM +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.
> > 
> > The APIs Looks good. One minor comment though,

One more suggestion,

How about adding the reset function for resetting the selective xstat counters similar to
ethdev? IMO, It will be useful.

> > 
> > Can you add statistics specific to per event queue and event
> > port?, To improve the cases like below in the application code(taken from
> > app/test/test_sw_eventdev.c).
> > 
> > IMO, it is useful because,
> > - ethdev has similar semantics
> > - majority of the implementations will have port and queue specific statistics counters
> 
> I'm not totally sure what you're asking but if I understand correctly, you're suggesting a struct based stats API like this?
> 
> struct rte_event_dev_port_stats {
>     uint64_t rx;
>     uint64_t tx;
>     ...
> };
> 
> struct rte_event_dev_queue_stats {
>     uint64_t rx;
>     uint64_t tx;
>     ...
> };
> 
> /** a get function to get a specific port's statistics. The *stats* pointer is filled in */ 
> int rte_event_dev_port_stats_get(dev, uint8_t port_id, struct rte_event_dev_port_stats *stats);
> 
> /** a get function to get a specific queue's statistics. The *stats* pointer is filled in */ 
> int rte_event_dev_queue_stats_get(dev, uint8_t queue_id, struct rte_event_dev_queue_stats *stats);
> 
> 
> Is this what you meant, or did I misunderstand?

I meant, queue and port specific "xstat" as each implementation may
have different statistics counters for queue/port.

Just to share my view, I have modified the exiting proposal.
Thoughts?

+enum rte_event_dev_xstats_mode {
+       RTE_EVENT_DEV_XSTAT_DEVICE; /* Event device specific global xstats */
+       RTE_EVENT_DEV_XSTAT_QUEUE; /* Event queue specific xstats */
+       RTE_EVENT_DEV_XSTAT_PORT; /* Event port specific xstats */
+};
+
 /**
  * Retrieve names of extended statistics of an event device.
  *
@@ -1436,9 +1442,11 @@ struct rte_event_dev_xstat_name {
  */
 int
 rte_event_dev_xstats_names_get(uint8_t dev_id,
+                              enum rte_event_dev_xstats_mode mode;
                               struct rte_event_dev_xstat_name *xstat_names,
                               unsigned int size);

 /**
  * Retrieve extended statistics of an event device.
  *
@@ -1458,7 +1466,10 @@ rte_event_dev_xstats_names_get(uint8_t dev_id,
  *     device doesn't support this function.
  */
 int
-rte_event_dev_xstats_get(uint8_t dev_id, const unsigned int ids[],
+rte_event_dev_xstats_get(uint8_t dev_id,
+                        enum rte_event_dev_xstats_mode mode,
+                        uint8_t queue_port_id; /* valid when RTE_EVENT_DEV_XSTAT_QUEUE or RTE_EVENT_DEV_XSTAT_PORT */
+                        const unsigned int ids[],
                         uint64_t values[], unsigned int n);
 
 /**
@@ -1478,7 +1489,9 @@ rte_event_dev_xstats_get(uint8_t dev_id, const
unsigned int ids[],
  *   - negative value: -EINVAL if stat not found, -ENOTSUP if not
  *   supported.
  */
 uint64_t
-rte_event_dev_xstats_by_name_get(uint8_t dev_id, const char *name,
+rte_event_dev_xstats_by_name_get(uint8_t dev_id,
+                                enum rte_event_dev_xstats_mode mode,
+                                const char *name,
                                 unsigned int *id);
> 
> 
> > 
> > +       for (i = 0; i < MAX_PORTS; i++) {
> > +               char name[32];
> > +               snprintf(name, sizeof(name), "port_%u_rx", i);
> > +               stats->port_rx_pkts[i] = rte_event_dev_xstats_by_name_get(
> > +                               dev_id, name, &port_rx_pkts_ids[i]);
> > 
> > +       for (i = 0; i < MAX_QIDS; i++) {
> > +               char name[32];
> > +               snprintf(name, sizeof(name), "qid_%u_rx", i);
> > +               stats->qid_rx_pkts[i] = rte_event_dev_xstats_by_name_get(
> > +                               dev_id, name, &qid_rx_pkts_ids[i]);
> > 
> 

Re: [dpdk-dev] [PATCH v2 02/15] eventdev: add APIs for extended stats

[Van Haaren, Harry]

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Tuesday, February 7, 2017 6:25 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: Re: [PATCH v2 02/15] eventdev: add APIs for extended stats

<snip>

> How about adding the reset function for resetting the selective xstat counters similar to
> ethdev? IMO, It will be useful.


Agreed, an ethdev like xstats_reset() is useful - will add it.


> > > Can you add statistics specific to per event queue and event
> > > port?, To improve the cases like below in the application code(taken from
> > > app/test/test_sw_eventdev.c).
> > >
> > > IMO, it is useful because,
> > > - ethdev has similar semantics
> > > - majority of the implementations will have port and queue specific statistics counters
> >
> > I'm not totally sure what you're asking but if I understand correctly, you're suggesting a
> struct based stats API like this?
> >
> > struct rte_event_dev_port_stats {
> >     uint64_t rx;
> >     uint64_t tx;
> >     ...
> > };
> >
> > struct rte_event_dev_queue_stats {
> >     uint64_t rx;
> >     uint64_t tx;
> >     ...
> > };
> >
> > /** a get function to get a specific port's statistics. The *stats* pointer is filled in */
> > int rte_event_dev_port_stats_get(dev, uint8_t port_id, struct rte_event_dev_port_stats
> *stats);
> >
> > /** a get function to get a specific queue's statistics. The *stats* pointer is filled in */
> > int rte_event_dev_queue_stats_get(dev, uint8_t queue_id, struct rte_event_dev_queue_stats
> *stats);
> >
> >
> > Is this what you meant, or did I misunderstand?
> 
> I meant, queue and port specific "xstat" as each implementation may
> have different statistics counters for queue/port.
> 
> Just to share my view, I have modified the exiting proposal.
> Thoughts?


That seems reasonable to me, thanks for the sample code - 1/2 the work done ;)
Will include in v3.


> +enum rte_event_dev_xstats_mode {
> +       RTE_EVENT_DEV_XSTAT_DEVICE; /* Event device specific global xstats */
> +       RTE_EVENT_DEV_XSTAT_QUEUE; /* Event queue specific xstats */
> +       RTE_EVENT_DEV_XSTAT_PORT; /* Event port specific xstats */
> +};
> +
>  /**
>   * Retrieve names of extended statistics of an event device.
>   *
> @@ -1436,9 +1442,11 @@ struct rte_event_dev_xstat_name {
>   */
>  int
>  rte_event_dev_xstats_names_get(uint8_t dev_id,
> +                              enum rte_event_dev_xstats_mode mode;
>                                struct rte_event_dev_xstat_name *xstat_names,
>                                unsigned int size);
> 
>  /**
>   * Retrieve extended statistics of an event device.
>   *
> @@ -1458,7 +1466,10 @@ rte_event_dev_xstats_names_get(uint8_t dev_id,
>   *     device doesn't support this function.
>   */
>  int
> -rte_event_dev_xstats_get(uint8_t dev_id, const unsigned int ids[],
> +rte_event_dev_xstats_get(uint8_t dev_id,
> +                        enum rte_event_dev_xstats_mode mode,
> +                        uint8_t queue_port_id; /* valid when RTE_EVENT_DEV_XSTAT_QUEUE or
> RTE_EVENT_DEV_XSTAT_PORT */
> +                        const unsigned int ids[],
>                          uint64_t values[], unsigned int n);
> 
>  /**
> @@ -1478,7 +1489,9 @@ rte_event_dev_xstats_get(uint8_t dev_id, const
> unsigned int ids[],
>   *   - negative value: -EINVAL if stat not found, -ENOTSUP if not
>   *   supported.
>   */
>  uint64_t
> -rte_event_dev_xstats_by_name_get(uint8_t dev_id, const char *name,
> +rte_event_dev_xstats_by_name_get(uint8_t dev_id,
> +                                enum rte_event_dev_xstats_mode mode,
> +                                const char *name,
>                                  unsigned int *id);
> >
> >
> > >
> > > +       for (i = 0; i < MAX_PORTS; i++) {
> > > +               char name[32];
> > > +               snprintf(name, sizeof(name), "port_%u_rx", i);
> > > +               stats->port_rx_pkts[i] = rte_event_dev_xstats_by_name_get(
> > > +                               dev_id, name, &port_rx_pkts_ids[i]);
> > >
> > > +       for (i = 0; i < MAX_QIDS; i++) {
> > > +               char name[32];
> > > +               snprintf(name, sizeof(name), "qid_%u_rx", i);
> > > +               stats->qid_rx_pkts[i] = rte_event_dev_xstats_by_name_get(
> > > +                               dev_id, name, &qid_rx_pkts_ids[i]);
> > >
> >