> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Friday, June 7, 2019 4:15 PM
> To: Stephen Hemminger <[email protected]>; Varghese, Vipin
> <[email protected]>
> Cc: [email protected]; Pattan, Reshma <[email protected]>;
> Wiles, Keith <[email protected]>; [email protected]; Padubidri, Sanjay A
> <[email protected]>
> Subject: RE: [dpdk-dev] [PATCH v1 1/2] lib/ring: add enqueue-dequeue callabck
>
>
>
> > -----Original Message-----
> > From: dev [mailto:[email protected]] On Behalf Of Stephen Hemminger
> > Sent: Thursday, June 6, 2019 8:49 PM
> > To: Varghese, Vipin <[email protected]>
> > Cc: [email protected]; Pattan, Reshma <[email protected]>;
> > Wiles, Keith <[email protected]>; [email protected]; Padubidri, Sanjay
> > A <[email protected]>
> > 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 <[email protected]> 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 <[email protected]>
> > > ---
> >
> > 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.
>
>
