[v3,2/2] eventdev: add support for enqueue reorder

Message ID 20240621222408.583464-3-abdullah.sevincer@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series DLB Enqueue Reorder Support |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation fail Compilation issues
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build fail github build: failed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-amd64-testing fail Testing issues
ci/iol-compile-amd64-testing fail Testing issues
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Abdullah Sevincer June 21, 2024, 10:24 p.m. UTC
This commit adds support flag to enable enqueue reorder
feature.

When this flag is enabled in the port configuration PMD
restores dequeue order on enqueue if applications happen to
change it.

Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
---
 lib/eventdev/rte_eventdev.h | 8 ++++++++
 1 file changed, 8 insertions(+)
  

Comments

Jerin Jacob June 24, 2024, 8:28 a.m. UTC | #1
On Sat, Jun 22, 2024 at 4:02 AM Abdullah Sevincer
<abdullah.sevincer@intel.com> wrote:
>
> This commit adds support flag to enable enqueue reorder
> feature.
>
> When this flag is enabled in the port configuration PMD
> restores dequeue order on enqueue if applications happen to
> change it.
>
> Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
> ---
>  lib/eventdev/rte_eventdev.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> index 08e5f9320b..f4220dd5dc 100644
> --- a/lib/eventdev/rte_eventdev.h
> +++ b/lib/eventdev/rte_eventdev.h
> @@ -1073,6 +1073,14 @@ rte_event_queue_attr_set(uint8_t dev_id, uint8_t queue_id, uint32_t attr_id,
>   *  @see rte_event_port_setup()
>   */
>
> +#define RTE_EVENT_PORT_CFG_RESTORE_DEQ_ORDER   (1ULL << 5)
> +/**< Flag to enable feature enqueue reordering to dequeue.
> + * The feature restores dequeue order on enqueue if applications
> + * happen to change the order.

# Is this feature or limitation?
# What is the use case for this feature?
# If application don't care about ORDER, they can use
RTE_SCHED_TYPE_PARALLEL. Right?
# Can you share the feature in the context of the below text in specification?

----------------
/* Scheduler type definitions */
#define RTE_SCHED_TYPE_ORDERED          0
/**< Ordered scheduling
 *
 * Events from an ordered flow of an event queue can be scheduled to multiple
 * ports for concurrent processing while maintaining the original event order,
 * i.e. the order in which they were first enqueued to that queue.
 * This scheme allows events pertaining to the same, potentially large, flow to
 * be processed in parallel on multiple cores without incurring any
 * application-level order restoration logic overhead.
 *
 * After events are dequeued from a set of ports, as those events are
re-enqueued
 * to another queue (with the op field set to @ref
RTE_EVENT_OP_FORWARD), the event
 * device restores the original event order - including events returned from all
 * ports in the set - before the events are placed on the destination queue,
 * for subsequent scheduling to ports

-----------------


> + *
> + *  @see rte_event_port_setup()
> + */
> +
>  /** Event port configuration structure */
>  struct rte_event_port_conf {
>         int32_t new_event_threshold;
> --
> 2.25.1
>
  
Abdullah Sevincer June 26, 2024, 6:31 p.m. UTC | #2
Hi Jerin my responses below:
>+# Is this feature or limitation?
This is a new feature to enable enqueuing to PMD in any order when the underlined hardware device needs enqueues in a strict dequeue order.
>+# What is the use case for this feature?
This is needed by the applications which processes events in batches based on their flow ids. Received burst is sorted based on flow ids.
>+# If application don't care about ORDER, they can use RTE_SCHED_TYPE_PARALLEL. Right?
This is the ordering between enqueue and dequeue and not across cores.
>+# Can you share the feature in the context of the below text in specification?
Since the feature is not across cores below context does not apply.

>+----------------
>+/* Scheduler type definitions */
>+#define RTE_SCHED_TYPE_ORDERED          0
>+/**< Ordered scheduling
>+ *
>+ * Events from an ordered flow of an event queue can be scheduled to multiple
>+ * ports for concurrent processing while maintaining the original event order,
>+ * i.e. the order in which they were first enqueued to that queue.
>+ * This scheme allows events pertaining to the same, potentially large, flow to
>+ * be processed in parallel on multiple cores without incurring any
>+* application-level order restoration logic overhead.
>+ *
>+* After events are dequeued from a set of ports, as those events are re-enqueued
>+* to another queue (with the op field set to @ref RTE_EVENT_OP_FORWARD), the event
>+* device restores the original event order - including events returned from all
>+* ports in the set - before the events are placed on the destination queue,
>+* for subsequent scheduling to ports


> + *
> + *  @see rte_event_port_setup()
> + */
> +
>  /** Event port configuration structure */  struct rte_event_port_conf 
> {
>         int32_t new_event_threshold;
> --
> 2.25.1
>
  
Jerin Jacob June 27, 2024, 1:13 p.m. UTC | #3
On Thu, Jun 27, 2024 at 12:01 AM Sevincer, Abdullah
<abdullah.sevincer@intel.com> wrote:
>
> Hi Jerin my responses below:
> >+# Is this feature or limitation?
> This is a new feature to enable enqueuing to PMD in any order when the underlined hardware device needs enqueues in a strict dequeue order.
> >+# What is the use case for this feature?
> This is needed by the applications which processes events in batches based on their flow ids. Received burst is sorted based on flow ids.


OK. It is not clear from the Doxygen comment, add more details in
comment in next version, especially if it is applicable for batch
mode.
In general, the concept looks good to me.

Add new  RTE_EVENT_DEV_CAP_* for this feature.
Update doc/guides/eventdevs/features/default.ini and your PMD feature list.

Adding other eventdev PMD maintainers if there are any comments.
  
Mattias Rönnblom July 1, 2024, 8:24 a.m. UTC | #4
On 2024-06-22 00:24, Abdullah Sevincer wrote:
> This commit adds support flag to enable enqueue reorder
> feature.
> 

"Enqueue reorder" is how this feature is implemented (on DLB2), but it's 
not a good description of what it does (or, allows for).

I've called this feature "independent enqueue" in the past. I have a 
vague memory of someone from Intel calling it something else 
("out-of-order enqueue" maybe?), but I can't seem to be able to find 
that e-mail.

> When this flag is enabled in the port configuration PMD
> restores dequeue order on enqueue if applications happen to
> change it.
> 

If this feature is enabled, the application is free to enqueue events in 
any order, while still maintaining ordered/atomic semantics. That's how 
I would characterize it.

You may also want to note that the DPDK dispatcher library depends on 
this flag to function properly on burst-capable event devices.

This patch set should also include a patch to DSW, where it advertises 
this capability.

Ideally, you should also include a patch to the dispatcher library, 
which checks for this flag on RTE_EVENT_DEV_CAP_BURST_MODE event devices.

> Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
> ---
>   lib/eventdev/rte_eventdev.h | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> index 08e5f9320b..f4220dd5dc 100644
> --- a/lib/eventdev/rte_eventdev.h
> +++ b/lib/eventdev/rte_eventdev.h
> @@ -1073,6 +1073,14 @@ rte_event_queue_attr_set(uint8_t dev_id, uint8_t queue_id, uint32_t attr_id,
>    *  @see rte_event_port_setup()
>    */
>   
> +#define RTE_EVENT_PORT_CFG_RESTORE_DEQ_ORDER   (1ULL << 5)
> +/**< Flag to enable feature enqueue reordering to dequeue.
> + * The feature restores dequeue order on enqueue if applications
> + * happen to change the order.
> + *
> + *  @see rte_event_port_setup()
> + */
> +
>   /** Event port configuration structure */
>   struct rte_event_port_conf {
>   	int32_t new_event_threshold;
  
Mattias Rönnblom July 1, 2024, 8:50 a.m. UTC | #5
On 2024-06-22 00:24, Abdullah Sevincer wrote:
> This commit adds support flag to enable enqueue reorder
> feature.
> 
> When this flag is enabled in the port configuration PMD
> restores dequeue order on enqueue if applications happen to
> change it.
> 
> Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
> ---
>   lib/eventdev/rte_eventdev.h | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> index 08e5f9320b..f4220dd5dc 100644
> --- a/lib/eventdev/rte_eventdev.h
> +++ b/lib/eventdev/rte_eventdev.h
> @@ -1073,6 +1073,14 @@ rte_event_queue_attr_set(uint8_t dev_id, uint8_t queue_id, uint32_t attr_id,
>    *  @see rte_event_port_setup()
>    */
>   
> +#define RTE_EVENT_PORT_CFG_RESTORE_DEQ_ORDER   (1ULL << 5)
> +/**< Flag to enable feature enqueue reordering to dequeue.
> + * The feature restores dequeue order on enqueue if applications
> + * happen to change the order.
> + *

Add a device-level
RTE_EVENT_DEV_CAP_INDEPENDENT_ENQ
as well.

The documentation of that flag should probably house the detailed 
description of this feature.

Here's how I would describe this feature:

#define RTE_EVENT_PORT_CFG_INDEPENDENT_ENQ   (1ULL << 5)

/**< Flag to enable independent enqueue. Must be unset if the device
  * is not RTE_EVENT_DEV_CAP_INDEPENDENT_ENQ capable. This feature
  * allows an application to enqueue RTE_EVENT_OP_FORWARD or
  * RTE_EVENT_OP_RELEASE in an order different than the order the
  * events were dequeued from the event device, while maintaining
  * RTE_SCHED_TYPE_ATOMIC or RTE_SCHED_TYPE_ORDERED semantics.
  *
  * If the application wish to change the order of two events *within
  * a flow*, it must both change the enqueue order and exchange the
  * impl_opaque field, to be portable across all event devices.
  */

That second paragraph allows DSW to support this feature without 
modification, since this is the only difference between DSW-style 
independent enqueue, and DLB enqueue reordering. DLB will restore a 
total order, while DSW doesn't (since it would be both pointless and 
costly, given its design).

The upside with DSW-style implementation is that it's very simple and 
efficient, and does not impose any head-of-line blocking (which follows 
from restoring a total order between dequeue and enqueue). The downside 
is it does not allow for a scenario where a particular flow is split 
across different modules, the application performs reordering (e.g., 
with the dispatcher library) *and* wants to maintain ordering between 
events pertaining to those "sub flows". I've never come across such a 
scenario, but it may well exist.

If we fail to make DLB2 and DSW compatible, we'll probably need another 
flag for DSW, because needlessly imposing a total order DSW does not 
make a lot of sense.

You may want to add an example as well. And a note on the importance of 
maintaining impl_opaque between dequeue and enqueue.

> + *  @see rte_event_port_setup()
> + */
> +
>   /** Event port configuration structure */
>   struct rte_event_port_conf {
>   	int32_t new_event_threshold;
  
Pathak, Pravin July 2, 2024, 5:25 p.m. UTC | #6
>+ Add a device-level
>+ RTE_EVENT_DEV_CAP_INDEPENDENT_ENQ
>+ as well.
>+ The documentation of that flag should probably house the detailed description of this feature.

So, this capability will be advertised by DSW and DLB devices with detailed documentation. 

>+ Here's how I would describe this feature:

>+ #define RTE_EVENT_PORT_CFG_INDEPENDENT_ENQ   (1ULL << 5)

>+/**< Flag to enable independent enqueue. Must be unset if the device
>+  * is not RTE_EVENT_DEV_CAP_INDEPENDENT_ENQ capable. This feature
>+  * allows an application to enqueue RTE_EVENT_OP_FORWARD or
>+  * RTE_EVENT_OP_RELEASE in an order different than the order the
>+  * events were dequeued from the event device, while maintaining
>+  * RTE_SCHED_TYPE_ATOMIC or RTE_SCHED_TYPE_ORDERED semantics.
>+  *
>+  * If the application wish to change the order of two events *within
>+  * a flow*, it must both change the enqueue order and exchange the
>+  * impl_opaque field, to be portable across all event devices.
>+  */

>+That second paragraph allows DSW to support this feature without modification since this is the only difference between DSW-style independent enqueue and DLB enqueue reordering. DLB will restore a total order, while DSW doesn't (since it would be both pointless and costly, given its design).

Can we skip mentioning this mechanism to change the order of any two events intentionally? For DLB, those two events should have been received from the same queue and, if atomic, with the same atomic flow ID. If there is no use case, we can skip it to avoid confusion. 

>+The upside with DSW-style implementation is that it's very simple and efficient, and does not impose any head-of-line blocking (which follows from restoring a total order between dequeue and enqueue). The downside is it does not allow for a scenario where a particular flow is split across different modules, the application performs reordering >+(e.g., with the dispatcher library) *and* wants to maintain ordering between events pertaining to those "sub flows". I've never come across such a scenario, but it may well exist.

>+If we fail to make DLB2 and DSW compatible, we'll probably need another flag for DSW, because needlessly imposing a total order DSW does not make a lot of sense.

If the device has the capability to support independent enqueue, it should enable it on applicable ports using the RTE_EVENT_PORT_CFG_INDEPENDENT_ENQ  flag. Some devices like DSW will not do any reordering inside as they can support it without changing the order whereas devices like DLB which depend on order will reorder events inside their PMD.


>+You may want to add an example as well. And a note on the importance of maintaining impl_opaque between dequeue and enqueue.

We will consider this a separate patch later with an example based on the dispatcher library, which can work with DSW and DLB.  Is the port provided to rte_dispatcher_bind_port_to_lcore() already set up by the application? In that case configuring this feature on the port becomes part of the application. 

> + *  @see rte_event_port_setup()
> + */
> +
>   /** Event port configuration structure */
>   struct rte_event_port_conf {
>   	int32_t new_event_threshold;
  
Pathak, Pravin July 11, 2024, 3:20 a.m. UTC | #7
>+ Add a device-level
>+ RTE_EVENT_DEV_CAP_INDEPENDENT_ENQ
>+ as well.
>+ The documentation of that flag should probably house the detailed description of this feature.

So, this capability will be advertised by DSW and DLB devices with detailed documentation. 
What about DPAA and OPDL devices supporting burst mode? Do these support independent enqueue ?
  

Patch

diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index 08e5f9320b..f4220dd5dc 100644
--- a/lib/eventdev/rte_eventdev.h
+++ b/lib/eventdev/rte_eventdev.h
@@ -1073,6 +1073,14 @@  rte_event_queue_attr_set(uint8_t dev_id, uint8_t queue_id, uint32_t attr_id,
  *  @see rte_event_port_setup()
  */
 
+#define RTE_EVENT_PORT_CFG_RESTORE_DEQ_ORDER   (1ULL << 5)
+/**< Flag to enable feature enqueue reordering to dequeue.
+ * The feature restores dequeue order on enqueue if applications
+ * happen to change the order.
+ *
+ *  @see rte_event_port_setup()
+ */
+
 /** Event port configuration structure */
 struct rte_event_port_conf {
 	int32_t new_event_threshold;