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: Fri, 15 Jul 2022 13:09:26 +0000 [thread overview]
Message-ID: <PH0PR18MB4086AB6A28EE66F239321F79DE8B9@PH0PR18MB4086.namprd18.prod.outlook.com> (raw)
In-Reply-To: <BN0PR11MB5712439D74ED5B0DC5D9E540D78B9@BN0PR11MB5712.namprd11.prod.outlook.com>
> -----Original Message-----
> From: Van Haaren, Harry <harry.van.haaren@intel.com>
> Sent: Friday, July 15, 2022 1:20 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 7:01 PM
> > 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; 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: 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.
>
> If this is the "main problem", then 2 steps:
> 1) Let the driver report it supports BURST, and app will try to use it
> 2A) Let user enq bursts of FWD/REL, and accept only 1 (expecting app to
> retry with rest of burst, as is common)
> 2B) Put a retry loop inside the PMD, until actual backpressure is hit in HW,
> then return to App.
>
Yeah, we could announce capability as burst supported, then set
max_dequeue_depth as 1
max_enqueue_depth as -1 (Infinite as open eventdev)
But this might be slightly misleading as -1 would be applicable only for OP_NEW.
We cannot put a retry loop for OP_FWD/OP_RELEASE as it simply doesn’t make sense
from HW PoV.
In CNXK each event port tracks only one scheduling context (held on dequeue),
and OP_FWD/OP_RELEASE translate to commands to the device to operate on the
the scheduling context. There can be only one pending command per a "scheduling context"
until the next dequeue.
I understand it can be done as stated above but it might mislead an application writer as he
might interpret the max_enqueue_depth to be applicable for OP_FWD/OP_NEW unless
explicitly stated.
>
> >
> > Currently burst check is done as follows:
> > https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__git.dpdk.org_dpdk_tree_app_test-2Deventdev_test-5Fperf-
> 5Fcommon.c-
> 23n545&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> fmvgGV3o-
> g_fjLhk5Pupi9ijohpc&m=vLB2LhvU8CB6ljBToaWpY30DLKEj1ELSFBx7CWAgF2
> MXInVg3fFQXw0iPtu_nKou&s=N4paenk2QVADf4igi-erY6XS9Ya-
> m5iMC19m2IMvpAM&e=
> > https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__git.dpdk.org_dpdk_tree_app_test-2Deventdev_evt-5Fcommon.h-
> 23n99&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> fmvgGV3o-
> g_fjLhk5Pupi9ijohpc&m=vLB2LhvU8CB6ljBToaWpY30DLKEj1ELSFBx7CWAgF2
> MXInVg3fFQXw0iPtu_nKou&s=eB2IU-
> ixeBz3473hwsL4erScT5XReSe3vk72hnku3zM&e=
> >
> > > > 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.
>
> See above suggestion; the application should already be burst-capable (if it
> wants to be)
> and hence there's "nothing to do" at the app level, if the PMD is reworked to
> 1 and 2B?
>
>
> > > 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>
next prev parent reply other threads:[~2022-07-15 13:11 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
2022-07-15 7:49 ` Van Haaren, Harry
2022-07-15 13:09 ` Pavan Nikhilesh Bhagavatula [this message]
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=PH0PR18MB4086AB6A28EE66F239321F79DE8B9@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).