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

[Van Haaren, Harry]

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


Let me be very clear, but also try to help here;
  I'm not in favour of the changes as proposed here, and feel that pushing
  the problem to Application API is NOT the right solution.

But *just in case* there is a *genuine* reason that we need an API/ABI
change, and that can be justified clearly in the upcoming weeks, then I'd
prefer not have problems with the deprecation notice not being included.

This Ack is *only* to allow possible API/ABI changes in 22.11, and not for
the proposal above.

Acked-by: Harry van Haaren <harry.van.haaren at intel.com>