Re: [dpdk-dev] [PATCH] eventdev: add producer enqueue hint

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Tue, 27 Jun 2017 08:44:34 +0000
> From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Eads, Gage"
>  <gage.eads@intel.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>, "Richardson, Bruce"
>  <bruce.richardson@intel.com>, "hemant.agrawal@nxp.com"
>  <hemant.agrawal@nxp.com>, "nipun.gupta@nxp.com" <nipun.gupta@nxp.com>,
>  "Vangati, Narender" <narender.vangati@intel.com>, "Rao, Nikhil"
>  <nikhil.rao@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] eventdev: add producer enqueue hint
> 
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Tuesday, June 27, 2017 9:08 AM
> > To: Eads, Gage <gage.eads@intel.com>
> > Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>; Van Haaren, Harry
> > <harry.van.haaren@intel.com>; hemant.agrawal@nxp.com; nipun.gupta@nxp.com; Vangati,
> > Narender <narender.vangati@intel.com>; Rao, Nikhil <nikhil.rao@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH] eventdev: add producer enqueue hint
> 
> <snip some patch code>
> 
> > > >  void
> > > > diff --git a/lib/librte_eventdev/rte_eventdev.h
> > > > b/lib/librte_eventdev/rte_eventdev.h
> > > > index a248fe90e..1c1a46593 100644
> > > > --- a/lib/librte_eventdev/rte_eventdev.h
> > > > +++ b/lib/librte_eventdev/rte_eventdev.h
> > > > @@ -933,7 +933,15 @@ struct rte_event {
> > > >  			 * and is undefined on dequeue.
> > > >  			 * @see RTE_EVENT_OP_NEW, (RTE_EVENT_OP_*)
> > > >  			 */
> > > > -			uint8_t rsvd:4;
> > > > +			uint8_t all_op_new:1;
> > > > +			/**< Valid only with event enqueue operation - This hint
> > > > +			 * indicates that the enqueue request has only the
> > > > +			 * events with op == RTE_EVENT_OP_NEW.
> > > > +			 * The event producer, typically use this pattern to
> > > > +			 * inject the events to eventdev.
> > > > +			 * @see RTE_EVENT_OP_NEW
> > > > rte_event_enqueue_burst()
> > > > +			 */
> > > > +			uint8_t rsvd:3;
> > > >  			/**< Reserved for future use */
> > > >  			uint8_t sched_type:2;
> > > >  			/**< Scheduler synchronization type
> > > > (RTE_SCHED_TYPE_*)
> > > > --
> > > > 2.13.1
> > >
> > > I slightly prefer the parallel enqueue API -- I can see folks making the mistake of
> > setting all_op_new without setting the op to RTE_EVENT_OP_NEW, and later adding a
> > "forward-only" enqueue API could be interesting for the sw PMD -- but this looks fine to
> > me. Curious if others have any thoughts.
> > 
> > If forward-only parallel enqueue API interesting for the SW PMD then I
> > can drop this one and introduce forward-only API. Let me know if others
> > have any thoughts?
> 
> 
> To make sure I understand correctly, the "parallel API" idea is to add a new function pointer per-PMD, and dedicate it to enqueueing a burst of packets with the same OP? So the end result would be function(s) in the public API like this:
> 
> rte_event_enqueue_burst_new(port, new_events, n_events);
> rte_event_enqueue_burst_forward(port, new_events, n_events);
> 
> Given these are a "specialization" of the generic enqueue_burst() function, the PMD is not obliged to implement them. If they are NULL, the eventdev.c infrastructure can just point the burst_new() and burst_forward() to the generic enqueue without any performance delta?
> 
> The cost is some added code in the public header and infrastructure.
> The gain is that we don't overload the current API with new behavior. 
> 
> 
> Assuming my description of the parallel proposal above is correct, +1 for the parallel function approach. I like APIs that "do what they say on the tin" :)

Yes. We are on the same page. I will send the v2.