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

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

On Thu, Dec 08, 2016 at 09:30:49AM +0000, Bruce Richardson wrote:
> 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?

If I understand it correctly, per "event port" their will be ONLY ONE
writer at time.

i.e, In the valid case, Following two can be invoked in parallel
rte_event_port_link(dev_id, 0 /*port_id*/,..)
rte_event_port_link(dev_id, 1 /*port_id*/,..)

But, not invoking rte_event_port_link() on the _same_ event port in parallel

Are we on same page?

Jerin 

> 
> /Bruce