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.
(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?)
/Bruce
