Re: [PATCH v2 1/6] eventdev: introduce event driven programming model

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Dec 08, 2016 at 12:23:03AM +0530, Jerin Jacob wrote:
> On Tue, Dec 06, 2016 at 04:51:19PM +0000, Bruce Richardson wrote:
> > On Tue, Dec 06, 2016 at 09:22:15AM +0530, Jerin Jacob wrote:
> > > In a polling model, lcores poll ethdev ports and associated
> > > rx queues directly to look for packet. In an event driven model,
> > > by contrast, lcores call the scheduler that selects packets for
> > > them based on programmer-specified criteria. Eventdev library
> > > adds support for event driven programming model, which offer
> > > applications automatic multicore scaling, dynamic load balancing,
> > > pipelining, packet ingress order maintenance and
> > > synchronization services to simplify application packet processing.
> > > 
> > > By introducing event driven programming model, DPDK can support
> > > both polling and event driven programming models for packet processing,
> > > and applications are free to choose whatever model
> > > (or combination of the two) that best suits their needs.
> > > 
> > > This patch adds the eventdev specification header file.
> > > 
> > > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > ---
> > > +	/** WORD1 */
> > > +	RTE_STD_C11
> > > +	union {
> > > +		uint64_t u64;
> > > +		/**< Opaque 64-bit value */
> > > +		uintptr_t event_ptr;
> > > +		/**< Opaque event pointer */
> > 
> > Since we have a uint64_t member of the union, might this be better as a
> > void * rather than uintptr_t?
> 
> No strong opinion here. For me, uintptr_t looks clean.
> But, It is OK to change to void* as per your input.
> 
> > 
> > > +		struct rte_mbuf *mbuf;
> > > +		/**< mbuf pointer if dequeued event is associated with mbuf */
> > > +	};
> > > +};
> > > +
> > <snip>
> > > +/**
> > > + * Link multiple source event queues supplied in *rte_event_queue_link*
> > > + * structure as *queue_id* to the destination event port designated by its
> > > + * *port_id* on the event device designated by its *dev_id*.
> > > + *
> > > + * The link establishment shall enable the event port *port_id* from
> > > + * receiving events from the specified event queue *queue_id*
> > > + *
> > > + * An event queue may link to one or more event ports.
> > > + * The number of links can be established from an event queue to event port is
> > > + * implementation defined.
> > > + *
> > > + * Event queue(s) to event port link establishment can be changed at runtime
> > > + * without re-configuring the device to support scaling and to reduce the
> > > + * latency of critical work by establishing the link with more event ports
> > > + * at runtime.
> > 
> > I think this might need to be clarified. The device doesn't need to be
> > reconfigured, but does it need to be stopped? In SW implementation, this
> > affects how much we have to make things thread-safe. At minimum I think
> > we should limit this to having only one thread call the function at a
> > time, but we may allow enqueue dequeue ops from the data plane to run
> > in parallel.
> 
> Cavium implementation can change it at runtime without re-configuring or stopping
> the device to support runtime load balancing from the application perspective.
> 
> AFAIK, link establishment is _NOT_ fast path API. But the application
> can invoke it from worker thread whenever there is a need for re-wiring
> the queue to port connection for better explicit load balancing. IMO, A
> software implementation with lock is fine here as we don't use this in
> fastpath.
> 
> Thoughts?
> >

I agree that it's obviously not fast-path. Therefore I suggest that we
document that this API should be safe to call while the data path is in
operation, but that it should not be called by multiple cores
simultaneously i.e. single-writer, multi-reader safe, but not
multi-writer safe. Does that seem reasonable to you?

/Bruce