Re: [PATCH 1/3] evendev: fix inconsistency in event queue config

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

-----Original Message-----
> Date: Sat, 21 Oct 2017 00:39:28 +0530
> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@caviumnetworks.com>
> To: "Van Haaren, Harry" <harry.van.haaren@intel.com>
> CC: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/3] evendev: fix inconsistency in event
>  queue config
> User-Agent: Mutt/1.5.24 (2015-08-30)
> 
> On Fri, Oct 20, 2017 at 04:38:57PM +0000, Van Haaren, Harry wrote:
> > > From: Pavan Nikhilesh Bhagavatula [mailto:pbhagavatula@caviumnetworks.com]
> > > Sent: Friday, October 20, 2017 11:31 AM
> > > To: Van Haaren, Harry <harry.van.haaren@intel.com>
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH 1/3] evendev: fix inconsistency in event
> > > queue config
> > >
> > > On Fri, Oct 20, 2017 at 09:54:36AM +0000, Van Haaren, Harry wrote:
> > > > > From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> > > > > Sent: Thursday, October 12, 2017 2:16 PM
> > > > > To: jerin.jacob@caviumnetworks.com; hemant.agrawal@nxp.com; Van Haaren,
> > > > > Harry <harry.van.haaren@intel.com>
> > > > > Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > > > > Subject: [dpdk-dev] [PATCH 1/3] evendev: fix inconsistency in event
> > > queue
> > > > > config
> > > > >
> 
> <snip>
> 
> > > > > -	}
> > > > > -	return ret;
> > > > > -}
> > > >
> > > >
> > > > We should note here, that SCHED_TYPE are integer values:
> > > > #define RTE_SCHED_TYPE_ORDERED          0
> > > > #define RTE_SCHED_TYPE_ATOMIC           1
> > > > #define RTE_SCHED_TYPE_PARALLEL         2
> > > >
> > > > While the EVENT_QUEUE_CFG_ types were bitmasks (before being removed in
> > > this patch)
> > > > #define RTE_EVENT_QUEUE_CFG_ATOMIC_ONLY        (1ULL << 0)
> > > > #define RTE_EVENT_QUEUE_CFG_ORDERED_ONLY       (2ULL << 0)
> > > > #define RTE_EVENT_QUEUE_CFG_PARALLEL_ONLY      (3ULL << 0)
> > > > #define RTE_EVENT_QUEUE_CFG_SINGLE_LINK        (1ULL << 2)
> > > >
> > > >
> > > > I'm not against this change - but we must be careful that if there was any
> > > bit-masking being used previously,
> > > > that that will subtly have broken if we change to sched types. I'm
> > > reviewing with that in mind..
> > > >
> > > > The RTE_EVENT_QUEUE_CFG_ALL_TYPES  config flag now means that all
> > > SCHED_TYPEs
> > > > are valid. Previously this was contained in the bitmask.. this may lead to
> > > issues.
> > > >
> > > > See patch 2/3, where *only* the schedule_type is read, and returned. What
> > > if it the "ALL_TYPES" flag is
> > > > set on the queue? It will be reported wrongly. Currently there is no
> > > integer value for "ALL_TYPES",
> > > > so we cannot represent "ALL TYPES" in the return value from get_attr().
> > > >
> > > > Am I explaining my reasoning clearly enough?
> > > >
> > >
> > > Hey Harry,
> > >
> > > I do understand what you mean, my initial thought was to include "ALL_TYPES"
> > > as
> > > a schedule_type in queue config but this would just complicate things.
> > >
> > > As these values are only used in config phase we could have a check to see
> > > if
> > > event_queue_cfg is not "ALL_TYPES" and only then return the value of
> > > sched_type
> > > else return a error value in case of get_attr().
> > >
> > > I think most of the places this specific check is handled, one such missed
> > > place would be get_attr(). If we could come to a conclusion to fix it in a
> > > correct way I will send out a v2.
> >
> >
> > Sure, I see two sane-ish options:
> >
> > 1) Return an error code from get_attr(), which actually means "ALL TYPES". Feels a bit weird, because an error value is really a valid return.
> >
> > 2) Return UINT_MAX (aka, -1) as the scheduling value. Applications that use/care about the scheduling type must check, others can ignore it.
> >
> > I'm not sure which of these is the better/less-bad solution. Opinions? -H
> >
> 
> I think 1st option would be good, we could use ENOTUNIQ to represent that the
> queue type is "ALL TYPE".
> 
> Thoughts?

If we were to choose between option 1 and option 2, I think,
option 1 is better instead of special interpretation of option 2.

Looks like ENOTUNIQ is not available for freebsd. Choose a errno that
works for linux and freebsd

https://www.freebsd.org/cgi/man.cgi?query=errno&sektion=2
http://man7.org/linux/man-pages/man3/errno.3.html