From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
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" <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 09:45:05 +0000 [thread overview]
Message-ID: <BN0PR11MB5712964A2B0F849A081D692DD7889@BN0PR11MB5712.namprd11.prod.outlook.com> (raw)
In-Reply-To: <PH0PR18MB40864E9F995AD8D8F10755D7DE889@PH0PR18MB4086.namprd18.prod.outlook.com>
> -----Original Message-----
> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> Sent: Thursday, July 14, 2022 7:33 AM
> 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
>
>
>
> > -----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.
> 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.
> 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://patches.dpdk.org/project/dpdk/patch/20220627095702.8047-2-
> pbhagavatula@marvell.com/
I've reviewed the above; I'm not in favour of adding even more APIs to the Eventdev level.
Adding even more enq APIs just overloads applications with options; today we already have
multiple APIs;
rte_event_enqueue_burst()
rte_event_enqueue_new_burst()
rte_event_enqueue_forward_burst()
Now the suggestion is to add another one,
rte_event_enqueue_new_queue_burst()?
For an Ethdev analogy: we have rx_burst() and tx_burst(). There is no tx_to_same_dest_ip_burst().
The driver already has all knowledge required if "all events going to same destination",
so it can optimize for that case already internally. I think Mattias was asking similar questions,
around PMD having enough info today already.
Pushing more APIs and complexity to Application level doesn't seem a good direction to me.
> >If the underlying hardware has some limitations,
> > why not let the driver loop until back pressure occurs? Then you can
> > 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.
> > >
next prev parent reply other threads:[~2022-07-14 9:45 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 [this message]
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
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=BN0PR11MB5712964A2B0F849A081D692DD7889@BN0PR11MB5712.namprd11.prod.outlook.com \
--to=harry.van.haaren@intel.com \
--cc=abhinandan.gujjar@intel.com \
--cc=anatoly.burakov@intel.com \
--cc=dev@dpdk.org \
--cc=erik.g.carrillo@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=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).