DPDK patches and discussions
 help / color / mirror / Atom feed
From: "David Harton (dharton)" <dharton@cisco.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>,
	"Van Haaren, Harry" <harry.van.haaren@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"Tahhan, Maryam" <maryam.tahhan@intel.com>,
	 "olivier.matz@6wind.com" <olivier.matz@6wind.com>
Subject: Re: [dpdk-dev] [PATCH] doc: announce xstats api change for 16.07
Date: Wed, 6 Apr 2016 13:49:28 +0000	[thread overview]
Message-ID: <9edf0e7ca1854e7cbb013359cb6423a8@XCH-RCD-016.cisco.com> (raw)
In-Reply-To: <23174662.vdJtoqjRUU@xps13>



> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, April 06, 2016 8:14 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: David Harton (dharton) <dharton@cisco.com>; dev@dpdk.org; Tahhan,
> Maryam <maryam.tahhan@intel.com>; olivier.matz@6wind.com
> Subject: Re: [dpdk-dev] [PATCH] doc: announce xstats api change for 16.07
> 
> 2016-04-06 11:16, Van Haaren, Harry:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > The issue we are going to fix is that currently PMDs copy strings
> > > > when retrieving
> > > statistics, which causes unnecessary overhead. The implementation is
> > > not decided yet, but using an int->value mapping seems logical.
> >
> > > I am not sure performance is so much critical when retrieving
> statistics.
> >
> > In the previous discussion David was concerned about performance
> > impact of string copies, are those concerns still present David?
> >
> > > The extended stats can be infinitely extended. So a string
> > > identifier seems a lot more natural.
> >
> > I'm not suggesting that the string identifier is removed totally.
> >
> > > I do not agree to add a new numeric identifier in the API each time
> > > a driver wants to report a specific statistic for debugging purpose.
> >
> > And I agree - the ints are just an index to xstats arrays, no eth-dev
> wide enums here.

Yes, I abandoned the idea of a set of stats ids.  I can see where registration will be problematic and cumbersome to driver developers.

> > The proposal is to make the API more flexible, see example:
> > http://thread.gmane.org/gmane.comp.networking.dpdk.devel/31728/focus=3
> > 2795
> >
> > This more flexible API would allow other types of information about
> > statistics be retrieved too.

I have prototyped this.  If there is interest/acceptance I can work on making an official patch to share back to the community.

Using this method still gives the flexibility the current API desires while giving the user the control to only obtain the counters.  This of course assumes that the counters per device are static but that seems a safe bet.

> 
> OK I think I start to understand.
> 
> > For now, the sent patch announces that the API/ABI may change, and we
> > can discuss details of API as development starts.
> 
> This should not be the normal process.
> It is important to understand what should be the changes to decide of
> announcing or not a deprecation.
> In the case of the mempool reworks, the patch have been sent and discussed
> on the mailing list.
> Given the previous explanations (and knowing you did good job on stats), I
> give my
> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Thanks for considering this.

Regards,
Dave

  reply	other threads:[~2016-04-06 13:49 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-05 17:58 Harry van Haaren
2016-04-05 18:45 ` Thomas Monjalon
2016-04-06  9:02   ` Van Haaren, Harry
2016-04-06  9:22     ` Thomas Monjalon
2016-04-06 11:16       ` Van Haaren, Harry
2016-04-06 12:14         ` Thomas Monjalon
2016-04-06 13:49           ` David Harton (dharton) [this message]
2016-04-06 13:53             ` Van Haaren, Harry
2016-04-06 13:46 ` Remy Horton
2016-04-06 14:00 ` David Harton (dharton)
2016-04-06 14:25 ` Tahhan, Maryam
2016-04-07 21:36   ` Thomas Monjalon

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=9edf0e7ca1854e7cbb013359cb6423a8@XCH-RCD-016.cisco.com \
    --to=dharton@cisco.com \
    --cc=dev@dpdk.org \
    --cc=harry.van.haaren@intel.com \
    --cc=maryam.tahhan@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=thomas.monjalon@6wind.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).