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

["Naga Harish K, S V" <s.v.naga.harish.k@intel.com>]

> -----Original Message-----
> From: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Sent: Friday, September 16, 2022 11:40 AM
> To: Jerin Jacob <jerinjacobk@gmail.com>; Naga Harish K, S V
> <s.v.naga.harish.k@intel.com>
> Cc: jerinj@marvell.com; dev@dpdk.org
> Subject: RE: [PATCH 1/3] eventdev/eth_tx: add queue start stop API
> 
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Wednesday, September 14, 2022 8:51 PM
> > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > Cc: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>;
> > jerinj@marvell.com; dev@dpdk.org
> > Subject: Re: [PATCH 1/3] eventdev/eth_tx: add queue start stop API
> >
> > 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
> 
> Yes @jerin, these APIs help avoid some unexpected behavior with
> application stopping ethdev Tx queues and Tx adapter being unaware of it.
> With these APIs, the application can call stop API to notify Tx adapter that
> corresponding ethdev Tx queue is stopped and any in-flight packets are
> freed by Tx adapter dataplane code. Tx adapter stop API is called before
> stopping the ethdev Tx queue. When ethdev Tx queue is enabled,
> application can notify Tx adapter to resume processing of the packets for that
> queue by calling the start API. The ethdev Tx queue is started before calling
> Tx adapter start API.
> 
> Perhaps, some of this usecase description can go into
> event_ethernet_tx_adapter.rst and doxygen comments for the APIs.
> @Harish, could you add please ?
> 

Included the above use case description in programmer guide and doxygen comments
In V4 version of the patch set.

> >
> > 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
> >
> > > +}