[dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

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.

Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
---
 lib/librte_eventdev/rte_eventdev.h     | 65 ++++++++++++++++++++++++++
 lib/librte_eventdev/rte_eventdev_pmd.h | 14 ++++++
 2 files changed, 79 insertions(+)

diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index 226f352ad..d69150792 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -289,6 +289,15 @@ struct rte_event;
  * single queue to each port or map a single queue to many port.
  */
 
+#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 9)
+/**< 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
@@ -1226,6 +1235,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 (*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 */
@@ -1301,6 +1313,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.
@@ -1634,6 +1648,57 @@ 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
+ * no-operation.
+ *
+ * @param dev_id
+ *   The identifier of the device.
+ * @param port_id
+ *   The identifier of the event port.
+ * @return
+ *  - 0 on success.
+ *  - -ENOTSUP if the device doesn't have RTE_EVENT_DEV_CAP_REQUIRES_MAINT set
+ *  - -EINVAL if *dev_id* or *port_id* is invalid
+ *
+ * @see RTE_EVENT_DEV_CAP_REQUIRES_MAINT
+ */
+static inline void
+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) {
+		rte_errno = EINVAL;
+		return;
+	}
+
+	if (port_id >= dev->data->nb_ports) {
+		rte_errno = EINVAL;
+		return;
+	}
+#endif
+	fn = *dev->maintain;
+
+	if (fn != NULL)
+		fn(dev->data->ports[port_id]);
+}
+
 /**
  * Link multiple source event queues supplied in *queues* to the destination
  * event port designated by its *port_id* with associated service priority
diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
index d118b9e5b..327e4a2ac 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd.h
@@ -364,6 +364,20 @@ typedef int (*eventdev_port_unlinks_in_progress_t)(struct rte_eventdev *dev,
 typedef int (*eventdev_dequeue_timeout_ticks_t)(struct rte_eventdev *dev,
 		uint64_t ns, uint64_t *timeout_ticks);
 
+/**
+ * Maintains an event port for RTE_EVENT_DEV_CAP_REQUIRES_MAINT devices.
+ *
+ * @param dev
+ *   Event device pointer
+ * @param port_id
+ *   Event port index
+ *
+ * @return
+ *   Returns 0 on success.
+ *
+ */
+typedef int (*eventdev_maintain_t)(struct rte_eventdev *dev, uint8_t port_id);
+
 /**
  * Dump internal information
  *
-- 
2.20.1

[dpdk-dev] [RFC 2/3] event/dsw: make use of eventdev maintenance facility

[Mattias Rönnblom]

Set the RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag, and perform DSW
background tasks on rte_event_maintain() calls.

Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
---
 drivers/event/dsw/dsw_evdev.c      | 4 +++-
 drivers/event/dsw/dsw_evdev.h      | 1 +
 drivers/event/dsw/dsw_event.c      | 8 ++++++++
 lib/librte_eventdev/rte_eventdev.h | 2 +-
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/event/dsw/dsw_evdev.c b/drivers/event/dsw/dsw_evdev.c
index 7798a38ad..ebacad2d6 100644
--- a/drivers/event/dsw/dsw_evdev.c
+++ b/drivers/event/dsw/dsw_evdev.c
@@ -223,7 +223,8 @@ dsw_info_get(struct rte_eventdev *dev __rte_unused,
 		.event_dev_cap = RTE_EVENT_DEV_CAP_BURST_MODE|
 		RTE_EVENT_DEV_CAP_DISTRIBUTED_SCHED|
 		RTE_EVENT_DEV_CAP_NONSEQ_MODE|
-		RTE_EVENT_DEV_CAP_MULTIPLE_QUEUE_PORT
+		RTE_EVENT_DEV_CAP_MULTIPLE_QUEUE_PORT|
+		RTE_EVENT_DEV_CAP_REQUIRES_MAINT
 	};
 }
 
@@ -411,6 +412,7 @@ dsw_probe(struct rte_vdev_device *vdev)
 	dev->enqueue_forward_burst = dsw_event_enqueue_forward_burst;
 	dev->dequeue = dsw_event_dequeue;
 	dev->dequeue_burst = dsw_event_dequeue_burst;
+	dev->maintain = dsw_event_maintain;
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
diff --git a/drivers/event/dsw/dsw_evdev.h b/drivers/event/dsw/dsw_evdev.h
index 5c7b6108d..deef5061a 100644
--- a/drivers/event/dsw/dsw_evdev.h
+++ b/drivers/event/dsw/dsw_evdev.h
@@ -246,6 +246,7 @@ uint16_t dsw_event_enqueue_forward_burst(void *port,
 uint16_t dsw_event_dequeue(void *port, struct rte_event *ev, uint64_t wait);
 uint16_t dsw_event_dequeue_burst(void *port, struct rte_event *events,
 				 uint16_t num, uint64_t wait);
+void dsw_event_maintain(void *port);
 
 int dsw_xstats_get_names(const struct rte_eventdev *dev,
 			 enum rte_event_dev_xstats_mode mode,
diff --git a/drivers/event/dsw/dsw_event.c b/drivers/event/dsw/dsw_event.c
index d68b71b98..9c0669e1d 100644
--- a/drivers/event/dsw/dsw_event.c
+++ b/drivers/event/dsw/dsw_event.c
@@ -1254,3 +1254,11 @@ dsw_event_dequeue_burst(void *port, struct rte_event *events, uint16_t num,
 
 	return dequeued;
 }
+
+void dsw_event_maintain(void *port)
+{
+	struct dsw_port *source_port = port;
+	struct dsw_evdev *dsw = source_port->dsw;
+
+	dsw_port_bg_process(dsw, source_port);
+}
diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index d69150792..00a9380d9 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -1235,7 +1235,7 @@ 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 (*maintain_t)(void *port);
+typedef void (*event_maintain_t)(void *port);
 /**< @internal Maintains a port */
 
 typedef uint16_t (*event_tx_adapter_enqueue)(void *port,
-- 
2.20.1

[dpdk-dev] [RFC 3/3] eventdev: allow devices requiring maintenance with adapters

[Mattias Rönnblom]

Introduce support for event devices requiring calls to
rte_event_maintain() in the Ethernet RX, Timer and Crypto Eventdev
adapters.

Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
---
 lib/librte_eventdev/rte_event_crypto_adapter.c | 16 +++++++++++-----
 lib/librte_eventdev/rte_event_eth_rx_adapter.c |  3 +++
 lib/librte_eventdev/rte_event_timer_adapter.c  |  4 +++-
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/lib/librte_eventdev/rte_event_crypto_adapter.c b/lib/librte_eventdev/rte_event_crypto_adapter.c
index 22d910816..e1bb9d4ab 100644
--- a/lib/librte_eventdev/rte_event_crypto_adapter.c
+++ b/lib/librte_eventdev/rte_event_crypto_adapter.c
@@ -625,19 +625,25 @@ static void
 eca_crypto_adapter_run(struct rte_event_crypto_adapter *adapter,
 			unsigned int max_ops)
 {
-	while (max_ops) {
+	unsigned int ops_left = max_ops;
+
+	while (ops_left > 0) {
 		unsigned int e_cnt, d_cnt;
 
-		e_cnt = eca_crypto_adapter_deq_run(adapter, max_ops);
-		max_ops -= RTE_MIN(max_ops, e_cnt);
+		e_cnt = eca_crypto_adapter_deq_run(adapter, ops_left);
+		ops_left -= RTE_MIN(ops_left, e_cnt);
 
-		d_cnt = eca_crypto_adapter_enq_run(adapter, max_ops);
-		max_ops -= RTE_MIN(max_ops, d_cnt);
+		d_cnt = eca_crypto_adapter_enq_run(adapter, ops_left);
+		ops_left -= RTE_MIN(ops_left, d_cnt);
 
 		if (e_cnt == 0 && d_cnt == 0)
 			break;
 
 	}
+
+	if (ops_left == max_ops)
+		rte_event_maintain(adapter->eventdev_id,
+				   adapter->event_port_id);
 }
 
 static int
diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
index 95dd47820..e6eb6e63f 100644
--- a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
@@ -852,6 +852,9 @@ rxa_eth_rx(struct rte_event_eth_rx_adapter *rx_adapter,
 
 	if (buf->count > 0)
 		rxa_flush_event_buffer(rx_adapter);
+	else
+		rte_event_maintain(rx_adapter->eventdev_id,
+				   rx_adapter->event_port_id);
 
 	return nb_rx;
 }
diff --git a/lib/librte_eventdev/rte_event_timer_adapter.c b/lib/librte_eventdev/rte_event_timer_adapter.c
index 161e21a68..66b42ee3d 100644
--- a/lib/librte_eventdev/rte_event_timer_adapter.c
+++ b/lib/librte_eventdev/rte_event_timer_adapter.c
@@ -741,7 +741,9 @@ swtim_service_func(void *arg)
 		sw->stats.ev_enq_count += nb_evs_flushed;
 		sw->stats.ev_inv_count += nb_evs_invalid;
 		sw->stats.adapter_tick_count++;
-	}
+	} else
+		rte_event_maintain(adapter->data->event_dev_id,
+				   adapter->data->event_port_id);
 
 	return 0;
 }
-- 
2.20.1

Re: [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Jerin Jacob]

On Wed, Apr 8, 2020 at 11:27 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.
>
> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> ---
>  lib/librte_eventdev/rte_eventdev.h     | 65 ++++++++++++++++++++++++++
>  lib/librte_eventdev/rte_eventdev_pmd.h | 14 ++++++
>  2 files changed, 79 insertions(+)
>
> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> index 226f352ad..d69150792 100644
> --- a/lib/librte_eventdev/rte_eventdev.h
> +++ b/lib/librte_eventdev/rte_eventdev.h
> @@ -289,6 +289,15 @@ struct rte_event;
>   * single queue to each port or map a single queue to many port.
>   */
>
> +#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 9)
> +/**< Event device requires calls to rte_event_maintain() during

This scheme would call for DSW specific API handling in fastpath.

> + * periods when neither rte_event_dequeue_burst() nor

The typical worker thread will be
while (1) {
                rte_event_dequeue_burst();
                 ..proess..
                rte_event_enqueue_burst();
}
If so, Why DSW driver can't do the maintenance in driver context in
dequeue() call.


> + * 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.
> + */

Re: [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

On 2020-04-08 21:36, Jerin Jacob wrote:
> On Wed, Apr 8, 2020 at 11:27 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.
>>
>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>> ---
>>   lib/librte_eventdev/rte_eventdev.h     | 65 ++++++++++++++++++++++++++
>>   lib/librte_eventdev/rte_eventdev_pmd.h | 14 ++++++
>>   2 files changed, 79 insertions(+)
>>
>> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
>> index 226f352ad..d69150792 100644
>> --- a/lib/librte_eventdev/rte_eventdev.h
>> +++ b/lib/librte_eventdev/rte_eventdev.h
>> @@ -289,6 +289,15 @@ struct rte_event;
>>    * single queue to each port or map a single queue to many port.
>>    */
>>
>> +#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 9)
>> +/**< Event device requires calls to rte_event_maintain() during
> This scheme would call for DSW specific API handling in fastpath.


Initially this would be so, but buffering events might yield performance 
benefits for more event devices than DSW.


In an application, it's often convenient, but sub-optimal from a 
performance point of view, to do single-event enqueue operations. The 
alternative is to use an application-level buffer, and the flush this 
buffer with rte_event_enqueue_burst(). If you allow the event device to 
buffer, you get the simplicity of single-event enqueue operations, but 
without taking any noticeable performance hit.


>> + * periods when neither rte_event_dequeue_burst() nor
> The typical worker thread will be
> while (1) {
>                  rte_event_dequeue_burst();
>                   ..proess..
>                  rte_event_enqueue_burst();
> }
> If so, Why DSW driver can't do the maintenance in driver context in
> dequeue() call.
>

DSW already does maintenance on dequeue, and works well in the above 
scenario. The typical worker does not need to care about the 
rte_event_maintain() functions, since it dequeues events on a regular basis.


What this RFC addresses is the more atypical (but still fairly common) 
case of a port being neither dequeued to or enqueued from on a regular 
basis. The timer and ethernet rx adapters are examples of such.


>> + * 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.
>> + */

Re: [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Jerin Jacob]

On Thu, Apr 9, 2020 at 5:51 PM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> On 2020-04-08 21:36, Jerin Jacob wrote:
> > On Wed, Apr 8, 2020 at 11:27 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.
> >>
> >> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >> ---
> >>   lib/librte_eventdev/rte_eventdev.h     | 65 ++++++++++++++++++++++++++
> >>   lib/librte_eventdev/rte_eventdev_pmd.h | 14 ++++++
> >>   2 files changed, 79 insertions(+)
> >>
> >> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> >> index 226f352ad..d69150792 100644
> >> --- a/lib/librte_eventdev/rte_eventdev.h
> >> +++ b/lib/librte_eventdev/rte_eventdev.h
> >> @@ -289,6 +289,15 @@ struct rte_event;
> >>    * single queue to each port or map a single queue to many port.
> >>    */
> >>
> >> +#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 9)
> >> +/**< Event device requires calls to rte_event_maintain() during
> > This scheme would call for DSW specific API handling in fastpath.
>
>
> Initially this would be so, but buffering events might yield performance
> benefits for more event devices than DSW.
>
>
> In an application, it's often convenient, but sub-optimal from a
> performance point of view, to do single-event enqueue operations. The
> alternative is to use an application-level buffer, and the flush this
> buffer with rte_event_enqueue_burst(). If you allow the event device to
> buffer, you get the simplicity of single-event enqueue operations, but
> without taking any noticeable performance hit.

IMO, It is better to aggregate the burst by the application,  as sending
event by event to the driver to aggregate has performance due to cost
function pointer overhead.

Another concern is the frequency of calling rte_event_maintain() function by
the application, as the timing requirements will vary differently by
the driver to driver and application to application.
IMO, It is not portable and I believe the application should not be
aware of those details. If the driver needs specific maintenance
function for any other reason then better to use DPDK SERVICE core infra.

>
>
> >> + * periods when neither rte_event_dequeue_burst() nor
> > The typical worker thread will be
> > while (1) {
> >                  rte_event_dequeue_burst();
> >                   ..proess..
> >                  rte_event_enqueue_burst();
> > }
> > If so, Why DSW driver can't do the maintenance in driver context in
> > dequeue() call.
> >
>
> DSW already does maintenance on dequeue, and works well in the above
> scenario. The typical worker does not need to care about the
> rte_event_maintain() functions, since it dequeues events on a regular basis.
>
>
> What this RFC addresses is the more atypical (but still fairly common)
> case of a port being neither dequeued to or enqueued from on a regular
> basis. The timer and ethernet rx adapters are examples of such.

If it is an Adapter specific use case problem then maybe, we have
an option to fix the problem in adapter specific API usage or in that area.


>
>
> >> + * 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.
> >> + */
>
>

Re: [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Eads, Gage]

> >> diff --git a/lib/librte_eventdev/rte_eventdev.h
> >> b/lib/librte_eventdev/rte_eventdev.h
> >> index 226f352ad..d69150792 100644
> >> --- a/lib/librte_eventdev/rte_eventdev.h
> >> +++ b/lib/librte_eventdev/rte_eventdev.h
> >> @@ -289,6 +289,15 @@ struct rte_event;
> >>    * single queue to each port or map a single queue to many port.
> >>    */
> >>
> >> +#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 9) /**<
> Event
> >> +device requires calls to rte_event_maintain() during
> > This scheme would call for DSW specific API handling in fastpath.
> 
> 
> Initially this would be so, but buffering events might yield performance
> benefits for more event devices than DSW.
> 

I agree. For applications that process and enqueue one event at a time, buffering in the PMD could give a performance boost with minimal code changes (assuming the application can tolerate higher packet latency caused by buffering).

> 
> In an application, it's often convenient, but sub-optimal from a
> performance point of view, to do single-event enqueue operations. The
> alternative is to use an application-level buffer, and the flush this
> buffer with rte_event_enqueue_burst(). If you allow the event device to
> buffer, you get the simplicity of single-event enqueue operations, but
> without taking any noticeable performance hit.
> 
> 
> >> + * periods when neither rte_event_dequeue_burst() nor
> > The typical worker thread will be
> > while (1) {
> >                  rte_event_dequeue_burst();
> >                   ..proess..
> >                  rte_event_enqueue_burst();
> > }
> > If so, Why DSW driver can't do the maintenance in driver context in
> > dequeue() call.
> >
> 
> DSW already does maintenance on dequeue, and works well in the above
> scenario. The typical worker does not need to care about the
> rte_event_maintain() functions, since it dequeues events on a regular basis.
> 
> 
> What this RFC addresses is the more atypical (but still fairly common)
> case of a port being neither dequeued to or enqueued from on a regular
> basis. The timer and ethernet rx adapters are examples of such.
> 

Those two adapters have application-level buffering already, so adding PMD-level buffering feels unnecessary. Could DSW support this behavior on a port-by-port basis?

If so, I'm picturing something like:
- Add a "PMD buffering" eventdev capability
- If an eventdev has that capability, its ports can be configured for PMD-level buffering (default: no buffering)
-- Convert " uint8_t disable_implicit_release" to a flags bitmap (e.g. "uint8_t event_port_cfg"), with one flag for implicit release disable and another for PMD-level buffering
-- I suspect we can maintain ABI compatibility with function versioning on rte_event_port_setup() and rte_event_port_default_conf_get(), and this flags bitmap could be extended out to 32 bits in 20.11.
- Add "flush" semantics either to a new interface or extend an existing one. I'm partial to a new interface, to avoid an additional check in e.g. the dequeue code. And putting the flush in dequeue doesn't allow an app to batch across multiple iterations of the dequeue-process-enqueue loop.
- Extend rte_event_port_attr_get() to allow users to query this new setting. Adapters that don't call the flush function could error out if the adapter's port is configured for PMD-level buffering.

(eventdev should also forbid "PMD-level buffering" and "implicit release" used together...it's easy to imagine double-release errors occurring otherwise.)

I think this accomplishes Mattias' objective, and there's no effect on existing apps or adapters unless they choose to enable this behavior.

Granted, existing apps would likely see performance loss with dsw until they enable this config option. But perhaps it's worth it to get this behavior properly supported in the interface.

Thanks,
Gage

Re: [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

On 2020-04-09 15:32, Jerin Jacob wrote:
> On Thu, Apr 9, 2020 at 5:51 PM Mattias Rönnblom
> <mattias.ronnblom@ericsson.com> wrote:
>> On 2020-04-08 21:36, Jerin Jacob wrote:
>>> On Wed, Apr 8, 2020 at 11:27 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.
>>>>
>>>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>> ---
>>>>    lib/librte_eventdev/rte_eventdev.h     | 65 ++++++++++++++++++++++++++
>>>>    lib/librte_eventdev/rte_eventdev_pmd.h | 14 ++++++
>>>>    2 files changed, 79 insertions(+)
>>>>
>>>> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
>>>> index 226f352ad..d69150792 100644
>>>> --- a/lib/librte_eventdev/rte_eventdev.h
>>>> +++ b/lib/librte_eventdev/rte_eventdev.h
>>>> @@ -289,6 +289,15 @@ struct rte_event;
>>>>     * single queue to each port or map a single queue to many port.
>>>>     */
>>>>
>>>> +#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 9)
>>>> +/**< Event device requires calls to rte_event_maintain() during
>>> This scheme would call for DSW specific API handling in fastpath.
>>
>> Initially this would be so, but buffering events might yield performance
>> benefits for more event devices than DSW.
>>
>>
>> In an application, it's often convenient, but sub-optimal from a
>> performance point of view, to do single-event enqueue operations. The
>> alternative is to use an application-level buffer, and the flush this
>> buffer with rte_event_enqueue_burst(). If you allow the event device to
>> buffer, you get the simplicity of single-event enqueue operations, but
>> without taking any noticeable performance hit.
> IMO, It is better to aggregate the burst by the application,  as sending
> event by event to the driver to aggregate has performance due to cost
> function pointer overhead.


That's a very slight overhead - but for optimal performance, sure. It'll 
come at a cost in terms of code complexity. Just look at the adapters. 
They do this already. I think some applications are ready to take the 
extra 5-10 clock cycles or so it'll cost them to do the function call 
(provided the event device had buffering support).


> Another concern is the frequency of calling rte_event_maintain() function by
> the application, as the timing requirements will vary differently by
> the driver to driver and application to application.
> IMO, It is not portable and I believe the application should not be
> aware of those details. If the driver needs specific maintenance
> function for any other reason then better to use DPDK SERVICE core infra.


The only thing the application needs to be aware of, is that it needs to 
call rte_event_maintain() as often as it would have called dequeue() in 
your "typical worker" example. To make sure this call is cheap-enough is 
up to the driver, and this needs to hold true for all event devices that 
needs maintenance.


If you plan to use a non-buffering hardware device driver or a soft, 
centralized scheduler that doesn't need this, it will also not set the 
flag, and thus the application needs not care about the 
rte_event_maintain() function. DPDK code such as the eventdev adapters 
do need to care, but the increase in complexity is slight, and the cost 
of calling rte_maintain_event() on a maintenance-free devices is very 
low (since the then-NULL function pointer is in the eventdev struct, 
likely on a cache-line already dragged in).


Unfortunately, DPDK doesn't have a per-core delayed-work mechanism. 
Flushing event buffers (and other DSW "background work") can't be done 
on a service core, since they would work on non-MT-safe data structures 
on the worker thread's event ports.


>>
>>>> + * periods when neither rte_event_dequeue_burst() nor
>>> The typical worker thread will be
>>> while (1) {
>>>                   rte_event_dequeue_burst();
>>>                    ..proess..
>>>                   rte_event_enqueue_burst();
>>> }
>>> If so, Why DSW driver can't do the maintenance in driver context in
>>> dequeue() call.
>>>
>> DSW already does maintenance on dequeue, and works well in the above
>> scenario. The typical worker does not need to care about the
>> rte_event_maintain() functions, since it dequeues events on a regular basis.
>>
>>
>> What this RFC addresses is the more atypical (but still fairly common)
>> case of a port being neither dequeued to or enqueued from on a regular
>> basis. The timer and ethernet rx adapters are examples of such.
> If it is an Adapter specific use case problem then maybe, we have
> an option to fix the problem in adapter specific API usage or in that area.
>

It's not adapter specific, I think. There might be producer-only ports, 
for example, which doesn't provide a constant stream of events, but 
rather intermittent bursts. A traffic generator is one example of such 
an application, and there might be other, less synthetic ones as well.


>>
>>>> + * 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.
>>>> + */
>>

Re: [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

On 2020-04-09 15:33, Eads, Gage wrote:
>>>> diff --git a/lib/librte_eventdev/rte_eventdev.h
>>>> b/lib/librte_eventdev/rte_eventdev.h
>>>> index 226f352ad..d69150792 100644
>>>> --- a/lib/librte_eventdev/rte_eventdev.h
>>>> +++ b/lib/librte_eventdev/rte_eventdev.h
>>>> @@ -289,6 +289,15 @@ struct rte_event;
>>>>     * single queue to each port or map a single queue to many port.
>>>>     */
>>>>
>>>> +#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 9) /**<
>> Event
>>>> +device requires calls to rte_event_maintain() during
>>> This scheme would call for DSW specific API handling in fastpath.
>>
>> Initially this would be so, but buffering events might yield performance
>> benefits for more event devices than DSW.
>>
> I agree. For applications that process and enqueue one event at a time, buffering in the PMD could give a performance boost with minimal code changes (assuming the application can tolerate higher packet latency caused by buffering).
>
>> In an application, it's often convenient, but sub-optimal from a
>> performance point of view, to do single-event enqueue operations. The
>> alternative is to use an application-level buffer, and the flush this
>> buffer with rte_event_enqueue_burst(). If you allow the event device to
>> buffer, you get the simplicity of single-event enqueue operations, but
>> without taking any noticeable performance hit.
>>
>>
>>>> + * periods when neither rte_event_dequeue_burst() nor
>>> The typical worker thread will be
>>> while (1) {
>>>                   rte_event_dequeue_burst();
>>>                    ..proess..
>>>                   rte_event_enqueue_burst();
>>> }
>>> If so, Why DSW driver can't do the maintenance in driver context in
>>> dequeue() call.
>>>
>> DSW already does maintenance on dequeue, and works well in the above
>> scenario. The typical worker does not need to care about the
>> rte_event_maintain() functions, since it dequeues events on a regular basis.
>>
>>
>> What this RFC addresses is the more atypical (but still fairly common)
>> case of a port being neither dequeued to or enqueued from on a regular
>> basis. The timer and ethernet rx adapters are examples of such.
>>
> Those two adapters have application-level buffering already, so adding PMD-level buffering feels unnecessary. Could DSW support this behavior on a port-by-port basis?


Flushing event buffers is just one of DSW's "background tasks". It also 
updates the load estimate, so that other ports, considering migrating 
flows to this port, has a recent-enough data to work with. In addition, 
DSW periodically considered flow migration (i.e. load balancing), which 
includes signaling between the ports. Even idle ports needs to respond 
to these signals, thus they need to be "maintained". While buffering can 
be made optional, the rest of the above can't.


DPDK eventdev seems to aspire to allow distributed scheduler 
implementation, considering it has the 
RTE_EVENT_DEV_CAP_DISTRIBUTED_SCHED flag since long. I've put some 
thought into this, and I have yet to find a solution which can avoid 
this kind "background tasks" for an efficient atomic scheduler with 
dynamic load balancing.


> If so, I'm picturing something like:
> - Add a "PMD buffering" eventdev capability
> - If an eventdev has that capability, its ports can be configured for PMD-level buffering (default: no buffering)
> -- Convert " uint8_t disable_implicit_release" to a flags bitmap (e.g. "uint8_t event_port_cfg"), with one flag for implicit release disable and another for PMD-level buffering
> -- I suspect we can maintain ABI compatibility with function versioning on rte_event_port_setup() and rte_event_port_default_conf_get(), and this flags bitmap could be extended out to 32 bits in 20.11.
> - Add "flush" semantics either to a new interface or extend an existing one. I'm partial to a new interface, to avoid an additional check in e.g. the dequeue code. And putting the flush in dequeue doesn't allow an app to batch across multiple iterations of the dequeue-process-enqueue loop.
> - Extend rte_event_port_attr_get() to allow users to query this new setting. Adapters that don't call the flush function could error out if the adapter's port is configured for PMD-level buffering.
>
> (eventdev should also forbid "PMD-level buffering" and "implicit release" used together...it's easy to imagine double-release errors occurring otherwise.)
> I think this accomplishes Mattias' objective, and there's no effect on existing apps or adapters unless they choose to enable this behavior.
>
> Granted, existing apps would likely see performance loss with dsw until they enable this config option. But perhaps it's worth it to get this behavior properly supported in the interface.
>
> Thanks,
> Gage

Re: [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Jerin Jacob]

On Thu, Apr 9, 2020 at 7:32 PM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> On 2020-04-09 15:32, Jerin Jacob wrote:
> > On Thu, Apr 9, 2020 at 5:51 PM Mattias Rönnblom
> > <mattias.ronnblom@ericsson.com> wrote:
> >> On 2020-04-08 21:36, Jerin Jacob wrote:
> >>> On Wed, Apr 8, 2020 at 11:27 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.
> >>>>
> >>>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >>>> ---
> >>>>    lib/librte_eventdev/rte_eventdev.h     | 65 ++++++++++++++++++++++++++
> >>>>    lib/librte_eventdev/rte_eventdev_pmd.h | 14 ++++++
> >>>>    2 files changed, 79 insertions(+)
> >>>>
> >>>> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> >>>> index 226f352ad..d69150792 100644
> >>>> --- a/lib/librte_eventdev/rte_eventdev.h
> >>>> +++ b/lib/librte_eventdev/rte_eventdev.h
> >>>> @@ -289,6 +289,15 @@ struct rte_event;
> >>>>     * single queue to each port or map a single queue to many port.
> >>>>     */
> >>>>
> >>>> +#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 9)
> >>>> +/**< Event device requires calls to rte_event_maintain() during
> >>> This scheme would call for DSW specific API handling in fastpath.
> >>
> >> Initially this would be so, but buffering events might yield performance
> >> benefits for more event devices than DSW.
> >>
> >>
> >> In an application, it's often convenient, but sub-optimal from a
> >> performance point of view, to do single-event enqueue operations. The
> >> alternative is to use an application-level buffer, and the flush this
> >> buffer with rte_event_enqueue_burst(). If you allow the event device to
> >> buffer, you get the simplicity of single-event enqueue operations, but
> >> without taking any noticeable performance hit.
> > IMO, It is better to aggregate the burst by the application,  as sending
> > event by event to the driver to aggregate has performance due to cost
> > function pointer overhead.
>
>
> That's a very slight overhead - but for optimal performance, sure. It'll
> come at a cost in terms of code complexity. Just look at the adapters.
> They do this already. I think some applications are ready to take the
> extra 5-10 clock cycles or so it'll cost them to do the function call
> (provided the event device had buffering support).

So Is there any advantage of moving aggregation logic to PMD? it is costly.

>
>
> > Another concern is the frequency of calling rte_event_maintain() function by
> > the application, as the timing requirements will vary differently by
> > the driver to driver and application to application.
> > IMO, It is not portable and I believe the application should not be
> > aware of those details. If the driver needs specific maintenance
> > function for any other reason then better to use DPDK SERVICE core infra.
>
>
> The only thing the application needs to be aware of, is that it needs to
> call rte_event_maintain() as often as it would have called dequeue() in
> your "typical worker" example. To make sure this call is cheap-enough is
> up to the driver, and this needs to hold true for all event devices that
> needs maintenance.

Why not rte_event_maintain() can't do either in dequeue() or enqueue()
in the driver context? Either one of them has to be called
periodically by application
in any case?


>
>
> If you plan to use a non-buffering hardware device driver or a soft,
> centralized scheduler that doesn't need this, it will also not set the
> flag, and thus the application needs not care about the
> rte_event_maintain() function. DPDK code such as the eventdev adapters
> do need to care, but the increase in complexity is slight, and the cost
> of calling rte_maintain_event() on a maintenance-free devices is very
> low (since the then-NULL function pointer is in the eventdev struct,
> likely on a cache-line already dragged in).
>
>
> Unfortunately, DPDK doesn't have a per-core delayed-work mechanism.
> Flushing event buffers (and other DSW "background work") can't be done
> on a service core, since they would work on non-MT-safe data structures
> on the worker thread's event ports.

Yes. Otherwise, DSW needs to update to support MT safe.

>
>
> >>
> >>>> + * periods when neither rte_event_dequeue_burst() nor
> >>> The typical worker thread will be
> >>> while (1) {
> >>>                   rte_event_dequeue_burst();
> >>>                    ..proess..
> >>>                   rte_event_enqueue_burst();
> >>> }
> >>> If so, Why DSW driver can't do the maintenance in driver context in
> >>> dequeue() call.
> >>>
> >> DSW already does maintenance on dequeue, and works well in the above
> >> scenario. The typical worker does not need to care about the
> >> rte_event_maintain() functions, since it dequeues events on a regular basis.
> >>
> >>
> >> What this RFC addresses is the more atypical (but still fairly common)
> >> case of a port being neither dequeued to or enqueued from on a regular
> >> basis. The timer and ethernet rx adapters are examples of such.
> > If it is an Adapter specific use case problem then maybe, we have
> > an option to fix the problem in adapter specific API usage or in that area.
> >
>
> It's not adapter specific, I think. There might be producer-only ports,
> for example, which doesn't provide a constant stream of events, but
> rather intermittent bursts. A traffic generator is one example of such
> an application, and there might be other, less synthetic ones as well.

In that case, the application knows the purpose of the eventdev port.
Is changing eventdev spec to configure "port" or "queue" for that use
case help? Meaning, DSW or
Any driver can get the hint and change the function pointers
accordingly for fastpath.
For instance, do maintenance on enqueue() for such ports or so.


>
>
> >>
> >>>> + * 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.
> >>>> + */
> >>
>

Re: [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

On 2020-04-10 15:00, Jerin Jacob wrote:
> On Thu, Apr 9, 2020 at 7:32 PM Mattias Rönnblom
> <mattias.ronnblom@ericsson.com> wrote:
>> On 2020-04-09 15:32, Jerin Jacob wrote:
>>> On Thu, Apr 9, 2020 at 5:51 PM Mattias Rönnblom
>>> <mattias.ronnblom@ericsson.com> wrote:
>>>> On 2020-04-08 21:36, Jerin Jacob wrote:
>>>>> On Wed, Apr 8, 2020 at 11:27 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.
>>>>>>
>>>>>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>>>> ---
>>>>>>     lib/librte_eventdev/rte_eventdev.h     | 65 ++++++++++++++++++++++++++
>>>>>>     lib/librte_eventdev/rte_eventdev_pmd.h | 14 ++++++
>>>>>>     2 files changed, 79 insertions(+)
>>>>>>
>>>>>> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
>>>>>> index 226f352ad..d69150792 100644
>>>>>> --- a/lib/librte_eventdev/rte_eventdev.h
>>>>>> +++ b/lib/librte_eventdev/rte_eventdev.h
>>>>>> @@ -289,6 +289,15 @@ struct rte_event;
>>>>>>      * single queue to each port or map a single queue to many port.
>>>>>>      */
>>>>>>
>>>>>> +#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 9)
>>>>>> +/**< Event device requires calls to rte_event_maintain() during
>>>>> This scheme would call for DSW specific API handling in fastpath.
>>>> Initially this would be so, but buffering events might yield performance
>>>> benefits for more event devices than DSW.
>>>>
>>>>
>>>> In an application, it's often convenient, but sub-optimal from a
>>>> performance point of view, to do single-event enqueue operations. The
>>>> alternative is to use an application-level buffer, and the flush this
>>>> buffer with rte_event_enqueue_burst(). If you allow the event device to
>>>> buffer, you get the simplicity of single-event enqueue operations, but
>>>> without taking any noticeable performance hit.
>>> IMO, It is better to aggregate the burst by the application,  as sending
>>> event by event to the driver to aggregate has performance due to cost
>>> function pointer overhead.
>>
>> That's a very slight overhead - but for optimal performance, sure. It'll
>> come at a cost in terms of code complexity. Just look at the adapters.
>> They do this already. I think some applications are ready to take the
>> extra 5-10 clock cycles or so it'll cost them to do the function call
>> (provided the event device had buffering support).
> So Is there any advantage of moving aggregation logic to PMD? it is costly.


What do you mean by aggregation logic?


>
>>
>>> Another concern is the frequency of calling rte_event_maintain() function by
>>> the application, as the timing requirements will vary differently by
>>> the driver to driver and application to application.
>>> IMO, It is not portable and I believe the application should not be
>>> aware of those details. If the driver needs specific maintenance
>>> function for any other reason then better to use DPDK SERVICE core infra.
>>
>> The only thing the application needs to be aware of, is that it needs to
>> call rte_event_maintain() as often as it would have called dequeue() in
>> your "typical worker" example. To make sure this call is cheap-enough is
>> up to the driver, and this needs to hold true for all event devices that
>> needs maintenance.
> Why not rte_event_maintain() can't do either in dequeue() or enqueue()
> in the driver context? Either one of them has to be called
> periodically by application
> in any case?


No, producer-only ports can go idle for long times. For applications 
that don't "go idle" need not worry about the maintain function.


>
>>
>> If you plan to use a non-buffering hardware device driver or a soft,
>> centralized scheduler that doesn't need this, it will also not set the
>> flag, and thus the application needs not care about the
>> rte_event_maintain() function. DPDK code such as the eventdev adapters
>> do need to care, but the increase in complexity is slight, and the cost
>> of calling rte_maintain_event() on a maintenance-free devices is very
>> low (since the then-NULL function pointer is in the eventdev struct,
>> likely on a cache-line already dragged in).
>>
>>
>> Unfortunately, DPDK doesn't have a per-core delayed-work mechanism.
>> Flushing event buffers (and other DSW "background work") can't be done
>> on a service core, since they would work on non-MT-safe data structures
>> on the worker thread's event ports.
> Yes. Otherwise, DSW needs to update to support MT safe.


I haven't been looking at this in detail, but I suspect it will be both 
complex and not very performant. One of problems that need to be solved 
in such a solution, is the "pausing" of flows during migration. All 
participating lcores needs to ACK that a flow is paused.


>
>>
>>>>>> + * periods when neither rte_event_dequeue_burst() nor
>>>>> The typical worker thread will be
>>>>> while (1) {
>>>>>                    rte_event_dequeue_burst();
>>>>>                     ..proess..
>>>>>                    rte_event_enqueue_burst();
>>>>> }
>>>>> If so, Why DSW driver can't do the maintenance in driver context in
>>>>> dequeue() call.
>>>>>
>>>> DSW already does maintenance on dequeue, and works well in the above
>>>> scenario. The typical worker does not need to care about the
>>>> rte_event_maintain() functions, since it dequeues events on a regular basis.
>>>>
>>>>
>>>> What this RFC addresses is the more atypical (but still fairly common)
>>>> case of a port being neither dequeued to or enqueued from on a regular
>>>> basis. The timer and ethernet rx adapters are examples of such.
>>> If it is an Adapter specific use case problem then maybe, we have
>>> an option to fix the problem in adapter specific API usage or in that area.
>>>
>> It's not adapter specific, I think. There might be producer-only ports,
>> for example, which doesn't provide a constant stream of events, but
>> rather intermittent bursts. A traffic generator is one example of such
>> an application, and there might be other, less synthetic ones as well.
> In that case, the application knows the purpose of the eventdev port.
> Is changing eventdev spec to configure "port" or "queue" for that use
> case help? Meaning, DSW or
> Any driver can get the hint and change the function pointers
> accordingly for fastpath.
> For instance, do maintenance on enqueue() for such ports or so.


This is what DSW does already today. A dequeue() call with a zero-length 
event array serves the purpose of rte_event_maintain(). It's a bit of a 
hack, in my opinion.


>
>>
>>>>>> + * 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.
>>>>>> + */

Re: [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Jerin Jacob]

On Tue, Apr 14, 2020 at 9:27 PM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> On 2020-04-10 15:00, Jerin Jacob wrote:
> > On Thu, Apr 9, 2020 at 7:32 PM Mattias Rönnblom
> > <mattias.ronnblom@ericsson.com> wrote:
> >> On 2020-04-09 15:32, Jerin Jacob wrote:
> >>> On Thu, Apr 9, 2020 at 5:51 PM Mattias Rönnblom
> >>> <mattias.ronnblom@ericsson.com> wrote:
> >>>> On 2020-04-08 21:36, Jerin Jacob wrote:
> >>>>> On Wed, Apr 8, 2020 at 11:27 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.
> >>>>>>
> >>>>>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >>>>>> ---
> >>>>>>     lib/librte_eventdev/rte_eventdev.h     | 65 ++++++++++++++++++++++++++
> >>>>>>     lib/librte_eventdev/rte_eventdev_pmd.h | 14 ++++++
> >>>>>>     2 files changed, 79 insertions(+)
> >>>>>>
> >>>>>> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> >>>>>> index 226f352ad..d69150792 100644
> >>>>>> --- a/lib/librte_eventdev/rte_eventdev.h
> >>>>>> +++ b/lib/librte_eventdev/rte_eventdev.h
> >>>>>> @@ -289,6 +289,15 @@ struct rte_event;
> >>>>>>      * single queue to each port or map a single queue to many port.
> >>>>>>      */
> >>>>>>
> >>>>>> +#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 9)
> >>>>>> +/**< Event device requires calls to rte_event_maintain() during
> >>>>> This scheme would call for DSW specific API handling in fastpath.
> >>>> Initially this would be so, but buffering events might yield performance
> >>>> benefits for more event devices than DSW.
> >>>>
> >>>>
> >>>> In an application, it's often convenient, but sub-optimal from a
> >>>> performance point of view, to do single-event enqueue operations. The
> >>>> alternative is to use an application-level buffer, and the flush this
> >>>> buffer with rte_event_enqueue_burst(). If you allow the event device to
> >>>> buffer, you get the simplicity of single-event enqueue operations, but
> >>>> without taking any noticeable performance hit.
> >>> IMO, It is better to aggregate the burst by the application,  as sending
> >>> event by event to the driver to aggregate has performance due to cost
> >>> function pointer overhead.
> >>
> >> That's a very slight overhead - but for optimal performance, sure. It'll
> >> come at a cost in terms of code complexity. Just look at the adapters.
> >> They do this already. I think some applications are ready to take the
> >> extra 5-10 clock cycles or so it'll cost them to do the function call
> >> (provided the event device had buffering support).
> > So Is there any advantage of moving aggregation logic to PMD? it is costly.
>
>
> What do you mean by aggregation logic?

aggregation == buffering.

>
>
> >
> >>
> >>> Another concern is the frequency of calling rte_event_maintain() function by
> >>> the application, as the timing requirements will vary differently by
> >>> the driver to driver and application to application.
> >>> IMO, It is not portable and I believe the application should not be
> >>> aware of those details. If the driver needs specific maintenance
> >>> function for any other reason then better to use DPDK SERVICE core infra.
> >>
> >> The only thing the application needs to be aware of, is that it needs to
> >> call rte_event_maintain() as often as it would have called dequeue() in
> >> your "typical worker" example. To make sure this call is cheap-enough is
> >> up to the driver, and this needs to hold true for all event devices that
> >> needs maintenance.
> > Why not rte_event_maintain() can't do either in dequeue() or enqueue()
> > in the driver context? Either one of them has to be called
> > periodically by application
> > in any case?
>
>
> No, producer-only ports can go idle for long times. For applications
> that don't "go idle" need not worry about the maintain function.

If I understand it correctly, If the worker does not call enqueue() or dequeue()
for a long time then maintain() needs to be called by the application.

That's where I concern with this API. What is the definition of long
time frame?(ns or ticks?)
Will it be changing driver to driver and arch to arch? And it is
leaking the driver requirements to the application.


>
>
> >
> >>
> >> If you plan to use a non-buffering hardware device driver or a soft,
> >> centralized scheduler that doesn't need this, it will also not set the
> >> flag, and thus the application needs not care about the
> >> rte_event_maintain() function. DPDK code such as the eventdev adapters
> >> do need to care, but the increase in complexity is slight, and the cost
> >> of calling rte_maintain_event() on a maintenance-free devices is very
> >> low (since the then-NULL function pointer is in the eventdev struct,
> >> likely on a cache-line already dragged in).
> >>
> >>
> >> Unfortunately, DPDK doesn't have a per-core delayed-work mechanism.
> >> Flushing event buffers (and other DSW "background work") can't be done
> >> on a service core, since they would work on non-MT-safe data structures
> >> on the worker thread's event ports.
> > Yes. Otherwise, DSW needs to update to support MT safe.
>
>
> I haven't been looking at this in detail, but I suspect it will be both
> complex and not very performant. One of problems that need to be solved
> in such a solution, is the "pausing" of flows during migration. All
> participating lcores needs to ACK that a flow is paused.

Could you share the patch on the details on how much it costs?

>
>
> >
> >>
> >>>>>> + * periods when neither rte_event_dequeue_burst() nor
> >>>>> The typical worker thread will be
> >>>>> while (1) {
> >>>>>                    rte_event_dequeue_burst();
> >>>>>                     ..proess..
> >>>>>                    rte_event_enqueue_burst();
> >>>>> }
> >>>>> If so, Why DSW driver can't do the maintenance in driver context in
> >>>>> dequeue() call.
> >>>>>
> >>>> DSW already does maintenance on dequeue, and works well in the above
> >>>> scenario. The typical worker does not need to care about the
> >>>> rte_event_maintain() functions, since it dequeues events on a regular basis.
> >>>>
> >>>>
> >>>> What this RFC addresses is the more atypical (but still fairly common)
> >>>> case of a port being neither dequeued to or enqueued from on a regular
> >>>> basis. The timer and ethernet rx adapters are examples of such.
> >>> If it is an Adapter specific use case problem then maybe, we have
> >>> an option to fix the problem in adapter specific API usage or in that area.
> >>>
> >> It's not adapter specific, I think. There might be producer-only ports,
> >> for example, which doesn't provide a constant stream of events, but
> >> rather intermittent bursts. A traffic generator is one example of such
> >> an application, and there might be other, less synthetic ones as well.
> > In that case, the application knows the purpose of the eventdev port.
> > Is changing eventdev spec to configure "port" or "queue" for that use
> > case help? Meaning, DSW or
> > Any driver can get the hint and change the function pointers
> > accordingly for fastpath.
> > For instance, do maintenance on enqueue() for such ports or so.
>
>
> This is what DSW does already today. A dequeue() call with a zero-length
> event array serves the purpose of rte_event_maintain(). It's a bit of a
> hack, in my opinion.

I agree that it is the hack.

One more concern we have we are adding API for the new driver requirements and
not updating the example application. Sharing the example application
patch would
enable us to understand the cost and usage.(i.e Cost wrt to other SW drivers)



>
>
> >
> >>
> >>>>>> + * 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.
> >>>>>> + */
>
>

Re: [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

On 2020-04-14 18:15, Jerin Jacob wrote:
> On Tue, Apr 14, 2020 at 9:27 PM Mattias Rönnblom
> <mattias.ronnblom@ericsson.com> wrote:
>> On 2020-04-10 15:00, Jerin Jacob wrote:
>>> On Thu, Apr 9, 2020 at 7:32 PM Mattias Rönnblom
>>> <mattias.ronnblom@ericsson.com> wrote:
>>>> On 2020-04-09 15:32, Jerin Jacob wrote:
>>>>> On Thu, Apr 9, 2020 at 5:51 PM Mattias Rönnblom
>>>>> <mattias.ronnblom@ericsson.com> wrote:
>>>>>> On 2020-04-08 21:36, Jerin Jacob wrote:
>>>>>>> On Wed, Apr 8, 2020 at 11:27 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.
>>>>>>>>
>>>>>>>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>>>>>> ---
>>>>>>>>      lib/librte_eventdev/rte_eventdev.h     | 65 ++++++++++++++++++++++++++
>>>>>>>>      lib/librte_eventdev/rte_eventdev_pmd.h | 14 ++++++
>>>>>>>>      2 files changed, 79 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
>>>>>>>> index 226f352ad..d69150792 100644
>>>>>>>> --- a/lib/librte_eventdev/rte_eventdev.h
>>>>>>>> +++ b/lib/librte_eventdev/rte_eventdev.h
>>>>>>>> @@ -289,6 +289,15 @@ struct rte_event;
>>>>>>>>       * single queue to each port or map a single queue to many port.
>>>>>>>>       */
>>>>>>>>
>>>>>>>> +#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 9)
>>>>>>>> +/**< Event device requires calls to rte_event_maintain() during
>>>>>>> This scheme would call for DSW specific API handling in fastpath.
>>>>>> Initially this would be so, but buffering events might yield performance
>>>>>> benefits for more event devices than DSW.
>>>>>>
>>>>>>
>>>>>> In an application, it's often convenient, but sub-optimal from a
>>>>>> performance point of view, to do single-event enqueue operations. The
>>>>>> alternative is to use an application-level buffer, and the flush this
>>>>>> buffer with rte_event_enqueue_burst(). If you allow the event device to
>>>>>> buffer, you get the simplicity of single-event enqueue operations, but
>>>>>> without taking any noticeable performance hit.
>>>>> IMO, It is better to aggregate the burst by the application,  as sending
>>>>> event by event to the driver to aggregate has performance due to cost
>>>>> function pointer overhead.
>>>> That's a very slight overhead - but for optimal performance, sure. It'll
>>>> come at a cost in terms of code complexity. Just look at the adapters.
>>>> They do this already. I think some applications are ready to take the
>>>> extra 5-10 clock cycles or so it'll cost them to do the function call
>>>> (provided the event device had buffering support).
>>> So Is there any advantage of moving aggregation logic to PMD? it is costly.
>>
>> What do you mean by aggregation logic?
> aggregation == buffering.


The reason I put it into DSW to begin with was that it yielded a 
significant performance benefit, for situations where the application 
would enqueue() few or even single events per call. For DSW it will 
translate to lower DPDK event ring overhead. I would imagine it could 
improve performance for hardware-based event devices as well.


>>
>>>>> Another concern is the frequency of calling rte_event_maintain() function by
>>>>> the application, as the timing requirements will vary differently by
>>>>> the driver to driver and application to application.
>>>>> IMO, It is not portable and I believe the application should not be
>>>>> aware of those details. If the driver needs specific maintenance
>>>>> function for any other reason then better to use DPDK SERVICE core infra.
>>>> The only thing the application needs to be aware of, is that it needs to
>>>> call rte_event_maintain() as often as it would have called dequeue() in
>>>> your "typical worker" example. To make sure this call is cheap-enough is
>>>> up to the driver, and this needs to hold true for all event devices that
>>>> needs maintenance.
>>> Why not rte_event_maintain() can't do either in dequeue() or enqueue()
>>> in the driver context? Either one of them has to be called
>>> periodically by application
>>> in any case?
>>
>> No, producer-only ports can go idle for long times. For applications
>> that don't "go idle" need not worry about the maintain function.
> If I understand it correctly, If the worker does not call enqueue() or dequeue()
> for a long time then maintain() needs to be called by the application.
>
> That's where I concern with this API. What is the definition of long
> time frame?(ns or ticks?)
> Will it be changing driver to driver and arch to arch? And it is
> leaking the driver requirements to the application.
>

It's difficult to quantify exactly, but the rate should be on the same 
order of magnitude as you would call dequeue() on a consumer-type worker 
port. All the RTE_EVENT_DEV_CAP_* are essentially represents such 
leakage, where the event device driver and/or the underlying hardware 
express various capabilities and limitations.


I'm not sure it needs to be much more complicated for the application to 
handle than the change to the event adapters I included in the patch. 
There, it boils down the service function call rate, which would be high 
during low load (causing buffers to be flush quickly etc), and a little 
lower during high lcore load.


>>
>>>> If you plan to use a non-buffering hardware device driver or a soft,
>>>> centralized scheduler that doesn't need this, it will also not set the
>>>> flag, and thus the application needs not care about the
>>>> rte_event_maintain() function. DPDK code such as the eventdev adapters
>>>> do need to care, but the increase in complexity is slight, and the cost
>>>> of calling rte_maintain_event() on a maintenance-free devices is very
>>>> low (since the then-NULL function pointer is in the eventdev struct,
>>>> likely on a cache-line already dragged in).
>>>>
>>>>
>>>> Unfortunately, DPDK doesn't have a per-core delayed-work mechanism.
>>>> Flushing event buffers (and other DSW "background work") can't be done
>>>> on a service core, since they would work on non-MT-safe data structures
>>>> on the worker thread's event ports.
>>> Yes. Otherwise, DSW needs to update to support MT safe.
>>
>> I haven't been looking at this in detail, but I suspect it will be both
>> complex and not very performant. One of problems that need to be solved
>> in such a solution, is the "pausing" of flows during migration. All
>> participating lcores needs to ACK that a flow is paused.
> Could you share the patch on the details on how much it costs?


I don't have a ready-made solution for making lcore ports thread-safe.


>
>>
>>>>>>>> + * periods when neither rte_event_dequeue_burst() nor
>>>>>>> The typical worker thread will be
>>>>>>> while (1) {
>>>>>>>                     rte_event_dequeue_burst();
>>>>>>>                      ..proess..
>>>>>>>                     rte_event_enqueue_burst();
>>>>>>> }
>>>>>>> If so, Why DSW driver can't do the maintenance in driver context in
>>>>>>> dequeue() call.
>>>>>>>
>>>>>> DSW already does maintenance on dequeue, and works well in the above
>>>>>> scenario. The typical worker does not need to care about the
>>>>>> rte_event_maintain() functions, since it dequeues events on a regular basis.
>>>>>>
>>>>>>
>>>>>> What this RFC addresses is the more atypical (but still fairly common)
>>>>>> case of a port being neither dequeued to or enqueued from on a regular
>>>>>> basis. The timer and ethernet rx adapters are examples of such.
>>>>> If it is an Adapter specific use case problem then maybe, we have
>>>>> an option to fix the problem in adapter specific API usage or in that area.
>>>>>
>>>> It's not adapter specific, I think. There might be producer-only ports,
>>>> for example, which doesn't provide a constant stream of events, but
>>>> rather intermittent bursts. A traffic generator is one example of such
>>>> an application, and there might be other, less synthetic ones as well.
>>> In that case, the application knows the purpose of the eventdev port.
>>> Is changing eventdev spec to configure "port" or "queue" for that use
>>> case help? Meaning, DSW or
>>> Any driver can get the hint and change the function pointers
>>> accordingly for fastpath.
>>> For instance, do maintenance on enqueue() for such ports or so.
>>
>> This is what DSW does already today. A dequeue() call with a zero-length
>> event array serves the purpose of rte_event_maintain(). It's a bit of a
>> hack, in my opinion.
> I agree that it is the hack.
>
> One more concern we have we are adding API for the new driver requirements and
> not updating the example application. Sharing the example application
> patch would
> enable us to understand the cost and usage.(i.e Cost wrt to other SW drivers)


Good point.



>
>>
>>>>>>>> + * 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.
>>>>>>>> + */
>>

Re: [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Jerin Jacob]

On Tue, Apr 14, 2020 at 11:25 PM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> On 2020-04-14 18:15, Jerin Jacob wrote:
> > On Tue, Apr 14, 2020 at 9:27 PM Mattias Rönnblom
> > <mattias.ronnblom@ericsson.com> wrote:
> >> On 2020-04-10 15:00, Jerin Jacob wrote:
> >>> On Thu, Apr 9, 2020 at 7:32 PM Mattias Rönnblom
> >>> <mattias.ronnblom@ericsson.com> wrote:
> >>>> On 2020-04-09 15:32, Jerin Jacob wrote:
> >>>>> On Thu, Apr 9, 2020 at 5:51 PM Mattias Rönnblom
> >>>>> <mattias.ronnblom@ericsson.com> wrote:
> >>>>>> On 2020-04-08 21:36, Jerin Jacob wrote:
> >>>>>>> On Wed, Apr 8, 2020 at 11:27 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.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >>>>>>>> ---
> >>>>>>>>      lib/librte_eventdev/rte_eventdev.h     | 65 ++++++++++++++++++++++++++
> >>>>>>>>      lib/librte_eventdev/rte_eventdev_pmd.h | 14 ++++++
> >>>>>>>>      2 files changed, 79 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> >>>>>>>> index 226f352ad..d69150792 100644
> >>>>>>>> --- a/lib/librte_eventdev/rte_eventdev.h
> >>>>>>>> +++ b/lib/librte_eventdev/rte_eventdev.h
> >>>>>>>> @@ -289,6 +289,15 @@ struct rte_event;
> >>>>>>>>       * single queue to each port or map a single queue to many port.
> >>>>>>>>       */
> >>>>>>>>
> >>>>>>>> +#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 9)
> >>>>>>>> +/**< Event device requires calls to rte_event_maintain() during
> >>>>>>> This scheme would call for DSW specific API handling in fastpath.
> >>>>>> Initially this would be so, but buffering events might yield performance
> >>>>>> benefits for more event devices than DSW.
> >>>>>>
> >>>>>>
> >>>>>> In an application, it's often convenient, but sub-optimal from a
> >>>>>> performance point of view, to do single-event enqueue operations. The
> >>>>>> alternative is to use an application-level buffer, and the flush this
> >>>>>> buffer with rte_event_enqueue_burst(). If you allow the event device to
> >>>>>> buffer, you get the simplicity of single-event enqueue operations, but
> >>>>>> without taking any noticeable performance hit.
> >>>>> IMO, It is better to aggregate the burst by the application,  as sending
> >>>>> event by event to the driver to aggregate has performance due to cost
> >>>>> function pointer overhead.
> >>>> That's a very slight overhead - but for optimal performance, sure. It'll
> >>>> come at a cost in terms of code complexity. Just look at the adapters.
> >>>> They do this already. I think some applications are ready to take the
> >>>> extra 5-10 clock cycles or so it'll cost them to do the function call
> >>>> (provided the event device had buffering support).
> >>> So Is there any advantage of moving aggregation logic to PMD? it is costly.
> >>
> >> What do you mean by aggregation logic?
> > aggregation == buffering.
>
>
> The reason I put it into DSW to begin with was that it yielded a
> significant performance benefit, for situations where the application
> would enqueue() few or even single events per call. For DSW it will
> translate to lower DPDK event ring overhead. I would imagine it could
> improve performance for hardware-based event devices as well.

Yes. we are already doing this in application. It makes sense for buffering.

>
>
> >>
> >>>>> Another concern is the frequency of calling rte_event_maintain() function by
> >>>>> the application, as the timing requirements will vary differently by
> >>>>> the driver to driver and application to application.
> >>>>> IMO, It is not portable and I believe the application should not be
> >>>>> aware of those details. If the driver needs specific maintenance
> >>>>> function for any other reason then better to use DPDK SERVICE core infra.
> >>>> The only thing the application needs to be aware of, is that it needs to
> >>>> call rte_event_maintain() as often as it would have called dequeue() in
> >>>> your "typical worker" example. To make sure this call is cheap-enough is
> >>>> up to the driver, and this needs to hold true for all event devices that
> >>>> needs maintenance.
> >>> Why not rte_event_maintain() can't do either in dequeue() or enqueue()
> >>> in the driver context? Either one of them has to be called
> >>> periodically by application
> >>> in any case?
> >>
> >> No, producer-only ports can go idle for long times. For applications
> >> that don't "go idle" need not worry about the maintain function.
> > If I understand it correctly, If the worker does not call enqueue() or dequeue()
> > for a long time then maintain() needs to be called by the application.
> >
> > That's where I concern with this API. What is the definition of long
> > time frame?(ns or ticks?)
> > Will it be changing driver to driver and arch to arch? And it is
> > leaking the driver requirements to the application.
> >
>
> It's difficult to quantify exactly, but the rate should be on the same
> order of magnitude as you would call dequeue() on a consumer-type worker

The challenge is if another driver has a different requirement for maintain()
interms of frequecey if is a public API then how we will abstract?

> port. All the RTE_EVENT_DEV_CAP_* are essentially represents such
> leakage, where the event device driver and/or the underlying hardware
> express various capabilities and limitations.

I agree. But in fastpath, we do not bring any such _functional_ difference.
If it is on a slow path then no issue at all.


>
>
> I'm not sure it needs to be much more complicated for the application to
> handle than the change to the event adapters I included in the patch.
> There, it boils down the service function call rate, which would be high
> during low load (causing buffers to be flush quickly etc), and a little
> lower during high lcore load.
>
>
> >>
> >>>> If you plan to use a non-buffering hardware device driver or a soft,
> >>>> centralized scheduler that doesn't need this, it will also not set the
> >>>> flag, and thus the application needs not care about the
> >>>> rte_event_maintain() function. DPDK code such as the eventdev adapters
> >>>> do need to care, but the increase in complexity is slight, and the cost
> >>>> of calling rte_maintain_event() on a maintenance-free devices is very
> >>>> low (since the then-NULL function pointer is in the eventdev struct,
> >>>> likely on a cache-line already dragged in).
> >>>>
> >>>>
> >>>> Unfortunately, DPDK doesn't have a per-core delayed-work mechanism.
> >>>> Flushing event buffers (and other DSW "background work") can't be done
> >>>> on a service core, since they would work on non-MT-safe data structures
> >>>> on the worker thread's event ports.
> >>> Yes. Otherwise, DSW needs to update to support MT safe.
> >>
> >> I haven't been looking at this in detail, but I suspect it will be both
> >> complex and not very performant. One of problems that need to be solved
> >> in such a solution, is the "pausing" of flows during migration. All
> >> participating lcores needs to ACK that a flow is paused.
> > Could you share the patch on the details on how much it costs?
>
>
> I don't have a ready-made solution for making lcore ports thread-safe.

OK

>
>
> >
> >>
> >>>>>>>> + * periods when neither rte_event_dequeue_burst() nor
> >>>>>>> The typical worker thread will be
> >>>>>>> while (1) {
> >>>>>>>                     rte_event_dequeue_burst();
> >>>>>>>                      ..proess..
> >>>>>>>                     rte_event_enqueue_burst();
> >>>>>>> }
> >>>>>>> If so, Why DSW driver can't do the maintenance in driver context in
> >>>>>>> dequeue() call.
> >>>>>>>
> >>>>>> DSW already does maintenance on dequeue, and works well in the above
> >>>>>> scenario. The typical worker does not need to care about the
> >>>>>> rte_event_maintain() functions, since it dequeues events on a regular basis.
> >>>>>>
> >>>>>>
> >>>>>> What this RFC addresses is the more atypical (but still fairly common)
> >>>>>> case of a port being neither dequeued to or enqueued from on a regular
> >>>>>> basis. The timer and ethernet rx adapters are examples of such.
> >>>>> If it is an Adapter specific use case problem then maybe, we have
> >>>>> an option to fix the problem in adapter specific API usage or in that area.
> >>>>>
> >>>> It's not adapter specific, I think. There might be producer-only ports,
> >>>> for example, which doesn't provide a constant stream of events, but
> >>>> rather intermittent bursts. A traffic generator is one example of such
> >>>> an application, and there might be other, less synthetic ones as well.
> >>> In that case, the application knows the purpose of the eventdev port.
> >>> Is changing eventdev spec to configure "port" or "queue" for that use
> >>> case help? Meaning, DSW or
> >>> Any driver can get the hint and change the function pointers
> >>> accordingly for fastpath.
> >>> For instance, do maintenance on enqueue() for such ports or so.
> >>
> >> This is what DSW does already today. A dequeue() call with a zero-length
> >> event array serves the purpose of rte_event_maintain(). It's a bit of a
> >> hack, in my opinion.
> > I agree that it is the hack.
> >
> > One more concern we have we are adding API for the new driver requirements and
> > not updating the example application. Sharing the example application
> > patch would
> > enable us to understand the cost and usage.(i.e Cost wrt to other SW drivers)
>
>
> Good point.

I suggest sharing patch based on existing app/adapter usage, based on that, we
can analyze more on abstraction and cost analysis.



>
>
>
> >
> >>
> >>>>>>>> + * 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.
> >>>>>>>> + */
> >>
>

Re: [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

On 2020-04-16 19:19, Jerin Jacob wrote:
> On Tue, Apr 14, 2020 at 11:25 PM Mattias Rönnblom
> <mattias.ronnblom@ericsson.com> wrote:
>> On 2020-04-14 18:15, Jerin Jacob wrote:
>>> On Tue, Apr 14, 2020 at 9:27 PM Mattias Rönnblom
>>> <mattias.ronnblom@ericsson.com> wrote:
>>>> On 2020-04-10 15:00, Jerin Jacob wrote:
>>>>> On Thu, Apr 9, 2020 at 7:32 PM Mattias Rönnblom
>>>>> <mattias.ronnblom@ericsson.com> wrote:
>>>>>> On 2020-04-09 15:32, Jerin Jacob wrote:
>>>>>>> On Thu, Apr 9, 2020 at 5:51 PM Mattias Rönnblom
>>>>>>> <mattias.ronnblom@ericsson.com> wrote:
>>>>>>>> On 2020-04-08 21:36, Jerin Jacob wrote:
>>>>>>>>> On Wed, Apr 8, 2020 at 11:27 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.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>>>>>>>> ---
>>>>>>>>>>       lib/librte_eventdev/rte_eventdev.h     | 65 ++++++++++++++++++++++++++
>>>>>>>>>>       lib/librte_eventdev/rte_eventdev_pmd.h | 14 ++++++
>>>>>>>>>>       2 files changed, 79 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
>>>>>>>>>> index 226f352ad..d69150792 100644
>>>>>>>>>> --- a/lib/librte_eventdev/rte_eventdev.h
>>>>>>>>>> +++ b/lib/librte_eventdev/rte_eventdev.h
>>>>>>>>>> @@ -289,6 +289,15 @@ struct rte_event;
>>>>>>>>>>        * single queue to each port or map a single queue to many port.
>>>>>>>>>>        */
>>>>>>>>>>
>>>>>>>>>> +#define RTE_EVENT_DEV_CAP_REQUIRES_MAINT (1ULL << 9)
>>>>>>>>>> +/**< Event device requires calls to rte_event_maintain() during
>>>>>>>>> This scheme would call for DSW specific API handling in fastpath.
>>>>>>>> Initially this would be so, but buffering events might yield performance
>>>>>>>> benefits for more event devices than DSW.
>>>>>>>>
>>>>>>>>
>>>>>>>> In an application, it's often convenient, but sub-optimal from a
>>>>>>>> performance point of view, to do single-event enqueue operations. The
>>>>>>>> alternative is to use an application-level buffer, and the flush this
>>>>>>>> buffer with rte_event_enqueue_burst(). If you allow the event device to
>>>>>>>> buffer, you get the simplicity of single-event enqueue operations, but
>>>>>>>> without taking any noticeable performance hit.
>>>>>>> IMO, It is better to aggregate the burst by the application,  as sending
>>>>>>> event by event to the driver to aggregate has performance due to cost
>>>>>>> function pointer overhead.
>>>>>> That's a very slight overhead - but for optimal performance, sure. It'll
>>>>>> come at a cost in terms of code complexity. Just look at the adapters.
>>>>>> They do this already. I think some applications are ready to take the
>>>>>> extra 5-10 clock cycles or so it'll cost them to do the function call
>>>>>> (provided the event device had buffering support).
>>>>> So Is there any advantage of moving aggregation logic to PMD? it is costly.
>>>> What do you mean by aggregation logic?
>>> aggregation == buffering.
>>
>> The reason I put it into DSW to begin with was that it yielded a
>> significant performance benefit, for situations where the application
>> would enqueue() few or even single events per call. For DSW it will
>> translate to lower DPDK event ring overhead. I would imagine it could
>> improve performance for hardware-based event devices as well.
> Yes. we are already doing this in application. It makes sense for buffering.


Should I read this as "we are assuming the application will do this"?


(I'm not saying it's a totally unreasonable assumption.)


>>
>>>>>>> Another concern is the frequency of calling rte_event_maintain() function by
>>>>>>> the application, as the timing requirements will vary differently by
>>>>>>> the driver to driver and application to application.
>>>>>>> IMO, It is not portable and I believe the application should not be
>>>>>>> aware of those details. If the driver needs specific maintenance
>>>>>>> function for any other reason then better to use DPDK SERVICE core infra.
>>>>>> The only thing the application needs to be aware of, is that it needs to
>>>>>> call rte_event_maintain() as often as it would have called dequeue() in
>>>>>> your "typical worker" example. To make sure this call is cheap-enough is
>>>>>> up to the driver, and this needs to hold true for all event devices that
>>>>>> needs maintenance.
>>>>> Why not rte_event_maintain() can't do either in dequeue() or enqueue()
>>>>> in the driver context? Either one of them has to be called
>>>>> periodically by application
>>>>> in any case?
>>>> No, producer-only ports can go idle for long times. For applications
>>>> that don't "go idle" need not worry about the maintain function.
>>> If I understand it correctly, If the worker does not call enqueue() or dequeue()
>>> for a long time then maintain() needs to be called by the application.
>>>
>>> That's where I concern with this API. What is the definition of long
>>> time frame?(ns or ticks?)
>>> Will it be changing driver to driver and arch to arch? And it is
>>> leaking the driver requirements to the application.
>>>
>> It's difficult to quantify exactly, but the rate should be on the same
>> order of magnitude as you would call dequeue() on a consumer-type worker
> The challenge is if another driver has a different requirement for maintain()
> interms of frequecey if is a public API then how we will abstract?


It's a challenge indeed, like so often is the case for poll-type APIs.


My thought, and what's implemented in DSW today, is that the application 
will call maintain() with a high frequency, as I've mentioned before. If 
the event device periodically needs to some perform heavy-weight 
operation, such an operation will not be perform on every call. Rather, 
the event device keeps a counter, and only perform these on every Nth 
call. For DSW, one such operation is when a DSW port considers if it 
needs to migrate flows, and in that case where those flows should emigrate.


Whatever maintain() is doing, it makes the event device machinery make 
progress in some way or the other. Calling it rarely will translate to 
latency. Failing to maintain a sufficient call rate shouldn't affect 
correctness.


>> port. All the RTE_EVENT_DEV_CAP_* are essentially represents such
>> leakage, where the event device driver and/or the underlying hardware
>> express various capabilities and limitations.
> I agree. But in fastpath, we do not bring any such _functional_ difference.
> If it is on a slow path then no issue at all.
>

I'm not sure I follow. What do you mean by functional here?


The capabilities certainly change API semantics, and as a result how 
applications need to behave - including fast-path behavior. For example, 
consider an application which prefers 
RTE_EVENT_DEV_CAP_IMPLICIT_RELEASE_DISABLE-capable event devices, but 
also need a code path for those that don't.


>>
>> I'm not sure it needs to be much more complicated for the application to
>> handle than the change to the event adapters I included in the patch.
>> There, it boils down the service function call rate, which would be high
>> during low load (causing buffers to be flush quickly etc), and a little
>> lower during high lcore load.
>>
>>
>>>>>> If you plan to use a non-buffering hardware device driver or a soft,
>>>>>> centralized scheduler that doesn't need this, it will also not set the
>>>>>> flag, and thus the application needs not care about the
>>>>>> rte_event_maintain() function. DPDK code such as the eventdev adapters
>>>>>> do need to care, but the increase in complexity is slight, and the cost
>>>>>> of calling rte_maintain_event() on a maintenance-free devices is very
>>>>>> low (since the then-NULL function pointer is in the eventdev struct,
>>>>>> likely on a cache-line already dragged in).
>>>>>>
>>>>>>
>>>>>> Unfortunately, DPDK doesn't have a per-core delayed-work mechanism.
>>>>>> Flushing event buffers (and other DSW "background work") can't be done
>>>>>> on a service core, since they would work on non-MT-safe data structures
>>>>>> on the worker thread's event ports.
>>>>> Yes. Otherwise, DSW needs to update to support MT safe.
>>>> I haven't been looking at this in detail, but I suspect it will be both
>>>> complex and not very performant. One of problems that need to be solved
>>>> in such a solution, is the "pausing" of flows during migration. All
>>>> participating lcores needs to ACK that a flow is paused.
>>> Could you share the patch on the details on how much it costs?
>>
>> I don't have a ready-made solution for making lcore ports thread-safe.
> OK
>
>>
>>>>>>>>>> + * periods when neither rte_event_dequeue_burst() nor
>>>>>>>>> The typical worker thread will be
>>>>>>>>> while (1) {
>>>>>>>>>                      rte_event_dequeue_burst();
>>>>>>>>>                       ..proess..
>>>>>>>>>                      rte_event_enqueue_burst();
>>>>>>>>> }
>>>>>>>>> If so, Why DSW driver can't do the maintenance in driver context in
>>>>>>>>> dequeue() call.
>>>>>>>>>
>>>>>>>> DSW already does maintenance on dequeue, and works well in the above
>>>>>>>> scenario. The typical worker does not need to care about the
>>>>>>>> rte_event_maintain() functions, since it dequeues events on a regular basis.
>>>>>>>>
>>>>>>>>
>>>>>>>> What this RFC addresses is the more atypical (but still fairly common)
>>>>>>>> case of a port being neither dequeued to or enqueued from on a regular
>>>>>>>> basis. The timer and ethernet rx adapters are examples of such.
>>>>>>> If it is an Adapter specific use case problem then maybe, we have
>>>>>>> an option to fix the problem in adapter specific API usage or in that area.
>>>>>>>
>>>>>> It's not adapter specific, I think. There might be producer-only ports,
>>>>>> for example, which doesn't provide a constant stream of events, but
>>>>>> rather intermittent bursts. A traffic generator is one example of such
>>>>>> an application, and there might be other, less synthetic ones as well.
>>>>> In that case, the application knows the purpose of the eventdev port.
>>>>> Is changing eventdev spec to configure "port" or "queue" for that use
>>>>> case help? Meaning, DSW or
>>>>> Any driver can get the hint and change the function pointers
>>>>> accordingly for fastpath.
>>>>> For instance, do maintenance on enqueue() for such ports or so.
>>>> This is what DSW does already today. A dequeue() call with a zero-length
>>>> event array serves the purpose of rte_event_maintain(). It's a bit of a
>>>> hack, in my opinion.
>>> I agree that it is the hack.
>>>
>>> One more concern we have we are adding API for the new driver requirements and
>>> not updating the example application. Sharing the example application
>>> patch would
>>> enable us to understand the cost and usage.(i.e Cost wrt to other SW drivers)
>>
>> Good point.
> I suggest sharing patch based on existing app/adapter usage, based on that, we
> can analyze more on abstraction and cost analysis.
>

Will do.


>
>>
>>
>>>>>>>>>> + * 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.
>>>>>>>>>> + */

Re: [dpdk-dev] [RFC 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

On 2020-04-14 19:55, Mattias Rönnblom wrote:
> On 2020-04-14 18:15, Jerin Jacob wrote:
>
<snip>
>> One more concern we have we are adding API for the new driver requirements and
>> not updating the example application. Sharing the example application
>> patch would
>> enable us to understand the cost and usage.(i.e Cost wrt to other SW drivers)
>
> Good point.
>
>

I went through the examples applications (l2fwd, l3fwd and 
eventdev_pipeline). From what I can tell, they require no changes. All 
follow the dequeue()+enqueue() pattern, and thus are both consumers and 
producers.

[dpdk-dev] [RFC v2 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

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(+)

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
-- 
2.25.1

[dpdk-dev] [RFC v2 2/3] event/dsw: make use of eventdev maintenance facility

[Mattias Rönnblom]

Set the RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag, and perform DSW
background tasks on rte_event_maintain() calls.

RFC v2: Have dsw_event_maintain() occasionally flush the port output
        buffers.

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>
---
 drivers/event/dsw/dsw_evdev.c | 4 +++-
 drivers/event/dsw/dsw_evdev.h | 1 +
 drivers/event/dsw/dsw_event.c | 9 +++++++++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/event/dsw/dsw_evdev.c b/drivers/event/dsw/dsw_evdev.c
index 2301a4b7a0..713a75bce8 100644
--- a/drivers/event/dsw/dsw_evdev.c
+++ b/drivers/event/dsw/dsw_evdev.c
@@ -222,7 +222,8 @@ dsw_info_get(struct rte_eventdev *dev __rte_unused,
 		RTE_EVENT_DEV_CAP_DISTRIBUTED_SCHED|
 		RTE_EVENT_DEV_CAP_NONSEQ_MODE|
 		RTE_EVENT_DEV_CAP_MULTIPLE_QUEUE_PORT|
-		RTE_EVENT_DEV_CAP_CARRY_FLOW_ID
+		RTE_EVENT_DEV_CAP_CARRY_FLOW_ID|
+		RTE_EVENT_DEV_CAP_REQUIRES_MAINT
 	};
 }
 
@@ -441,6 +442,7 @@ dsw_probe(struct rte_vdev_device *vdev)
 	dev->enqueue_forward_burst = dsw_event_enqueue_forward_burst;
 	dev->dequeue = dsw_event_dequeue;
 	dev->dequeue_burst = dsw_event_dequeue_burst;
+	dev->maintain = dsw_event_maintain;
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
diff --git a/drivers/event/dsw/dsw_evdev.h b/drivers/event/dsw/dsw_evdev.h
index 08889a0990..9769d9740e 100644
--- a/drivers/event/dsw/dsw_evdev.h
+++ b/drivers/event/dsw/dsw_evdev.h
@@ -269,6 +269,7 @@ uint16_t dsw_event_enqueue_forward_burst(void *port,
 uint16_t dsw_event_dequeue(void *port, struct rte_event *ev, uint64_t wait);
 uint16_t dsw_event_dequeue_burst(void *port, struct rte_event *events,
 				 uint16_t num, uint64_t wait);
+void dsw_event_maintain(void *port);
 
 int dsw_xstats_get_names(const struct rte_eventdev *dev,
 			 enum rte_event_dev_xstats_mode mode,
diff --git a/drivers/event/dsw/dsw_event.c b/drivers/event/dsw/dsw_event.c
index 1f09816945..f16d9d463b 100644
--- a/drivers/event/dsw/dsw_event.c
+++ b/drivers/event/dsw/dsw_event.c
@@ -1400,3 +1400,12 @@ dsw_event_dequeue_burst(void *port, struct rte_event *events, uint16_t num,
 
 	return dequeued;
 }
+
+void dsw_event_maintain(void *port)
+{
+	struct dsw_port *source_port = port;
+	struct dsw_evdev *dsw = source_port->dsw;
+
+	dsw_port_note_op(source_port, 0);
+	dsw_port_bg_process(dsw, source_port);
+}
-- 
2.25.1

[dpdk-dev] [RFC v2 3/3] eventdev: have adapters support device maintenance

[Mattias Rönnblom]

Introduce support for event devices requiring calls to
rte_event_maintain() in the Ethernet RX, Timer and Crypto Eventdev
adapters.

RFC v2:
  - For simplicity, the timer adapter now unconditionally calls
    rte_event_maintain().
  - The RX adapter now only calls rte_event_maintain() when it has not
    enqueued any events.

Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
Tested-by: Richard Eklycke <richard.eklycke@ericsson.com>
---
 lib/eventdev/rte_event_crypto_adapter.c | 16 +++++++++++-----
 lib/eventdev/rte_event_eth_rx_adapter.c |  9 +++++++--
 lib/eventdev/rte_event_timer_adapter.c  |  3 +++
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/lib/eventdev/rte_event_crypto_adapter.c b/lib/eventdev/rte_event_crypto_adapter.c
index e1d38d383d..8dd2f7853a 100644
--- a/lib/eventdev/rte_event_crypto_adapter.c
+++ b/lib/eventdev/rte_event_crypto_adapter.c
@@ -630,19 +630,25 @@ static void
 eca_crypto_adapter_run(struct rte_event_crypto_adapter *adapter,
 			unsigned int max_ops)
 {
-	while (max_ops) {
+	unsigned int ops_left = max_ops;
+
+	while (ops_left > 0) {
 		unsigned int e_cnt, d_cnt;
 
-		e_cnt = eca_crypto_adapter_deq_run(adapter, max_ops);
-		max_ops -= RTE_MIN(max_ops, e_cnt);
+		e_cnt = eca_crypto_adapter_deq_run(adapter, ops_left);
+		ops_left -= RTE_MIN(ops_left, e_cnt);
 
-		d_cnt = eca_crypto_adapter_enq_run(adapter, max_ops);
-		max_ops -= RTE_MIN(max_ops, d_cnt);
+		d_cnt = eca_crypto_adapter_enq_run(adapter, ops_left);
+		ops_left -= RTE_MIN(ops_left, d_cnt);
 
 		if (e_cnt == 0 && d_cnt == 0)
 			break;
 
 	}
+
+	if (ops_left == max_ops)
+		rte_event_maintain(adapter->eventdev_id,
+				   adapter->event_port_id);
 }
 
 static int
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index 13dfb28401..017b6db6e9 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -923,6 +923,7 @@ rxa_eth_rx(struct rte_event_eth_rx_adapter *rx_adapter,
 					&rx_adapter->stats;
 	uint16_t n;
 	uint32_t nb_rx = 0;
+	uint32_t nb_flushed = 0;
 
 	if (rxq_empty)
 		*rxq_empty = 0;
@@ -931,7 +932,7 @@ rxa_eth_rx(struct rte_event_eth_rx_adapter *rx_adapter,
 	 */
 	while (BATCH_SIZE <= (RTE_DIM(buf->events) - buf->count)) {
 		if (buf->count >= BATCH_SIZE)
-			rxa_flush_event_buffer(rx_adapter);
+			nb_flushed += rxa_flush_event_buffer(rx_adapter);
 
 		stats->rx_poll_count++;
 		n = rte_eth_rx_burst(port_id, queue_id, mbufs, BATCH_SIZE);
@@ -947,7 +948,11 @@ rxa_eth_rx(struct rte_event_eth_rx_adapter *rx_adapter,
 	}
 
 	if (buf->count > 0)
-		rxa_flush_event_buffer(rx_adapter);
+		nb_flushed += rxa_flush_event_buffer(rx_adapter);
+
+	if (nb_flushed == 0)
+		rte_event_maintain(rx_adapter->eventdev_id,
+				   rx_adapter->event_port_id);
 
 	return nb_rx;
 }
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index ee20b39f4b..ccff2c687a 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -754,6 +754,9 @@ swtim_service_func(void *arg)
 		sw->stats.adapter_tick_count++;
 	}
 
+	rte_event_maintain(adapter->data->event_dev_id,
+			   adapter->data->event_port_id);
+
 	return 0;
 }
 
-- 
2.25.1

Re: [dpdk-dev] [RFC v2 1/3] eventdev: allow for event devices requiring maintenance

[Jerin Jacob]

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.

Re: [dpdk-dev] [RFC v2 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

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.

Re: [dpdk-dev] [EXT] Re: [RFC v2 1/3] eventdev: allow for event devices requiring maintenance

[Jerin Jacob Kollanukkaran]

> -----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.

[dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

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>
---
 lib/eventdev/eventdev_pmd.h          |  2 +
 lib/eventdev/eventdev_private.c      |  9 ++++
 lib/eventdev/eventdev_trace_points.c |  3 ++
 lib/eventdev/rte_eventdev.h          | 63 ++++++++++++++++++++++++++++
 lib/eventdev/rte_eventdev_core.h     |  5 +++
 lib/eventdev/rte_eventdev_trace_fp.h |  7 ++++
 6 files changed, 89 insertions(+)

diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h
index d009e24309..82a5c4db33 100644
--- a/lib/eventdev/eventdev_pmd.h
+++ b/lib/eventdev/eventdev_pmd.h
@@ -164,6 +164,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;
+	/**< Pointer to PMD port maintenance function. */
 	event_tx_adapter_enqueue_t txa_enqueue_same_dest;
 	/**< Pointer to PMD eth Tx adapter burst enqueue function with
 	 * events destined to same Eth port & Tx queue.
diff --git a/lib/eventdev/eventdev_private.c b/lib/eventdev/eventdev_private.c
index 9084833847..6395c21dce 100644
--- a/lib/eventdev/eventdev_private.c
+++ b/lib/eventdev/eventdev_private.c
@@ -44,6 +44,13 @@ dummy_event_dequeue_burst(__rte_unused void *port,
 	return 0;
 }
 
+static void
+dummy_event_maintain(__rte_unused void *port)
+{
+	RTE_EDEV_LOG_ERR(
+		"maintenance requested for unconfigured event device");
+}
+
 static uint16_t
 dummy_event_tx_adapter_enqueue(__rte_unused void *port,
 			       __rte_unused struct rte_event ev[],
@@ -85,6 +92,7 @@ event_dev_fp_ops_reset(struct rte_event_fp_ops *fp_op)
 		.enqueue_forward_burst = dummy_event_enqueue_burst,
 		.dequeue = dummy_event_dequeue,
 		.dequeue_burst = dummy_event_dequeue_burst,
+		.maintain = dummy_event_maintain,
 		.txa_enqueue = dummy_event_tx_adapter_enqueue,
 		.txa_enqueue_same_dest =
 			dummy_event_tx_adapter_enqueue_same_dest,
@@ -105,6 +113,7 @@ event_dev_fp_ops_set(struct rte_event_fp_ops *fp_op,
 	fp_op->enqueue_forward_burst = dev->enqueue_forward_burst;
 	fp_op->dequeue = dev->dequeue;
 	fp_op->dequeue_burst = dev->dequeue_burst;
+	fp_op->maintain = dev->maintain;
 	fp_op->txa_enqueue = dev->txa_enqueue;
 	fp_op->txa_enqueue_same_dest = dev->txa_enqueue_same_dest;
 	fp_op->ca_enqueue = dev->ca_enqueue;
diff --git a/lib/eventdev/eventdev_trace_points.c b/lib/eventdev/eventdev_trace_points.c
index 237d9383fd..de6b1f4417 100644
--- a/lib/eventdev/eventdev_trace_points.c
+++ b/lib/eventdev/eventdev_trace_points.c
@@ -37,6 +37,9 @@ RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_enq_burst,
 RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_deq_burst,
 	lib.eventdev.deq.burst)
 
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_maintain,
+	lib.eventdev.maintain)
+
 /* Eventdev Rx adapter trace points */
 RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_create,
 	lib.eventdev.rx.adapter.create)
diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index 0abed56bef..aad89c66f3 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
@@ -2063,6 +2072,60 @@ 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 require the
+ * application to call rte_event_maintain() on a port during periods
+ * which it is neither enqueuing nor dequeuing events from that
+ * port. 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

[dpdk-dev] [PATCH 2/3] event/dsw: make use of eventdev maintenance facility

[Mattias Rönnblom]

Set the RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag, and perform DSW
background tasks on rte_event_maintain() calls.

RFC v2: Have dsw_event_maintain() occasionally flush the port output
        buffers.

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>
---
 drivers/event/dsw/dsw_evdev.c | 4 +++-
 drivers/event/dsw/dsw_evdev.h | 1 +
 drivers/event/dsw/dsw_event.c | 9 +++++++++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/event/dsw/dsw_evdev.c b/drivers/event/dsw/dsw_evdev.c
index 0652d83ad6..5ff8fcc6a9 100644
--- a/drivers/event/dsw/dsw_evdev.c
+++ b/drivers/event/dsw/dsw_evdev.c
@@ -222,7 +222,8 @@ dsw_info_get(struct rte_eventdev *dev __rte_unused,
 		RTE_EVENT_DEV_CAP_DISTRIBUTED_SCHED|
 		RTE_EVENT_DEV_CAP_NONSEQ_MODE|
 		RTE_EVENT_DEV_CAP_MULTIPLE_QUEUE_PORT|
-		RTE_EVENT_DEV_CAP_CARRY_FLOW_ID
+		RTE_EVENT_DEV_CAP_CARRY_FLOW_ID|
+		RTE_EVENT_DEV_CAP_REQUIRES_MAINT
 	};
 }
 
@@ -441,6 +442,7 @@ dsw_probe(struct rte_vdev_device *vdev)
 	dev->enqueue_forward_burst = dsw_event_enqueue_forward_burst;
 	dev->dequeue = dsw_event_dequeue;
 	dev->dequeue_burst = dsw_event_dequeue_burst;
+	dev->maintain = dsw_event_maintain;
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
diff --git a/drivers/event/dsw/dsw_evdev.h b/drivers/event/dsw/dsw_evdev.h
index 631daea55c..31af4ede0f 100644
--- a/drivers/event/dsw/dsw_evdev.h
+++ b/drivers/event/dsw/dsw_evdev.h
@@ -271,6 +271,7 @@ uint16_t dsw_event_enqueue_forward_burst(void *port,
 uint16_t dsw_event_dequeue(void *port, struct rte_event *ev, uint64_t wait);
 uint16_t dsw_event_dequeue_burst(void *port, struct rte_event *events,
 				 uint16_t num, uint64_t wait);
+void dsw_event_maintain(void *port);
 
 int dsw_xstats_get_names(const struct rte_eventdev *dev,
 			 enum rte_event_dev_xstats_mode mode,
diff --git a/drivers/event/dsw/dsw_event.c b/drivers/event/dsw/dsw_event.c
index 1f09816945..f16d9d463b 100644
--- a/drivers/event/dsw/dsw_event.c
+++ b/drivers/event/dsw/dsw_event.c
@@ -1400,3 +1400,12 @@ dsw_event_dequeue_burst(void *port, struct rte_event *events, uint16_t num,
 
 	return dequeued;
 }
+
+void dsw_event_maintain(void *port)
+{
+	struct dsw_port *source_port = port;
+	struct dsw_evdev *dsw = source_port->dsw;
+
+	dsw_port_note_op(source_port, 0);
+	dsw_port_bg_process(dsw, source_port);
+}
-- 
2.25.1

[dpdk-dev] [PATCH 3/3] eventdev: have adapters support device maintenance

[Mattias Rönnblom]

Introduce support for event devices requiring calls to
rte_event_maintain() in the Ethernet RX, Timer and Crypto Eventdev
adapters.

RFC v2:
  - For simplicity, the timer adapter now unconditionally calls
    rte_event_maintain().
  - The RX adapter now only calls rte_event_maintain() when it has not
    enqueued any events.

Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
Tested-by: Richard Eklycke <richard.eklycke@ericsson.com>
---
 lib/eventdev/rte_event_crypto_adapter.c | 16 +++++++++++-----
 lib/eventdev/rte_event_eth_rx_adapter.c |  9 +++++++--
 lib/eventdev/rte_event_timer_adapter.c  |  3 +++
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/lib/eventdev/rte_event_crypto_adapter.c b/lib/eventdev/rte_event_crypto_adapter.c
index ae1151fb75..a6bd48da81 100644
--- a/lib/eventdev/rte_event_crypto_adapter.c
+++ b/lib/eventdev/rte_event_crypto_adapter.c
@@ -630,19 +630,25 @@ static void
 eca_crypto_adapter_run(struct event_crypto_adapter *adapter,
 		       unsigned int max_ops)
 {
-	while (max_ops) {
+	unsigned int ops_left = max_ops;
+
+	while (ops_left > 0) {
 		unsigned int e_cnt, d_cnt;
 
-		e_cnt = eca_crypto_adapter_deq_run(adapter, max_ops);
-		max_ops -= RTE_MIN(max_ops, e_cnt);
+		e_cnt = eca_crypto_adapter_deq_run(adapter, ops_left);
+		ops_left -= RTE_MIN(ops_left, e_cnt);
 
-		d_cnt = eca_crypto_adapter_enq_run(adapter, max_ops);
-		max_ops -= RTE_MIN(max_ops, d_cnt);
+		d_cnt = eca_crypto_adapter_enq_run(adapter, ops_left);
+		ops_left -= RTE_MIN(ops_left, d_cnt);
 
 		if (e_cnt == 0 && d_cnt == 0)
 			break;
 
 	}
+
+	if (ops_left == max_ops)
+		rte_event_maintain(adapter->eventdev_id,
+				   adapter->event_port_id);
 }
 
 static int
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index a175c61551..adf20f8b0c 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -992,6 +992,7 @@ rxa_eth_rx(struct event_eth_rx_adapter *rx_adapter, uint16_t port_id,
 					&rx_adapter->stats;
 	uint16_t n;
 	uint32_t nb_rx = 0;
+	uint32_t nb_flushed = 0;
 
 	if (rxq_empty)
 		*rxq_empty = 0;
@@ -1000,7 +1001,7 @@ rxa_eth_rx(struct event_eth_rx_adapter *rx_adapter, uint16_t port_id,
 	 */
 	while (rxa_pkt_buf_available(buf)) {
 		if (buf->count >= BATCH_SIZE)
-			rxa_flush_event_buffer(rx_adapter, buf);
+			nb_flushed += rxa_flush_event_buffer(rx_adapter, buf);
 
 		stats->rx_poll_count++;
 		n = rte_eth_rx_burst(port_id, queue_id, mbufs, BATCH_SIZE);
@@ -1016,7 +1017,11 @@ rxa_eth_rx(struct event_eth_rx_adapter *rx_adapter, uint16_t port_id,
 	}
 
 	if (buf->count > 0)
-		rxa_flush_event_buffer(rx_adapter, buf);
+		nb_flushed += rxa_flush_event_buffer(rx_adapter, buf);
+
+	if (nb_flushed == 0)
+		rte_event_maintain(rx_adapter->eventdev_id,
+				   rx_adapter->event_port_id);
 
 	return nb_rx;
 }
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index 0e21b7c475..a89cf8b978 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -786,6 +786,9 @@ swtim_service_func(void *arg)
 		sw->stats.adapter_tick_count++;
 	}
 
+	rte_event_maintain(adapter->data->event_dev_id,
+			   adapter->data->event_port_id);
+
 	return 0;
 }
 
-- 
2.25.1

Re: [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance

[Jerin Jacob]

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()
# 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?


+  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
>

Re: [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

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.


> # 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.


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
>>

Re: [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance

[Jerin Jacob]

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


>
>
> 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
> >>
>

Re: [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance

[Van Haaren, Harry]

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Friday, October 29, 2021 4:18 PM
> To: mattias.ronnblom <mattias.ronnblom@ericsson.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>;
> Carrillo, Erik G <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
> 
> 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.

Applications can identify if this "maintain()" call is required by
the RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag correct?

So the call through the Eventdev function pointer could be branched over at application level if required.
Or if the PMD doesn't actually set the ".maintain" function pointer to a valid value, then it will just return?

Is it easy/possible to test/benchmark this with test-eventdev to measure perf-impact?


> 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.

The "port hints" that was recently added could be used to communicate such a concept.
http://patches.dpdk.org/project/dpdk/patch/20211014145141.679372-1-harry.van.haaren@intel.com/

Note that today the "port hints" are *hints*, and must not be *relied on* to do anything.
This may be useful as (one of) the tools/concepts we could use to try abstract the "maintain" requirement.


> 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

Above, thanks for the +CC.

<snip remaining patch>

Re: [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

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.


Applications that are using some other event device today, and don't 
care about DSW or potential future event devices 
requiringRTE_EVENT_DEV_CAP_REQUIRES_MAINT, won't be affected at all, 
except the ops struct will be 8 bytes larger.


A rte_event_maintain() call on a device which doesn't need maintenance 
is just an inlined NULL compare on the ops struct field, which is 
frequently used and should be in a cache close to the core. In my 
benchmarks, I've been unable to measure any additional cost at all.


I reviewed the test and example applications last time I attempted to 
upstream this patch set, and from what I remember there was nothing to 
update. Things might have changed and I might misremember, so I'll have 
a look again.


What's important to keep in mind is that applications (DPDK tests, 
examples, user applications etc.) that have producer-only ports or 
otherwise potentially leave eventdev ports "unattended" don't work with 
DSW today, unless the take the measures described in the DSW 
documentation (which for example the eventdev adapters do not). So 
rte_event_maintain() will not break anything that's not already broken.


> 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.


rte_event_maintain() is called when no interaction with the event device 
has been done, during that service function call. That's the overall 
intention.

In the RX adapter, zero flushed events can also mean that the RX adapter 
had buffered events it wanted to flush, but the event device didn't 
accept new events (i.e, back pressure). In that case, the 
rte_event_maintain() call is redundant, but harmless (both because it's 
very low overhead on DSW, and near-zero overhead on any other current 
event device). Plus, if you are back-pressured by the pipeline, RX is 
not the bottleneck so a tiny bit of extra overhead is not an issue.


> In such a case, if the port is configured as producer and then
> internally it can call maintain.


To be able to perform maintenance (flushing, migration etc.), it needs 
cycles from the thread that "owns" the port. If the thread neither does 
enqueue (because it doesn't have anything to enqueue), nor dequeue, the 
driver will never get the chance to run. If DPDK had a delayed work 
mechanism that somehow could be tied to the "owning" port, then you 
could use that. But it doesn't.


> 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
>
>
>>
>> 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
>>>>

Re: [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

On 2021-10-29 18:02, Van Haaren, Harry wrote:
>> -----Original Message-----
>> From: Jerin Jacob <jerinjacobk@gmail.com>
>> Sent: Friday, October 29, 2021 4:18 PM
>> To: mattias.ronnblom <mattias.ronnblom@ericsson.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>;
>> Carrillo, Erik G <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
>>
>> 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.
> Applications can identify if this "maintain()" call is required by
> the RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag correct?


Yes.


> So the call through the Eventdev function pointer could be branched over at application level if required.


Yes, you could use the constant propagation trick to eliminate all 
overhead, if so would be preferred.


> Or if the PMD doesn't actually set the ".maintain" function pointer to a valid value, then it will just return?

That's another alternative, but a slightly more costly one. I fail to 
see the benefit of taking that approach.


> Is it easy/possible to test/benchmark this with test-eventdev to measure perf-impact?
>

I've benchmarked it in a different test bed, and as I've mentioned, for 
event devices which doesn't set RTE_EVENT_DEV_CAP_REQUIRES_MAINT and 
thus have NULL pointer as the maintain function in the ops struct, I was 
unable to measure any increase at all in overhead, even if they called 
rte_event_maintain() for every iteration in their 
"dequeue+process+enqueue" loop. Obviously, there are a couple of more 
instructions in the code path, so surely there is some cost, but ISP 
managed to hide everything on the system I was using.


>> 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.
> The "port hints" that was recently added could be used to communicate such a concept.
> https://protect2.fireeye.com/v1/url?k=131d62df-4c865bdd-131d2244-869a14f4b08c-e995dd419beebf7c&q=1&e=75c16b2e-15e8-4a1a-ab9f-6f80ec7138e6&u=http%3A%2F%2Fpatches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20211014145141.679372-1-harry.van.haaren%40intel.com%2F
>
> Note that today the "port hints" are *hints*, and must not be *relied on* to do anything.
> This may be useful as (one of) the tools/concepts we could use to try abstract the "maintain" requirement.
>
>
>> 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
> Above, thanks for the +CC.
>
> <snip remaining patch>


Sorry, I should have included all from the original (RFC) discussion.

Re: [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance

[Jerin Jacob]

On Sat, Oct 30, 2021 at 10:49 PM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> 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.
>
>
> Applications that are using some other event device today, and don't
> care about DSW or potential future event devices
> requiringRTE_EVENT_DEV_CAP_REQUIRES_MAINT, won't be affected at all,
> except the ops struct will be 8 bytes larger.

Ack. That's not an issue.

>
>
> A rte_event_maintain() call on a device which doesn't need maintenance
> is just an inlined NULL compare on the ops struct field, which is
> frequently used and should be in a cache close to the core. In my
> benchmarks, I've been unable to measure any additional cost at all.
>
>
> I reviewed the test and example applications last time I attempted to
> upstream this patch set, and from what I remember there was nothing to
> update. Things might have changed and I might misremember, so I'll have
> a look again.


OK.

>
>
> What's important to keep in mind is that applications (DPDK tests,
> examples, user applications etc.) that have producer-only ports or
> otherwise potentially leave eventdev ports "unattended" don't work with
> DSW today, unless the take the measures described in the DSW
> documentation (which for example the eventdev adapters do not). So
> rte_event_maintain() will not break anything that's not already broken.


OK.

>
>
> > 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.
>
>
> rte_event_maintain() is called when no interaction with the event device
> has been done, during that service function call. That's the overall
> intention.
>
> In the RX adapter, zero flushed events can also mean that the RX adapter
> had buffered events it wanted to flush, but the event device didn't
> accept new events (i.e, back pressure). In that case, the
> rte_event_maintain() call is redundant, but harmless (both because it's
> very low overhead on DSW, and near-zero overhead on any other current
> event device). Plus, if you are back-pressured by the pipeline, RX is
> not the bottleneck so a tiny bit of extra overhead is not an issue.

OK.

Looks good to me. Since DSW has better performance than other SW
drivers, I think, it is OK
to take some limitations to the application.

Please send the next version with the suggested documentation change.

If there is no objection, I will merge it on Wednesday. (3rd  Nov)

Re: [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

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
>>>>

[dpdk-dev] [PATCH v2 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

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 v2:
  - Mark rte_event_maintain() experimental.
  - Make clear it's the thread that owns the port that needs to call
    rte_event_maintain().
  - Make clear rte_event_maintain() may be called also during periods
    when enqueue/dequeue operations *are* performed.
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>
---
 lib/eventdev/eventdev_pmd.h          |  2 +
 lib/eventdev/eventdev_private.c      |  9 ++++
 lib/eventdev/eventdev_trace_points.c |  3 ++
 lib/eventdev/rte_eventdev.h          | 70 ++++++++++++++++++++++++++++
 lib/eventdev/rte_eventdev_core.h     |  5 ++
 lib/eventdev/rte_eventdev_trace_fp.h |  7 +++
 6 files changed, 96 insertions(+)

diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h
index d009e24309..82a5c4db33 100644
--- a/lib/eventdev/eventdev_pmd.h
+++ b/lib/eventdev/eventdev_pmd.h
@@ -164,6 +164,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;
+	/**< Pointer to PMD port maintenance function. */
 	event_tx_adapter_enqueue_t txa_enqueue_same_dest;
 	/**< Pointer to PMD eth Tx adapter burst enqueue function with
 	 * events destined to same Eth port & Tx queue.
diff --git a/lib/eventdev/eventdev_private.c b/lib/eventdev/eventdev_private.c
index 9084833847..6395c21dce 100644
--- a/lib/eventdev/eventdev_private.c
+++ b/lib/eventdev/eventdev_private.c
@@ -44,6 +44,13 @@ dummy_event_dequeue_burst(__rte_unused void *port,
 	return 0;
 }
 
+static void
+dummy_event_maintain(__rte_unused void *port)
+{
+	RTE_EDEV_LOG_ERR(
+		"maintenance requested for unconfigured event device");
+}
+
 static uint16_t
 dummy_event_tx_adapter_enqueue(__rte_unused void *port,
 			       __rte_unused struct rte_event ev[],
@@ -85,6 +92,7 @@ event_dev_fp_ops_reset(struct rte_event_fp_ops *fp_op)
 		.enqueue_forward_burst = dummy_event_enqueue_burst,
 		.dequeue = dummy_event_dequeue,
 		.dequeue_burst = dummy_event_dequeue_burst,
+		.maintain = dummy_event_maintain,
 		.txa_enqueue = dummy_event_tx_adapter_enqueue,
 		.txa_enqueue_same_dest =
 			dummy_event_tx_adapter_enqueue_same_dest,
@@ -105,6 +113,7 @@ event_dev_fp_ops_set(struct rte_event_fp_ops *fp_op,
 	fp_op->enqueue_forward_burst = dev->enqueue_forward_burst;
 	fp_op->dequeue = dev->dequeue;
 	fp_op->dequeue_burst = dev->dequeue_burst;
+	fp_op->maintain = dev->maintain;
 	fp_op->txa_enqueue = dev->txa_enqueue;
 	fp_op->txa_enqueue_same_dest = dev->txa_enqueue_same_dest;
 	fp_op->ca_enqueue = dev->ca_enqueue;
diff --git a/lib/eventdev/eventdev_trace_points.c b/lib/eventdev/eventdev_trace_points.c
index 237d9383fd..de6b1f4417 100644
--- a/lib/eventdev/eventdev_trace_points.c
+++ b/lib/eventdev/eventdev_trace_points.c
@@ -37,6 +37,9 @@ RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_enq_burst,
 RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_deq_burst,
 	lib.eventdev.deq.burst)
 
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_maintain,
+	lib.eventdev.maintain)
+
 /* Eventdev Rx adapter trace points */
 RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_create,
 	lib.eventdev.rx.adapter.create)
diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index 0abed56bef..3a2ec2230a 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
@@ -2063,6 +2072,67 @@ 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 have the
+ * RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag set. Such devices require an
+ * application thread using a particular port to periodically call
+ * rte_event_maintain() on that port during periods which it is
+ * neither attempting to enqueue events to nor dequeue events from the
+ * port. rte_event_maintain() is a low-overhead function and should be
+ * called at a high rate (e.g., in the application's poll loop).
+ *
+ * No port may be left unmaintained.
+ *
+ * At the application thread's convenience, rte_event_maintain() may
+ * (but is not required to) be called even during periods when enqueue
+ * or dequeue functions are being called, at the cost of a slight
+ * increase in overhead.
+ *
+ * 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
+ */
+__rte_experimental
+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

[dpdk-dev] [PATCH v2 2/3] event/dsw: make use of eventdev maintenance facility

[Mattias Rönnblom]

Set the RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag, and perform DSW
background tasks on rte_event_maintain() calls.

PATCH v2:
  - Update DSW documentation.
RFC v2:
  - Have dsw_event_maintain() occasionally flush the port output
    buffers.

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>
---
 doc/guides/eventdevs/dsw.rst  | 25 ++++++++++++++-----------
 drivers/event/dsw/dsw_evdev.c |  4 +++-
 drivers/event/dsw/dsw_evdev.h |  1 +
 drivers/event/dsw/dsw_event.c |  9 +++++++++
 4 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/doc/guides/eventdevs/dsw.rst b/doc/guides/eventdevs/dsw.rst
index 6653f501c1..af6ca714a2 100644
--- a/doc/guides/eventdevs/dsw.rst
+++ b/doc/guides/eventdevs/dsw.rst
@@ -40,20 +40,20 @@ Example:
 Limitations
 -----------
 
-Unattended Ports
+Port Maintenance
 ~~~~~~~~~~~~~~~~
 
-The distributed software eventdev uses an internal signaling schema
-between the ports to achieve load balancing. In order for this to
-work, the application must perform enqueue and/or dequeue operations
-on all ports.
+The distributed software eventdev uses an internal signaling scheme
+between the ports to achieve load balancing. Therefor, it sets the
+``RTE_EVENT_DEV_CAP_REQUIRES_MAINT`` flag.
 
-Producer-only ports which currently have no events to enqueue should
-periodically call rte_event_enqueue_burst() with a zero-sized burst.
+During periods when the application thread using a particular port is
+neither attempting to enqueue nor to dequeue events, it must
+repeatedly call rte_event_maintain() on that port.
 
-Ports left unattended for longer periods of time will prevent load
-balancing, and also cause traffic interruptions on the flows which
-are in the process of being migrated.
+Ports left unmaintained for long periods of time will prevent load
+balancing and cause traffic interruptions on flows which are in the
+process of being migrated.
 
 Output Buffering
 ~~~~~~~~~~~~~~~~
@@ -66,9 +66,12 @@ In case no more events are enqueued on a port with buffered events,
 these events will be sent after the application has performed a number
 of enqueue and/or dequeue operations.
 
-For explicit flushing, an application may call
+To explicitly flush a port's output buffer, an application may call
 rte_event_enqueue_burst() with a zero-sized burst.
 
+Repeated calls to rte_event_maintain() will also flush the output
+buffers.
+
 
 Priorities
 ~~~~~~~~~~
diff --git a/drivers/event/dsw/dsw_evdev.c b/drivers/event/dsw/dsw_evdev.c
index 0652d83ad6..5ff8fcc6a9 100644
--- a/drivers/event/dsw/dsw_evdev.c
+++ b/drivers/event/dsw/dsw_evdev.c
@@ -222,7 +222,8 @@ dsw_info_get(struct rte_eventdev *dev __rte_unused,
 		RTE_EVENT_DEV_CAP_DISTRIBUTED_SCHED|
 		RTE_EVENT_DEV_CAP_NONSEQ_MODE|
 		RTE_EVENT_DEV_CAP_MULTIPLE_QUEUE_PORT|
-		RTE_EVENT_DEV_CAP_CARRY_FLOW_ID
+		RTE_EVENT_DEV_CAP_CARRY_FLOW_ID|
+		RTE_EVENT_DEV_CAP_REQUIRES_MAINT
 	};
 }
 
@@ -441,6 +442,7 @@ dsw_probe(struct rte_vdev_device *vdev)
 	dev->enqueue_forward_burst = dsw_event_enqueue_forward_burst;
 	dev->dequeue = dsw_event_dequeue;
 	dev->dequeue_burst = dsw_event_dequeue_burst;
+	dev->maintain = dsw_event_maintain;
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
diff --git a/drivers/event/dsw/dsw_evdev.h b/drivers/event/dsw/dsw_evdev.h
index 631daea55c..31af4ede0f 100644
--- a/drivers/event/dsw/dsw_evdev.h
+++ b/drivers/event/dsw/dsw_evdev.h
@@ -271,6 +271,7 @@ uint16_t dsw_event_enqueue_forward_burst(void *port,
 uint16_t dsw_event_dequeue(void *port, struct rte_event *ev, uint64_t wait);
 uint16_t dsw_event_dequeue_burst(void *port, struct rte_event *events,
 				 uint16_t num, uint64_t wait);
+void dsw_event_maintain(void *port);
 
 int dsw_xstats_get_names(const struct rte_eventdev *dev,
 			 enum rte_event_dev_xstats_mode mode,
diff --git a/drivers/event/dsw/dsw_event.c b/drivers/event/dsw/dsw_event.c
index 1f09816945..f16d9d463b 100644
--- a/drivers/event/dsw/dsw_event.c
+++ b/drivers/event/dsw/dsw_event.c
@@ -1400,3 +1400,12 @@ dsw_event_dequeue_burst(void *port, struct rte_event *events, uint16_t num,
 
 	return dequeued;
 }
+
+void dsw_event_maintain(void *port)
+{
+	struct dsw_port *source_port = port;
+	struct dsw_evdev *dsw = source_port->dsw;
+
+	dsw_port_note_op(source_port, 0);
+	dsw_port_bg_process(dsw, source_port);
+}
-- 
2.25.1

[dpdk-dev] [PATCH v2 3/3] eventdev: have adapters support device maintenance

[Mattias Rönnblom]

Introduce support for event devices requiring calls to
rte_event_maintain() in the Ethernet RX, Timer and Crypto Eventdev
adapters.

RFC v2:
  - For simplicity, the timer adapter now unconditionally calls
    rte_event_maintain().
  - The RX adapter now only calls rte_event_maintain() when it has not
    enqueued any events.

Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
Tested-by: Richard Eklycke <richard.eklycke@ericsson.com>
---
 lib/eventdev/rte_event_crypto_adapter.c | 16 +++++++++++-----
 lib/eventdev/rte_event_eth_rx_adapter.c |  9 +++++++--
 lib/eventdev/rte_event_timer_adapter.c  |  3 +++
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/lib/eventdev/rte_event_crypto_adapter.c b/lib/eventdev/rte_event_crypto_adapter.c
index ae1151fb75..a6bd48da81 100644
--- a/lib/eventdev/rte_event_crypto_adapter.c
+++ b/lib/eventdev/rte_event_crypto_adapter.c
@@ -630,19 +630,25 @@ static void
 eca_crypto_adapter_run(struct event_crypto_adapter *adapter,
 		       unsigned int max_ops)
 {
-	while (max_ops) {
+	unsigned int ops_left = max_ops;
+
+	while (ops_left > 0) {
 		unsigned int e_cnt, d_cnt;
 
-		e_cnt = eca_crypto_adapter_deq_run(adapter, max_ops);
-		max_ops -= RTE_MIN(max_ops, e_cnt);
+		e_cnt = eca_crypto_adapter_deq_run(adapter, ops_left);
+		ops_left -= RTE_MIN(ops_left, e_cnt);
 
-		d_cnt = eca_crypto_adapter_enq_run(adapter, max_ops);
-		max_ops -= RTE_MIN(max_ops, d_cnt);
+		d_cnt = eca_crypto_adapter_enq_run(adapter, ops_left);
+		ops_left -= RTE_MIN(ops_left, d_cnt);
 
 		if (e_cnt == 0 && d_cnt == 0)
 			break;
 
 	}
+
+	if (ops_left == max_ops)
+		rte_event_maintain(adapter->eventdev_id,
+				   adapter->event_port_id);
 }
 
 static int
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index a175c61551..adf20f8b0c 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -992,6 +992,7 @@ rxa_eth_rx(struct event_eth_rx_adapter *rx_adapter, uint16_t port_id,
 					&rx_adapter->stats;
 	uint16_t n;
 	uint32_t nb_rx = 0;
+	uint32_t nb_flushed = 0;
 
 	if (rxq_empty)
 		*rxq_empty = 0;
@@ -1000,7 +1001,7 @@ rxa_eth_rx(struct event_eth_rx_adapter *rx_adapter, uint16_t port_id,
 	 */
 	while (rxa_pkt_buf_available(buf)) {
 		if (buf->count >= BATCH_SIZE)
-			rxa_flush_event_buffer(rx_adapter, buf);
+			nb_flushed += rxa_flush_event_buffer(rx_adapter, buf);
 
 		stats->rx_poll_count++;
 		n = rte_eth_rx_burst(port_id, queue_id, mbufs, BATCH_SIZE);
@@ -1016,7 +1017,11 @@ rxa_eth_rx(struct event_eth_rx_adapter *rx_adapter, uint16_t port_id,
 	}
 
 	if (buf->count > 0)
-		rxa_flush_event_buffer(rx_adapter, buf);
+		nb_flushed += rxa_flush_event_buffer(rx_adapter, buf);
+
+	if (nb_flushed == 0)
+		rte_event_maintain(rx_adapter->eventdev_id,
+				   rx_adapter->event_port_id);
 
 	return nb_rx;
 }
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index 0e21b7c475..a89cf8b978 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -786,6 +786,9 @@ swtim_service_func(void *arg)
 		sw->stats.adapter_tick_count++;
 	}
 
+	rte_event_maintain(adapter->data->event_dev_id,
+			   adapter->data->event_port_id);
+
 	return 0;
 }
 
-- 
2.25.1

[dpdk-dev] [PATCH v3 1/3] eventdev: allow for event devices requiring maintenance

[Mattias Rönnblom]

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 v3:
  - Introduced 'op' parameter to rte_event_maintain(), to allow for
    forcing an immediate flush of buffered events (and potentially
    other future maintenance-related operations).
  - Return -EINVAL in case rte_event_maintain() is passed invalid
    arguments. Do not set rte_errno.
PATCH v2:
  - Mark rte_event_maintain() experimental.
  - Make clear it's the thread that owns the port that needs to call
    rte_event_maintain().
  - Make clear rte_event_maintain() may be called also during periods
    when enqueue/dequeue operations *are* performed.
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>
---
 lib/eventdev/eventdev_pmd.h          |  2 +
 lib/eventdev/eventdev_private.c      |  9 ++++
 lib/eventdev/eventdev_trace_points.c |  3 ++
 lib/eventdev/rte_eventdev.h          | 79 ++++++++++++++++++++++++++++
 lib/eventdev/rte_eventdev_core.h     |  5 ++
 lib/eventdev/rte_eventdev_trace_fp.h |  8 +++
 6 files changed, 106 insertions(+)

diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h
index d009e24309..82a5c4db33 100644
--- a/lib/eventdev/eventdev_pmd.h
+++ b/lib/eventdev/eventdev_pmd.h
@@ -164,6 +164,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;
+	/**< Pointer to PMD port maintenance function. */
 	event_tx_adapter_enqueue_t txa_enqueue_same_dest;
 	/**< Pointer to PMD eth Tx adapter burst enqueue function with
 	 * events destined to same Eth port & Tx queue.
diff --git a/lib/eventdev/eventdev_private.c b/lib/eventdev/eventdev_private.c
index 9084833847..1d3d9d357e 100644
--- a/lib/eventdev/eventdev_private.c
+++ b/lib/eventdev/eventdev_private.c
@@ -44,6 +44,13 @@ dummy_event_dequeue_burst(__rte_unused void *port,
 	return 0;
 }
 
+static void
+dummy_event_maintain(__rte_unused void *port, __rte_unused int op)
+{
+	RTE_EDEV_LOG_ERR(
+		"maintenance requested for unconfigured event device");
+}
+
 static uint16_t
 dummy_event_tx_adapter_enqueue(__rte_unused void *port,
 			       __rte_unused struct rte_event ev[],
@@ -85,6 +92,7 @@ event_dev_fp_ops_reset(struct rte_event_fp_ops *fp_op)
 		.enqueue_forward_burst = dummy_event_enqueue_burst,
 		.dequeue = dummy_event_dequeue,
 		.dequeue_burst = dummy_event_dequeue_burst,
+		.maintain = dummy_event_maintain,
 		.txa_enqueue = dummy_event_tx_adapter_enqueue,
 		.txa_enqueue_same_dest =
 			dummy_event_tx_adapter_enqueue_same_dest,
@@ -105,6 +113,7 @@ event_dev_fp_ops_set(struct rte_event_fp_ops *fp_op,
 	fp_op->enqueue_forward_burst = dev->enqueue_forward_burst;
 	fp_op->dequeue = dev->dequeue;
 	fp_op->dequeue_burst = dev->dequeue_burst;
+	fp_op->maintain = dev->maintain;
 	fp_op->txa_enqueue = dev->txa_enqueue;
 	fp_op->txa_enqueue_same_dest = dev->txa_enqueue_same_dest;
 	fp_op->ca_enqueue = dev->ca_enqueue;
diff --git a/lib/eventdev/eventdev_trace_points.c b/lib/eventdev/eventdev_trace_points.c
index 237d9383fd..de6b1f4417 100644
--- a/lib/eventdev/eventdev_trace_points.c
+++ b/lib/eventdev/eventdev_trace_points.c
@@ -37,6 +37,9 @@ RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_enq_burst,
 RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_deq_burst,
 	lib.eventdev.deq.burst)
 
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_maintain,
+	lib.eventdev.maintain)
+
 /* Eventdev Rx adapter trace points */
 RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_create,
 	lib.eventdev.rx.adapter.create)
diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index 0abed56bef..e026486ca5 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
@@ -2063,6 +2072,76 @@ rte_event_dequeue_burst(uint8_t dev_id, uint8_t port_id, struct rte_event ev[],
 					       timeout_ticks);
 }
 
+#define RTE_EVENT_DEV_MAINT_OP_FLUSH          (1 << 0)
+/**< Force an immediately flush of any buffered events in the port,
+ * potentially at the cost of additional overhead.
+ *
+ * @see rte_event_maintain()
+ */
+
+/**
+ * Maintain an event device.
+ *
+ * This function is only relevant for event devices which have the
+ * @ref RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag set. Such devices
+ * require an application thread using a particular port to
+ * periodically call rte_event_maintain() on that port during periods
+ * which it is neither attempting to enqueue events to nor dequeue
+ * events from the port. rte_event_maintain() is a low-overhead
+ * function and should be called at a high rate (e.g., in the
+ * application's poll loop).
+ *
+ * No port may be left unmaintained.
+ *
+ * At the application thread's convenience, rte_event_maintain() may
+ * (but is not required to) be called even during periods when enqueue
+ * or dequeue functions are being called, at the cost of a slight
+ * increase in overhead.
+ *
+ * rte_event_maintain() may be called on event devices which haven't
+ * set @ref 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.
+ * @param op
+ *   0, or @ref RTE_EVENT_DEV_MAINT_OP_FLUSH.
+ * @return
+ *  - 0 on success.
+ *  - -EINVAL if *dev_id*,  *port_id*, or *op* is invalid.
+ *
+ * @see RTE_EVENT_DEV_CAP_REQUIRES_MAINT
+ */
+__rte_experimental
+static inline int
+rte_event_maintain(uint8_t dev_id, uint8_t port_id, int op)
+{
+	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)
+		return -EINVAL;
+
+	if (port == NULL)
+		return -EINVAL;
+
+	if (op & (~RTE_EVENT_DEV_MAINT_OP_FLUSH))
+		return -EINVAL;
+#endif
+	rte_eventdev_trace_maintain(dev_id, port_id, op);
+
+	if (fp_ops->maintain != NULL)
+		fp_ops->maintain(port, op);
+
+	return 0;
+}
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/eventdev/rte_eventdev_core.h b/lib/eventdev/rte_eventdev_core.h
index 61d5ebdc44..c328bdbc82 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, int op);
+/**< @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..af2172d2a5 100644
--- a/lib/eventdev/rte_eventdev_trace_fp.h
+++ b/lib/eventdev/rte_eventdev_trace_fp.h
@@ -38,6 +38,14 @@ 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, int op),
+	rte_trace_point_emit_u8(dev_id);
+	rte_trace_point_emit_u8(port_id);
+	rte_trace_point_emit_int(op);
+)
+
 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

[dpdk-dev] [PATCH v3 2/3] event/dsw: make use of eventdev maintenance facility

[Mattias Rönnblom]

Set the RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag, and perform DSW
background tasks on rte_event_maintain() calls.

PATCH v3:
  - Add support for output buffer flushing by means of the new
    rte_event_maintain() op parameter, and RTE_EVENT_DEV_MAINT_OP_FLUSH.
PATCH v2:
  - Update DSW documentation.
RFC v2:
  - Have dsw_event_maintain() occasionally flush the port output
    buffers.

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>
---
 doc/guides/eventdevs/dsw.rst  | 27 +++++++++++++++------------
 drivers/event/dsw/dsw_evdev.c |  4 +++-
 drivers/event/dsw/dsw_evdev.h |  1 +
 drivers/event/dsw/dsw_event.c | 12 ++++++++++++
 4 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/doc/guides/eventdevs/dsw.rst b/doc/guides/eventdevs/dsw.rst
index 6653f501c1..18f7e9588f 100644
--- a/doc/guides/eventdevs/dsw.rst
+++ b/doc/guides/eventdevs/dsw.rst
@@ -40,20 +40,20 @@ Example:
 Limitations
 -----------
 
-Unattended Ports
+Port Maintenance
 ~~~~~~~~~~~~~~~~
 
-The distributed software eventdev uses an internal signaling schema
-between the ports to achieve load balancing. In order for this to
-work, the application must perform enqueue and/or dequeue operations
-on all ports.
+The distributed software eventdev uses an internal signaling scheme
+between the ports to achieve load balancing. Therefore, it sets the
+``RTE_EVENT_DEV_CAP_REQUIRES_MAINT`` flag.
 
-Producer-only ports which currently have no events to enqueue should
-periodically call rte_event_enqueue_burst() with a zero-sized burst.
+During periods when the application thread using a particular port is
+neither attempting to enqueue nor to dequeue events, it must
+repeatedly call rte_event_maintain() on that port.
 
-Ports left unattended for longer periods of time will prevent load
-balancing, and also cause traffic interruptions on the flows which
-are in the process of being migrated.
+Ports left unmaintained for long periods of time will prevent load
+balancing and cause traffic interruptions on flows which are in the
+process of being migrated.
 
 Output Buffering
 ~~~~~~~~~~~~~~~~
@@ -66,8 +66,11 @@ In case no more events are enqueued on a port with buffered events,
 these events will be sent after the application has performed a number
 of enqueue and/or dequeue operations.
 
-For explicit flushing, an application may call
-rte_event_enqueue_burst() with a zero-sized burst.
+To immediately flush a port's output buffer, an application may call
+rte_event_maintain() with op set to ``RTE_EVENT_DEV_MAINT_OP_FLUSH``.
+
+Repeated calls to rte_event_maintain() will also flush the output
+buffers.
 
 
 Priorities
diff --git a/drivers/event/dsw/dsw_evdev.c b/drivers/event/dsw/dsw_evdev.c
index 0652d83ad6..5ff8fcc6a9 100644
--- a/drivers/event/dsw/dsw_evdev.c
+++ b/drivers/event/dsw/dsw_evdev.c
@@ -222,7 +222,8 @@ dsw_info_get(struct rte_eventdev *dev __rte_unused,
 		RTE_EVENT_DEV_CAP_DISTRIBUTED_SCHED|
 		RTE_EVENT_DEV_CAP_NONSEQ_MODE|
 		RTE_EVENT_DEV_CAP_MULTIPLE_QUEUE_PORT|
-		RTE_EVENT_DEV_CAP_CARRY_FLOW_ID
+		RTE_EVENT_DEV_CAP_CARRY_FLOW_ID|
+		RTE_EVENT_DEV_CAP_REQUIRES_MAINT
 	};
 }
 
@@ -441,6 +442,7 @@ dsw_probe(struct rte_vdev_device *vdev)
 	dev->enqueue_forward_burst = dsw_event_enqueue_forward_burst;
 	dev->dequeue = dsw_event_dequeue;
 	dev->dequeue_burst = dsw_event_dequeue_burst;
+	dev->maintain = dsw_event_maintain;
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
diff --git a/drivers/event/dsw/dsw_evdev.h b/drivers/event/dsw/dsw_evdev.h
index 631daea55c..e64ae26f6e 100644
--- a/drivers/event/dsw/dsw_evdev.h
+++ b/drivers/event/dsw/dsw_evdev.h
@@ -271,6 +271,7 @@ uint16_t dsw_event_enqueue_forward_burst(void *port,
 uint16_t dsw_event_dequeue(void *port, struct rte_event *ev, uint64_t wait);
 uint16_t dsw_event_dequeue_burst(void *port, struct rte_event *events,
 				 uint16_t num, uint64_t wait);
+void dsw_event_maintain(void *port, int op);
 
 int dsw_xstats_get_names(const struct rte_eventdev *dev,
 			 enum rte_event_dev_xstats_mode mode,
diff --git a/drivers/event/dsw/dsw_event.c b/drivers/event/dsw/dsw_event.c
index 1f09816945..c6ed470286 100644
--- a/drivers/event/dsw/dsw_event.c
+++ b/drivers/event/dsw/dsw_event.c
@@ -1400,3 +1400,15 @@ dsw_event_dequeue_burst(void *port, struct rte_event *events, uint16_t num,
 
 	return dequeued;
 }
+
+void dsw_event_maintain(void *port, int op)
+{
+	struct dsw_port *source_port = port;
+	struct dsw_evdev *dsw = source_port->dsw;
+
+	dsw_port_note_op(source_port, 0);
+	dsw_port_bg_process(dsw, source_port);
+
+	if (op & RTE_EVENT_DEV_MAINT_OP_FLUSH)
+		dsw_port_flush_out_buffers(dsw, source_port);
+}
-- 
2.25.1

[dpdk-dev] [PATCH v3 3/3] eventdev: have adapters support device maintenance

[Mattias Rönnblom]

Introduce support for event devices requiring calls to
rte_event_maintain() in the Ethernet RX, Timer and Crypto Eventdev
adapters.

PATCH v3:
  - Set the new 'op' rte_event_maintain() parameter to zero.
RFC v2:
  - For simplicity, the timer adapter now unconditionally calls
    rte_event_maintain().
  - The RX adapter now only calls rte_event_maintain() when it has not
    enqueued any events.

Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
Tested-by: Richard Eklycke <richard.eklycke@ericsson.com>
---
 lib/eventdev/rte_event_crypto_adapter.c | 16 +++++++++++-----
 lib/eventdev/rte_event_eth_rx_adapter.c |  9 +++++++--
 lib/eventdev/rte_event_timer_adapter.c  |  3 +++
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/lib/eventdev/rte_event_crypto_adapter.c b/lib/eventdev/rte_event_crypto_adapter.c
index ae1151fb75..d84080355d 100644
--- a/lib/eventdev/rte_event_crypto_adapter.c
+++ b/lib/eventdev/rte_event_crypto_adapter.c
@@ -630,19 +630,25 @@ static void
 eca_crypto_adapter_run(struct event_crypto_adapter *adapter,
 		       unsigned int max_ops)
 {
-	while (max_ops) {
+	unsigned int ops_left = max_ops;
+
+	while (ops_left > 0) {
 		unsigned int e_cnt, d_cnt;
 
-		e_cnt = eca_crypto_adapter_deq_run(adapter, max_ops);
-		max_ops -= RTE_MIN(max_ops, e_cnt);
+		e_cnt = eca_crypto_adapter_deq_run(adapter, ops_left);
+		ops_left -= RTE_MIN(ops_left, e_cnt);
 
-		d_cnt = eca_crypto_adapter_enq_run(adapter, max_ops);
-		max_ops -= RTE_MIN(max_ops, d_cnt);
+		d_cnt = eca_crypto_adapter_enq_run(adapter, ops_left);
+		ops_left -= RTE_MIN(ops_left, d_cnt);
 
 		if (e_cnt == 0 && d_cnt == 0)
 			break;
 
 	}
+
+	if (ops_left == max_ops)
+		rte_event_maintain(adapter->eventdev_id,
+				   adapter->event_port_id, 0);
 }
 
 static int
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index a175c61551..d3af469aec 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -992,6 +992,7 @@ rxa_eth_rx(struct event_eth_rx_adapter *rx_adapter, uint16_t port_id,
 					&rx_adapter->stats;
 	uint16_t n;
 	uint32_t nb_rx = 0;
+	uint32_t nb_flushed = 0;
 
 	if (rxq_empty)
 		*rxq_empty = 0;
@@ -1000,7 +1001,7 @@ rxa_eth_rx(struct event_eth_rx_adapter *rx_adapter, uint16_t port_id,
 	 */
 	while (rxa_pkt_buf_available(buf)) {
 		if (buf->count >= BATCH_SIZE)
-			rxa_flush_event_buffer(rx_adapter, buf);
+			nb_flushed += rxa_flush_event_buffer(rx_adapter, buf);
 
 		stats->rx_poll_count++;
 		n = rte_eth_rx_burst(port_id, queue_id, mbufs, BATCH_SIZE);
@@ -1016,7 +1017,11 @@ rxa_eth_rx(struct event_eth_rx_adapter *rx_adapter, uint16_t port_id,
 	}
 
 	if (buf->count > 0)
-		rxa_flush_event_buffer(rx_adapter, buf);
+		nb_flushed += rxa_flush_event_buffer(rx_adapter, buf);
+
+	if (nb_flushed == 0)
+		rte_event_maintain(rx_adapter->eventdev_id,
+				   rx_adapter->event_port_id, 0);
 
 	return nb_rx;
 }
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index 0e21b7c475..e5572e2add 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -786,6 +786,9 @@ swtim_service_func(void *arg)
 		sw->stats.adapter_tick_count++;
 	}
 
+	rte_event_maintain(adapter->data->event_dev_id,
+			   adapter->data->event_port_id, 0);
+
 	return 0;
 }
 
-- 
2.25.1

Re: [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance

[Jerin Jacob]

On Sun, Oct 31, 2021 at 6:47 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Sat, Oct 30, 2021 at 10:49 PM Mattias Rönnblom
> <mattias.ronnblom@ericsson.com> wrote:
> >
> > 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.
> >
> >
> > Applications that are using some other event device today, and don't
> > care about DSW or potential future event devices
> > requiringRTE_EVENT_DEV_CAP_REQUIRES_MAINT, won't be affected at all,
> > except the ops struct will be 8 bytes larger.
>
> Ack. That's not an issue.
>
> >
> >
> > A rte_event_maintain() call on a device which doesn't need maintenance
> > is just an inlined NULL compare on the ops struct field, which is
> > frequently used and should be in a cache close to the core. In my
> > benchmarks, I've been unable to measure any additional cost at all.
> >
> >
> > I reviewed the test and example applications last time I attempted to
> > upstream this patch set, and from what I remember there was nothing to
> > update. Things might have changed and I might misremember, so I'll have
> > a look again.
>
>
> OK.
>
> >
> >
> > What's important to keep in mind is that applications (DPDK tests,
> > examples, user applications etc.) that have producer-only ports or
> > otherwise potentially leave eventdev ports "unattended" don't work with
> > DSW today, unless the take the measures described in the DSW
> > documentation (which for example the eventdev adapters do not). So
> > rte_event_maintain() will not break anything that's not already broken.
>
>
> OK.
>
> >
> >
> > > 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.
> >
> >
> > rte_event_maintain() is called when no interaction with the event device
> > has been done, during that service function call. That's the overall
> > intention.
> >
> > In the RX adapter, zero flushed events can also mean that the RX adapter
> > had buffered events it wanted to flush, but the event device didn't
> > accept new events (i.e, back pressure). In that case, the
> > rte_event_maintain() call is redundant, but harmless (both because it's
> > very low overhead on DSW, and near-zero overhead on any other current
> > event device). Plus, if you are back-pressured by the pipeline, RX is
> > not the bottleneck so a tiny bit of extra overhead is not an issue.
>
> OK.
>
> Looks good to me. Since DSW has better performance than other SW
> drivers, I think, it is OK
> to take some limitations to the application.
>
> Please send the next version with the suggested documentation change.
>
> If there is no objection, I will merge it on Wednesday. (3rd  Nov)


Applied v3 version  to dpdk-next-net-eventdev/for-main. Thanks