Re: [dpdk-dev] [RFC] ethdev: support Tx queue free descriptor query

[Jerin Jacob <jerinjacobk@gmail.com>]

On Tue, Jan 9, 2024 at 2:24 AM Morten Brørup <mb@smartsharesystems.com> wrote:
>
> > From: Konstantin Ananyev [mailto:konstantin.ananyev@huawei.com]
> > Sent: Friday, 5 January 2024 12.13
> >
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > Sent: Friday, January 5, 2024 10:04 AM
> > >
> > > 05/01/2024 10:57, Jerin Jacob:
> > > > On Thu, Jan 4, 2024 at 11:59 PM Thomas Monjalon
> > <thomas@monjalon.net> wrote:
> > > > >
> > > > > 04/01/2024 15:21, Konstantin Ananyev:
> > > > > >
> > > > > > > > > Introduce a new API to retrieve the number of available
> > free descriptors
> > > > > > > > > in a Tx queue. Applications can leverage this API in the
> > fast path to
> > > > > > > > > inspect the Tx queue occupancy and take appropriate
> > actions based on the
> > > > > > > > > available free descriptors.
> > > > > > > > >
> > > > > > > > > A notable use case could be implementing Random Early
> > Discard (RED)
> > > > > > > > > in software based on Tx queue occupancy.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > > > > >
> > > > > > > > I think having an API to get the number of free descriptors
> > per queue is a good idea. Why have it only for TX queues and not
> > > for RX
> > > > > > > queues as well?
> > > > > > >
> > > > > > > I see no harm in adding for Rx as well. I think, it is better
> > to have
> > > > > > > separate API for each instead of adding argument as it is
> > fast path
> > > > > > > API.
> > > > > > > If so, we could add a new API when there is any PMD
> > implementation or
> > > > > > > need for this.
> > > > > >
> > > > > > I think for RX we already have similar one:
> > > > > > /** @internal Get number of used descriptors on a receive
> > queue. */
> > > > > > typedef uint32_t (*eth_rx_queue_count_t)(void *rxq);
> > > > >
> > > > > rte_eth_rx_queue_count() gives the number of Rx used descriptors
> > > > > rte_eth_rx_descriptor_status() gives the status of one Rx
> > descriptor
> > > > > rte_eth_tx_descriptor_status() gives the status of one Tx
> > descriptor
> > > > >
> > > > > This patch is adding a function to get Tx available descriptors,
> > > > > rte_eth_tx_queue_free_desc_get().
> > > > > I can see a symmetry with rte_eth_rx_queue_count().
> > > > > For consistency I would rename it to
> > rte_eth_tx_queue_free_count().
>
> For consistency with rte_eth_rx_queue_count() regarding both naming and behavior of the API, I would prefer:
>
> rte_eth_tx_queue_count(), returning the number of used descriptors.

Ack. I will change to "used" version.

>
> > > > >
> > > > > Should we add rte_eth_tx_queue_count() and
> > rte_eth_rx_queue_free_count()?
> > > >
> > > > IMO, rte_eth_rx_queue_free_count() is enough as
> > > > used count =  total desc number(configured via nb_tx_desc with
> > > > rte_eth_tx_queue_setup())  - free count
> > >
> > > I'm fine with that.
> > >
> >
> > Yep, agree.
> > If we ever need  rte_eth_rx_queue_free_count() and
> > rte_eth_tx_queue_used_count(),
> > it could be done via slow-path as Jerin outlined above, no need to
> > waste entries in fp_ops
> > for that.
>
> Yes, rte_eth_rx/tx_queue_avail_count() could be added in the ethdev library, without driver callbacks, but simply getting data from configured descriptor ring sizes and rte_eth_rx/tx_queue_count().
>
> PS: For naming conventions, I sought inspiration from the mempool library. Also, I don't see any need to use "descs" as part of the names of the proposed functions.
>
> The driver callback can be either "free count" or "used count", whichever is easier for the drivers to implement, or (preferably) whichever is likelier to be faster to execute in the most common case. I would assume that the TX descriptor "used count" is relatively low most of the time.

I will change driver callback to "used count" to have better synergy
with public ethdev API.


>