RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type

[Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>]

> -----Original Message-----
> From: Van Haaren, Harry <harry.van.haaren@intel.com>
> Sent: Thursday, July 14, 2022 10:24 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>; dev@dpdk.org; McDaniel, Timothy
> <timothy.mcdaniel@intel.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>;
> Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> 
> > -----Original Message-----
> > From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > Sent: Thursday, July 14, 2022 5:42 PM
> > To: mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas
> Monjalon
> > <thomas@monjalon.net>
> > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>;
> > dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>;
> Hemant
> > Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> > liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>; Van
> Haaren,
> > Harry <harry.van.haaren@intel.com>; Carrillo, Erik G
> <erik.g.carrillo@intel.com>;
> > Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> > <jay.jayatheerthan@intel.com>; Burakov, Anatoly
> <anatoly.burakov@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.

Currently burst check is done as follows:
http://git.dpdk.org/dpdk/tree/app/test-eventdev/test_perf_common.c#n545
http://git.dpdk.org/dpdk/tree/app/test-eventdev/evt_common.h#n99

> > 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.

> 
> 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>