From: Thomas Monjalon <thomas@monjalon.net>
To: "David Harton (dharton)" <dharton@cisco.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2 1/2] ethdev: stop overriding rx_nombuf by rte_eth_stats_get
Date: Wed, 23 Aug 2017 15:24:12 +0200 [thread overview]
Message-ID: <3459676.trPQTB1ucG@xps> (raw)
In-Reply-To: <fea2a2ca77ce4d03963dfd6702641e38@XCH-RCD-016.cisco.com>
23/08/2017 14:18, David Harton (dharton):
>
> > -----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?
Not asking, just giving my thought :)
> 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
next prev parent reply other threads:[~2017-08-23 13:24 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-23 1:19 [dpdk-dev] [PATCH] " David Harton
2017-08-23 2:55 ` [dpdk-dev] [PATCH v2 1/2] " David Harton
2017-08-23 7:51 ` Thomas Monjalon
2017-08-23 12:18 ` David Harton (dharton)
2017-08-23 13:24 ` Thomas Monjalon [this message]
2017-08-23 21:27 ` David Harton (dharton)
2017-08-23 21:35 ` Thomas Monjalon
2017-08-23 21:56 ` Stephen Hemminger
2017-08-23 22:19 ` David Harton (dharton)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3459676.trPQTB1ucG@xps \
--to=thomas@monjalon.net \
--cc=dev@dpdk.org \
--cc=dharton@cisco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).