From: "Zhang, Qi Z" <qi.z.zhang@intel.com>
To: "Zhao1, Wei" <wei.zhao1@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [PATCH] net/ixgbe: fix Tx check descriptor status APIs error
Date: Fri, 22 Jun 2018 13:47:22 +0000 [thread overview]
Message-ID: <039ED4275CED7440929022BC67E706115323BAD2@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <1529656727-40207-1-git-send-email-wei.zhao1@intel.com>
Hi Wei:
> -----Original Message-----
> From: Zhao1, Wei
> Sent: Friday, June 22, 2018 4:39 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; stable@dpdk.org; Zhao1, Wei <wei.zhao1@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.zhao1@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
next prev parent reply other threads:[~2018-06-22 13:47 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-22 8:38 Wei Zhao
2018-06-22 13:47 ` Zhang, Qi Z [this message]
2018-06-25 1:57 ` Zhao1, Wei
2018-06-25 2:48 ` Zhang, Qi Z
2018-06-25 5:58 ` Zhao1, Wei
2018-06-25 6:49 ` Zhao1, Wei
2018-06-25 7:47 ` Zhang, Qi Z
2018-06-25 7:52 ` Zhao1, Wei
2018-06-25 7:55 ` Zhang, Qi Z
2018-06-25 8:41 ` Zhao1, Wei
2018-06-26 1:24 ` [dpdk-stable] [PATCH v2] " Wei Zhao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=039ED4275CED7440929022BC67E706115323BAD2@SHSMSX103.ccr.corp.intel.com \
--to=qi.z.zhang@intel.com \
--cc=dev@dpdk.org \
--cc=stable@dpdk.org \
--cc=wei.zhao1@intel.com \
--cc=wenzhuo.lu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).