ethdev: document xstats API assumption

Message ID 20180628141602.91695-1-ferruh.yigit@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: document xstats API assumption |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Ferruh Yigit June 28, 2018, 2:16 p.m. UTC
  Documents the assumption that 'xstats[i].id == i' and
key=xstats_names[i].name, value=xstats[i].value

xstats[i].id is still used for xstats _by_id() APIs.

This patch reverts some part of the commit 6d52d1d4afb4 ("ethdev:
clarify extended statistics documentation")

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: David Marchand <david.marchand@6wind.com>
Cc: Olivier Matz <olivier.matz@6wind.com>
Cc: Remy Horton <remy.horton@intel.com>
---
 lib/librte_ethdev/rte_ethdev.h | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)
  

Comments

David Marchand June 29, 2018, 7:15 a.m. UTC | #1
On Thu, Jun 28, 2018 at 4:16 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> Documents the assumption that 'xstats[i].id == i' and
> key=xstats_names[i].name, value=xstats[i].value
>
> xstats[i].id is still used for xstats _by_id() APIs.
>
> This patch reverts some part of the commit 6d52d1d4afb4 ("ethdev:
> clarify extended statistics documentation")
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Reviewed-by: David Marchand <david.marchand@6wind.com>

So my testpmd patch can be dropped.
  
Van Haaren, Harry July 3, 2018, 4:11 p.m. UTC | #2
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Thursday, June 28, 2018 3:16 PM
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; David Marchand
> <david.marchand@6wind.com>; Olivier Matz <olivier.matz@6wind.com>; Horton,
> Remy <remy.horton@intel.com>
> Subject: [dpdk-dev] [PATCH] ethdev: document xstats API assumption
> 
> Documents the assumption that 'xstats[i].id == i' and
> key=xstats_names[i].name, value=xstats[i].value
> 
> xstats[i].id is still used for xstats _by_id() APIs.
> 
> This patch reverts some part of the commit 6d52d1d4afb4 ("ethdev:
> clarify extended statistics documentation")
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

I'm also in favor of simplifying xstats APIs if possible, given we have a number of PMDs using it, and they all adhere to this simplification;

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
  
Ferruh Yigit July 4, 2018, 5:38 p.m. UTC | #3
On 6/29/2018 8:15 AM, David Marchand wrote:
> On Thu, Jun 28, 2018 at 4:16 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>> Documents the assumption that 'xstats[i].id == i' and
>> key=xstats_names[i].name, value=xstats[i].value
>>
>> xstats[i].id is still used for xstats _by_id() APIs.
>>
>> This patch reverts some part of the commit 6d52d1d4afb4 ("ethdev:
>> clarify extended statistics documentation")
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Reviewed-by: David Marchand <david.marchand@6wind.com>

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

Applied to dpdk-next-net/master, thanks.
  
Andrew Rybchenko July 7, 2018, 5:45 p.m. UTC | #4
On 28.06.2018 17:16, Ferruh Yigit wrote:
> Documents the assumption that 'xstats[i].id == i' and
> key=xstats_names[i].name, value=xstats[i].value
>
> xstats[i].id is still used for xstats _by_id() APIs.
>
> This patch reverts some part of the commit 6d52d1d4afb4 ("ethdev:
> clarify extended statistics documentation")
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
  

Patch

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 5760f45d3..261dabafb 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -2008,6 +2008,15 @@  int rte_eth_stats_reset(uint16_t port_id);
 /**
  * Retrieve names of extended statistics of an Ethernet device.
  *
+ * There is an assumption that 'xstat_names' and 'xstats' arrays are matched
+ * by array index:
+ *  xstats_names[i].name => xstats[i].value
+ *
+ * And the array index is same with id field of 'struct rte_eth_xstat':
+ *  xstats[i].id == i
+ *
+ * This assumption makes key-value pair matching less flexible but simpler.
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param xstats_names
@@ -2032,13 +2041,20 @@  int rte_eth_xstats_get_names(uint16_t port_id,
 /**
  * Retrieve extended statistics of an Ethernet device.
  *
+ * There is an assumption that 'xstat_names' and 'xstats' arrays are matched
+ * by array index:
+ *  xstats_names[i].name => xstats[i].value
+ *
+ * And the array index is same with id field of 'struct rte_eth_xstat':
+ *  xstats[i].id == i
+ *
+ * This assumption makes key-value pair matching less flexible but simpler.
+ *
  * @param 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: id is the
- *   index of the name string in xstats_names (see rte_eth_xstats_get_names()),
- *   and value is the statistic counter.
+ *   to be filled with device statistics ids and values.
  *   This parameter can be set to NULL if n is 0.
  * @param n
  *   The size of the xstats array (number of elements).