> -----Original Message----- > From: Pavan Nikhilesh Bhagavatula <[email protected]> > Sent: Thursday, July 14, 2022 7:33 AM > To: mattias.ronnblom <[email protected]>; Thomas Monjalon > <[email protected]> > Cc: Jerin Jacob Kollanukkaran <[email protected]>; Ray Kinsella > <[email protected]>; > [email protected]; McDaniel, Timothy <[email protected]>; Hemant > Agrawal <[email protected]>; [email protected]; > [email protected]; Mccarthy, Peter <[email protected]>; Van Haaren, > Harry <[email protected]>; Carrillo, Erik G > <[email protected]>; > Gujjar, Abhinandan S <[email protected]>; Jayatheerthan, Jay > <[email protected]>; Burakov, Anatoly <[email protected]> > Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type > > > > > -----Original Message----- > > From: Mattias Rönnblom <[email protected]> > > Sent: Wednesday, July 13, 2022 5:45 PM > > To: Pavan Nikhilesh Bhagavatula <[email protected]>; Thomas > > Monjalon <[email protected]> > > Cc: Jerin Jacob Kollanukkaran <[email protected]>; Ray Kinsella > > <[email protected]>; [email protected]; [email protected]; Hemant > > Agrawal <[email protected]>; [email protected]; > > [email protected]; [email protected]; > > [email protected]; [email protected]; > > [email protected]; [email protected]; > > [email protected] > > 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 <[email protected]> > > >> Sent: Wednesday, July 13, 2022 2:39 PM > > >> To: Pavan Nikhilesh Bhagavatula <[email protected]>; Thomas > > >> Monjalon <[email protected]> > > >> Cc: Jerin Jacob Kollanukkaran <[email protected]>; Ray Kinsella > > >> <[email protected]>; [email protected]; [email protected]; > > Hemant > > >> Agrawal <[email protected]>; [email protected]; > > >> [email protected]; [email protected]; > > >> [email protected]; [email protected]; > > >> [email protected]; [email protected]; > > >> [email protected] > > >> 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 > > >>> [email protected]; > > >>> [email protected]; > > >>> [email protected]; > > >>> [email protected]; > > >>> [email protected]; > > >>> [email protected]; > > >>> [email protected]; > > >>> [email protected]; > > >>> [email protected]; > > >>> [email protected]; > > >>> [email protected]; > > >>> [email protected]; > > >>> [email protected]; > > >>> > > >>> > > >>>> -----Original Message----- > > >>>> From: Thomas Monjalon <[email protected]> > > >>>> Sent: Tuesday, July 12, 2022 8:31 PM > > >>>> To: Pavan Nikhilesh Bhagavatula <[email protected]> > > >>>> Cc: Jerin Jacob Kollanukkaran <[email protected]>; Ray Kinsella > > >>>> <[email protected]>; [email protected]; Pavan Nikhilesh Bhagavatula > > >>>> <[email protected]> > > >>>> 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, [email protected]: > > >>>>> From: Pavan Nikhilesh <[email protected]> > > >>>>> > > >>>>> 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 <[email protected]> > > >>>>> --- > > >>>>> 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- > [email protected]/
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. > > >
