DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] ethdev: fix name index in xstats Api
@ 2016-12-16  9:44 Olivier Matz
  2016-12-16  9:44 ` [dpdk-dev] [PATCH 2/2] ethdev: clarify xstats Api documentation Olivier Matz
  2017-01-03 10:03 ` [dpdk-dev] [PATCH 1/2] ethdev: fix name index in xstats Api Remy Horton
  0 siblings, 2 replies; 8+ messages in thread
From: Olivier Matz @ 2016-12-16  9:44 UTC (permalink / raw)
  To: dev, thomas.monjalon; +Cc: remy.horton, stable

The function rte_eth_xstats_get() return an array of tuples (id,
value). The value is the statistic counter, while the id references a
name in the array returned by rte_eth_xstats_get_name().

Today, each 'id' returned by rte_eth_xstats_get() is equal to the index
in the returned array, making this value useless. It also prevents a
driver from having different indexes for names and value, like in the
example below:

  rte_eth_xstats_get_name() returns:
    0: "rx0_stat"
    1: "rx1_stat"
    2: ...
    7: "rx7_stat"
    8: "tx0_stat"
    9: "tx1_stat"
    ...
    15: "tx7_stat"

  rte_eth_xstats_get() returns:
    0: id=0, val=<stat>    ("rx0_stat")
    1: id=1, val=<stat>    ("rx1_stat")
    2: id=8, val=<stat>    ("tx0_stat")
    3: id=9, val=<stat>    ("tx1_stat")

This patch fixes the drivers to set the 'id' in their ethdev->xstats_get()
(except e1000 which was already doing it), and fixes ethdev by not setting
the 'id' field to the index of the table for pmd-specific stats: instead,
they should just be shifted by the max number of generic statistics.

CC: stable@dpdk.org
Fixes: bd6aa172cf35 ("ethdev: fetch extended statistics with integer ids")

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 drivers/net/bnx2x/bnx2x_ethdev.c   | 1 +
 drivers/net/fm10k/fm10k_ethdev.c   | 3 +++
 drivers/net/i40e/i40e_ethdev.c     | 4 ++++
 drivers/net/ixgbe/ixgbe_ethdev.c   | 3 +++
 drivers/net/qede/qede_ethdev.c     | 2 ++
 drivers/net/vhost/rte_eth_vhost.c  | 2 ++
 drivers/net/virtio/virtio_ethdev.c | 2 ++
 lib/librte_ether/rte_ethdev.c      | 5 ++++-
 8 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 0f1e4a2..f01047f 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -422,6 +422,7 @@ bnx2x_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 			xstats[num].value =
 					  *(uint64_t *)((char *)&sc->eth_stats +
 					  bnx2x_xstats_strings[num].offset_lo);
+		xstats[num].id = num;
 	}
 
 	return num;
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index fe74f6d..0d3098e 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1315,6 +1315,7 @@ fm10k_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	for (i = 0; i < FM10K_NB_HW_XSTATS; i++) {
 		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 			fm10k_hw_stats_strings[count].offset);
+		xstats[count].id = count;
 		count++;
 	}
 
@@ -1324,12 +1325,14 @@ fm10k_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 			xstats[count].value =
 				*(uint64_t *)(((char *)&hw_stats->q[q]) +
 				fm10k_hw_stats_rx_q_strings[i].offset);
+			xstats[count].id = count;
 			count++;
 		}
 		for (i = 0; i < FM10K_NB_TX_Q_XSTATS; i++) {
 			xstats[count].value =
 				*(uint64_t *)(((char *)&hw_stats->q[q]) +
 				fm10k_hw_stats_tx_q_strings[i].offset);
+			xstats[count].id = count;
 			count++;
 		}
 	}
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index b0c0fbf..f01455e 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2533,6 +2533,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	for (i = 0; i < I40E_NB_ETH_XSTATS; i++) {
 		xstats[count].value = *(uint64_t *)(((char *)&hw_stats->eth) +
 			rte_i40e_stats_strings[i].offset);
+		xstats[count].id = count;
 		count++;
 	}
 
@@ -2540,6 +2541,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	for (i = 0; i < I40E_NB_HW_PORT_XSTATS; i++) {
 		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 			rte_i40e_hw_port_strings[i].offset);
+		xstats[count].id = count;
 		count++;
 	}
 
@@ -2549,6 +2551,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 				*(uint64_t *)(((char *)hw_stats) +
 				rte_i40e_rxq_prio_strings[i].offset +
 				(sizeof(uint64_t) * prio));
+			xstats[count].id = count;
 			count++;
 		}
 	}
@@ -2559,6 +2562,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 				*(uint64_t *)(((char *)hw_stats) +
 				rte_i40e_txq_prio_strings[i].offset +
 				(sizeof(uint64_t) * prio));
+			xstats[count].id = count;
 			count++;
 		}
 	}
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index baffc71..cf68fa9 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2902,6 +2902,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	for (i = 0; i < IXGBE_NB_HW_STATS; i++) {
 		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 				rte_ixgbe_stats_strings[i].offset);
+		xstats[count].id = count;
 		count++;
 	}
 
@@ -2911,6 +2912,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 			xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 					rte_ixgbe_rxq_strings[stat].offset +
 					(sizeof(uint64_t) * i));
+			xstats[count].id = count;
 			count++;
 		}
 	}
@@ -2921,6 +2923,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 			xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 					rte_ixgbe_txq_strings[stat].offset +
 					(sizeof(uint64_t) * i));
+			xstats[count].id = count;
 			count++;
 		}
 	}
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 001166a..6f3b10c 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -973,6 +973,7 @@ qede_get_xstats(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	for (i = 0; i < RTE_DIM(qede_xstats_strings); i++) {
 		xstats[stat_idx].value = *(uint64_t *)(((char *)&stats) +
 					     qede_xstats_strings[i].offset);
+		xstats[stat_idx].id = stat_idx;
 		stat_idx++;
 	}
 
@@ -982,6 +983,7 @@ qede_get_xstats(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 				xstats[stat_idx].value = *(uint64_t *)(
 					((char *)(qdev->fp_array[(qid)].rxq)) +
 					 qede_rxq_xstats_strings[i].offset);
+				xstats[stat_idx].id = stat_idx;
 				stat_idx++;
 			}
 		}
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 766d4ef..dd79ccf 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -324,6 +324,7 @@ vhost_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 				*(uint64_t *)(((char *)vq)
 				+ vhost_rxport_stat_strings[t].offset);
 		}
+		xstats[count].id = count;
 		count++;
 	}
 	for (t = 0; t < VHOST_NB_XSTATS_TXPORT; t++) {
@@ -336,6 +337,7 @@ vhost_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 				*(uint64_t *)(((char *)vq)
 				+ vhost_txport_stat_strings[t].offset);
 		}
+		xstats[count].id = count;
 		count++;
 	}
 	return count;
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 1bd60e9..f6335aa 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -893,6 +893,7 @@ virtio_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		for (t = 0; t < VIRTIO_NB_RXQ_XSTATS; t++) {
 			xstats[count].value = *(uint64_t *)(((char *)rxvq) +
 				rte_virtio_rxq_stat_strings[t].offset);
+			xstats[count].id = count;
 			count++;
 		}
 	}
@@ -908,6 +909,7 @@ virtio_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		for (t = 0; t < VIRTIO_NB_TXQ_XSTATS; t++) {
 			xstats[count].value = *(uint64_t *)(((char *)txvq) +
 				rte_virtio_txq_stat_strings[t].offset);
+			xstats[count].id = count;
 			count++;
 		}
 	}
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 1e0f206..45d8286 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1491,8 +1491,11 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
 		}
 	}
 
-	for (i = 0; i < count + xcount; i++)
+	for (i = 0; i < count; i++)
 		xstats[i].id = i;
+	/* add an offset to driver-specific stats */
+	for ( ; i < count + xcount; i++)
+		xstats[i].id += count;
 
 	return count + xcount;
 }
-- 
2.8.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH 2/2] ethdev: clarify xstats Api documentation
  2016-12-16  9:44 [dpdk-dev] [PATCH 1/2] ethdev: fix name index in xstats Api Olivier Matz
@ 2016-12-16  9:44 ` Olivier Matz
  2016-12-16 14:28   ` [dpdk-dev] [dpdk-stable] " Mcnamara, John
  2016-12-23 20:35   ` [dpdk-dev] [PATCH v2 " Olivier Matz
  2017-01-03 10:03 ` [dpdk-dev] [PATCH 1/2] ethdev: fix name index in xstats Api Remy Horton
  1 sibling, 2 replies; 8+ messages in thread
From: Olivier Matz @ 2016-12-16  9:44 UTC (permalink / raw)
  To: dev, thomas.monjalon; +Cc: remy.horton, stable

Reword the Api documentation of xstats ethdev.

CC: stable@dpdk.org
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_ether/rte_ethdev.h | 45 ++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9678179..4479553 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -938,23 +938,26 @@ struct rte_eth_txq_info {
 /**
  * An Ethernet device extended statistic structure
  *
- * This structure is used by ethdev->eth_xstats_get() to provide
- * statistics that are not provided in the generic rte_eth_stats
+ * This structure is used by rte_eth_xstats_get() to provide
+ * statistics that are not provided in the generic *rte_eth_stats*
  * structure.
+ * It maps a name id, corresponding to an index in the array returned
+ * by rte_eth_xstats_get_names(), to a statistic value.
  */
 struct rte_eth_xstat {
-	uint64_t id;
-	uint64_t value;
+	uint64_t id;        /**< The index in xstats name array. */
+	uint64_t value;     /**< The statistic counter value. */
 };
 
 /**
- * A name-key lookup element for extended statistics.
+ * A name element for extended statistics.
  *
- * This structure is used to map between names and ID numbers
- * for extended ethernet statistics.
+ * An array of this structure is returned by rte_eth_xstats_get_names().
+ * It lists the names of extended statistics for a PMD. The *rte_eth_xstat*
+ * structure references these names by their array index.
  */
 struct rte_eth_xstat_name {
-	char name[RTE_ETH_XSTATS_NAME_SIZE];
+	char name[RTE_ETH_XSTATS_NAME_SIZE]; /**< The statistic name. */
 };
 
 #define ETH_DCB_NUM_TCS    8
@@ -2272,18 +2275,19 @@ void rte_eth_stats_reset(uint8_t port_id);
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param xstats_names
- *  Block of memory to insert names into. Must be at least size in capacity.
- *  If set to NULL, function returns required capacity.
+ *   An rte_eth_xstat_name array of at least *size* elements to
+ *   be filled. If set to NULL, the function returns the required number
+ *   of elements.
  * @param size
- *  Capacity of xstats_names (number of names).
+ *   The size of the xstats_names array (number of elements).
  * @return
- *   - positive value lower or equal to size: success. The return value
+ *   - A 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
+ *   - A 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)
+ *   - A negative value on error (invalid port id).
  */
 int rte_eth_xstats_get_names(uint8_t port_id,
 		struct rte_eth_xstat_name *xstats_names,
@@ -2296,19 +2300,20 @@ int rte_eth_xstats_get_names(uint8_t port_id,
  *   The port identifier of the Ethernet device.
  * @param xstats
  *   A pointer to a table of structure of type *rte_eth_xstat*
- *   to be filled with device statistics ids and values.
+ *   to be filled with device statistics ids and values: id is the
+ *   index of the name string in xstats_names (@see rte_eth_xstats_get_names),
+ *   and value is the statistic counter.
  *   This parameter can be set to NULL if n is 0.
  * @param n
- *   The size of the stats table, which should be large enough to store
- *   all the statistics of the device.
+ *   The size of the xstats array (number of elements).
  * @return
- *   - positive value lower or equal to n: success. The return value
+ *   - A positive value lower or equal to n: success. The return value
  *     is the number of entries filled in the stats table.
- *   - positive value higher than n: error, the given statistics table
+ *   - A positive value higher than n: 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)
+ *   - A negative value on error (invalid port id).
  */
 int rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
 		unsigned n);
-- 
2.8.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] ethdev: clarify xstats Api documentation
  2016-12-16  9:44 ` [dpdk-dev] [PATCH 2/2] ethdev: clarify xstats Api documentation Olivier Matz
@ 2016-12-16 14:28   ` Mcnamara, John
  2016-12-16 14:36     ` Olivier Matz
  2016-12-23 20:35   ` [dpdk-dev] [PATCH v2 " Olivier Matz
  1 sibling, 1 reply; 8+ messages in thread
From: Mcnamara, John @ 2016-12-16 14:28 UTC (permalink / raw)
  To: Olivier Matz, dev, thomas.monjalon; +Cc: Horton, Remy, stable

> -----Original Message-----
> From: stable [mailto:stable-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Friday, December 16, 2016 9:44 AM
> To: dev@dpdk.org; thomas.monjalon@6wind.com
> Cc: Horton, Remy <remy.horton@intel.com>; stable@dpdk.org
> Subject: [dpdk-stable] [PATCH 2/2] ethdev: clarify xstats Api
> documentation
> 
> Reword the Api documentation of xstats ethdev.
> 
> CC: stable@dpdk.org
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  lib/librte_ether/rte_ethdev.h | 45 ++++++++++++++++++++++++--------------
> -----
>  ...
>  int rte_eth_xstats_get_names(uint8_t port_id,
>  		struct rte_eth_xstat_name *xstats_names, @@ -2296,19 +2300,20
> @@ int rte_eth_xstats_get_names(uint8_t port_id,
>   *   The port identifier of the Ethernet device.
>   * @param xstats
>   *   A pointer to a table of structure of type *rte_eth_xstat*
> - *   to be filled with device statistics ids and values.
> + *   to be filled with device statistics ids and values: id is the
> + *   index of the name string in xstats_names (@see rte_eth_xstats_get_names),

The @see directive starts a new "See also" section and breaks/interrupts the 
parameter description. Probably what you want is:

    index of the name string in xstats_names (see rte_eth_xstats_get_names()), 

Otherwise it is a good update.

John

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] ethdev: clarify xstats Api documentation
  2016-12-16 14:28   ` [dpdk-dev] [dpdk-stable] " Mcnamara, John
@ 2016-12-16 14:36     ` Olivier Matz
  0 siblings, 0 replies; 8+ messages in thread
From: Olivier Matz @ 2016-12-16 14:36 UTC (permalink / raw)
  To: Mcnamara, John; +Cc: dev, thomas.monjalon, Horton, Remy, stable

Hi John,

On Fri, 16 Dec 2016 14:28:21 +0000, "Mcnamara, John"
<john.mcnamara@intel.com> wrote:
> > -----Original Message-----
> > From: stable [mailto:stable-bounces@dpdk.org] On Behalf Of Olivier
> > Matz Sent: Friday, December 16, 2016 9:44 AM
> > To: dev@dpdk.org; thomas.monjalon@6wind.com
> > Cc: Horton, Remy <remy.horton@intel.com>; stable@dpdk.org
> > Subject: [dpdk-stable] [PATCH 2/2] ethdev: clarify xstats Api
> > documentation
> > 
> > Reword the Api documentation of xstats ethdev.
> > 
> > CC: stable@dpdk.org
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> >  lib/librte_ether/rte_ethdev.h | 45
> > ++++++++++++++++++++++++-------------- -----
> >  ...
> >  int rte_eth_xstats_get_names(uint8_t port_id,
> >  		struct rte_eth_xstat_name *xstats_names, @@
> > -2296,19 +2300,20 @@ int rte_eth_xstats_get_names(uint8_t port_id,
> >   *   The port identifier of the Ethernet device.
> >   * @param xstats
> >   *   A pointer to a table of structure of type *rte_eth_xstat*
> > - *   to be filled with device statistics ids and values.
> > + *   to be filled with device statistics ids and values: id is the
> > + *   index of the name string in xstats_names (@see
> > rte_eth_xstats_get_names),  
> 
> The @see directive starts a new "See also" section and
> breaks/interrupts the parameter description. Probably what you want
> is:
> 
>     index of the name string in xstats_names (see
> rte_eth_xstats_get_names()), 
> 
> Otherwise it is a good update.
> 
> John

Thank you for the review. I'll send a new version of the patch
addressing this.


Regards,
Olivier

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH v2 2/2] ethdev: clarify xstats Api documentation
  2016-12-16  9:44 ` [dpdk-dev] [PATCH 2/2] ethdev: clarify xstats Api documentation Olivier Matz
  2016-12-16 14:28   ` [dpdk-dev] [dpdk-stable] " Mcnamara, John
@ 2016-12-23 20:35   ` Olivier Matz
  2017-01-03 10:03     ` Remy Horton
  1 sibling, 1 reply; 8+ messages in thread
From: Olivier Matz @ 2016-12-23 20:35 UTC (permalink / raw)
  To: dev, thomas.monjalon; +Cc: remy.horton, john.mcnamara, stable

Reword the Api documentation of xstats ethdev.

CC: stable@dpdk.org
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---

v1-> v2:
- replace "@see rte_eth_xstats_get_names" by "see rte_eth_xstats_get_names()"
  as suggested by John

 lib/librte_ether/rte_ethdev.h | 45 ++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 272fd41..e4796e9 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -938,23 +938,26 @@ struct rte_eth_txq_info {
 /**
  * An Ethernet device extended statistic structure
  *
- * This structure is used by ethdev->eth_xstats_get() to provide
- * statistics that are not provided in the generic rte_eth_stats
+ * This structure is used by rte_eth_xstats_get() to provide
+ * statistics that are not provided in the generic *rte_eth_stats*
  * structure.
+ * It maps a name id, corresponding to an index in the array returned
+ * by rte_eth_xstats_get_names(), to a statistic value.
  */
 struct rte_eth_xstat {
-	uint64_t id;
-	uint64_t value;
+	uint64_t id;        /**< The index in xstats name array. */
+	uint64_t value;     /**< The statistic counter value. */
 };
 
 /**
- * A name-key lookup element for extended statistics.
+ * A name element for extended statistics.
  *
- * This structure is used to map between names and ID numbers
- * for extended ethernet statistics.
+ * An array of this structure is returned by rte_eth_xstats_get_names().
+ * It lists the names of extended statistics for a PMD. The *rte_eth_xstat*
+ * structure references these names by their array index.
  */
 struct rte_eth_xstat_name {
-	char name[RTE_ETH_XSTATS_NAME_SIZE];
+	char name[RTE_ETH_XSTATS_NAME_SIZE]; /**< The statistic name. */
 };
 
 #define ETH_DCB_NUM_TCS    8
@@ -2281,18 +2284,19 @@ void rte_eth_stats_reset(uint8_t port_id);
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param xstats_names
- *  Block of memory to insert names into. Must be at least size in capacity.
- *  If set to NULL, function returns required capacity.
+ *   An rte_eth_xstat_name array of at least *size* elements to
+ *   be filled. If set to NULL, the function returns the required number
+ *   of elements.
  * @param size
- *  Capacity of xstats_names (number of names).
+ *   The size of the xstats_names array (number of elements).
  * @return
- *   - positive value lower or equal to size: success. The return value
+ *   - A 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
+ *   - A 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)
+ *   - A negative value on error (invalid port id).
  */
 int rte_eth_xstats_get_names(uint8_t port_id,
 		struct rte_eth_xstat_name *xstats_names,
@@ -2305,19 +2309,20 @@ int rte_eth_xstats_get_names(uint8_t port_id,
  *   The port identifier of the Ethernet device.
  * @param xstats
  *   A pointer to a table of structure of type *rte_eth_xstat*
- *   to be filled with device statistics ids and values.
+ *   to be filled with device statistics ids and values: id is the
+ *   index of the name string in xstats_names (see rte_eth_xstats_get_names()),
+ *   and value is the statistic counter.
  *   This parameter can be set to NULL if n is 0.
  * @param n
- *   The size of the stats table, which should be large enough to store
- *   all the statistics of the device.
+ *   The size of the xstats array (number of elements).
  * @return
- *   - positive value lower or equal to n: success. The return value
+ *   - A positive value lower or equal to n: success. The return value
  *     is the number of entries filled in the stats table.
- *   - positive value higher than n: error, the given statistics table
+ *   - A positive value higher than n: 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)
+ *   - A negative value on error (invalid port id).
  */
 int rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
 		unsigned n);
-- 
2.8.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] ethdev: fix name index in xstats Api
  2016-12-16  9:44 [dpdk-dev] [PATCH 1/2] ethdev: fix name index in xstats Api Olivier Matz
  2016-12-16  9:44 ` [dpdk-dev] [PATCH 2/2] ethdev: clarify xstats Api documentation Olivier Matz
@ 2017-01-03 10:03 ` Remy Horton
  1 sibling, 0 replies; 8+ messages in thread
From: Remy Horton @ 2017-01-03 10:03 UTC (permalink / raw)
  To: Olivier Matz, dev, thomas.monjalon; +Cc: stable

Been away, hence the somewhat late review..

On 16/12/2016 09:44, Olivier Matz wrote:
[..]
> Today, each 'id' returned by rte_eth_xstats_get() is equal to the index
> in the returned array, making this value useless. It also prevents a
> driver from having different indexes for names and value, like in the
> example below:

My original intention was to give free reign over what id numbers are 
used, but for reasons I've now forgotten the implementation ended up 
making everything sequential.

> CC: stable@dpdk.org
> Fixes: bd6aa172cf35 ("ethdev: fetch extended statistics with integer ids")
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

Acked-by: Remy Horton <remy.horton@intel.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/2] ethdev: clarify xstats Api documentation
  2016-12-23 20:35   ` [dpdk-dev] [PATCH v2 " Olivier Matz
@ 2017-01-03 10:03     ` Remy Horton
  2017-01-04 18:10       ` Thomas Monjalon
  0 siblings, 1 reply; 8+ messages in thread
From: Remy Horton @ 2017-01-03 10:03 UTC (permalink / raw)
  To: Olivier Matz, dev, thomas.monjalon; +Cc: john.mcnamara, stable


On 23/12/2016 20:35, Olivier Matz wrote:
> Reword the Api documentation of xstats ethdev.
>
> CC: stable@dpdk.org
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

Acked-by: Remy Horton <remy.horton@intel.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/2] ethdev: clarify xstats Api documentation
  2017-01-03 10:03     ` Remy Horton
@ 2017-01-04 18:10       ` Thomas Monjalon
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2017-01-04 18:10 UTC (permalink / raw)
  To: Olivier Matz; +Cc: Remy Horton, dev, john.mcnamara, stable

2017-01-03 10:03, Remy Horton:
> 
> On 23/12/2016 20:35, Olivier Matz wrote:
> > Reword the Api documentation of xstats ethdev.
> >
> > CC: stable@dpdk.org
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Acked-by: Remy Horton <remy.horton@intel.com>

Series applied, thanks

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-01-04 18:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-16  9:44 [dpdk-dev] [PATCH 1/2] ethdev: fix name index in xstats Api Olivier Matz
2016-12-16  9:44 ` [dpdk-dev] [PATCH 2/2] ethdev: clarify xstats Api documentation Olivier Matz
2016-12-16 14:28   ` [dpdk-dev] [dpdk-stable] " Mcnamara, John
2016-12-16 14:36     ` Olivier Matz
2016-12-23 20:35   ` [dpdk-dev] [PATCH v2 " Olivier Matz
2017-01-03 10:03     ` Remy Horton
2017-01-04 18:10       ` Thomas Monjalon
2017-01-03 10:03 ` [dpdk-dev] [PATCH 1/2] ethdev: fix name index in xstats Api Remy Horton

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).