Re: [dpdk-dev] [PATCH v1 1/2] lib/ring: add enqueue-dequeue callabck

["Varghese, Vipin" <vipin.varghese@intel.com>]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Friday, June 7, 2019 4:15 PM
> To: Stephen Hemminger <stephen@networkplumber.org>; Varghese, Vipin
> <vipin.varghese@intel.com>
> Cc: olivier.matz@6wind.com; Pattan, Reshma <reshma.pattan@intel.com>;
> Wiles, Keith <keith.wiles@intel.com>; dev@dpdk.org; Padubidri, Sanjay A
> <sanjay.padubidri@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v1 1/2] lib/ring: add enqueue-dequeue callabck
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> > Sent: Thursday, June 6, 2019 8:49 PM
> > To: Varghese, Vipin <vipin.varghese@intel.com>
> > Cc: olivier.matz@6wind.com; Pattan, Reshma <reshma.pattan@intel.com>;
> > Wiles, Keith <keith.wiles@intel.com>; dev@dpdk.org; Padubidri, Sanjay
> > A <sanjay.padubidri@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v1 1/2] lib/ring: add enqueue-dequeue
> > callabck
> >
> > On Fri,  7 Jun 2019 00:03:54 +0530
> > Vipin Varghese <vipin.varghese@intel.com> wrote:
> >
> > > Add callback event handler for enqueue dequeue operation on ring.
> > > The pre-enqueue and post-dequeue operation on ring is selected to
> > > invoke user callback handler.
> > >
> > > Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> > > ---
> >
> > NAK
> > What is the use case for this? Rings are one of the most used elements
> > of DPDK and anything like this will have performance impact.
> >
> > And as DPDK goes to more of distribution model, all features have to
> > be enabled.
> 
> +1 here.
I am not sure if I have the understanding correct,, so if all features are to enabled 'why is features set as no in `common/config_base`'.

> Approach in general looks quite expensive, most users wouldn't need it.
> For those who want such functionality, it is not big deal to create a wrapper on
> top of current rte_ring (drivers/net/ring could be one example).
Thanks, will explore and get back.

> Konstantin
> 
> >
> >
> > Also, this looks like a problem:
> > +struct rte_ring_callback {
> > +	TAILQ_ENTRY(rte_ring_callback) next; /* Callbacks list */
> > +	rte_ring_cb_fn cb_fn; /* Callback address */
> > +	void *cb_arg; /* Parameter for callback */
> > +	uint32_t active; /* Callback is executing */
> >
> > The active element is likely updated in one thread and queried in
> > another it needs to be volatile, or better yet an atomic to ensure memory
> ordering.
> 
>