[RFC,0/1] ring: add callback infrastructire to ring library

  This is an RFC proposing the addition of a callback infrastructure to the ring
library, particularly in the ring dequeue functions, but they could also be
added to the enqueue functions if desired.

Callbacks in the ring dequeue functions would be beneficial for a number of
reasons including but not limited to the following:
- would allow users to register specific functions to be called on dequeue of a
  ring avoiding the need to call the function within application code on several
  threads reading from said ring.
- would mirror the callback functionality offered by the ethdev library for
  threads that read exclusively from a ring and process packets based off that,
  thus allowing for the same threads to read from either a NIC i/f or directly
  from a ring without needing a different codepath.

The addition of callbacks wouldn't impact the reading of rings by more than 1
cycle when no callbacks are registered. They could also additionally be compiled
in/out as desired to give more confidence in maintaining performance when
callbacks are not required.

This RFC is to give a feel for what the additional APIs would be and how they
would be integrated within the ring dequeue functions. As such only function
declarations are present. If there is a willingness within the community to add
callback infrastructure to the ring library I will implement further code.

Rory Sexton (1):
  ring: add infrastructure to allow callbacks within the ring library

 lib/ring/rte_ring.h      | 133 ++++++++++++++++++++++++++++++++++++++-
 lib/ring/rte_ring_core.h |   3 +
 2 files changed, 135 insertions(+), 1 deletion(-)
  

> From: Rory Sexton [mailto:rory.sexton@intel.com]
> Sent: Thursday, 23 March 2023 12.38
> 
> This is an RFC proposing the addition of a callback infrastructure to the ring
> library, particularly in the ring dequeue functions, but they could also be
> added to the enqueue functions if desired.
> 
> Callbacks in the ring dequeue functions would be beneficial for a number of
> reasons including but not limited to the following:
> - would allow users to register specific functions to be called on dequeue of
> a
>   ring avoiding the need to call the function within application code on
> several
>   threads reading from said ring.
> - would mirror the callback functionality offered by the ethdev library for
>   threads that read exclusively from a ring and process packets based off
> that,
>   thus allowing for the same threads to read from either a NIC i/f or directly
>   from a ring without needing a different codepath.
> 
> The addition of callbacks wouldn't impact the reading of rings by more than 1
> cycle when no callbacks are registered. They could also additionally be
> compiled
> in/out as desired to give more confidence in maintaining performance when
> callbacks are not required.

On the condition that they can be omitted at build time, as you mention here, I don't mind having more callbacks.

> 
> This RFC is to give a feel for what the additional APIs would be and how they
> would be integrated within the ring dequeue functions. As such only function
> declarations are present. If there is a willingness within the community to
> add
> callback infrastructure to the ring library I will implement further code.

I like the idea that these callbacks mimic the ethdev callbacks, so if proceeding with this, I think that both enqueue and dequeue callbacks should be added. The ethdev callbacks share one RTE_ETHDEV_RXTX_CALLBACKS define, so the ring callbacks can probably do the same.

> 
> Rory Sexton (1):
>   ring: add infrastructure to allow callbacks within the ring library
> 
>  lib/ring/rte_ring.h      | 133 ++++++++++++++++++++++++++++++++++++++-
>  lib/ring/rte_ring_core.h |   3 +
>  2 files changed, 135 insertions(+), 1 deletion(-)
> 
> --
> 2.34.1
>
  

Hi Roxy,
	Thanks for the work, few questions inline.

> -----Original Message-----
> From: Rory Sexton <rory.sexton@intel.com>
> Sent: Thursday, March 23, 2023 6:38 AM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> konstantin.v.ananyev@yandex.ru
> Cc: dev@dpdk.org; Rory Sexton <rory.sexton@intel.com>
> Subject: [RFC 0/1] ring: add callback infrastructire to ring library
> 
> This is an RFC proposing the addition of a callback infrastructure to the ring
> library, particularly in the ring dequeue functions, but they could also be added
> to the enqueue functions if desired.
> 
> Callbacks in the ring dequeue functions would be beneficial for a number of
> reasons including but not limited to the following:
> - would allow users to register specific functions to be called on dequeue of a
>   ring avoiding the need to call the function within application code on several
>   threads reading from said ring.
I do not completely understand the 'avoiding the need to call the function within application code on several threads reading from said ring'. Irrespective of where the feature is implemented (either in ring library or the application), the call back function will be called on all the threads that receive from this queue. 

> - would mirror the callback functionality offered by the ethdev library for
>   threads that read exclusively from a ring and process packets based off that,
>   thus allowing for the same threads to read from either a NIC i/f or directly
>   from a ring without needing a different codepath.
Do you plan to support a chain of callbacks?

> 
> The addition of callbacks wouldn't impact the reading of rings by more than 1
> cycle when no callbacks are registered. They could also additionally be compiled
> in/out as desired to give more confidence in maintaining performance when
> callbacks are not required.
I would prefer to keep this feature on always as maintenance is easier. But, I think we should make that decision only after we have performance data. Is it possible to provide some performance data with this feature on but no callbacks registered?

> 
> This RFC is to give a feel for what the additional APIs would be and how they
> would be integrated within the ring dequeue functions. As such only function
> declarations are present. If there is a willingness within the community to add
> callback infrastructure to the ring library I will implement further code.
> 
> Rory Sexton (1):
>   ring: add infrastructure to allow callbacks within the ring library
> 
>  lib/ring/rte_ring.h      | 133 ++++++++++++++++++++++++++++++++++++++-
>  lib/ring/rte_ring_core.h |   3 +
>  2 files changed, 135 insertions(+), 1 deletion(-)
> 
> --
> 2.34.1
  

Hi Honnappa,

Responses inline.
I will continue with an initial implementation of this feature and capture performance data as suggested. 

Rgds,
Rory

> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> 
> Sent: Wednesday, April 5, 2023 1:46 AM
> To: Sexton, Rory <rory.sexton@intel.com>; konstantin.v.ananyev@yandex.ru
> Cc: dev@dpdk.org; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: RE: [RFC 0/1] ring: add callback infrastructire to ring library
> 
> Hi Roxy,
> 	Thanks for the work, few questions inline.
> 
> > -----Original Message-----
> > From: Rory Sexton <rory.sexton@intel.com>
> > Sent: Thursday, March 23, 2023 6:38 AM
> > To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; 
> > konstantin.v.ananyev@yandex.ru
> > Cc: dev@dpdk.org; Rory Sexton <rory.sexton@intel.com>
> > Subject: [RFC 0/1] ring: add callback infrastructire to ring library
> > 
> > This is an RFC proposing the addition of a callback infrastructure to 
> > the ring library, particularly in the ring dequeue functions, but they 
> > could also be added to the enqueue functions if desired.
> > 
> > Callbacks in the ring dequeue functions would be beneficial for a 
> > number of reasons including but not limited to the following:
> > - would allow users to register specific functions to be called on dequeue of a
> >   ring avoiding the need to call the function within application code on several
> >   threads reading from said ring.
> I do not completely understand the 'avoiding the need to call the function within application code on several threads reading from said ring'. Irrespective of where the feature is implemented (either in ring library or the application), the call back function will be called on all the threads that receive from this queue. 
I just mean the callback infrastructure would handle the call back function rather than developer needing to call their implemented function explicitely 

> 
> > - would mirror the callback functionality offered by the ethdev library for
> >   threads that read exclusively from a ring and process packets based off that,
> >   thus allowing for the same threads to read from either a NIC i/f or directly
> >   from a ring without needing a different codepath.
> Do you plan to support a chain of callbacks?
Yes I would plan to support a chain of callbacks 

> 
> > 
> > The addition of callbacks wouldn't impact the reading of rings by more 
> > than 1 cycle when no callbacks are registered. They could also 
> > additionally be compiled in/out as desired to give more confidence in 
> > maintaining performance when callbacks are not required.
> I would prefer to keep this feature on always as maintenance is easier. But, I think we should make that decision only after we have performance data. Is it possible to provide some performance data with this feature on but no callbacks registered?
Agreed that keeping the feature always on would be easier long-term.
I can capture performance data with the feature on but no callbacks registered.
We can base the final implementation with that data in mind. 

> 
> > 
> > This RFC is to give a feel for what the additional APIs would be and 
> > how they would be integrated within the ring dequeue functions. As 
> > such only function declarations are present. If there is a willingness 
> > within the community to add callback infrastructure to the ring library I will implement further code.
> > 
> > Rory Sexton (1):
> >   ring: add infrastructure to allow callbacks within the ring library
> > 
> >  lib/ring/rte_ring.h      | 133 ++++++++++++++++++++++++++++++++++++++-
> >  lib/ring/rte_ring_core.h |   3 +
> >  2 files changed, 135 insertions(+), 1 deletion(-)
> > 
> > --
> > 2.34.1
  

<snip>

> 
> Hi Honnappa,
> 
> Responses inline.
> I will continue with an initial implementation of this feature and capture
> performance data as suggested.
> 
> Rgds,
> Rory
> 
> > -----Original Message-----
> > From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > Sent: Wednesday, April 5, 2023 1:46 AM
> > To: Sexton, Rory <rory.sexton@intel.com>;
> > konstantin.v.ananyev@yandex.ru
> > Cc: dev@dpdk.org; nd <nd@arm.com>; nd <nd@arm.com>
> > Subject: RE: [RFC 0/1] ring: add callback infrastructire to ring
> > library
> >
> > Hi Roxy,
Sincere apologies for spelling your name incorrectly.

> > 	Thanks for the work, few questions inline.
> >
> > > -----Original Message-----
> > > From: Rory Sexton <rory.sexton@intel.com>
> > > Sent: Thursday, March 23, 2023 6:38 AM
> > > To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> > > konstantin.v.ananyev@yandex.ru
> > > Cc: dev@dpdk.org; Rory Sexton <rory.sexton@intel.com>
> > > Subject: [RFC 0/1] ring: add callback infrastructire to ring library
> > >
> > > This is an RFC proposing the addition of a callback infrastructure
> > > to the ring library, particularly in the ring dequeue functions, but
> > > they could also be added to the enqueue functions if desired.
> > >
> > > Callbacks in the ring dequeue functions would be beneficial for a
> > > number of reasons including but not limited to the following:
> > > - would allow users to register specific functions to be called on dequeue
> of a
> > >   ring avoiding the need to call the function within application code on
> several
> > >   threads reading from said ring.
> > I do not completely understand the 'avoiding the need to call the function
> within application code on several threads reading from said ring'.
> Irrespective of where the feature is implemented (either in ring library or the
> application), the call back function will be called on all the threads that
> receive from this queue.
> I just mean the callback infrastructure would handle the call back function
> rather than developer needing to call their implemented function explicitly
Thanks for clarifying

> 
> >
<snip>

> 
> >
> > >
> > > The addition of callbacks wouldn't impact the reading of rings by
> > > more than 1 cycle when no callbacks are registered. They could also
> > > additionally be compiled in/out as desired to give more confidence
> > > in maintaining performance when callbacks are not required.
> > I would prefer to keep this feature on always as maintenance is easier. But, I
> think we should make that decision only after we have performance data. Is it
> possible to provide some performance data with this feature on but no
> callbacks registered?
> Agreed that keeping the feature always on would be easier long-term.
> I can capture performance data with the feature on but no callbacks
> registered.
> We can base the final implementation with that data in mind.
Ack

<snip>