On Thu, Feb 01, 2024 at 05:02:44PM +0000, Bruce Richardson wrote:
> On Wed, Jan 24, 2024 at 12:34:50PM +0100, Mattias Rönnblom wrote:
> > On 2024-01-19 18:43, Bruce Richardson wrote:
> > > Clarify the meaning of the NEW, FORWARD and RELEASE event types.
> > > For the fields in "rte_event" struct, enhance the comments on each to
> > > clarify the field's use, and whether it is preserved between enqueue and
> > > dequeue, and it's role, if any, in scheduling.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richard...@intel.com>
> > > ---
> > >
> > > As with the previous patch, please review this patch to ensure that the
> > > expected semantics of the various event types and event fields have not
> > > changed in an unexpected way.
> > > ---
> > > lib/eventdev/rte_eventdev.h | 105 ++++++++++++++++++++++++++----------
> > > 1 file changed, 77 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> > > index cb13602ffb..4eff1c4958 100644
> > > --- a/lib/eventdev/rte_eventdev.h
> > > +++ b/lib/eventdev/rte_eventdev.h
> <snip>
>
> > > /**
> > > @@ -1473,53 +1475,100 @@ struct rte_event {
> > > /**< Targeted flow identifier for the enqueue
> > > and
> > > * dequeue operation.
> > > * The value must be in the range of
> > > - * [0, nb_event_queue_flows - 1] which
> > > + * [0, @ref rte_event_dev_config.nb_event_queue_flows -
> > > 1] which
> >
> > The same comment as I had before about ranges for unsigned types.
> >
> Actually, is this correct, does a range actually apply here?
>
> I thought that the number of queue flows supported was a guide as to how
> internal HW resources were to be allocated, and that the flow_id was always
> a 20-bit value, where it was up to the scheduler to work out how to map
> that to internal atomic locks (when combined with queue ids etc.). It
> should not be up to the app to have to do the range limiting itself!
>
Looking at the RX adapter in eventdev, I don't see any obvious clamping of
the flow ids to the range of 0-nb_event_queue_flows, though I'm not that
familiar with that code, so I may have missed something. If I'm right,
it looks like this doc line may indeed by a mistake.
@Jerin, can you comment again here. Is flow_id really meant to be limited
to the specified range, or is it a full 20-bit value supplied in all cases?
Thanks,
/Bruce
