From: "Zhang, Qi Z" <qi.z.zhang@intel.com>
To: Olivier Matz <olivier.matz@6wind.com>,
"Zhao1, Wei" <wei.zhao1@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "Lu, Wenzhuo" <wenzhuo.lu@intel.com>
Subject: Re: [dpdk-dev] [RFC] net/ixgbe: fix Tx descriptor status api
Date: Wed, 27 Jun 2018 13:38:36 +0000 [thread overview]
Message-ID: <039ED4275CED7440929022BC67E706115323F421@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <20180626084608.c4kfsc5ccnfwypgw@platinum>
Hi Oliver
> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Tuesday, June 26, 2018 4:46 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Subject: Re: [RFC] net/ixgbe: fix Tx descriptor status api
>
> Hi Wei,
>
> On Tue, Jun 26, 2018 at 01:38:22AM +0000, Zhao1, Wei wrote:
> > Hi, Olivier Matz
> >
> > Will you commit fix patch for i40e and ixgbe and em?
>
> If you think the patch are relevant, yes :)
>
> Here is a pre-version (last 5 patches):
> http://git.droids-corp.org/?p=dpdk.git;a=shortlog;h=refs/heads/tx-desc
>
> It still need to fix checkpatch issues, few more tests, and rebase on next-net.
>
> > And the code " dd = (desc / txq->tx_rs_thresh + 1) * txq->tx_rs_thresh - 1;"
> > Is only proper for tx function ixgbe_xmit_pkts_simple and
> ixgbe_xmit_pkts_vec ().
> > But not proper for ixgbe_xmit_pkts (), the RS bit set rule is different from all
> these two.
>
> Can you please give more detail please?
I think the problem in ixgbe_xmit_pkts is, we cannot guarantee tx_rs_thresh *n -1 will always get RS bit.
Because the next tx_rs_thresh cycle counted start from the last segment of the packet that cross the boundary, it could be any value.
So probably we should return -ENOSUP if pkt_tx_burst = ixgbe_xmit_pkts, or we need to change the method that write RS in ixgbe_xmit_pkts.
Btw, yes, we should look before the tail but not after, you fix is correct.
Thanks
Qi
>
> Note this code, maybe you are talking about this?
>
> + /* 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;
>
> Maybe there is something better to test?
>
> Just to ensure we are on the same line, here are some more infos.
>
> ===
>
> - sw advances the tail pointer
> - hw advances the head pointer
> - the software populates the ring with full buffers to be sent by
> the hw
> - head points to the in-progress descriptor.
> - sw writes new descriptors at tail
> - head == tail means that the transmit queue is empty
> - when the hw has processed a descriptor, it sets the DD bit if
> the descriptor has the RS (report status) bit.
> - the driver never reads the head (needs a pci transaction), instead it monitors
> the DD bit of a descriptor that has the RS bit
>
> txq->tx_tail: sw value for tail register
> txq->tx_free_thresh: free buffers if count(free descs) < this value
> txq->tx_rs_thresh: RS bit is set every rs_thresh descriptor
> txq->tx_next_dd: next desc to scan for DD bit
> txq->tx_next_rs: next desc to set RS bit
> txq->last_desc_cleaned: last descriptor that have been cleaned
> txq->nb_tx_free: number of free descriptors
>
> Example:
>
> |----------------------------------------------------------------|
> | D R R R
> |
> | ............xxxxxxxxxxxxxxxxxxxxxxxxx |
> | <descs sent><- descs not sent yet -> |
> | ............xxxxxxxxxxxxxxxxxxxxxxxxx |
> |----------------------------------------------------------------|
> ^last_desc_cleaned=8 ^next_rs=47
> ^next_dd=15 ^sw_tail=45
> ^hw_head=20
>
> <---- nb_used --------->
>
> The hardware is currently processing the descriptor 20 'R' means the
> descriptor has the RS bit 'D' means the descriptor has the DD + RS bits 'x' are
> packets in txq (not sent) '.' are packet already sent but not freed by sw
>
> In this example, we have rs_thres=8. On next call to ixgbe_tx_free_bufs(),
> some buffers will be freed.
>
> ===
>
> Let's call ixgbe_dev_tx_descriptor_status(10):
>
>
> - original version:
>
> desc = 45 + 10 = 55
> desc = ((55 + 8 - 1) / 8) * 8 = (62 / 8) * 8 = 56
>
> wrong because it goes in the wrong direction, and because
> 56 does not have the RS bit
>
> - after your patch:
>
> desc = 45 + 10 = 55
> desc = (((55 / 8) + 1) * 8) - 1 = (7 * 8) - 1 = 55
>
> wrong because it goes in the wrong direction
>
> - after my patch
>
> desc = 45 - 10 - 1 = 34
> desc = (((34 / 8) + 1) * 8) - 1 = (5 * 8) - 1 = 39
>
> looks correct
>
>
>
> Regards,
> Olivier
next prev parent reply other threads:[~2018-06-27 13:38 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-22 8:38 [dpdk-dev] [PATCH] net/ixgbe: fix Tx check descriptor status APIs error Wei Zhao
2018-06-22 13:47 ` Zhang, Qi Z
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-25 14:20 ` [dpdk-dev] [RFC] net/ixgbe: fix Tx descriptor status api Olivier Matz
2018-06-25 14:30 ` Olivier Matz
2018-06-26 1:38 ` Zhao1, Wei
2018-06-26 8:46 ` Olivier Matz
2018-06-27 2:07 ` Zhao1, Wei
2018-06-27 13:38 ` Zhang, Qi Z [this message]
2018-06-26 1:24 ` [dpdk-dev] [PATCH v2] net/ixgbe: fix Tx check descriptor status APIs error 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=039ED4275CED7440929022BC67E706115323F421@SHSMSX103.ccr.corp.intel.com \
--to=qi.z.zhang@intel.com \
--cc=dev@dpdk.org \
--cc=olivier.matz@6wind.com \
--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).