Re: [dpdk-dev] [PATCH v2] eventdev: flag to identify same destined packets enqueue

[Jerin Jacob <jerinjacobk@gmail.com>]

On Fri, Oct 4, 2019 at 12:41 PM Nipun Gupta <nipun.gupta@nxp.com> wrote:
>
> This patch introduces a `flag` in the Eth TX adapter enqueue API.
> Some drivers may support burst functionality only with the packets
> having same destination device and queue.
>
> The flag `RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_SAME_DEST` can be used
> to indicate this so the underlying driver, for drivers to utilize
> burst functionality appropriately.
>
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> ---
>
> Changes in v2:
>  - have separate internal API in tx adapter for sending burst packets to
>    same eth dev, queue pair on the basis of the passed flag
>  - fix compilation of examples/eventdev_pipeline/
>
>  app/test-eventdev/test_pipeline_common.h      |  6 +++---
>  .../prog_guide/event_ethernet_tx_adapter.rst  |  3 ++-
>  .../eventdev_pipeline/pipeline_worker_tx.c    |  2 +-
>  .../rte_event_eth_tx_adapter.h                | 19 +++++++++++++++++--
>  lib/librte_eventdev/rte_eventdev.c            | 12 ++++++++++++
>  lib/librte_eventdev/rte_eventdev.h            |  9 +++++++++

It is an ABI change so please update

a) lib/librte_eventdev/Makefile
b) lib/librte_eventdev/meson.build
c) doc/guides/rel_notes/release_19_11.rst

Please see c2189c907dd191b909ce1f18487d46410782e370 as reference.

If anyone has any objection for treating this ABI change as an
exception(ABI change without depreciation notice in advance)
then please shout.



>  6 files changed, 44 insertions(+), 7 deletions(-)
>

> diff --git a/lib/librte_eventdev/rte_event_eth_tx_adapter.h b/lib/librte_eventdev/rte_event_eth_tx_adapter.h
> index c848261c4..f552b6023 100644
> --- a/lib/librte_eventdev/rte_event_eth_tx_adapter.h
> +++ b/lib/librte_eventdev/rte_event_eth_tx_adapter.h
> @@ -300,6 +300,11 @@ rte_event_eth_tx_adapter_txq_get(struct rte_mbuf *pkt)
>  int
>  rte_event_eth_tx_adapter_event_port_get(uint8_t id, uint8_t *event_port_id);
>
> +#define RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_SAME_DEST     0x1
> +/**< This flag is used when all the packets enqueued in the tx adapter are
> + * destined for the same Ethernet device, queue pair.

I would say "Ethernet port and Tx queue"

> + */
> +
>  /**
>   * Enqueue a burst of events objects or an event object supplied in *rte_event*
>   * structure on an  event device designated by its *dev_id* through the event
> @@ -324,6 +329,10 @@ rte_event_eth_tx_adapter_event_port_get(uint8_t id, uint8_t *event_port_id);
>   *  The number of event objects to enqueue, typically number of
>   *  rte_event_port_attr_get(...RTE_EVENT_PORT_ATTR_ENQ_DEPTH...)
>   *  available for this port.
> + * @param flags
> + *  See RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_ flags.

Change to @see

> + *  #RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_SAME_DEST signifies that all the packets
> + *  which are enqueued are destined for the same Ethernet device, queue pair.

Same as above. Please check generated doxgen API output.

>   *
>   * @return
>   *   The number of event objects actually enqueued on the event device. The
> @@ -343,7 +352,8 @@ static inline uint16_t
>  rte_event_eth_tx_adapter_enqueue(uint8_t dev_id,
>                                 uint8_t port_id,
>                                 struct rte_event ev[],
> -                               uint16_t nb_events)
> +                               uint16_t nb_events,
> +                               const uint8_t flags)
>  {
>         const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
>
> @@ -359,7 +369,12 @@ rte_event_eth_tx_adapter_enqueue(uint8_t dev_id,
>                 return 0;
>         }
>  #endif
> -       return dev->txa_enqueue(dev->data->ports[port_id], ev, nb_events);
> +       if (flags)
> +               return dev->txa_enqueue_same_dest(dev->data->ports[port_id],
> +                                                 ev, nb_events, flags);
> +       else
> +               return dev->txa_enqueue(dev->data->ports[port_id], ev,
> +                                       nb_events);
>  }
>
>  /**
> diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
> index f44c869cb..7c8ffdfef 100644
> --- a/lib/librte_eventdev/rte_eventdev.c
> +++ b/lib/librte_eventdev/rte_eventdev.c
> @@ -1330,6 +1330,16 @@ rte_event_tx_adapter_enqueue(__rte_unused void *port,
>         return 0;
>  }
>
> +static uint16_t
> +rte_event_tx_adapter_enqueue_same_dest(__rte_unused void *port,
> +                       __rte_unused struct rte_event ev[],
> +                       __rte_unused uint16_t nb_events,
> +                       __rte_unused const uint8_t flags)
> +{
> +       rte_errno = ENOTSUP;
> +       return 0;
> +}
> +
>  struct rte_eventdev *
>  rte_event_pmd_allocate(const char *name, int socket_id)
>  {
> @@ -1351,6 +1361,8 @@ rte_event_pmd_allocate(const char *name, int socket_id)
>         eventdev = &rte_eventdevs[dev_id];
>
>         eventdev->txa_enqueue = rte_event_tx_adapter_enqueue;
> +       eventdev->txa_enqueue_same_dest =
> +                       rte_event_tx_adapter_enqueue_same_dest;
>
>         if (eventdev->data == NULL) {
>                 struct rte_eventdev_data *eventdev_data = NULL;
> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> index 5044a13d0..ea61f52f6 100644
> --- a/lib/librte_eventdev/rte_eventdev.h
> +++ b/lib/librte_eventdev/rte_eventdev.h
> @@ -1230,6 +1230,13 @@ 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 */
>
> +typedef uint16_t (*event_tx_adapter_enqueue_same_dest)(void *port,
> +               struct rte_event ev[], uint16_t nb_events,
> +               const uint8_t flags);
> +/**< @internal Enqueue burst of events on port of a device supporting flag to
> + * indicate burst having same dest eth port and queue.

s/queue/Tx queue

> + */
> +
>  #define RTE_EVENTDEV_NAME_MAX_LEN      (64)
>  /**< @internal Max length of name of event PMD */
>
> @@ -1292,6 +1299,8 @@ struct rte_eventdev {
>         /**< Pointer to PMD dequeue function. */
>         event_dequeue_burst_t dequeue_burst;
>         /**< Pointer to PMD dequeue burst function. */
> +       event_tx_adapter_enqueue_same_dest txa_enqueue_same_dest;
> +       /**< Pointer to PMD eth Tx adapter enqueue burst function. */

Is it really burst function? enqueue supports already support burst.
Please change the description.

>         event_tx_adapter_enqueue txa_enqueue;
>         /**< Pointer to PMD eth Tx adapter enqueue function. */
>         struct rte_eventdev_data *data;
> --
> 2.17.1
>