All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] ethdev: fix name index in xstats Api
@ 2016-12-16  9:44 Olivier Matz
  2016-12-16  9:44 ` [PATCH 2/2] ethdev: clarify xstats Api documentation Olivier Matz
  2017-01-03 10:03 ` [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 related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] ethdev: clarify xstats Api documentation
  2016-12-16  9:44 [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-stable] " Mcnamara, John
  2016-12-23 20:35   ` [PATCH v2 " Olivier Matz
  2017-01-03 10:03 ` [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 related	[flat|nested] 8+ messages in thread

* Re: [dpdk-stable] [PATCH 2/2] ethdev: clarify xstats Api documentation
  2016-12-16  9:44 ` [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   ` [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-stable] [PATCH 2/2] ethdev: clarify xstats Api documentation
  2016-12-16 14:28   ` [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

* [PATCH v2 2/2] ethdev: clarify xstats Api documentation
  2016-12-16  9:44 ` [PATCH 2/2] ethdev: clarify xstats Api documentation Olivier Matz
  2016-12-16 14:28   ` [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 related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] ethdev: fix name index in xstats Api
  2016-12-16  9:44 [PATCH 1/2] ethdev: fix name index in xstats Api Olivier Matz
  2016-12-16  9:44 ` [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: [PATCH v2 2/2] ethdev: clarify xstats Api documentation
  2016-12-23 20:35   ` [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: [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 [PATCH 1/2] ethdev: fix name index in xstats Api Olivier Matz
2016-12-16  9:44 ` [PATCH 2/2] ethdev: clarify xstats Api documentation Olivier Matz
2016-12-16 14:28   ` [dpdk-stable] " Mcnamara, John
2016-12-16 14:36     ` Olivier Matz
2016-12-23 20:35   ` [PATCH v2 " Olivier Matz
2017-01-03 10:03     ` Remy Horton
2017-01-04 18:10       ` Thomas Monjalon
2017-01-03 10:03 ` [PATCH 1/2] ethdev: fix name index in xstats Api Remy Horton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.