Re: [PATCH v2 1/2] ethdev: stop overriding rx_nombuf by rte_eth_stats_get

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 22 Aug 2017 22:55:55 -0400
David Harton <dharton@cisco.com> wrote:

> rte_eth_stats_get() unconditonally would set rx_nombuf
> even if the device was setting the value.  A check has
> been added in rte_eth_stats_get() to leave the device
> value in-tact when non-zero.
> 
> Signed-off-by: David Harton <dharton@cisco.com>
> ---
> 
> v2: Fixed braces complaint required by other coding standards.
> 
>  lib/librte_ether/rte_ethdev.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 0597641..0a1d3b8 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -1336,8 +1336,11 @@ struct rte_eth_dev *
>  	memset(stats, 0, sizeof(*stats));
>  
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->stats_get, -ENOTSUP);
> -	stats->rx_nombuf = dev->data->rx_mbuf_alloc_failed;
>  	(*dev->dev_ops->stats_get)(dev, stats);
> +	/* only set rx_nombuf if not set by the device */
> +	if (!stats->rx_nombuf)
> +		stats->rx_nombuf = dev->data->rx_mbuf_alloc_failed;
> +
>  	return 0;
>  }
>  

This seems backwards. It seems like the original way worked fine.
If device specific code wanted to override rx_nombuf it could do so either
by adding it's additional value or just setting rx_nombuf.

Adding special cases seems like it would start a bad precedent and the
could would end up quite complex as some values had one semantic and others
were only from driver.