From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
To: "Mattias Rönnblom" <mattias.ronnblom@ericsson.com>,
"Van Haaren, Harry" <harry.van.haaren@intel.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 14:44:18 +0000 [thread overview]
Message-ID: <PH0PR18MB408651AECD6FFD10D03B21A1DE889@PH0PR18MB4086.namprd18.prod.outlook.com> (raw)
In-Reply-To: <dec48a01-09c6-e020-9d9d-c89d5fd11b16@ericsson.com>
> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Sent: Thursday, July 14, 2022 4:24 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; 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; 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
>
> On 2022-07-14 11:45, Van Haaren, Harry wrote:
> >> -----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://urldefense.proofpoint.com/v2/url?u=https-
> 3A__protect2.fireeye.com_v1_url-3Fk-3D31323334-2D501d5122-2D313273af-
> 2D454445555731-2D6bd5fd62af0f8992-26q-3D1-26e-3Dc4f1686f-2D725e-
> 2D48bf-2Daa62-2D069489e74009-26u-3Dhttps-253A-252F-
> 252Fpatches.dpdk.org-252Fproject-252Fdpdk-252Fpatch-
> 252F20220627095702.8047-2D2-
> 2D&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> fmvgGV3o-g_fjLhk5Pupi9ijohpc&m=-Mr6gUdwMnOZbXSlWeHhYpTVt7UdA-
> VHDmIC_MuUjne3U5nqwFeoOatsEX10pzow&s=Khz6GF4271RAiO7-
> 5BY1J2u0BvT8CwWvfwvRSnzeeeM&e=
> >> 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()?
> >
>
> Why not three new ones? And why not also
> rte_event_queue(_new_|_forward_|)(_queue_|)priority_burst()?
>
> Plus maybe you want functions that enqueue to the same flow id as well?
>
> It's like you can almost hear the combinatorial explosion go off. :)
>
> Btw, isn't this the problem that the event vector functionality is
> supposed to solve? Enqueue many similar events to the same destination.
Maybe we could move this to rte_event_enqueue_new_burst() by adding an
additional flags param?
This is already done for an existing API rte_event_eth_tx_adapter_enqueue
where flags is used to signify same Tx queue destination.
* @param flags
* RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_ flags.
* #RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_SAME_DEST signifies that all the packets
* which are enqueued are destined for the same Ethernet port & Tx queue.
static inline uint16_t
rte_event_eth_tx_adapter_enqueue(uint8_t dev_id,
uint8_t port_id,
struct rte_event ev[],
uint16_t nb_events,
const uint8_t flags)
>
> > 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 14:46 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 [this message]
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=PH0PR18MB408651AECD6FFD10D03B21A1DE889@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).