On 2022-07-14 11:45, Van Haaren, Harry wrote: >> -----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://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-454445555731-6bd5fd62af0f8992&q=1&e=c4f1686f-725e-48bf-aa62-069489e74009&u=https%3A%2F%2Fpatches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20220627095702.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()? >
Why not three new ones? And why not also rte_event_queue(_new_|_forward_|)(_queue_|)priority_burst()? Plus maybe you want functions that enqueue to the same flow id as well? It's like you can almost hear the combinatorial explosion go off. :) Btw, isn't this the problem that the event vector functionality is supposed to solve? Enqueue many similar events to the same destination. > 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. >>>> >
