[EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type

[Pavan Nikhilesh Bhagavatula]

> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnblom at ericsson.com>
> Sent: Wednesday, July 13, 2022 5:45 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula at marvell.com>; Thomas
> Monjalon <thomas at monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj at marvell.com>; Ray Kinsella
> <mdr at ashroe.eu>; dev at dpdk.org; timothy.mcdaniel at intel.com; Hemant
> Agrawal <hemant.agrawal at nxp.com>; sachin.saxena at oss.nxp.com;
> liangma at liangbit.com; peter.mccarthy at intel.com;
> harry.van.haaren at intel.com; erik.g.carrillo at intel.com;
> abhinandan.gujjar at intel.com; jay.jayatheerthan at intel.com;
> anatoly.burakov at intel.com
> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> 
> On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote:
> >
> >
> >> -----Original Message-----
> >> From: Mattias Rönnblom <mattias.ronnblom at ericsson.com>
> >> Sent: Wednesday, July 13, 2022 2:39 PM
> >> To: Pavan Nikhilesh Bhagavatula <pbhagavatula at marvell.com>; Thomas
> >> Monjalon <thomas at monjalon.net>
> >> Cc: Jerin Jacob Kollanukkaran <jerinj at marvell.com>; Ray Kinsella
> >> <mdr at ashroe.eu>; dev at dpdk.org; timothy.mcdaniel at intel.com;
> Hemant
> >> Agrawal <hemant.agrawal at nxp.com>; sachin.saxena at oss.nxp.com;
> >> liangma at liangbit.com; peter.mccarthy at intel.com;
> >> harry.van.haaren at intel.com; erik.g.carrillo at intel.com;
> >> abhinandan.gujjar at intel.com; jay.jayatheerthan at intel.com;
> >> anatoly.burakov at intel.com
> >> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> event
> >> type
> >>
> >> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
> >>> +Cc
> >>> timothy.mcdaniel at intel.com;
> >>> hemant.agrawal at nxp.com;
> >>> sachin.saxena at oss.nxp.com;
> >>> mattias.ronnblom at ericsson.com;
> >>> jerinj at marvell.com;
> >>> liangma at liangbit.com;
> >>> peter.mccarthy at intel.com;
> >>> harry.van.haaren at intel.com;
> >>> erik.g.carrillo at intel.com;
> >>> abhinandan.gujjar at intel.com;
> >>> jay.jayatheerthan at intel.com;
> >>> mdr at ashroe.eu;
> >>> anatoly.burakov at intel.com;
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Thomas Monjalon <thomas at monjalon.net>
> >>>> Sent: Tuesday, July 12, 2022 8:31 PM
> >>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula at marvell.com>
> >>>> Cc: Jerin Jacob Kollanukkaran <jerinj at marvell.com>; Ray Kinsella
> >>>> <mdr at ashroe.eu>; dev at dpdk.org; Pavan Nikhilesh Bhagavatula
> >>>> <pbhagavatula at marvell.com>
> >>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> >> type
> >>>>
> >>>> External Email
> >>>>
> >>>> ----------------------------------------------------------------------
> >>>> I'm not your teacher, but please consider Cc'ing people outside of your
> >>>> company.
> >>>>
> >>>
> >>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks like it's
> >> useless for
> >>> sending deprecation notices.
> >>>
> >>> Maybe we should update it to include lib/driver maintainers when diff
> sees
> >> deprecation.rst
> >>>
> >>>>
> >>>> 27/06/2022 11:57, pbhagavatula at marvell.com:
> >>>>> From: Pavan Nikhilesh <pbhagavatula at marvell.com>
> >>>>>
> >>>>> A new field ``max_event_port_enqueue_new_burst`` will be added
> to
> >> the
> >>>>> structure ``rte_event_dev_info``. The field defines the max enqueue
> >>>>> burst size of new events (OP_NEW) supported by the underlying
> event
> >>>>> device.
> >>>>>
> >>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula at marvell.com>
> >>>>> ---
> >>>>>    doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>>>>    1 file changed, 5 insertions(+)
> >>>>>
> >>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>> b/doc/guides/rel_notes/deprecation.rst
> >>>>> index 4e5b23c53d..071317e8e3 100644
> >>>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>> @@ -125,3 +125,8 @@ Deprecation Notices
> >>>>>      applications should be updated to use the ``dmadev`` library
> instead,
> >>>>>      with the underlying HW-functionality being provided by the ``ioat``
> or
> >>>>>      ``idxd`` dma drivers
> >>>>> +
> >>>>> +* eventdev: The structure ``rte_event_dev_info`` will be extended
> to
> >>>> include the
> >>>>> +  max enqueue burst size of new events supported by the
> underlying
> >>>> event device.
> >>>>> +  A new field ``max_event_port_enqueue_new_burst`` will be
> added
> >> to
> >>>> the structure
> >>>>> +  ``rte_event_dev_info`` in DPDK 22.11.
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>
> >> Why is this needed, or useful? Why isn't called something with
> >> "enqueue_depth" in it, like the already-present field?
> >>
> >
> > This is for a case where enqueues with OP_FORWARD/OP_RELEASE only
> supports
> > enqueue depth of 1.
> 
> I assume it's for other cases as well? Any case when the event device
> has a max forward enqueue depth != max new enqueue depth?
> 

Yes, generally new events have much more flexibility than forwards event.

> I don't see why an event device would have such low max limit on the
> number events enqueued. 

It depends on the number of scheduling contexts a given event port can track.
Anyway this would align to the current existing feature definitions. The existing
API only defines the enqueue size of fwd and release events i.e. scheduling contexts
already tracked by event device.
NEW is always a special case as we are adding new scheduling contexts to the event 
device, the idea of this patch is to specify that NEW events wouldn’t have the same
restrictions of fwd/release events.

This would also allow us to craft optimized APIs such as 
https://patches.dpdk.org/project/dpdk/patch/20220627095702.8047-2-pbhagavatula@marvell.com/


>If the underlying hardware has some limitations,
> why not let the driver loop until back pressure occurs? Then you can
> amortize the function call overhead and potentially other software
> operations (credit system management etc) over multiple events. Plus,
> the application doesn't need a special case for new events versus
> forward type events, or this-versus-that event device.
> 
> > Where as OP_NEW supports higher burst size.
> >
> > This is already tied into capability description :
> >
> > #define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
> > /**< Event device is capable of operating in burst mode for
> enqueue(forward,
> >   * release) and dequeue operation. If this capability is not set, application
> >   * still uses the rte_event_dequeue_burst() and
> rte_event_enqueue_burst() but
> >   * PMD accepts only one event at a time.
> >   *
> >   * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
> >   */
> >
> >> I think I would rather remove all fields related to the max
> >> enqueue/dequeue burst sizes. They serve no purpose, as far as I see. If
> >> you have some HW limit on the maximum number of new events it can
> >> accept, have the driver retry until backpressure occurs.
> >