> -----Original Message----- > From: Van Haaren, Harry <harry.van.haa...@intel.com> > Sent: Friday, July 15, 2022 1:20 PM > To: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com>; > 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>; > 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: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com> > > Sent: Thursday, July 14, 2022 7:01 PM > > To: Van Haaren, Harry <harry.van.haa...@intel.com>; 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>; > 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: Van Haaren, Harry <harry.van.haa...@intel.com> > > > Sent: Thursday, July 14, 2022 10:24 PM > > > To: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com>; > > > 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>; > > > 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: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com> > > > > Sent: Thursday, July 14, 2022 5:42 PM > > > > 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 > > > > > > <snip old conversation> > > > > > > > > >> If the underlying hardware has some limitations, > > > > > >> why not let the driver loop until back pressure occurs? Then you > can > > > > > > > > > > You didn't answer this question. Why not let the driver loop, until > > > > > you > > > > > for some reason or the other can't accept more events? > > > > > > > > CNXK event driver cannot accept forwarding(enq) more than one event > > > that has > > > > been dequeued. Enqueueing more than one event for > > > forwarding/releasing > > > > is a violation from HW perspective, this is currently announced by > BURST > > > capability. > > > > But It can enqueue a burst if new events. > > > > > > Can't the driver just backpressure NEW events? that's what the event/sw > > > driver > > > does in order to limit "new" inflight events. App attempts to enq > FWD/REL, > > > no > > > problem. App enqueues burst of NEW (and there's only N spaces) then > the > > > first N events pass, and the rest are returned to the application. > > > > > > > Yes, driver can backpressure NEW events, in-fact that’s what we do today > even > > with burst size 1 as we need to check if target queue has space. > > > > The main problem is app needs to know that enqueue NEW supports > > burst of events even when capability doesn't report BURST support. > > If this is the "main problem", then 2 steps: > 1) Let the driver report it supports BURST, and app will try to use it > 2A) Let user enq bursts of FWD/REL, and accept only 1 (expecting app to > retry with rest of burst, as is common) > 2B) Put a retry loop inside the PMD, until actual backpressure is hit in HW, > then return to App. >
Yeah, we could announce capability as burst supported, then set max_dequeue_depth as 1 max_enqueue_depth as -1 (Infinite as open eventdev) But this might be slightly misleading as -1 would be applicable only for OP_NEW. We cannot put a retry loop for OP_FWD/OP_RELEASE as it simply doesn’t make sense from HW PoV. In CNXK each event port tracks only one scheduling context (held on dequeue), and OP_FWD/OP_RELEASE translate to commands to the device to operate on the the scheduling context. There can be only one pending command per a "scheduling context" until the next dequeue. I understand it can be done as stated above but it might mislead an application writer as he might interpret the max_enqueue_depth to be applicable for OP_FWD/OP_NEW unless explicitly stated. > > > > > Currently burst check is done as follows: > > https://urldefense.proofpoint.com/v2/url?u=http- > 3A__git.dpdk.org_dpdk_tree_app_test-2Deventdev_test-5Fperf- > 5Fcommon.c- > 23n545&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB- > fmvgGV3o- > g_fjLhk5Pupi9ijohpc&m=vLB2LhvU8CB6ljBToaWpY30DLKEj1ELSFBx7CWAgF2 > MXInVg3fFQXw0iPtu_nKou&s=N4paenk2QVADf4igi-erY6XS9Ya- > m5iMC19m2IMvpAM&e= > > https://urldefense.proofpoint.com/v2/url?u=http- > 3A__git.dpdk.org_dpdk_tree_app_test-2Deventdev_evt-5Fcommon.h- > 23n99&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB- > fmvgGV3o- > g_fjLhk5Pupi9ijohpc&m=vLB2LhvU8CB6ljBToaWpY30DLKEj1ELSFBx7CWAgF2 > MXInVg3fFQXw0iPtu_nKou&s=eB2IU- > ixeBz3473hwsL4erScT5XReSe3vk72hnku3zM&e= > > > > > > If you see the current example implementation we pick the worker > based > > > on > > > > BURST capability for optimizing the enqueue/dequeue by providing a > hint > > > > to the driver layer. > > > > > > Please provide a link to the code? Others are not familiar with the CNXK > > > driver, > > > or the sample code you're referring to... > > > > > > > See above. > > > > > > > > > Although, we could live with aggregating the events at driver layer > based > > > on > > > > queue. We would still require announce burst capability for new events > i.e. > > > > changes to the info structure. > > > > > > As per above, I still don't see a reason why this HW > optimization/limitation > > > needs to be pushed to the application layer. Why can the driver not > handle > > > things by allowing/backpressure to the events it can/can't handle? > > > > > > > We can handle aggregation in the driver i.e. the new API is not needed > although > > doing so is inefficient, our synthetic benchmark shows ~20% drop. > > > > The main issue is that application needs to know that burst enqueue is > supported > > for event with op_type as NEW even when capability doesn’t report BURST > support. > > I think this can only be done if driver reports it via info structure. > > See above suggestion; the application should already be burst-capable (if it > wants to be) > and hence there's "nothing to do" at the app level, if the PMD is reworked to > 1 and 2B? > > > > > In this email thread[1] you've suggested reworking the rx_burst API with > a > > > flag to indicate "same destination". This still pushes the problem to the > > > application, > > > and exposes more HW/PMD specific options. This impl is *slightly* better > > > because it > > > wont' require new APIs for each mode, but also *breaks all existing > apps*!? > > > > > > I'm just not understanding why the application needs to change, and why > it > > > cannot be optimized/handled in the driver layer. > > > > > > [1] https://urldefense.proofpoint.com/v2/url?u=http- > > > 3A__mails.dpdk.org_archives_dev_2022- > > > > 2DJuly_246717.html&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjt > > > KCMVsB-fmvgGV3o- > > > > g_fjLhk5Pupi9ijohpc&m=CyiCnnBdRFmd0maK3yHCkM7_3fDnVGGCeHteXAb > > > > I6DvehYrkk6BvyrMsV_NKsUGs&s=SbdMMotdrG_yzjCRgJc7h_Oq9Jtfl_8V06 > > > QsyPqUfro&e= > > > > > > <snip old conversation>
