[RFC,v2,1/3] eventdev: allow for event devices requiring maintenance

Message ID 20210802161501.155797-1-mattias.ronnblom@ericsson.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series [RFC,v2,1/3] eventdev: allow for event devices requiring maintenance |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Mattias Rönnblom Aug. 2, 2021, 4:14 p.m. UTC
  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.

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>
---
 lib/eventdev/rte_eventdev.h | 62 +++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)
  

Comments

Jerin Jacob Aug. 3, 2021, 4:39 a.m. UTC | #1
On Mon, Aug 2, 2021 at 9:45 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.
>
> 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>
> ---
>  lib/eventdev/rte_eventdev.h | 62 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>
> +/**
> + * 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 requires
> + * the application to call rte_event_maintain() on a port during periods
> + * which it is neither enqueuing nor dequeuing events from this
> + * port. No port may be left unattended.
> + *
> + * An event device's rte_event_maintain() is a low overhead function. In
> + * situations when rte_event_maintain() must be called, the application
> + * should do so often.

See rte_service_component_register() scheme, If a driver needs additional house
keeping it can use DPDK's service core scheme to abstract different driver
requirements.We may not need any public API for this.
  
Mattias Rönnblom Aug. 3, 2021, 8:26 a.m. UTC | #2
On 2021-08-03 06:39, Jerin Jacob wrote:
> On Mon, Aug 2, 2021 at 9:45 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.
>>
>> 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>
>> ---
>>   lib/eventdev/rte_eventdev.h | 62 +++++++++++++++++++++++++++++++++++++
>>   1 file changed, 62 insertions(+)
>>
>> +/**
>> + * 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 requires
>> + * the application to call rte_event_maintain() on a port during periods
>> + * which it is neither enqueuing nor dequeuing events from this
>> + * port. No port may be left unattended.
>> + *
>> + * An event device's rte_event_maintain() is a low overhead function. In
>> + * situations when rte_event_maintain() must be called, the application
>> + * should do so often.
> 
> See rte_service_component_register() scheme, If a driver needs additional house
> keeping it can use DPDK's service core scheme to abstract different driver
> requirements.We may not need any public API for this.
> 

What DSW requires, and indeed any event device that does software-level 
event buffering, is a way schedule the execution of some function to 
some time later, on the lcore that currently "owns" that port.

Put differently; it's not that the driver "needs some cycles at time T", 
but "it needs some cycles at time T on the lcore thread that currently 
is the user of eventdev port X".

The DSW output buffers and other per-port data structures aren't, for 
simplicity and performance, MT safe. That's one of the reasons the 
processing can't be done by a random service lcore.

Pushing output buffering into the application (or whatever is accessing 
the event device) is not a solution to the DSW<->adapter integration 
issue, since DSW also requires per-port deferred work for the flow 
migration machinery. In addition, if you have a look at the RX adapter, 
for example, you'll see that the buffering logic adds complexity to the 
"application".

The services cores are a rather course-grained deferred work construct. 
A more elaborate one might well have been the basis of a better solution 
than the proposed rte_event_maintain(), user-driven API.

rte_event_maintain() is a crude way to make the Ethernet/Crypto/Timer 
adapters work with DSW. I would argue it still puts us in a better 
position than we are today, where the DSW+adapter combo doesn't work at all.

If/when a more fancy DPDK deferred work framework comes along, 
rte_event_maintain() may be deprecated. Something like work queues in 
Linux could work, run as a DPDK service. In such a case, you might also 
need to require a service-cores-only deployment, and thus disallow the 
use of user-launched lcore threads.

That, however, is not a couple of tiny patches.
  
Jerin Jacob Kollanukkaran Aug. 3, 2021, 10:06 a.m. UTC | #3
> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Sent: Tuesday, August 3, 2021 1:57 PM
> To: Jerin Jacob <jerinjacobk@gmail.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; dpdk-dev
> <dev@dpdk.org>; Richard Eklycke <richard.eklycke@ericsson.com>; Liron Himi
> <lironh@marvell.com>
> Subject: [EXT] Re: [dpdk-dev] [RFC v2 1/3] eventdev: allow for event devices
> requiring maintenance
> 
> External Email
> 
> ----------------------------------------------------------------------
> On 2021-08-03 06:39, Jerin Jacob wrote:
> > On Mon, Aug 2, 2021 at 9:45 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.
> >>
> >> 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>
> >> ---
> >>   lib/eventdev/rte_eventdev.h | 62
> +++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 62 insertions(+)
> >>
> >> +/**
> >> + * 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 requires
> >> + * the application to call rte_event_maintain() on a port during
> >> +periods
> >> + * which it is neither enqueuing nor dequeuing events from this
> >> + * port. No port may be left unattended.
> >> + *
> >> + * An event device's rte_event_maintain() is a low overhead
> >> +function. In
> >> + * situations when rte_event_maintain() must be called, the
> >> +application
> >> + * should do so often.
> >
> > See rte_service_component_register() scheme, If a driver needs
> > additional house keeping it can use DPDK's service core scheme to
> > abstract different driver requirements.We may not need any public API for
> this.
> >
> 
> What DSW requires, and indeed any event device that does software-level
> event buffering, is a way schedule the execution of some function to some time
> later, on the lcore that currently "owns" that port.
> 
> Put differently; it's not that the driver "needs some cycles at time T", but "it
> needs some cycles at time T on the lcore thread that currently is the user of
> eventdev port X".
> 
> The DSW output buffers and other per-port data structures aren't, for simplicity
> and performance, MT safe. That's one of the reasons the processing can't be
> done by a random service lcore.
> 
> Pushing output buffering into the application (or whatever is accessing the
> event device) is not a solution to the DSW<->adapter integration issue, since
> DSW also requires per-port deferred work for the flow migration machinery. In
> addition, if you have a look at the RX adapter, for example, you'll see that the
> buffering logic adds complexity to the "application".
> 
> The services cores are a rather course-grained deferred work construct.
> A more elaborate one might well have been the basis of a better solution than
> the proposed rte_event_maintain(), user-driven API.
> 
> rte_event_maintain() is a crude way to make the Ethernet/Crypto/Timer
> adapters work with DSW. I would argue it still puts us in a better position than
> we are today, where the DSW+adapter combo doesn't work at all.

+ Adapter maintainers

- May only concern of this making as public API where application does not know what
Interval and when to call it.

- We can create an internal API which call be used by Adapters API. No
Need to expose public evendev API for this.

> 
> If/when a more fancy DPDK deferred work framework comes along,
> rte_event_maintain() may be deprecated. Something like work queues in Linux
> could work, run as a DPDK service. In such a case, you might also need to
> require a service-cores-only deployment, and thus disallow the use of user-
> launched lcore threads.
> 
> That, however, is not a couple of tiny patches.
  

Patch

diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index a9c496fb62..2c17d9272e 100644
--- a/lib/eventdev/rte_eventdev.h
+++ b/lib/eventdev/rte_eventdev.h
@@ -299,6 +299,15 @@  struct rte_event;
  * the content of this field is implementation dependent.
  */
 
+#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 10)
+/**< Event device requires calls to rte_event_maintain() during
+ * periods when neither rte_event_dequeue_burst() nor
+ * rte_event_enqueue_burst() are called on a port. This will allow the
+ * event device to perform internal processing, such as flushing
+ * buffered events, return credits to a global pool, or process
+ * signaling related to load balancing.
+ */
+
 /* Event device priority levels */
 #define RTE_EVENT_DEV_PRIORITY_HIGHEST   0
 /**< Highest priority expressed across eventdev subsystem
@@ -1342,6 +1351,9 @@  typedef uint16_t (*event_dequeue_burst_t)(void *port, struct rte_event ev[],
 		uint16_t nb_events, 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)(void *port,
 				struct rte_event ev[], uint16_t nb_events);
 /**< @internal Enqueue burst of events on port of a device */
@@ -1421,6 +1433,8 @@  struct rte_eventdev {
 	/**< Pointer to PMD dequeue function. */
 	event_dequeue_burst_t dequeue_burst;
 	/**< Pointer to PMD dequeue burst function. */
+	event_maintain_t maintain;
+	/**< Maintains an event port. */
 	event_tx_adapter_enqueue_same_dest txa_enqueue_same_dest;
 	/**< Pointer to PMD eth Tx adapter burst enqueue function with
 	 * events destined to same Eth port & Tx queue.
@@ -1758,6 +1772,54 @@  rte_event_dequeue_burst(uint8_t dev_id, uint8_t port_id, struct rte_event ev[],
 				timeout_ticks);
 }
 
+/**
+ * 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 requires
+ * the application to call rte_event_maintain() on a port during periods
+ * which it is neither enqueuing nor dequeuing events from this
+ * port. No port may be left unattended.
+ *
+ * An event device's rte_event_maintain() is a low overhead function. In
+ * situations when rte_event_maintain() must be called, the application
+ * should do so often.
+ *
+ * rte_event_maintain() may be called on event devices which hasn't
+ * set RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag, in which case it is a
+ * successful 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)
+{
+	struct rte_eventdev *dev = &rte_eventdevs[dev_id];
+	event_maintain_t fn;
+
+#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
+	if (dev_id >= RTE_EVENT_MAX_DEVS || !rte_eventdevs[dev_id].attached)
+		return -EINVAL;
+
+	if (port_id >= dev->data->nb_ports)
+		return -EINVAL;
+#endif
+	fn = *dev->maintain;
+
+	if (fn != NULL)
+		fn(dev->data->ports[port_id]);
+
+	return 0;
+}
+
 /**
  * Link multiple source event queues supplied in *queues* to the destination
  * event port designated by its *port_id* with associated service priority