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

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

On Wed, Nov 02, 2016 at 04:55:22PM +0530, Jerin Jacob wrote:
> On Fri, Oct 28, 2016 at 02:36:48PM +0530, Jerin Jacob wrote:
> > On Fri, Oct 28, 2016 at 09:36:46AM +0100, Bruce Richardson wrote:
> > > On Fri, Oct 28, 2016 at 08:31:41AM +0530, Jerin Jacob wrote:
> > > > On Wed, Oct 26, 2016 at 01:54:14PM +0100, Bruce Richardson wrote:
> > > > > On Wed, Oct 26, 2016 at 05:54:17PM +0530, Jerin Jacob wrote:
> > > > > > On Wed, Oct 26, 2016 at 12:11:03PM +0000, Van Haaren, Harry wrote:
> > > > > > > > -----Original Message-----
> > > > > rte_event_queue_conf, with possible values:
> > > > > * atomic
> > > > > * ordered
> > > > > * parallel
> > > > > * mixed - allowing all 3 types. I think allowing 2 of three types might
> > > > >     make things too complicated.
> > > > > 
> > > > > An open question would then be how to behave when the queue type and
> > > > > requested event type conflict. We can either throw an error, or just
> > > > > ignore the event type and always treat enqueued events as being of the
> > > > > queue type. I prefer the latter, because it's faster not having to
> > > > > error-check, and it pushes the responsibility on the app to know what
> > > > > it's doing.
> > > > 
> > > > How about making default as "mixed" and let application configures what
> > > > is not required?. That way application responsibility is clear.
> > > > something similar to ETH_TXQ_FLAGS_NOMULTSEGS, ETH_TXQ_FLAGS_NOREFCOUNT
> > > > with default.
> > > > 
> > > I suppose it could work, but why bother doing that? If an app knows it's
> > > only going to use one traffic type, why not let it just state what it
> > > will do rather than try to specify what it won't do. If mixed is needed,
> > 
> > My thought was more inline with ethdev spec, like, ref-count is default,
> > if application need exception then set ETH_TXQ_FLAGS_NOREFCOUNT. But it is OK, if
> > you need other way.
> > 
> > > then it's easy enough to specify - and we can make it the zero/default
> > > value too.
> > 
> > OK. Then we will make MIX as zero/default and add "allowed_event_types" in
> > event queue config.
> >
> 
> Bruce,
> 
> I have tried to make it as "allowed_event_types" in event queue config.
> However, rte_event_queue_default_conf_get() can also take NULL for default
> configuration. So I think, It makes sense to go with negation approach
> like ethdev to define the default to avoid confusion on the default. So
> I am thinking like below now,
> 
> ➜ [master][libeventdev] $ git diff
> diff --git a/rte_eventdev.h b/rte_eventdev.h
> index cf22b0e..cac4642 100644
> --- a/rte_eventdev.h
> +++ b/rte_eventdev.h
> @@ -429,6 +429,12 @@ rte_event_dev_configure(uint8_t dev_id, struct
> rte_event_dev_config *config);
>   *
>   *  \see rte_event_port_setup(), rte_event_port_link()
>   */
> +#define RTE_EVENT_QUEUE_CFG_NOATOMIC_TYPE      (1ULL << 1)
> +/**< Skip configuring atomic schedule type resources */
> +#define RTE_EVENT_QUEUE_CFG_NOORDERED_TYPE     (1ULL << 2)
> +/**< Skip configuring ordered schedule type resources */
> +#define RTE_EVENT_QUEUE_CFG_NOPARALLEL_TYPE    (1ULL << 3)
> +/**< Skip configuring parallel schedule type resources */
> 
>  /** Event queue configuration structure */
>  struct rte_event_queue_conf {
> 
> Thoughts?
> 

I'm ok with the default as being all types, in the case where NULL is
specified for the parameter. It does make the most sense.

However, for the cases where the user does specify what they want, I
think it does make more sense, and is easier on the user for things to
be specified in a positive, rather than negative sense. For a user who
wants to just use atomic events, having to specify that as "not-reordered
and not-unordered" just isn't as clear! :-)

/Bruce