diff mbox series

[19.11,V3,02/12] ethdev: fix description of tx descriptor status

Message ID c269a506083c54182af874bbb22a8f31fe23b61e.1565252336.git.thierry.herbelot@6wind.com (mailing list archive)
State Superseded, archived
Headers
Series Miscellaneous fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Thierry Herbelot Aug. 8, 2019, 8:22 a.m. UTC 
  From: Olivier Matz <olivier.matz@6wind.com>

The API comment of rte_eth_tx_descriptor_status() was incorrect. The
reference descriptor (when offset = 0) is not where the next packet
will be sent, but where the latest packet has been enqueued.

Fixes: 52f5cdd2e897 ("ethdev: add descriptor status API")
Cc: stable@dpdk.org

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_ethdev/rte_ethdev.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andrew Rybchenko Aug. 8, 2019, 10:37 a.m. UTC | #1 
On 8/8/19 11:22 AM, Thierry Herbelot wrote:
> From: Olivier Matz <olivier.matz@6wind.com>
>
> The API comment of rte_eth_tx_descriptor_status() was incorrect. The
> reference descriptor (when offset = 0) is not where the next packet
> will be sent, but where the latest packet has been enqueued.
>
> Fixes: 52f5cdd2e897 ("ethdev: add descriptor status API")
> Cc: stable@dpdk.org
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>   lib/librte_ethdev/rte_ethdev.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index dc6596bc93b4..b423e71050e9 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -4245,8 +4245,8 @@ rte_eth_rx_descriptor_status(uint16_t port_id, uint16_t queue_id,
>    * @param queue_id
>    *  A valid Tx queue identifier on this port.
>    * @param offset
> - *  The offset of the descriptor starting from tail (0 is the place where
> - *  the next packet will be send).
> + *  The offset of the descriptor starting from tail (0 is the last written
> + *  descriptor).
>    *
>    * @return
>    *  - (RTE_ETH_TX_DESC_FULL) Descriptor is being processed by the hw, i.e.

The patch lacks explanation why it was wrong before and why it is
correct after the patch.

I think there is a space here for description improvements since it is
still confusing. I'd suggest to remove 'tail' term from the description or
write down full description to explain what/where is head and tail.

Rx case:

          V-->offset
=========***********----------------00000000000==
  UNAVAIL     DONE        AVAIL        UNUSED

i.e. V points to the next packet to be received by the driver, i.e. the 
first *
(the descriptor is done by HW and packet is ready to be processed).
DONE(*) are descriptors filled in by HW, but not processed by the driver 
yet.
AVAIL(-) are available for HW to fill in with received packets.
UNUSED(0) (right now it is UNAVAIL as well) are descriptors which can be
done AVAIL on Rx queue refill
UNAVAIL(=) are reserved Rx ring space which cannot be used

The key question for Rx is the length of DONE. Is CPU fast enough to
handle incoming traffic?

Tx case (where is tail V and why):

=========************--------------00000000000==
  UNAVAIL     DONE          FULL       UNUSED

DONE(*) are descriptors processed by HW and should be processed by the 
driver to
free associated mbuf (rte_eth_tx_done_cleanup())
FULL(-) are available for HW to send these packets out, but not 
processed yet.
UNUSED(0) is available space in Tx ring to make it FULL
UNAVAIL(=) is reserved Tx ring space which cannot be used

I'd say that it should be similar to Rx here, but it looks different in 
accordance with
current description. Above patch fixes it from the first '-' (or the 
first '0'?) to be
the last '-'.

What is the main question for Tx status descriptor user?
diff mbox series

Patch

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index dc6596bc93b4..b423e71050e9 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -4245,8 +4245,8 @@  rte_eth_rx_descriptor_status(uint16_t port_id, uint16_t queue_id,
  * @param queue_id
  *  A valid Tx queue identifier on this port.
  * @param offset
- *  The offset of the descriptor starting from tail (0 is the place where
- *  the next packet will be send).
+ *  The offset of the descriptor starting from tail (0 is the last written
+ *  descriptor).
  *
  * @return
  *  - (RTE_ETH_TX_DESC_FULL) Descriptor is being processed by the hw, i.e.