DPDK patches and discussions
 help / color / mirror / Atom feed
From: Neil Horman <nhorman@tuxdriver.com>
To: "Mcnamara, John" <john.mcnamara@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] ethdev: additional parameter in RX callback
Date: Fri, 13 Mar 2015 19:15:26 -0400	[thread overview]
Message-ID: <20150313231526.GA14641@neilslaptop.think-freely.org> (raw)
In-Reply-To: <B27915DBBA3421428155699D51E4CFE2ECEC0A@IRSMSX103.ger.corp.intel.com>

On Fri, Mar 13, 2015 at 06:28:31PM +0000, Mcnamara, John wrote:
> 
> 
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Friday, March 13, 2015 5:32 PM
> > To: Mcnamara, John
> > Cc: Richardson, Bruce; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] ethdev: additional parameter in RX
> > callback
> > 
> > > Is encoding the information in the array really a better solution here?
> > The cb->param already exists for passing in user defined information to
> > the callback. The proposed patch merely transmits the parent function
> > arguments to the enclosed callback.
> > >
> > The cb->param can't be used here, because its opaque to the internals of
> > the DPDK.  rte_eth_rx_burst doesn't (and can't) know where in the cb-
> > >params pointer to store that information.  Thats why you added an
> > additional parameter in the first place, isn't it?
> 
> Yes. That is correct.
> 
Then why did you suggest doing so?

> > My point is that using
> > an array terminator keeps us out of this habbit of just adding parameters
> > to communicate more information (as thats an ABI breaking method, and not
> > particularly scalable if there is more information to be transmitted in
> > the future).  Using a context sensitive API set goes beyond even that, and
> > allows to retrieve arbitrary information form callbacks as needed in an
> > ABI safe manner
> 
> Again I can agree with this in the general case, but it isn't necessary, in this case, to encode the information in the array since it is already local to and available in the function. It seems artificial, at this point, to implement an array terminator solution to protect an API that, effectively, hasn't been published yet.
> 
You indicate that you agree an alternate solution is preferable in the general
case, so as to provide an API that is extensible in a way that isn't subject to
ABI breakage, correct?  If so, why do assert that its not necessecary in this
specific case?  If you feel you need to add information so that callbacks can be
more flexible (in this case specifying the size of a passed in array), why
immediately shoehorn another parmeter in place, and break the consistency
between rx and tx callbacks, when you don't have to?  I don't care if you break
ABI today (although to call it unpublished I think is disingenuous, as lots of
testing and development has already taken place with the ABI as it currently
stands).  I care, as I noted above about not getting into the habbit of just
assuming a change like this requires that you invaliate ABI somehow.  You don't
have to, you can create an API that is fairly invariant to it here if you like.
The question in my mind is, why don't you?

Neil


> John
> 
>  
> 
> 
> 

  reply	other threads:[~2015-03-13 23:15 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
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 [this message]
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=20150313231526.GA14641@neilslaptop.think-freely.org \
    --to=nhorman@tuxdriver.com \
    --cc=dev@dpdk.org \
    --cc=john.mcnamara@intel.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).