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

["David Harton (dharton)" <dharton@cisco.com>]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, August 23, 2017 3:52 AM
> To: David Harton (dharton) <dharton@cisco.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v2 1/2] ethdev: stop overriding rx_nombuf by
> rte_eth_stats_get
> 
> 23/08/2017 04:55, David Harton:
> > 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.
> 
> If we get this counter from stats->rx_nombuf, why keeping
> dev->data->rx_mbuf_alloc_failed ?
> We could rework other PMDs to not use this global variable.
> It is inconsistent to use it for some PMDs but not all.
> And it seems not used outside of PMDs.

Are you also asking to remove dev->data->rx_mbuf_alloc_failed as well since we will have an ABI breakage anyway?

On an somewhat related note, since we are introducing an ABI breakage how do you feel about re-adding the return code for the vlan_offload_set vector?  Some devices conditionally provide the ability to modify some offload and the caller should know.  Since I've got your attention thought I'd ask here before posting the patch.

<soapbox>
In fact, I believe all the API function calls should provide a return code to help mitigate ABI breakages and also provide the ability to let the caller distinguish between - no device, not supported and some other error.  A control plane often needs to understand these distinctions to properly orchestrate the system and/or report real errors.  This is more than I'm willing to take on myself but believe it's a principle I'd like to discuss (can start separate thread if desired).
</soapbox>