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

["Mattias Rönnblom" <mattias.ronnblom@ericsson.com>]

On 2022-07-14 18:57, Van Haaren, Harry wrote:
>> -----Original Message-----
>> From: Van Haaren, Harry
>> Sent: Thursday, July 14, 2022 5:54 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.
>>
>>> 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] https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-454445555731-e92a9cef34b4dac6&q=1&e=fb252b76-35e7-4476-b756-c64849e9bf54&u=http%3A%2F%2Fmails.dpdk.org%2Farchives%2Fdev%2F2022-July%2F246717.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.

Quantifying the performance benefits would be helpful, I think.

> 
> 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@intel.com>
>