Re: [dpdk-dev] [PATCH 2/7] eventdev: introduce event vector Rx capability

[Jerin Jacob <jerinjacobk@gmail.com>]

On Sun, Feb 21, 2021 at 3:40 AM <pbhagavatula@marvell.com> wrote:
>
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> Introduce event ethernet Rx adapter event vector capability.
>
> If an event eth Rx adapter has the capability of
> RTE_EVENT_ETH_RX_ADAPTER_CAP_EVENT_VECTOR then a given Rx queue
> can be configured to enable event vectorization by passing the
> flag RTE_EVENT_ETH_RX_ADAPTER_QUEUE_EVENT_VECTOR to
> rte_event_eth_rx_adapter_queue_conf::rx_queue_flags while configuring
> Rx adapter through rte_event_eth_rx_adapter_queue_add.
>
> The max vector size, vector timeout define the vector size and
> mempool used for allocating vector event are configured through
> rte_event_eth_rx_adapter_queue_add. The element size of the element
> in the vector pool should be equal to
>     sizeof(struct rte_event_vector) + (vector_sz * sizeof(uintptr_t))
>
> Application can use `rte_event_get_event_vector_memory_footprint`

This comment is outdated. There is no
rte_event_get_event_vector_memory_footprint() in series.

> to get the element size before creating the vector mempool
> rte_event_eth_rx_adapter_queue_conf::vector_mp.
>
> The Rx adapter would be responsible for vectorizing the mbufs
> based on the flow, the vector limits configured by the application
> and add the vector event of mbufs to the event queue set via
> rte_event_eth_rx_adapter_queue_conf::ev::queue_id.
> It should also mark rte_event_vector::union_valid and fill
> rte_event_vector::port, rte_event_vector::queue.
>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>  .../prog_guide/event_ethernet_rx_adapter.rst  | 38 +++++++++++
>  .../rte_event_eth_rx_adapter.h                | 66 +++++++++++++++++++
>  lib/librte_eventdev/rte_eventdev.h            | 30 ++++++++-
>  lib/librte_eventdev/version.map               |  1 +
>  4 files changed, 133 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> index cb44ce0e4..735ea2439 100644
> --- a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> +++ b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> @@ -186,3 +186,41 @@ the event buffer fill level is low. The
>  ``rte_event_eth_rx_adapter_cb_register()`` function allow the application
>  to register a callback that selects which packets to enqueue to the event
>  device.
> +
> +Rx event vectorization
> +~~~~~~~~~~~~~~~~~~~~~~
> +
> +The event devices, ethernet device pairs which support the capability
> +``RTE_EVENT_ETH_RX_ADAPTER_CAP_EVENT_VECTOR`` can aggregate packets based on
> +flow characteristics and generate a ``rte_event`` containing ``rte_event_vector``
> +whose event type is either ``RTE_EVENT_TYPE_ETHDEV_VECTOR`` or
> +``RTE_EVENT_TYPE_ETH_RX_ADAPTER_VECTOR``.
> +The aggregation size and timeout are configurable at a queue level and the
> +maximum, minimum vector sizes and timeouts vary based on the device capability
> +and can be queried using ``rte_event_eth_rx_adapter_vector_limits_get``.
> +The Rx adapter additionally might include useful data such as ethernet device
> +port and queue identifier in the ``rte_event_vector::port`` and
> +``rte_event_vector::queue`` and mark ``rte_event_vector::attr_valid`` as true.
> +
> +A loop processing ``rte_event_vector`` containing mbufs is shown below.
> +
> +.. code-block:: c
> +
> +        event = rte_event_dequeue_burst(event_dev, event_port, &event,
> +                                        1, 0);
> +        if (!event)
> +                continue;
> +
> +        switch (ev.event_type) {
> +        case RTE_EVENT_TYPE_ETH_RX_ADAPTER_VECTOR:
> +        case RTE_EVENT_TYPE_ETHDEV_VECTOR:
> +                struct rte_mbufs **mbufs;
> +
> +                mbufs = (struct rte_mbufs **)ev[i].vec->mbufs;
> +                for (i = 0; i < ev.vec->nb_elem; i++) {
> +                        // Process each mbuf.

Change to C style /* */ comments.

> +                }
> +        break;
> +        case ...
> +        ...
> +        }
> diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.h b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> index 21bb1e54c..4bdb38f08 100644
> --- a/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> +++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.h
> @@ -92,6 +92,10 @@ extern "C" {
>  /**< This flag indicates the flow identifier is valid
>   * @see rte_event_eth_rx_adapter_queue_conf::rx_queue_flags
>   */
> +#define RTE_EVENT_ETH_RX_ADAPTER_QUEUE_EVENT_VECTOR    0x2
> +/**< This flag indicates that mbufs arriving on the queue need to be vectorized
> + * @see rte_event_eth_rx_adapter_queue_conf::rx_queue_flags
> + */

>  /**
> @@ -199,6 +230,20 @@ struct rte_event_eth_rx_adapter_stats {
>         /**< Received packet count for interrupt mode Rx queues */
>  };
>
> +/**
> + * A structure used to retrieve eth rx adapter vector limits.
> + */
> +struct rte_event_eth_rx_adapter_vector_limits {
> +       uint16_t min_sz;
> +       /**< Minimum vector limit configurable. */
> +       uint16_t max_sz;

I think, it will common to have constraints such as size needs to be log2.
Please add a  bool type to express that limit/contrain.