[24.03,v2,2/9] eventdev: increase flexibility of all-types flag

  Rather than requiring that any device advertising the
RTE_EVENT_DEV_CAP_QUEUE_ALL_TYPES flag support all of atomic, ordered
and parallel scheduling, we can redefine the field so that it basically
means that you don't need to specify the queue scheduling type at config
time. Instead all types of supported events can be sent to all queues.

Suggested-by: Mattias Rönnblom <hofors@lysator.liu.se>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eventdev/rte_eventdev.h | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

--
2.39.2
  

On Tue, Nov 21, 2023 at 5:25 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> Rather than requiring that any device advertising the
> RTE_EVENT_DEV_CAP_QUEUE_ALL_TYPES flag support all of atomic, ordered
> and parallel scheduling, we can redefine the field so that it basically
> means that you don't need to specify the queue scheduling type at config
> time. Instead all types of supported events can be sent to all queues.
>
> Suggested-by: Mattias Rönnblom <hofors@lysator.liu.se>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/eventdev/rte_eventdev.h | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> index d48957362c..1c5043de26 100644
> --- a/lib/eventdev/rte_eventdev.h
> +++ b/lib/eventdev/rte_eventdev.h
> @@ -250,11 +250,20 @@ struct rte_event;
>   * @see rte_event_dequeue_burst()
>   */
>  #define RTE_EVENT_DEV_CAP_QUEUE_ALL_TYPES     (1ULL << 3)
> -/**< Event device is capable of enqueuing events of any type to any queue.
> +/**< Event device is capable of accepting enqueued events, of any type
> + * advertised as supported by the device, to all destination queues.
> + *
> + * When this capability is set, the "schedule_type" field of the
> + * rte_event_queue_conf structure is ignored when a queue is being configured.

can we also add something like below or so to above line

rte_event_queue_conf structure is ignored when a queue is being
configured instead rte_event::sched_type
shall be used.

Also, git commit subject "eventdev: increase flexibility of all-types
flag", This patch is just documentation clarification. Right? It was
like this from day 1.
I would suggest the subject as "eventdev: clarify all-types flag
documentation" or so.

With above changes,

Acked-by: Jerin Jacob <jerinj@marvell.com>
  

On Thu, Nov 23, 2023 at 09:37:58AM +0530, Jerin Jacob wrote:
> On Tue, Nov 21, 2023 at 5:25 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > Rather than requiring that any device advertising the
> > RTE_EVENT_DEV_CAP_QUEUE_ALL_TYPES flag support all of atomic, ordered
> > and parallel scheduling, we can redefine the field so that it basically
> > means that you don't need to specify the queue scheduling type at config
> > time. Instead all types of supported events can be sent to all queues.
> >
> > Suggested-by: Mattias Rönnblom <hofors@lysator.liu.se>
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  lib/eventdev/rte_eventdev.h | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> > index d48957362c..1c5043de26 100644
> > --- a/lib/eventdev/rte_eventdev.h
> > +++ b/lib/eventdev/rte_eventdev.h
> > @@ -250,11 +250,20 @@ struct rte_event;
> >   * @see rte_event_dequeue_burst()
> >   */
> >  #define RTE_EVENT_DEV_CAP_QUEUE_ALL_TYPES     (1ULL << 3)
> > -/**< Event device is capable of enqueuing events of any type to any queue.
> > +/**< Event device is capable of accepting enqueued events, of any type
> > + * advertised as supported by the device, to all destination queues.
> > + *
> > + * When this capability is set, the "schedule_type" field of the
> > + * rte_event_queue_conf structure is ignored when a queue is being configured.
> 
> can we also add something like below or so to above line
> 
> rte_event_queue_conf structure is ignored when a queue is being
> configured instead rte_event::sched_type
> shall be used.
> 
Preparing v3 now and including this doc change. However, I'm also wondering
about the correct behaviour when this flag is not set. When the flag is not
set, the events enqueued should match the event type of the queue, but do
we need to enforce this, or should we?

Couple of options:
1. Actually enforce this, and state that it is an error to enqueue events
   with another scheduling type.
2. Explicitly not enforce this, and just state instead that the sched_type
   of events will be ignored.

Personally, I'd tend very much towards #2. because:
* it's easier for the app, as they can ignore the sched_type field through
  the pipeline if they want, relying on each queues type to do the right
  thing. This could be especially useful if they have fallback mechanisms
  to e.g. configure a queue as atomic if reordered is not supported etc.
  The downside is that for portable applications the sched type
  should always be set anyway, but the app doesn't lose anything in this
  case with #2 over #1.

* It's easier and more performant for the drivers, since it's one less
  check that should be performed on enqueue. The driver can just blindly
  override the sched_type provided with the queue config version.

I actually think an extension of #2 would also be nice to have for
portability, whereby an app could explicitly configure a queue to only have
a scheduling type - event if it's all-types-capable - and thereafter never
have to set the sched_type field on events. The drivers would always
remember the queue-type and explicitly set that if necessary on enqueue

Thoughts?
/Bruce

PS: Going to send v3 now anyway, based on feedback thus far. If we get
quick consensus on above, I can roll it into a v4, otherwise we can look to
clarify that situation in a separate patch later.
  

On Tue, Dec 12, 2023 at 4:58 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Thu, Nov 23, 2023 at 09:37:58AM +0530, Jerin Jacob wrote:
> > On Tue, Nov 21, 2023 at 5:25 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > Rather than requiring that any device advertising the
> > > RTE_EVENT_DEV_CAP_QUEUE_ALL_TYPES flag support all of atomic, ordered
> > > and parallel scheduling, we can redefine the field so that it basically
> > > means that you don't need to specify the queue scheduling type at config
> > > time. Instead all types of supported events can be sent to all queues.
> > >
> > > Suggested-by: Mattias Rönnblom <hofors@lysator.liu.se>
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > >  lib/eventdev/rte_eventdev.h | 15 ++++++++++++---
> > >  1 file changed, 12 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> > > index d48957362c..1c5043de26 100644
> > > --- a/lib/eventdev/rte_eventdev.h
> > > +++ b/lib/eventdev/rte_eventdev.h
> > > @@ -250,11 +250,20 @@ struct rte_event;
> > >   * @see rte_event_dequeue_burst()
> > >   */
> > >  #define RTE_EVENT_DEV_CAP_QUEUE_ALL_TYPES     (1ULL << 3)
> > > -/**< Event device is capable of enqueuing events of any type to any queue.
> > > +/**< Event device is capable of accepting enqueued events, of any type
> > > + * advertised as supported by the device, to all destination queues.
> > > + *
> > > + * When this capability is set, the "schedule_type" field of the
> > > + * rte_event_queue_conf structure is ignored when a queue is being configured.
> >
> > can we also add something like below or so to above line
> >
> > rte_event_queue_conf structure is ignored when a queue is being
> > configured instead rte_event::sched_type
> > shall be used.
> >
> Preparing v3 now and including this doc change. However, I'm also wondering
> about the correct behaviour when this flag is not set. When the flag is not
> set, the events enqueued should match the event type of the queue, but do
> we need to enforce this, or should we?
>
> Couple of options:
> 1. Actually enforce this, and state that it is an error to enqueue events
>    with another scheduling type.
> 2. Explicitly not enforce this, and just state instead that the sched_type
>    of events will be ignored.
>
> Personally, I'd tend very much towards #2. because:
> * it's easier for the app, as they can ignore the sched_type field through
>   the pipeline if they want, relying on each queues type to do the right
>   thing. This could be especially useful if they have fallback mechanisms
>   to e.g. configure a queue as atomic if reordered is not supported etc.
>   The downside is that for portable applications the sched type
>   should always be set anyway, but the app doesn't lose anything in this
>   case with #2 over #1.
>
> * It's easier and more performant for the drivers, since it's one less
>   check that should be performed on enqueue. The driver can just blindly
>   override the sched_type provided with the queue config version.
>
> I actually think an extension of #2 would also be nice to have for
> portability, whereby an app could explicitly configure a queue to only have
> a scheduling type - event if it's all-types-capable - and thereafter never
> have to set the sched_type field on events. The drivers would always
> remember the queue-type and explicitly set that if necessary on enqueue
>
> Thoughts?


HW queues with All-type support will be costly resources, so making a
portable program
we need to spare 3 queues instead of 1 queue.

Considering the difference in eventdev capabilities, I see the most
reliable option is
end user focus on reusable packet processing stages. Have worker skeleton based
on HW device capabilities, as there are enough HW differences across
event devices.
That is the kind of theme followed in testeventdev.


> /Bruce
>
> PS: Going to send v3 now anyway, based on feedback thus far. If we get
> quick consensus on above, I can roll it into a v4, otherwise we can look to

Let's go with v3 now. Later we can discuss more on this latter.

> clarify that situation in a separate patch later.