Re: [PATCH 1/3] eventdev/eth_tx: add queue start stop API

[Jerin Jacob <jerinjacobk@gmail.com>]

On Fri, Sep 9, 2022 at 9:12 AM Naga Harish K S V
<s.v.naga.harish.k@intel.com> wrote:
>
> This patch adds support to start or stop a particular queue
> that is associated with the adapter.
>
> Start function enables the Tx Adapter to start enqueueing
> packets to the Tx queue.
>
> Stop function stops the Tx Adapter from transmitting any
> mbufs to the Tx queue. The Tx Adapter also frees any mbufs
> that it may have buffered for this queue. All inflight packets
> destined to the queue are freed until the queue is started again.
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>

In general, new APIs look good to me if it is helping the SW driver.
I will wait for comment from @Jayatheerthan, Jay

Some comments below

> ---
>  lib/eventdev/eventdev_pmd.h             |  41 +++++++++
>  lib/eventdev/rte_event_eth_tx_adapter.c | 114 +++++++++++++++++++++++-
>  lib/eventdev/rte_event_eth_tx_adapter.h |  39 ++++++++
>  lib/eventdev/version.map                |   2 +

Please squash 3/3 to 1/3.

Please update doc/guides/prog_guide/event_ethernet_tx_adapter.rst for
new APIs doc for
What is this API and when to use this API etc.


>  4 files changed, 192 insertions(+), 4 deletions(-)
>
> diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h
> index f514a37575..a27c0883c6 100644
> --- a/lib/eventdev/eventdev_pmd.h
> +++ b/lib/eventdev/eventdev_pmd.h
> @@ -1294,6 +1294,43 @@ typedef int (*eventdev_eth_tx_adapter_stats_reset_t)(uint8_t id,
>  typedef int (*eventdev_eth_tx_adapter_instance_get_t)
>         (uint16_t eth_dev_id, uint16_t tx_queue_id, uint8_t *txa_inst_id);
>
> +/**
> + * Start a Tx queue that is assigned to TX adapter instance

Tx adapter

> + *
> + * @param id
> + *  Adapter identifier
> + *
> + * @param eth_dev_id
> + *  Port identifier of Ethernet device
> + *
> + * @param tx_queue_id
> + *  Ethernet device TX queue index

Tx

> + *
> + * @return
> + *  -  0: Success
> + *  - <0: Error code on failure
> + */
> +typedef int (*eventdev_eth_tx_adapter_queue_start)
> +       (uint8_t id, uint16_t eth_dev_id, uint16_t tx_queue_id);
> +
> +/**
> + * Stop a Tx queue that is assigned to TX adapter instance

Tx

> +int
> +rte_event_eth_tx_adapter_queue_start(uint16_t eth_dev_id, uint16_t tx_queue_id)
> +{
> +       return txa_queue_state_set(eth_dev_id, tx_queue_id, true);

Make it is NOP with return 0 if eventdev_eth_tx_adapter_queue_start is NULL

> +}
> +
> +int
> +rte_event_eth_tx_adapter_queue_stop(uint16_t eth_dev_id, uint16_t tx_queue_id)
> +{
> +       return txa_queue_state_set(eth_dev_id, tx_queue_id, false);

Make it is NOP with return 0 if eventdev_eth_tx_adapter_queue_start is NULL

> +}