From: "Mattias Rönnblom" <mattias.ronnblom@ericsson.com>
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" <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 10:53:41 +0000 [thread overview]
Message-ID: <dec48a01-09c6-e020-9d9d-c89d5fd11b16@ericsson.com> (raw)
In-Reply-To: <BN0PR11MB5712964A2B0F849A081D692DD7889@BN0PR11MB5712.namprd11.prod.outlook.com>
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://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-454445555731-6bd5fd62af0f8992&q=1&e=c4f1686f-725e-48bf-aa62-069489e74009&u=https%3A%2F%2Fpatches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20220627095702.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()?
>
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.
> 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 10:53 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 [this message]
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=dec48a01-09c6-e020-9d9d-c89d5fd11b16@ericsson.com \
--to=mattias.ronnblom@ericsson.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=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).