DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] ethdev: additional parameter in RX callback
Date: Fri, 13 Mar 2015 09:41:33 +0000	[thread overview]
Message-ID: <20150313094133.GA5056@bricha3-MOBL3> (raw)
In-Reply-To: <20150312191540.GB15260@hmsreliant.think-freely.org>

On Thu, Mar 12, 2015 at 03:15:40PM -0400, Neil Horman wrote:
> On Thu, Mar 12, 2015 at 04:54:27PM +0000, John McNamara wrote:
> > 
> > This patch is a minor extension to the recent patchset for RX/TX callbacks
> > based on feedback from users implementing solutions based on it.
> > 
> > The patch adds a new parameter to the RX callback to pass in the number of
> > available RX packets in addition to the number of dequeued packets.
> > This provides the RX callback functions with additional information
> > that can be used to decide how packets from a burst are handled.
> > 
> > The TX callback doesn't require this additional parameter so the RX
> > and TX callbacks no longer have the same function parameters. As such
> > the single RX/TX callback has been refactored into two separate callbacks.
> > 
> > Since this is an API change we hope it can be included in 2.0.0 to avoid
> > changing the API in a subsequent release.    
> > 
> > 
> > John McNamara (1):
> >   ethdev: added additional packet count parameter to RX callbacks
> > 
> >  examples/rxtx_callbacks/main.c |    3 +-
> >  lib/librte_ether/rte_ethdev.c  |    8 ++--
> >  lib/librte_ether/rte_ethdev.h  |   74 ++++++++++++++++++++++++++--------------
> >  3 files changed, 54 insertions(+), 31 deletions(-)
> > 
> > -- 
> > 1.7.4.1
> > 
> > 
> 
> 
> Well, we're well past the new feature phase of this cycle, so I would say NACK.
> I would also suggest that you don't need to modify ABI to accomodate this
> feature.  Instead just document the pkts array to be terminated by a reserved
> value, so that the callback can determine its size dynamically.  You could
> alternatively create a new api call that allows you to retrieve that information
> from the context of the callback.
> 
> Neil
> 

Yes, I would agree we are past the new feature phase. However, given that we
are making a change to the API, and a fairly small change too - adding one extra
parameter - we think that the benefit of including this now outweighs any risk
of merging the patch. It seems a bit crazy to ship a release with a new API and
then immediately change the API straight after release. Is it not better to
take the received feedback on the API and fix/improve it pre-release before it
gets set-in-stone?

/Bruce

  parent reply	other threads:[~2015-03-13  9:41 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-12 16:54 John McNamara
2015-03-12 16:54 ` [dpdk-dev] [PATCH] ethdev: added additional packet count parameter to RX callbacks John McNamara
2015-03-12 19:15 ` [dpdk-dev] [PATCH] ethdev: additional parameter in RX callback Neil Horman
2015-03-12 23:24   ` Mcnamara, John
2015-03-13  9:41   ` Bruce Richardson [this message]
2015-03-13 13:45     ` Neil Horman
2015-03-13 14:50       ` Bruce Richardson
2015-03-13 15:09         ` Neil Horman
2015-03-13 16:26           ` Mcnamara, John
2015-03-13 17:31             ` Neil Horman
2015-03-13 18:28               ` Mcnamara, John
2015-03-13 23:15                 ` Neil Horman
2015-03-23 15:16                   ` Thomas Monjalon
2015-03-23 15:29                     ` Bruce Richardson
2015-03-23 16:00                     ` Neil Horman
2015-03-30 19:52                       ` 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=20150313094133.GA5056@bricha3-MOBL3 \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=nhorman@tuxdriver.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).