On Sep 29, 2014, at 7:30 AM, Ananyev, Konstantin <konstantin.ananyev at intel.com> wrote:
> > >> -----Original Message----- >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson >> Sent: Monday, September 29, 2014 1:10 PM >> To: Wiles, Roger Keith (Wind River) >> Cc: dev at dpdk.org >> Subject: Re: [dpdk-dev] Bulk dequeue of packets and the returned values, >> question >> >> 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. > > It is doable, I suppose, but might become quite messy: > Don't know how many people are using rte_ring_dequeue_bulk() all over the > place. > I suspect quite a lot. > From other side - what the real gain we'll have from it? > I don't see much so far. > Konstantin > I see two possible gains one is a consistent return method for Fixed/Variable and some code reduction in a few places. Let me see if I can create a patch we can review and see if it seems reasonable. >> >> (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?) Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
