DPDK patches and discussions
 help / color / mirror / Atom feed
From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
To: "Mattias Rönnblom" <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>,
	"timothy.mcdaniel@intel.com" <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>,
	"peter.mccarthy@intel.com" <peter.mccarthy@intel.com>,
	"harry.van.haaren@intel.com" <harry.van.haaren@intel.com>,
	"erik.g.carrillo@intel.com" <erik.g.carrillo@intel.com>,
	"abhinandan.gujjar@intel.com" <abhinandan.gujjar@intel.com>,
	"jay.jayatheerthan@intel.com" <jay.jayatheerthan@intel.com>,
	"anatoly.burakov@intel.com" <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:16:19 +0000	[thread overview]
Message-ID: <PH0PR18MB4086A11EB5CC1FFE0D48F637DE8B9@PH0PR18MB4086.namprd18.prod.outlook.com> (raw)
In-Reply-To: <ec4922ea-74e5-dbee-02ee-ff4140cacec2@ericsson.com>



> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Sent: Friday, July 15, 2022 1:26 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
> Monjalon <thomas@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com; Hemant
> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> liangma@liangbit.com; peter.mccarthy@intel.com;
> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> anatoly.burakov@intel.com
> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> 
> On 2022-07-14 18:42, Pavan Nikhilesh Bhagavatula wrote:
> >
> >
> >> -----Original Message-----
> >> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >> Sent: Thursday, July 14, 2022 4:13 PM
> >> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
> >> Monjalon <thomas@monjalon.net>
> >> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
> Hemant
> >> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> >> liangma@liangbit.com; peter.mccarthy@intel.com;
> >> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> >> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> >> anatoly.burakov@intel.com
> >> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> event
> >> type
> >>
> >> On 2022-07-14 08:32, Pavan Nikhilesh Bhagavatula wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >>>> Sent: Wednesday, July 13, 2022 5:45 PM
> >>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> Thomas
> >>>> Monjalon <thomas@monjalon.net>
> >>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
> >> Hemant
> >>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> >>>> liangma@liangbit.com; peter.mccarthy@intel.com;
> >>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> >>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> >>>> anatoly.burakov@intel.com
> >>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> >> event
> >>>> type
> >>>>
> >>>> On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote:
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >>>>>> Sent: Wednesday, July 13, 2022 2:39 PM
> >>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> >> Thomas
> >>>>>> Monjalon <thomas@monjalon.net>
> >>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >>>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
> >>>> Hemant
> >>>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> >>>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
> >>>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> >>>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> >>>>>> anatoly.burakov@intel.com
> >>>>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> >>>> event
> >>>>>> type
> >>>>>>
> >>>>>> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
> >>>>>>> +Cc
> >>>>>>> timothy.mcdaniel@intel.com;
> >>>>>>> hemant.agrawal@nxp.com;
> >>>>>>> sachin.saxena@oss.nxp.com;
> >>>>>>> mattias.ronnblom@ericsson.com;
> >>>>>>> jerinj@marvell.com;
> >>>>>>> liangma@liangbit.com;
> >>>>>>> peter.mccarthy@intel.com;
> >>>>>>> harry.van.haaren@intel.com;
> >>>>>>> erik.g.carrillo@intel.com;
> >>>>>>> abhinandan.gujjar@intel.com;
> >>>>>>> jay.jayatheerthan@intel.com;
> >>>>>>> mdr@ashroe.eu;
> >>>>>>> anatoly.burakov@intel.com;
> >>>>>>>
> >>>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>>>>> Sent: Tuesday, July 12, 2022 8:31 PM
> >>>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> >>>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >>>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
> >>>>>>>> <pbhagavatula@marvell.com>
> >>>>>>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> >> event
> >>>>>> type
> >>>>>>>>
> >>>>>>>> External Email
> >>>>>>>>
> >>>>>>>> ----------------------------------------------------------------------
> >>>>>>>> I'm not your teacher, but please consider Cc'ing people outside of
> >> your
> >>>>>>>> company.
> >>>>>>>>
> >>>>>>>
> >>>>>>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks
> like
> >> it's
> >>>>>> useless for
> >>>>>>> sending deprecation notices.
> >>>>>>>
> >>>>>>> Maybe we should update it to include lib/driver maintainers when
> diff
> >>>> sees
> >>>>>> deprecation.rst
> >>>>>>>
> >>>>>>>>
> >>>>>>>> 27/06/2022 11:57, pbhagavatula@marvell.com:
> >>>>>>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>>>>>>>
> >>>>>>>>> A new field ``max_event_port_enqueue_new_burst`` will be
> >> added
> >>>> to
> >>>>>> the
> >>>>>>>>> structure ``rte_event_dev_info``. The field defines the max
> >> enqueue
> >>>>>>>>> burst size of new events (OP_NEW) supported by the underlying
> >>>> event
> >>>>>>>>> device.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>>>>>>> ---
> >>>>>>>>>      doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>>>>>>>>      1 file changed, 5 insertions(+)
> >>>>>>>>>
> >>>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>>>>>> b/doc/guides/rel_notes/deprecation.rst
> >>>>>>>>> index 4e5b23c53d..071317e8e3 100644
> >>>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>>>>>> @@ -125,3 +125,8 @@ Deprecation Notices
> >>>>>>>>>        applications should be updated to use the ``dmadev`` library
> >>>> instead,
> >>>>>>>>>        with the underlying HW-functionality being provided by the
> >> ``ioat``
> >>>> or
> >>>>>>>>>        ``idxd`` dma drivers
> >>>>>>>>> +
> >>>>>>>>> +* eventdev: The structure ``rte_event_dev_info`` will be
> >> extended
> >>>> to
> >>>>>>>> include the
> >>>>>>>>> +  max enqueue burst size of new events supported by the
> >>>> underlying
> >>>>>>>> event device.
> >>>>>>>>> +  A new field ``max_event_port_enqueue_new_burst`` will be
> >>>> added
> >>>>>> to
> >>>>>>>> the structure
> >>>>>>>>> +  ``rte_event_dev_info`` in DPDK 22.11.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>>> Why is this needed, or useful? Why isn't called something with
> >>>>>> "enqueue_depth" in it, like the already-present field?
> >>>>>>
> >>>>>
> >>>>> This is for a case where enqueues with OP_FORWARD/OP_RELEASE
> only
> >>>> supports
> >>>>> enqueue depth of 1.
> >>>>
> >>>> I assume it's for other cases as well? Any case when the event device
> >>>> has a max forward enqueue depth != max new enqueue depth?
> >>>>
> >>>
> >>> Yes, generally new events have much more flexibility than forwards
> event.
> >>>
> >>>> I don't see why an event device would have such low max limit on the
> >>>> number events enqueued.
> >>>
> >>> It depends on the number of scheduling contexts a given event port can
> >> track.
> >>
> >> I have no idea what a "scheduling context" is (it's not something
> >> related to the Eventdev APIs), but if you have a shortage of them (for
> >> the moment, or always), the driver would just return a short count from
> >> the enqueue burst function. What use has the application of knowing that
> >> the event device can accept at most a certain number of events?
> >>
> >>> Anyway this would align to the current existing feature definitions. The
> >> existing
> >>> API only defines the enqueue size of fwd and release events i.e.
> >> scheduling contexts
> >>> already tracked by event device.
> >>
> >> The documentation of max_event_port_enqueue_depth says:
> >> "Maximum number of events can be enqueued at a time from an event
> port
> >> by this device."
> >>
> >>   From what I can tell, it says nothing about new versus forward, or
> >> release type events.
> >>
> >>> NEW is always a special case as we are adding new scheduling contexts
> to
> >> the event
> >>> device, the idea of this patch is to specify that NEW events wouldn’t
> have
> >> the same
> >>> restrictions of fwd/release events.
> >>>
> >>> This would also allow us to craft optimized APIs such as
> >>> https://urldefense.proofpoint.com/v2/url?u=https-
> >> 3A__protect2.fireeye.com_v1_url-3Fk-3D31323334-2D501d5122-
> 2D313273af-
> >> 2D454445555731-2Def12ffaf4bdb568f-26q-3D1-26e-3Df0a92ad3-2D1167-
> >> 2D448d-2Dbe14-2D0987e939f019-26u-3Dhttps-253A-252F-
> >> 252Fpatches.dpdk.org-252Fproject-252Fdpdk-252Fpatch-
> >> 252F20220627095702.8047-2D2-2Dpbhagavatula-2540marvell.com-
> >> 252F&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> >> fmvgGV3o-g_fjLhk5Pupi9ijohpc&m=iazRQtbylIuGXRfj2NPpf_wwG-
> >>
> ppSwFOPKclj5zcwu2pQG9b7ovePiSBn9k4PjQZ&s=sTrEcWbst59oK_jYe6jXhya
> >> CWwELib6Yr-3d9BccQt4&e=
> >>>
> >>>
> >>>> 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.
> >
> 
> OK, so the limitation we are discussing here is not really related to
> enqueue bursts, but the number of allowed events that can be in-flight
> (in-progress) on the eventdev port?

Yes that’s correct.

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.

> 
> So what happens if you announce some larger max enqueue depth in
> dev_info? If the application can't dequeue more than one event at a
> time, which means it can't possible enqueue more than one forward event
> at a time. Or am I missing something? Even with implicit release turned
> off, the PMD can deny the application having more than one outstanding
> event.

My intention was to cleanly differentiate between OP_FWD and OP_NEW as 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.

> 
> One issue is head-of-line blocking if you mix new and forward events,
> but that you can solve on the application level by separating new and
> forward bursts. That problem already exists, but for back pressure
> scenarios, where new events are disallowed, but forward are not.
> 
> > 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.
> >
> 
> What example? What does "pick the worker" mean?

Pick worker functions:
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

> 
> > 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.
> >
> >>
> >>>> amortize the function call overhead and potentially other software
> >>>> operations (credit system management etc) over multiple events. Plus,
> >>>> the application doesn't need a special case for new events versus
> >>>> forward type events, or this-versus-that event device.
> >>>>
> >>>>> Where as OP_NEW supports higher burst size.
> >>>>>
> >>>>> This is already tied into capability description :
> >>>>>
> >>>>> #define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
> >>>>> /**< Event device is capable of operating in burst mode for
> >>>> enqueue(forward,
> >>>>>     * release) and dequeue operation. If this capability is not set,
> >> application
> >>>>>     * still uses the rte_event_dequeue_burst() and
> >>>> rte_event_enqueue_burst() but
> >>>>>     * PMD accepts only one event at a time.
> >>>>>     *
> >>>>>     * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
> >>>>>     */
> >>>>>
> >>>>>> I think I would rather remove all fields related to the max
> >>>>>> enqueue/dequeue burst sizes. They serve no purpose, as far as I
> see.
> >> If
> >>>>>> you have some HW limit on the maximum number of new events it
> can
> >>>>>> accept, have the driver retry until backpressure occurs.
> >>>>>
> >>>
> >


  reply	other threads:[~2022-07-15 13:18 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
2022-07-15  7:56                 ` Mattias Rönnblom
2022-07-15 13:16                   ` Pavan Nikhilesh Bhagavatula [this message]
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=PH0PR18MB4086A11EB5CC1FFE0D48F637DE8B9@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).