On Tue, Jan 9, 2024 at 2:24 AM Morten Brørup <m...@smartsharesystems.com> wrote: > > > From: Konstantin Ananyev [mailto:konstantin.anan...@huawei.com] > > Sent: Friday, 5 January 2024 12.13 > > > > > From: Thomas Monjalon <tho...@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 > > <tho...@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 <jer...@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. >
