[dpdk-dev] Bulk dequeue of packets and the returned values, question

Neil Horman nhorman at tuxdriver.com
Mon Sep 29 14:26:59 CEST 2014


On Mon, Sep 29, 2014 at 01:10:22PM +0100, Bruce Richardson wrote:
> On Sun, Sep 28, 2014 at 11:06:17PM +0000, Wiles, Roger Keith wrote:
> > Thanks Venky,
> > On Sep 28, 2014, at 5:23 PM, Venkatesan, Venky <venky.venkatesan at intel.com> wrote:
> > 
> > > Keith,
> > > 
> > > On 9/28/2014 11:04 AM, Wiles, Roger Keith wrote:
> > >> I am also looking at the bulk dequeue routines, which the ring can be fixed or variable. On fixed  < 0 on error is returned and 0 if successful. On a variable ring < 0 on error or n on success, but I think n can be zero in the variable case, correct?
> > >> 
> > >> If these are true then why not have the routines return  < 0 on error and >= 0 on success. Which means a dequeue from a fixed ring would return only ’requested size n’ or < 0 if you error off the 0 case. The 0 case could be OK, if you allow zero to be return on a empty ring for the fixed ring case.
> > >> 
> > >> Does this make sense to anyone?
> > > It won't make sense unless you're aware of the history behind these functions. The original functions that were implemented for the ring were only the bulk functions (i.e. FIXED). They would return exactly the number of items requested for dequeue (0 if success, negative if error), and not return any if the required number were not available.
> > > 
> > > The burst (i.e. VARIABLE) functions came in much later (think it was r1.3 where we introduced them), and by that time, there were already quite a number of deployments of DPDK in the field using the legacy ring functions. Therefore we made the decision to keep the legacy behavior intact & not impacting deployed code - and merging the burst functions into the code. Given that there was no "versioning" of the API/ABI in those releases :).
> > 
> > I see why the code is this way. If the developers used ‘if ( ret == 0 ) { /* do something */ }’ then it would break if it returned a positive value on success. I would expect the normal behavior to be ‘if ( ret < 0 ) { /* error case */ }’ and fall thru for the success case. I would love to change the code to just return <0 on error or >= 0 on success. I wonder how many customers code would break changing the code to do just just the two steps. I think it will remove some code in a couple places that were testing for FIXED or VARIABLE?
> > > 
> > > Hope that helps.
> > > -Venky
> > > 
> > >> 
> > >> Thanks
> > >> ++Keith
> > >> 
> > >> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> > 
> > Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> > 
> 
> Since we are looking at making considerable ABI changes in this release and 
> (hopefully) also looking to version our ABI going forward, I would be in 
> favour of making any changes to these APIs in this current release if 
> possible. While the current behaviour makes sense for historical reason, I 
> think an overall change to the behaviour as Keith describes would be more 
> sensible long-term. 
> 
I agree, this seems like a sensible time to make these sorts of changes as we
identify them.

> (Also to note my previous suggestion about upping the major version to 2.0 
> if we continue to increase the number of ABI/API changes in this release.  
> Anyone else any thoughts on that?)
> 
I feel like this is a policy decision, as I vew the versioning as arbitrary.
I'm really fine with it either way.  Presumably moving to 2.0 would represent a
major shift in design, and I suppose adding versioning does amount to something
like that, so I could be supportive.
Neil

> /Bruce
> 


More information about the dev mailing list