[dpdk-dev] [RFC] net/ixgbe: fix Tx descriptor status api

[Olivier Matz <olivier.matz@6wind.com>]

The Tx descriptor status api was not behaving as expected. This API is
used to inspect the content of the descriptors in the Tx ring to
determine the length of the Tx queue.

Since the software advances the tail pointer and the hardware advances
the head pointer, the Tx queue is located before txq->tx_tail in the
ring. Therefore, a call to rte_eth_tx_descriptor_status(..., offset=20)
should inspect the 20th descriptor before the tail, not after.

As before, we still need to take care about only checking descriptors
that have the RS bit.

Additionally, we can avoid an access to the ring if offset is greater or
equal to nb_tx_desc - nb_tx_free.

Fixes: a2919e13d95e ("net/ixgbe: implement descriptor status API")
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
---

Hi Wei, Hi Qi,

We also recently found some issues in Tx descriptor status API for ixgbe,
i40, e1000, igb. I'm preparing a clean patchset for all of them.

Here is already the patch for ixgbe, please let me know what you think.

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

Regards,
Olivier



 drivers/net/ixgbe/ixgbe_rxtx.c | 45 +++++++++++++++++++++++++++++++-----------
 drivers/net/ixgbe/ixgbe_rxtx.h |  1 +
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 3e13d26ae..384587cc6 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2606,10 +2606,15 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev,
 	    hw->mac.type == ixgbe_mac_X540_vf ||
 	    hw->mac.type == ixgbe_mac_X550_vf ||
 	    hw->mac.type == ixgbe_mac_X550EM_x_vf ||
-	    hw->mac.type == ixgbe_mac_X550EM_a_vf)
+	    hw->mac.type == ixgbe_mac_X550EM_a_vf) {
 		txq->tdt_reg_addr = IXGBE_PCI_REG_ADDR(hw, IXGBE_VFTDT(queue_idx));
-	else
+		txq->tdh_reg_addr = IXGBE_PCI_REG_ADDR(hw,
+				IXGBE_VFTDH(queue_idx));
+	} else {
 		txq->tdt_reg_addr = IXGBE_PCI_REG_ADDR(hw, IXGBE_TDT(txq->reg_idx));
+		txq->tdh_reg_addr = IXGBE_PCI_REG_ADDR(hw,
+				IXGBE_TDH(txq->reg_idx));
+	}
 
 	txq->tx_ring_phys_addr = tz->iova;
 	txq->tx_ring = (union ixgbe_adv_tx_desc *) tz->addr;
@@ -3140,22 +3145,38 @@ ixgbe_dev_tx_descriptor_status(void *tx_queue, uint16_t offset)
 {
 	struct ixgbe_tx_queue *txq = tx_queue;
 	volatile uint32_t *status;
-	uint32_t desc;
+	int32_t desc, dd;
 
 	if (unlikely(offset >= txq->nb_tx_desc))
 		return -EINVAL;
+	if (offset >= txq->nb_tx_desc - txq->nb_tx_free)
+		return RTE_ETH_TX_DESC_DONE;
+
+	desc = txq->tx_tail - offset - 1;
+	if (desc < 0)
+		desc += txq->nb_tx_desc;
 
-	desc = txq->tx_tail + offset;
-	/* go to next desc that has the RS bit */
-	desc = ((desc + txq->tx_rs_thresh - 1) / txq->tx_rs_thresh) *
-		txq->tx_rs_thresh;
-	if (desc >= txq->nb_tx_desc) {
-		desc -= txq->nb_tx_desc;
-		if (desc >= txq->nb_tx_desc)
-			desc -= txq->nb_tx_desc;
+	/* offset is too small, no other way than reading PCI reg */
+	if (unlikely(offset < txq->tx_rs_thresh)) {
+		int16_t tx_head, queue_size;
+		tx_head = ixgbe_read_addr(txq->tdh_reg_addr);
+		queue_size = txq->tx_tail - tx_head;
+		if (queue_size < 0)
+			queue_size += txq->nb_tx_desc;
+		return queue_size > offset ? RTE_ETH_TX_DESC_FULL :
+			RTE_ETH_TX_DESC_DONE;
 	}
 
-	status = &txq->tx_ring[desc].wb.status;
+	/* index of the dd bit to look at */
+	dd = (desc / txq->tx_rs_thresh + 1) * txq->tx_rs_thresh - 1;
+
+	/* In full featured mode, RS bit is only set in the last descriptor */
+	/* of a multisegments packet */
+	if (!((txq->offloads == 0) &&
+	      (txq->tx_rs_thresh >= RTE_PMD_IXGBE_TX_MAX_BURST)))
+		dd = txq->sw_ring[dd].last_id;
+
+	status = &txq->tx_ring[dd].wb.status;
 	if (*status & rte_cpu_to_le_32(IXGBE_ADVTXD_STAT_DD))
 		return RTE_ETH_TX_DESC_DONE;
 
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 39378f754..384f6324d 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -201,6 +201,7 @@ struct ixgbe_tx_queue {
 		struct ixgbe_tx_entry_v *sw_ring_v; /**< address of SW ring for vector PMD */
 	};
 	volatile uint32_t   *tdt_reg_addr; /**< Address of TDT register. */
+	volatile uint32_t   *tdh_reg_addr; /**< Address of TDH register. */
 	uint16_t            nb_tx_desc;    /**< number of TX descriptors. */
 	uint16_t            tx_tail;       /**< current value of TDT reg. */
 	/**< Start freeing TX buffers if there are less free descriptors than
-- 
2.11.0