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

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

On Thu, Nov 24, 2016 at 04:24:11PM +0000, Bruce Richardson wrote:
> On Fri, Nov 18, 2016 at 11:14:59AM +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.
> > 
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > ---
> 
> Hi Jerin,
> 
> Thanks for the patchset. A few minor comments in general on the API that
> we found from working with it (thus far - more may follow :-) ).

Thanks Bruce.

> 
> 1. Priorities: priorities are used in a number of places in the API, but
>    all are uint8_t types and have their own MAX/NORMAL/MIN values. I think
>    it would be simpler for the user just to have one priority type in the
>    library, and use that everywhere. I suggest using RTE_EVENT_PRIORITY_*
>    and drop the separate defines for SERVICE_PRIORITY, and QUEUE_PRIORITY
>    etc. Ideally, I'd see things like this converted to enums too, rather
>    than defines, but I'm not sure it's possible in this case.

OK. I will address it in v2

> 
> 2. Functions for config and setup can have their structure parameter
>    types as const as they don't/shouldn't change the values internally.
>    So add "const" to parameters to:
>      rte_event_dev_configure()
>      rte_event_queue_setup()
>      rte_event_port_setup()
>      rte_event_port_link()
> 

OK. I will address it in v2

> 3. in event schedule() function, the dev->schedule() function needs the
>    dev instance pointer passed in as parameter.

OK. I will address it in v2

> 
> 4. The event op values and the event type values would be better as
>    enums rather than as a set of #defines.

OK. I will address it in v2

I will reply to your other comments in Thomas's email.

> 
> Regards,
> /Bruce