-----Original Message----- > Date: Sat, 21 Oct 2017 00:39:28 +0530 > From: Pavan Nikhilesh Bhagavatula <pbhagavat...@caviumnetworks.com> > To: "Van Haaren, Harry" <harry.van.haa...@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:pbhagavat...@caviumnetworks.com] > > > Sent: Friday, October 20, 2017 11:31 AM > > > To: Van Haaren, Harry <harry.van.haa...@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:pbhagavat...@caviumnetworks.com] > > > > > Sent: Thursday, October 12, 2017 2:16 PM > > > > > To: jerin.ja...@caviumnetworks.com; hemant.agra...@nxp.com; Van > > > > > Haaren, > > > > > Harry <harry.van.haa...@intel.com> > > > > > Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavat...@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
