DPDK patches and discussions
 help / color / mirror / Atom feed
From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
To: "Van Haaren, Harry" <harry.van.haaren@intel.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" <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: Thu, 14 Jul 2022 18:01:11 +0000	[thread overview]
Message-ID: <PH0PR18MB40862A13084949B65F977A8FDE889@PH0PR18MB4086.namprd18.prod.outlook.com> (raw)
In-Reply-To: <BN0PR11MB57129F76A265FB44CC53541FD7889@BN0PR11MB5712.namprd11.prod.outlook.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>

  parent reply	other threads:[~2022-07-14 18:03 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
2022-07-14 18:01                   ` Pavan Nikhilesh Bhagavatula [this message]
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=PH0PR18MB40862A13084949B65F977A8FDE889@PH0PR18MB4086.namprd18.prod.outlook.com \
    --to=pbhagavatula@marvell.com \
    --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=peter.mccarthy@intel.com \
    --cc=sachin.saxena@oss.nxp.com \
    --cc=thomas@monjalon.net \
    --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).