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
>
