[v2,1/9] ethdev: define retval when xstats is null of get xstats

Message ID 20220428131600.41032-2-fengchengwen@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Andrew Rybchenko
Headers
Series bugfix for ethdev telemetry |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

fengchengwen April 28, 2022, 1:15 p.m. UTC
  Currently the value returned when xstats is NULL of rte_eth_xstats_get()
is not specified, some PMDs (eg. hns3/ipn3ke/mvpp2/axgbe) return zero
while others return the required number of elements.

This patch defines that the return value should be the required number of
elements when xstats is NULL of rte_eth_xstats_get().

Fixes: ce757f5c9a4d ("ethdev: new method to retrieve extended statistics")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Andrew Rybchenko May 4, 2022, 10:32 a.m. UTC | #1
On 4/28/22 16:15, Chengwen Feng wrote:
> Currently the value returned when xstats is NULL of rte_eth_xstats_get()
> is not specified, some PMDs (eg. hns3/ipn3ke/mvpp2/axgbe) return zero
> while others return the required number of elements.
> 
> This patch defines that the return value should be the required number of
> elements when xstats is NULL of rte_eth_xstats_get().
> 
> Fixes: ce757f5c9a4d ("ethdev: new method to retrieve extended statistics")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>   lib/ethdev/rte_ethdev.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 04cff8ee10..0b18297c95 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -3174,7 +3174,7 @@ int rte_eth_xstats_get_names(uint16_t port_id,
>    * @param xstats
>    *   A pointer to a table of structure of type *rte_eth_xstat*
>    *   to be filled with device statistics ids and values.
> - *   This parameter can be set to NULL if n is 0.
> + *   If set to NULL, the function returns the required number of elements.

I'm sorry, but I disagree with the patch. First of all it
removes limitation when xstats may be NULL. Second, I think
that clarification is not required since:
if xstats is NULL, n must be 0 as defined above and return
value description says:
3183  *   - A positive value higher than n: error, the given statistics 
table
3184  *     is too small. The return value corresponds to the size that 
should
3185  *     be given to succeed. The entries in the table are not valid and
3186  *     shall not be used by the caller.


>    * @param n
>    *   The size of the xstats array (number of elements).
>    * @return
  

Patch

diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 04cff8ee10..0b18297c95 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -3174,7 +3174,7 @@  int rte_eth_xstats_get_names(uint16_t port_id,
  * @param xstats
  *   A pointer to a table of structure of type *rte_eth_xstat*
  *   to be filled with device statistics ids and values.
- *   This parameter can be set to NULL if n is 0.
+ *   If set to NULL, the function returns the required number of elements.
  * @param n
  *   The size of the xstats array (number of elements).
  * @return