Re: [dpdk-dev] [RFC] [PATCH v2] libeventdev: event driven programming model framework for DPDK

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

On Fri, Oct 28, 2016 at 03:16:18PM +0100, Bruce Richardson wrote:
> On Fri, Oct 28, 2016 at 02:48:57PM +0100, Van Haaren, Harry wrote:
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jerin Jacob
> > > Sent: Tuesday, October 25, 2016 6:49 PM
> > <snip>
> > > 
> > > Hi Community,
> > > 
> > > So far, I have received constructive feedback from Intel, NXP and Linaro folks.
> > > Let me know, if anyone else interested in contributing to the definition of eventdev?
> > > 
> > > If there are no major issues in proposed spec, then Cavium would like work on
> > > implementing and up-streaming the common code(lib/librte_eventdev/) and
> > > an associated HW driver.(Requested minor changes of v2 will be addressed
> > > in next version).
> > 
> > 
> > Hi All,
> > 
> > I've been looking at the eventdev API from a use-case point of view, and I'm unclear on a how the API caters for two uses. I have simplified these as much as possible, think of them as a theoretical unit-test for the API :)
> > 
> > 
> > Fragmentation:
> > 1. Dequeue 8 packets
> > 2. Process 2 packets
> > 3. Processing 3rd, this packet needs fragmentation into two packets
> > 4. Process remaining 5 packets as normal
> > 
> > What function calls does the application make to achieve this?
> > In particular, I'm referring to how can the scheduler know that the 3rd packet is the one being fragmented, and how to keep packet order valid. 
> > 
> > 
> > Dropping packets:
> > 1. Dequeue 8 packets
> > 2. Process 2 packets
> > 3. Processing 3rd, this packet needs to be dropped
> > 4. Process remaining 5 packets as normal
> > 
> > What function calls does the application make to achieve this?
> > Again, in particular how does the scheduler know that the 3rd packet is being dropped.
> > 
> > 
> > Regards, -Harry
> 
> Hi,
> 
> these questions apply particularly to reordered which has a lot more
> complications than the other types in terms of sending packets back into
> the scheduler. However, atomic types will still suffer from problems
> with things the way they are - again if we assume a burst of 8 packets,
> then to forward those packets, we need to re-enqueue them again to the
> scheduler, and also then send 8 releases to the scheduler as well, to
> release the atomic locks for those packets.
> This means that for each packet we have to send two messages to a
> scheduler core, something that is really inefficient.
> 
> This number of messages is critical for any software implementation, as
> the cost of moving items core-to-core is going to be a big bottleneck
> (perhaps the biggest bottleneck) in the system. It's for this reason we
> need to use burst APIs - as with rte_rings.

I agree, That the reason why we have rte_event_*_burst()

> 
> How we have solved this in our implementation, is to allow there to be
> an event operation type. The four operations we implemented are as below
> (using packet as a synonym for event here, since these would mostly
> apply to packets flowing through a system):
> 
> * NEW     - just a regular enqueue of a packet, without any previous context

Makes sense. I was trying derive it.Make sense for application
requesting it.

> * FORWARD - enqueue a packet, and mark the flow processing for the
>             equivalent packet that was dequeued as completed, i.e.
> 	    release any atomic locks, or reorder this packet with
> 	    respect to any other outstanding packets from the event queue.

Default case

> * DROP    - this is roughtly equivalent to the existing "release" API call,
>             except that having it as an enqueue type allows us to
> 	    release multiple items in a single call, and also to mix
> 	    releases with new packets and forwarded packets

Yes. Maps to rte_event_release(), with index parameter, its kind doing
the job. But, Makes sense as flag to enable burst.
But it calls for removing the index parameter. Looks like index parameter
has issue in Intel implementation. If so, may be we(Cavium) can fill the
index in the dequeue as implementation specific bits like Harry
suggested and use it in enqueue.
http://dpdk.org/ml/archives/dev/2016-October/049459.html

Any thoughts from NXP?

> * PARTIAL - this indicates that the packet being enqueued should be
> 	    treated according to the context of the current packet, but
> 	    that that context should not be released/completed by the
> 	    enqueue of this packet. This only really applies for
> 	    reordered events, and is needed to do fragmentation and or
> 	    multicast of packets with reordering.

I believe PARTIAL is something, HW implementation will have trouble.
I have outlined other way to fix without coupling fragmentation logic in
scheduler.
http://dpdk.org/ml/archives/dev/2016-November/049707.html

If it makes sense for everyone then may be can
- Introduce "event operation type" bits (NEW, DROP, FORWARD(may not required as it is default) in enqueue
- remaining bits in "struct rte_event"(128B) for "implementation" defined
- remove rte_event_release() and use event operation type(DROP) as
  replacement.

/Jerin

> 
> 
> Therefore, I think we need to use some of the bits just freed up in the
> event structure to include an enqueue operation type. Without it, I just
> can't see how the API can ever support burst operation on packets.
> 
> Regards,
> /Bruce
>