From: "Mattias Rönnblom" <mattias.ronnblom@ericsson.com>
To: Jerin Jacob <jerinjacobk@gmail.com>,
"Van Haaren, Harry" <harry.van.haaren@intel.com>,
"McDaniel, Timothy" <timothy.mcdaniel@intel.com>,
Pavan Nikhilesh <pbhagavatula@marvell.com>,
Hemant Agrawal <hemant.agrawal@nxp.com>,
Liang Ma <liangma@liangbit.com>
Cc: "Richardson, Bruce" <bruce.richardson@intel.com>,
Jerin Jacob <jerinj@marvell.com>,
"Gujjar, Abhinandan S" <abhinandan.gujjar@intel.com>,
Erik Gabriel Carrillo <erik.g.carrillo@intel.com>,
"Jayatheerthan, Jay" <jay.jayatheerthan@intel.com>,
dpdk-dev <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance
Date: Mon, 1 Nov 2021 09:26:38 +0000 [thread overview]
Message-ID: <3a6d30ca-3335-ee06-c0a9-8f2b0ade2c40@ericsson.com> (raw)
In-Reply-To: <CALBAE1N6-Z1Pk4rF-fqCrY+BLEQs1-2HWwXgy7om0zBgDGG-Pw@mail.gmail.com>
On 2021-10-29 17:17, Jerin Jacob wrote:
> On Fri, Oct 29, 2021 at 8:33 PM Mattias Rönnblom
> <mattias.ronnblom@ericsson.com> wrote:
>> On 2021-10-29 16:38, Jerin Jacob wrote:
>>> On Tue, Oct 26, 2021 at 11:02 PM Mattias Rönnblom
>>> <mattias.ronnblom@ericsson.com> wrote:
>>>> Extend Eventdev API to allow for event devices which require various
>>>> forms of internal processing to happen, even when events are not
>>>> enqueued to or dequeued from a port.
>>>>
>>>> PATCH v1:
>>>> - Adapt to the move of fastpath function pointers out of
>>>> rte_eventdev struct
>>>> - Attempt to clarify how often the application is expected to
>>>> call rte_event_maintain()
>>>> - Add trace point
>>>> RFC v2:
>>>> - Change rte_event_maintain() return type to be consistent
>>>> with the documentation.
>>>> - Remove unused typedef from eventdev_pmd.h.
>>>>
>>>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>> Tested-by: Richard Eklycke <richard.eklycke@ericsson.com>
>>>> Tested-by: Liron Himi <lironh@marvell.com>
>>>> ---
>>>>
>>>> +/**
>>>> + * Maintain an event device.
>>>> + *
>>>> + * This function is only relevant for event devices which has the
>>>> + * RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag set. Such devices require the
>>>> + * application to call rte_event_maintain() on a port during periods
>>>> + * which it is neither enqueuing nor dequeuing events from that
>>>> + * port.
>>> # We need to add "by the same core". Right? As other core such as
>>> service core can not call rte_event_maintain()
>>
>> Do you mean by the same lcore thread that "owns" (dequeues and enqueues
>> to) the port? Yes. I thought that was implicit, since eventdev port are
>> not MT safe. I'll try to figure out some wording that makes that more clear.
> OK.
>
>>
>>> # Also, Incase of Adapters enqueue() happens, right? If so, either
>>> above text is not correct.
>>> # @Erik Gabriel Carrillo @Jayatheerthan, Jay @Gujjar, Abhinandan S
>>> Please review 3/3 patch on adapter change.
>>> Let me know you folks are OK with change or not or need more time to analyze.
>>>
>>> If it need only for the adapter subsystem then can we make it an
>>> internal API between DSW and adapters?
>>
>> No, it's needed for any producer-only eventdev ports, including any such
>> ports used by the application.
>
> In that case, the code path in testeventdev, eventdev_pipeline, etc needs
> to be updated. I am worried about the performance impact for the drivers they
> don't have such limitations.
>
> Why not have an additional config option in port_config which says
> it is a producer-only port by an application and takes care of the driver.
>
> In the current adapters code, you are calling maintain() when enqueue
> returns zero.
> In such a case, if the port is configured as producer and then
> internally it can call maintain.
>
> Thoughts from other eventdev maintainers?
> Cc+ @Van Haaren, Harry @Richardson, Bruce @Gujjar, Abhinandan S
> @Jayatheerthan, Jay @Erik Gabriel Carrillo @McDaniel, Timothy @Pavan
> Nikhilesh @Hemant Agrawal @Liang Ma
>
One more thing to consider: should we add a "int op" parameter to
rte_event_maintain()? It would also solve hack #2 in DSW eventdev API
integration: forcing an output buffer flush. This is today done with a
zero-sized rte_event_enqueue() call.
You could have something like:
#define RTE_EVENT_DEV_MAINT_FLUSH (1)
int
rte_event_maintain(int op);
It would also allow future extensions of "maintain", without ABI breakage.
Explicit flush is rare in real applications, in my experience, but
useful for test cases. I suspect for DSW to work with the DPDK eventdev
test suite, flushing buffered events (either zero-sized enqueue,
repeated rte_event_maintain() calls, or a single of the
rte_event_maintain(RTE_EVENT_DEV_MAINT_FLUSH) call [assuming the above
API]) is required in the test code.
>>
>> Should rte_event_maintain() be marked experimental? I don't know how
>> that works for inline functions.
>>
>>
>>> + rte_event_maintain() is a low-overhead function and should be
>>>> + * called at a high rate (e.g., in the applications poll loop).
>>>> + *
>>>> + * No port may be left unmaintained.
>>>> + *
>>>> + * rte_event_maintain() may be called on event devices which haven't
>>>> + * set RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag, in which case it is a
>>>> + * no-operation.
>>>> + *
>>>> + * @param dev_id
>>>> + * The identifier of the device.
>>>> + * @param port_id
>>>> + * The identifier of the event port.
>>>> + * @return
>>>> + * - 0 on success.
>>>> + * - -EINVAL if *dev_id* or *port_id* is invalid
>>>> + *
>>>> + * @see RTE_EVENT_DEV_CAP_REQUIRES_MAINT
>>>> + */
>>>> +static inline int
>>>> +rte_event_maintain(uint8_t dev_id, uint8_t port_id)
>>>> +{
>>>> + const struct rte_event_fp_ops *fp_ops;
>>>> + void *port;
>>>> +
>>>> + fp_ops = &rte_event_fp_ops[dev_id];
>>>> + port = fp_ops->data[port_id];
>>>> +#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
>>>> + if (dev_id >= RTE_EVENT_MAX_DEVS ||
>>>> + port_id >= RTE_EVENT_MAX_PORTS_PER_DEV) {
>>>> + rte_errno = EINVAL;
>>>> + return 0;
>>>> + }
>>>> +
>>>> + if (port == NULL) {
>>>> + rte_errno = EINVAL;
>>>> + return 0;
>>>> + }
>>>> +#endif
>>>> + rte_eventdev_trace_maintain(dev_id, port_id);
>>>> +
>>>> + if (fp_ops->maintain != NULL)
>>>> + fp_ops->maintain(port);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> #ifdef __cplusplus
>>>> }
>>>> #endif
>>>> diff --git a/lib/eventdev/rte_eventdev_core.h b/lib/eventdev/rte_eventdev_core.h
>>>> index 61d5ebdc44..61fa65cab3 100644
>>>> --- a/lib/eventdev/rte_eventdev_core.h
>>>> +++ b/lib/eventdev/rte_eventdev_core.h
>>>> @@ -29,6 +29,9 @@ typedef uint16_t (*event_dequeue_burst_t)(void *port, struct rte_event ev[],
>>>> uint64_t timeout_ticks);
>>>> /**< @internal Dequeue burst of events from port of a device */
>>>>
>>>> +typedef void (*event_maintain_t)(void *port);
>>>> +/**< @internal Maintains a port */
>>>> +
>>>> typedef uint16_t (*event_tx_adapter_enqueue_t)(void *port,
>>>> struct rte_event ev[],
>>>> uint16_t nb_events);
>>>> @@ -54,6 +57,8 @@ struct rte_event_fp_ops {
>>>> /**< PMD dequeue function. */
>>>> event_dequeue_burst_t dequeue_burst;
>>>> /**< PMD dequeue burst function. */
>>>> + event_maintain_t maintain;
>>>> + /**< PMD port maintenance function. */
>>>> event_tx_adapter_enqueue_t txa_enqueue;
>>>> /**< PMD Tx adapter enqueue function. */
>>>> event_tx_adapter_enqueue_t txa_enqueue_same_dest;
>>>> diff --git a/lib/eventdev/rte_eventdev_trace_fp.h b/lib/eventdev/rte_eventdev_trace_fp.h
>>>> index 5639e0b83a..c5a79a14d8 100644
>>>> --- a/lib/eventdev/rte_eventdev_trace_fp.h
>>>> +++ b/lib/eventdev/rte_eventdev_trace_fp.h
>>>> @@ -38,6 +38,13 @@ RTE_TRACE_POINT_FP(
>>>> rte_trace_point_emit_ptr(enq_mode_cb);
>>>> )
>>>>
>>>> +RTE_TRACE_POINT_FP(
>>>> + rte_eventdev_trace_maintain,
>>>> + RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint8_t port_id),
>>>> + rte_trace_point_emit_u8(dev_id);
>>>> + rte_trace_point_emit_u8(port_id);
>>>> +)
>>>> +
>>>> RTE_TRACE_POINT_FP(
>>>> rte_eventdev_trace_eth_tx_adapter_enqueue,
>>>> RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint8_t port_id, void *ev_table,
>>>> --
>>>> 2.25.1
>>>>
next prev parent reply other threads:[~2021-11-01 9:26 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-08 17:56 [dpdk-dev] [RFC " Mattias Rönnblom
2020-04-08 17:56 ` [dpdk-dev] [RFC 2/3] event/dsw: make use of eventdev maintenance facility Mattias Rönnblom
2020-04-08 17:56 ` [dpdk-dev] [RFC 3/3] eventdev: allow devices requiring maintenance with adapters Mattias Rönnblom
2020-04-08 19:36 ` [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance Jerin Jacob
2020-04-09 12:21 ` Mattias Rönnblom
2020-04-09 13:32 ` Jerin Jacob
2020-04-09 14:02 ` Mattias Rönnblom
2020-04-10 13:00 ` Jerin Jacob
2020-04-14 15:57 ` Mattias Rönnblom
2020-04-14 16:15 ` Jerin Jacob
2020-04-14 17:55 ` Mattias Rönnblom
2020-04-16 17:19 ` Jerin Jacob
2020-04-20 9:05 ` Mattias Rönnblom
2020-05-13 18:56 ` Mattias Rönnblom
2021-08-02 16:14 ` [dpdk-dev] [RFC v2 " Mattias Rönnblom
2021-08-02 16:15 ` [dpdk-dev] [RFC v2 2/3] event/dsw: make use of eventdev maintenance facility Mattias Rönnblom
2021-08-02 16:15 ` [dpdk-dev] [RFC v2 3/3] eventdev: have adapters support device maintenance Mattias Rönnblom
2021-08-03 4:39 ` [dpdk-dev] [RFC v2 1/3] eventdev: allow for event devices requiring maintenance Jerin Jacob
2021-08-03 8:26 ` Mattias Rönnblom
2021-08-03 10:06 ` [dpdk-dev] [EXT] " Jerin Jacob Kollanukkaran
2021-10-26 17:31 ` [dpdk-dev] [PATCH " Mattias Rönnblom
2021-10-26 17:31 ` [dpdk-dev] [PATCH 2/3] event/dsw: make use of eventdev maintenance facility Mattias Rönnblom
2021-10-26 17:31 ` [dpdk-dev] [PATCH 3/3] eventdev: have adapters support device maintenance Mattias Rönnblom
2021-10-29 14:38 ` [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance Jerin Jacob
2021-10-29 15:03 ` Mattias Rönnblom
2021-10-29 15:17 ` Jerin Jacob
2021-10-29 16:02 ` Van Haaren, Harry
2021-10-31 9:29 ` Mattias Rönnblom
2021-10-30 17:19 ` Mattias Rönnblom
2021-10-31 13:17 ` Jerin Jacob
2021-11-01 13:28 ` [dpdk-dev] [PATCH v2 " Mattias Rönnblom
2021-11-01 13:28 ` [dpdk-dev] [PATCH v2 2/3] event/dsw: make use of eventdev maintenance facility Mattias Rönnblom
2021-11-01 13:28 ` [dpdk-dev] [PATCH v2 3/3] eventdev: have adapters support device maintenance Mattias Rönnblom
2021-11-04 12:33 ` [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance Jerin Jacob
2021-11-01 9:26 ` Mattias Rönnblom [this message]
2021-11-01 18:40 ` [dpdk-dev] [PATCH v3 " Mattias Rönnblom
2021-11-01 18:40 ` [dpdk-dev] [PATCH v3 2/3] event/dsw: make use of eventdev maintenance facility Mattias Rönnblom
2021-11-01 18:40 ` [dpdk-dev] [PATCH v3 3/3] eventdev: have adapters support device maintenance Mattias Rönnblom
2020-04-09 13:33 ` [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance Eads, Gage
2020-04-09 14:14 ` 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=3a6d30ca-3335-ee06-c0a9-8f2b0ade2c40@ericsson.com \
--to=mattias.ronnblom@ericsson.com \
--cc=abhinandan.gujjar@intel.com \
--cc=bruce.richardson@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=jerinjacobk@gmail.com \
--cc=liangma@liangbit.com \
--cc=pbhagavatula@marvell.com \
--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).