Bulk dequeue of packets and the returned values, question

Bulk dequeue of packets and the returned values, question

[Wiles, Roger Keith]

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?

Thanks
++Keith

Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533

Re: Bulk dequeue of packets and the returned values, question

[Neil Horman]

On Sun, Sep 28, 2014 at 06:04:42PM +0000, 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?
> 
Yes, that seems right.

> 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?
> 
I agree, I don't see why the behavior is bifurcated the way it is, its
nonsensical.
Neil

> Thanks
> ++Keith
> 
> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> 
> 

Re: Bulk dequeue of packets and the returned values, question

[Venkatesan, Venky]

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 :).

Hope that helps.
-Venky

>
> Thanks
> ++Keith
>
> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
>

Re: Bulk dequeue of packets and the returned values, question

[Neil Horman]

On Sun, Sep 28, 2014 at 03:23:44PM -0700, Venkatesan, Venky 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 :).
> 
Hehe :)

Yes, API versioning would be a great benefit to this sort of problem.  If only
there were a patchset available to create that ability ;)

Neil

Re: Bulk dequeue of packets and the returned values, question

[Wiles, Roger Keith]

Thanks Venky,
On Sep 28, 2014, at 5:23 PM, Venkatesan, Venky <venky.venkatesan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 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

Re: Bulk dequeue of packets and the returned values, question

[Bruce Richardson]

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@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

Re: Bulk dequeue of packets and the returned values, question

[Neil Horman]

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@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
> 

Re: Bulk dequeue of packets and the returned values, question

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Monday, September 29, 2014 1:10 PM
> To: Wiles, Roger Keith (Wind River)
> Cc: dev@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@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

> 
> (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?)
> 
>

Re: Bulk dequeue of packets and the returned values, question

[Wiles, Roger Keith]

On Sep 29, 2014, at 7:30 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:

> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces-VfR2kkLFssw@public.gmane.org] On Behalf Of Bruce Richardson
>> Sent: Monday, September 29, 2014 1:10 PM
>> To: Wiles, Roger Keith (Wind River)
>> Cc: dev-VfR2kkLFssw@public.gmane.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@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