Hi Wei:
> -----Original Message-----
> From: Zhao1, Wei
> Sent: Friday, June 22, 2018 4:39 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo...@intel.com>; Zhang, Qi Z
> <qi.z.zh...@intel.com>; sta...@dpdk.org; Zhao1, Wei <wei.zh...@intel.com>
> Subject: [PATCH] net/ixgbe: fix Tx check descriptor status APIs error
>
> This is a issue involve RS bit set rule in ixgbe.
> Let us take function ixgbe_xmit_pkts_vec () as an example, in this function RS
> bit will be set for descriptor with index
> txq->tx_next_rs, and also descriptor free function
> ixgbe_tx_free_bufs() also check RS bit for descriptor with index
> txq->tx_next_rs, This is perfect ok. Let us take an example,
> if app set tx_rs_thresh = 32 and nb_desc = 512, then ixgbe PMD code will init
> txq->tx_next_rs = 31 in function ixgbe_reset_tx_queue when tx queue setup.
> And also txq->tx_next_rs will be update as 63, 95 and so on. But, in the
> function ixgbe_dev_tx_descriptor_status(), the RS bit to check is " desc =
> ((desc
> + txq->tx_rs_thresh - 1) /
> txq->tx_rs_thresh) * txq-tx_rs_thresh", which is 32 ,64, 96 and so on.
> So, they are all wrong! In tx function of ixgbe_xmit_pkts_simple, the RS bit
> rule
> is also the same, it also set index 31 ,64, 95.
> we need to correct it.
One question:
does only the descriptor with RS bit will have DD status, or NIC will always
update all descriptor's DD status but this happens when the next descriptor
with RS bit has been sent?
If it is the first case, I think you fix still have problem, because multi-seg
mbuf or tso offload will break the 31, 63, 95 pattern
See:
nb_used =
(uint16_t)(tx_pkt->nb_segs + new_ctx);
if (txp != NULL &&
nb_used + txq->nb_tx_used >= txq->tx_rs_thresh)
/* set RS on the previous packet in the burst */
txp->read.cmd_type_len |=
rte_cpu_to_le_32(IXGBE_TXD_CMD_RS);
so the possible solution is store each RS position in a list at tx, and find
the next RS from the list in ixgbe_dev_tx_descriptor_status
If it is the second case, it will be simple we don't need to check forward with
tx_rs_thresh, just check the exact position ( I hope it is this case :))
Regards
Qi
>
> Fixes: a2919e13d95e ("net/ixgbe: implement descriptor status API")
>
> Signed-off-by: Wei Zhao <wei.zh...@intel.com>
> ---
> drivers/net/ixgbe/ixgbe_rxtx.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 3e13d26..f185219 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -3146,15 +3146,15 @@ ixgbe_dev_tx_descriptor_status(void *tx_queue,
> uint16_t offset)
> return -EINVAL;
>
> desc = txq->tx_tail + offset;
> + if (desc >= txq->nb_tx_desc)
> + desc -= txq->nb_tx_desc;
> /* 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 = (desc / txq->tx_rs_thresh + 1) *
> + txq->tx_rs_thresh - 1;
> + if (desc >= txq->nb_tx_desc)
> desc -= txq->nb_tx_desc;
> - if (desc >= txq->nb_tx_desc)
> - desc -= txq->nb_tx_desc;
> - }
>
> + desc = txq->sw_ring[desc].last_id;
> status = &txq->tx_ring[desc].wb.status;
> if (*status & rte_cpu_to_le_32(IXGBE_ADVTXD_STAT_DD))
> return RTE_ETH_TX_DESC_DONE;
> --
> 2.7.5
