RE: [EXTERNAL] [PATCH v12 2/3] event/dlb2: add support for independent enqueue

[Jerin Jacob <jerinj@marvell.com>]

> -----Original Message-----
> From: Abdullah Sevincer <abdullah.sevincer@intel.com>
> Sent: Monday, September 9, 2024 9:35 PM
> To: dev@dpdk.org
> Cc: Jerin Jacob <jerinj@marvell.com>; bruce.richardson@intel.com;
> pravin.pathak@intel.com; mattias.ronnblom@ericsson.com;
> manish.aggarwal@intel.com; Abdullah Sevincer <abdullah.sevincer@intel.com>
> Subject: [EXTERNAL] [PATCH v12 2/3] event/dlb2: add support for independent
> enqueue
> 
> DLB devices need events to be enqueued in the same order they are dequeued.
> Applications are not suppose to change event order between dequeue and to
> enqueue. Since Eventdev standard does not add such restrictions independent
> enqueue support 
> DLB devices need events to be enqueued in the same order they are dequeued.
> Applications are not suppose to change event order between dequeue and to
> enqueue. Since Eventdev standard does not add such restrictions independent
> enqueue support is needed for DLB PMD so that it restores dequeue order on
> enqueue if applications happen to change it. It also adds missing releases in
> places where events are dropped by the application and it expects implicit
> release to handle it.
> 
> By default the feature will be off on all DLB ports and they will behave the same
> as older releases. To enable reordering feature, applications need to add the
> flag RTE_EVENT_PORT_CFG_INDEPENDENT_ENQ
> to port configuration if only the device advertises the capability
> RTE_EVENT_DEV_CAP_INDEPENDENT_ENQ.
> 
> Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
> Acked-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> ---
>  doc/guides/eventdevs/dlb2.rst          |  41 +++
>  doc/guides/rel_notes/release_24_11.rst |   6 +
>  drivers/event/dlb2/dlb2.c              | 492 ++++++++++++++++---------
>  drivers/event/dlb2/dlb2_avx512.c       |  27 +-
>  drivers/event/dlb2/dlb2_inline_fns.h   |   8 +
>  drivers/event/dlb2/dlb2_priv.h         |  25 +-
>  drivers/event/dlb2/rte_pmd_dlb2.h      |  10 +
>  7 files changed, 419 insertions(+), 190 deletions(-)
> 
> diff --git a/doc/guides/eventdevs/dlb2.rst b/doc/guides/eventdevs/dlb2.rst
> index 2532d92888..8b973cf81e 100644
> --- a/doc/guides/eventdevs/dlb2.rst
> +++ b/doc/guides/eventdevs/dlb2.rst
> @@ -456,6 +456,47 @@ Example command to enable QE Weight feature:
> 
>         --allow ea:00.0,enable_cq_weight=<y/Y>
> 
> +Independent Enqueue Capability
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +DLB2 hardware device expects all forwarded events to be enqueued in the
> +same order as they are dequeued. For dropped events, their releases
> +should come at the same location as the original event was expected.
> +Hardware has this restriction as it uses the order to retrieve
> +information about the original event that was sent to the CPU.  This
> +contains information like atomic flow ID to release the flow lock and
> +ordered events sequence number to restore the original order.
> +
> +Some applications, like those based on the DPDK dispatcher library,
> +want enqueue order independence. To support this, DLB2 PMD supports the
> +``RTE_EVENT_DEV_CAP_INDEPENDENT_ENQ`` capability.
> +
> +This capability applies to Eventdevs supporting burst mode. On ports
> +where the application is going to change enqueue order,
> +``RTE_EVENT_PORT_CFG_INDEPENDENT_ENQ`` support should be enabled.
> +
> +Example code to inform PMD that the application plans to use
> +independent enqueue order on a port:
> +
> +    .. code-block:: c
> +
> +       if (capability & RTE_EVENT_DEV_CAP_INDEPENDENT_ENQ)
> +         port_config = port_config |
> + RTE_EVENT_PORT_CFG_INDEPENDENT_ENQ;


A lot of thing in this is generic, update and move that to  doc/guides/prog_guide/eventdev/eventdev.rst
And add DLB2 specific items here.


> +
> +This code example enables enqueue event reordering inside DLB2 PMD
> +before the events are sent to the DLB2 hardware. If the application is
> +not going to change the enqueue order, this flag should not be enabled
> +to get better performance. DLB2 PMD saves ordering information inside
> +the impl_opaque field of the event, and this field should be preserved
> +for all FORWARD or RELEASE events. Following MACROs are provided to get
> +and set this field inside the event in case the same event is not used
> +for forwarding (e.g., a new RELEASE event is created when the original event is
> dropped instead of reusing the same event).
> +
> +    .. code-block:: c
> +
> +       #define RTE_EVENT_GET_IMPL_OPAQUE(ev)      (ev->impl_opaque)
> +       #define RTE_EVENT_SET_IMPL_OPAQUE(ev, val)  (ev->impl_opaque =
> + val)

Don’t add public RTE_EVENT_ symbol as example macro, use some other prefix like DLB2.


> +
>  Running Eventdev Applications with DLB Device
>  ---------------------------------------------
> 
> diff --git a/doc/guides/rel_notes/release_24_11.rst
> b/doc/guides/rel_notes/release_24_11.rst
> index 89c6a67e6a..61796f24e6 100644
> --- a/doc/guides/rel_notes/release_24_11.rst
> +++ b/doc/guides/rel_notes/release_24_11.rst
> @@ -60,6 +60,12 @@ New Features
>    * Added support for independent enqueue feature. Updated Event Device and
>      PMD feature list.
> 
> +* **Updated DLB2 Driver for independent enqueue feature**

Driver->driver

> +
> +  Added support for DLB independent enqueue feature. Applications
> + should use  ``RTE_EVENT_PORT_CFG_INDEPENDENT_ENQ`` to enable the
> + feature if the capability  ``RTE_EVENT_DEV_CAP_INDEPENDENT_ENQ`` exists.

Next patch add one more item to release notes. Instead, IMO, it better to update the generic section.
for this PMD support. I.e. add one item under 
"*Updated eventdev to support independent enqueue feature" section. As add DLB2 support.


> 
>  #include <rte_compat.h>
> 
> +/**
> + * Macros to get/set QID depth and QE weight from rte_event metadata.
> + * Currently 'rsvd' field is used for these. Lower 2 bits are used to
> +store
> + * QID depth while the upper 2 bits are used for QER weight.
> + */
> +#define RTE_PMD_DLB2_GET_QID_DEPTH(x) ((x)->rsvd & 0x3) #define
> +RTE_PMD_DLB2_SET_QID_DEPTH(x, v) ((x)->rsvd = ((x)->rsvd & ~0x3) | (v &
> +0x3)) #define RTE_PMD_DLB2_GET_QE_WEIGHT(x) (((x)->rsvd >> 2) & 0x3)
> +#define RTE_PMD_DLB2_SET_QE_WEIGHT(x, v) ((x)->rsvd = ((x)->rsvd & 0x3)
> +| ((v & 0x3) << 2))


All of the above MACROs are public symbols, Add Doxygen comment for each of them.

> +
>  /**
>   * @warning
>   * @b EXPERIMENTAL: this API may change, or be removed, without prior
> notice
> --
> 2.25.1