> -----Original Message----- > From: Mattias Rönnblom <mattias.ronnb...@ericsson.com> > Sent: Thursday, July 14, 2022 4:24 PM > To: Van Haaren, Harry <harry.van.haa...@intel.com>; 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; 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>; > 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 > > 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://urldefense.proofpoint.com/v2/url?u=https- > 3A__protect2.fireeye.com_v1_url-3Fk-3D31323334-2D501d5122-2D313273af- > 2D454445555731-2D6bd5fd62af0f8992-26q-3D1-26e-3Dc4f1686f-2D725e- > 2D48bf-2Daa62-2D069489e74009-26u-3Dhttps-253A-252F- > 252Fpatches.dpdk.org-252Fproject-252Fdpdk-252Fpatch- > 252F20220627095702.8047-2D2- > 2D&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB- > fmvgGV3o-g_fjLhk5Pupi9ijohpc&m=-Mr6gUdwMnOZbXSlWeHhYpTVt7UdA- > VHDmIC_MuUjne3U5nqwFeoOatsEX10pzow&s=Khz6GF4271RAiO7- > 5BY1J2u0BvT8CwWvfwvRSnzeeeM&e= > >> 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.
Maybe we could move this to rte_event_enqueue_new_burst() by adding an additional flags param? This is already done for an existing API rte_event_eth_tx_adapter_enqueue where flags is used to signify same Tx queue destination. * @param flags * RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_ flags. * #RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_SAME_DEST signifies that all the packets * which are enqueued are destined for the same Ethernet port & Tx queue. static inline uint16_t rte_event_eth_tx_adapter_enqueue(uint8_t dev_id, uint8_t port_id, struct rte_event ev[], uint16_t nb_events, const uint8_t flags) > > > 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. > >>>> > >