From: "David Harton (dharton)" <dharton@cisco.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: "dev@dpdk.org" <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 21:27:35 +0000 [thread overview]
Message-ID: <840715e081a542788c217ef639dac766@XCH-RCD-016.cisco.com> (raw)
In-Reply-To: <3459676.trPQTB1ucG@xps>
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, August 23, 2017 9:24 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 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 :)
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
next prev parent reply other threads:[~2017-08-23 21:27 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
2017-08-23 21:27 ` David Harton (dharton) [this message]
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=840715e081a542788c217ef639dac766@XCH-RCD-016.cisco.com \
--to=dharton@cisco.com \
--cc=dev@dpdk.org \
--cc=thomas@monjalon.net \
/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).