DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>,
	"Van Haaren, Harry" <harry.van.haaren@intel.com>
Cc: "mattias. ronnblom" <mattias.ronnblom@ericsson.com>,
	dev@dpdk.org, Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	Ray Kinsella <mdr@ashroe.eu>, "dev@dpdk.org" <dev@dpdk.org>,
	"McDaniel, Timothy" <timothy.mcdaniel@intel.com>,
	Hemant Agrawal <hemant.agrawal@nxp.com>,
	"sachin.saxena@oss.nxp.com" <sachin.saxena@oss.nxp.com>,
	"liangma@liangbit.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
Date: Sun, 17 Jul 2022 14:38:02 +0200	[thread overview]
Message-ID: <1754321.3VsfAaAtOV@thomas> (raw)
In-Reply-To: <BN0PR11MB5712ED8A8C4EF03B9A5CAA0CD7889@BN0PR11MB5712.namprd11.prod.outlook.com>

14/07/2022 18:57, Van Haaren, Harry:
> > -----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] 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@intel.com>

It doesn't make sense to add a deprecation notice
if the direction is not agreed.

Let's discuss further and ask for techboard help if needed.
In general, I would not be surprised that it's time for a cleanup in this API.



  parent reply	other threads:[~2022-07-17 12:38 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-27  9:57 pbhagavatula
2022-06-27  9:57 ` [PATCH 2/2] eventdev: add function to enq new events to the same queue pbhagavatula
2022-07-11 14:54 ` [PATCH 1/2] doc: add enqueue depth for new event type Jerin Jacob
2022-07-12 15:01 ` Thomas Monjalon
2022-07-12 18:11   ` [EXT] " Pavan Nikhilesh Bhagavatula
2022-07-12 20:47     ` Thomas Monjalon
2022-07-13  3:15     ` Hemant Agrawal
2022-07-13  9:08     ` Mattias Rönnblom
2022-07-13 10:40       ` Pavan Nikhilesh Bhagavatula
2022-07-13 12:15         ` Mattias Rönnblom
2022-07-14  6:32           ` Pavan Nikhilesh Bhagavatula
2022-07-14  9:45             ` Van Haaren, Harry
2022-07-14 10:53               ` Mattias Rönnblom
2022-07-14 14:44                 ` Pavan Nikhilesh Bhagavatula
2022-07-15  7:43                   ` Mattias Rönnblom
2022-07-14 10:43             ` Mattias Rönnblom
2022-07-14 16:42               ` Pavan Nikhilesh Bhagavatula
2022-07-14 16:53                 ` Van Haaren, Harry
2022-07-14 16:57                   ` Van Haaren, Harry
2022-07-15  8:13                     ` Mattias Rönnblom
2022-07-17 12:38                     ` Thomas Monjalon [this message]
2022-07-14 18:01                   ` Pavan Nikhilesh Bhagavatula
2022-07-15  7:49                     ` Van Haaren, Harry
2022-07-15 13:09                       ` Pavan Nikhilesh Bhagavatula
2022-07-15  7:56                 ` Mattias Rönnblom
2022-07-15 13:16                   ` Pavan Nikhilesh Bhagavatula
2022-07-17 20:23                     ` Mattias Rönnblom

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1754321.3VsfAaAtOV@thomas \
    --to=thomas@monjalon.net \
    --cc=abhinandan.gujjar@intel.com \
    --cc=anatoly.burakov@intel.com \
    --cc=dev@dpdk.org \
    --cc=erik.g.carrillo@intel.com \
    --cc=harry.van.haaren@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=jay.jayatheerthan@intel.com \
    --cc=jerinj@marvell.com \
    --cc=liangma@liangbit.com \
    --cc=mattias.ronnblom@ericsson.com \
    --cc=mdr@ashroe.eu \
    --cc=pbhagavatula@marvell.com \
    --cc=peter.mccarthy@intel.com \
    --cc=sachin.saxena@oss.nxp.com \
    --cc=timothy.mcdaniel@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).