> -----Original Message----- > From: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com> > Sent: Thursday, July 14, 2022 7:33 AM > To: mattias.ronnblom <mattias.ronnb...@ericsson.com>; Thomas Monjalon > <tho...@monjalon.net> > Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Ray Kinsella > <m...@ashroe.eu>; > dev@dpdk.org; McDaniel, Timothy <timothy.mcdan...@intel.com>; Hemant > Agrawal <hemant.agra...@nxp.com>; sachin.sax...@oss.nxp.com; > lian...@liangbit.com; Mccarthy, Peter <peter.mccar...@intel.com>; Van Haaren, > Harry <harry.van.haa...@intel.com>; Carrillo, Erik G > <erik.g.carri...@intel.com>; > Gujjar, Abhinandan S <abhinandan.guj...@intel.com>; Jayatheerthan, Jay > <jay.jayatheert...@intel.com>; Burakov, Anatoly <anatoly.bura...@intel.com> > Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type > > > > > -----Original Message----- > > From: Mattias Rönnblom <mattias.ronnb...@ericsson.com> > > Sent: Wednesday, July 13, 2022 5:45 PM > > To: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com>; Thomas > > Monjalon <tho...@monjalon.net> > > Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Ray Kinsella > > <m...@ashroe.eu>; dev@dpdk.org; timothy.mcdan...@intel.com; Hemant > > Agrawal <hemant.agra...@nxp.com>; sachin.sax...@oss.nxp.com; > > lian...@liangbit.com; peter.mccar...@intel.com; > > harry.van.haa...@intel.com; erik.g.carri...@intel.com; > > abhinandan.guj...@intel.com; jay.jayatheert...@intel.com; > > anatoly.bura...@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.ronnb...@ericsson.com> > > >> Sent: Wednesday, July 13, 2022 2:39 PM > > >> To: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com>; Thomas > > >> Monjalon <tho...@monjalon.net> > > >> Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Ray Kinsella > > >> <m...@ashroe.eu>; dev@dpdk.org; timothy.mcdan...@intel.com; > > Hemant > > >> Agrawal <hemant.agra...@nxp.com>; sachin.sax...@oss.nxp.com; > > >> lian...@liangbit.com; peter.mccar...@intel.com; > > >> harry.van.haa...@intel.com; erik.g.carri...@intel.com; > > >> abhinandan.guj...@intel.com; jay.jayatheert...@intel.com; > > >> anatoly.bura...@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.mcdan...@intel.com; > > >>> hemant.agra...@nxp.com; > > >>> sachin.sax...@oss.nxp.com; > > >>> mattias.ronnb...@ericsson.com; > > >>> jer...@marvell.com; > > >>> lian...@liangbit.com; > > >>> peter.mccar...@intel.com; > > >>> harry.van.haa...@intel.com; > > >>> erik.g.carri...@intel.com; > > >>> abhinandan.guj...@intel.com; > > >>> jay.jayatheert...@intel.com; > > >>> m...@ashroe.eu; > > >>> anatoly.bura...@intel.com; > > >>> > > >>> > > >>>> -----Original Message----- > > >>>> From: Thomas Monjalon <tho...@monjalon.net> > > >>>> Sent: Tuesday, July 12, 2022 8:31 PM > > >>>> To: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com> > > >>>> Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Ray Kinsella > > >>>> <m...@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula > > >>>> <pbhagavat...@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, pbhagavat...@marvell.com: > > >>>>> From: Pavan Nikhilesh <pbhagavat...@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 <pbhagavat...@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- > pbhagavat...@marvell.com/
I've reviewed the above; I'm not in favour of adding even more APIs to the Eventdev level. Adding even more enq APIs just overloads applications with options; today we already have multiple APIs; rte_event_enqueue_burst() rte_event_enqueue_new_burst() rte_event_enqueue_forward_burst() Now the suggestion is to add another one, rte_event_enqueue_new_queue_burst()? For an Ethdev analogy: we have rx_burst() and tx_burst(). There is no tx_to_same_dest_ip_burst(). The driver already has all knowledge required if "all events going to same destination", so it can optimize for that case already internally. I think Mattias was asking similar questions, around PMD having enough info today already. Pushing more APIs and complexity to Application level doesn't seem a good direction to me. > >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. > > >
