> -----Original Message----- > From: Thomas Monjalon [mailto:tho...@monjalon.net] > Sent: Wednesday, August 23, 2017 9:24 AM > To: David Harton (dharton) <dhar...@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 14:18, David Harton (dharton): > > > > > -----Original Message----- > > > From: Thomas Monjalon [mailto:tho...@monjalon.net] > > > Sent: Wednesday, August 23, 2017 3:52 AM > > > To: David Harton (dharton) <dhar...@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? > > Not asking, just giving my thought :)
I did some more digging. For this count it looks like some devices: - have their own internal version - have a count shared with the pf - rely on this field to maintain the count - don't count this failure at all :( With that said I'd like to keep with the requested changes. Thoughts? Dave > > > 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. > > Seems reasonnable > > > <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> > > Yes you're right