DPDK patches and discussions
 help / color / mirror / Atom feed
From: Olivier Matz <olivier.matz@6wind.com>
To: "Zhao1, Wei" <wei.zhao1@intel.com>
Cc: "Zhang, Qi Z" <qi.z.zhang@intel.com>,
	"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: Tue, 26 Jun 2018 10:46:08 +0200	[thread overview]
Message-ID: <20180626084608.c4kfsc5ccnfwypgw@platinum> (raw)
In-Reply-To: <A2573D2ACFCADC41BB3BE09C6DE313CA07DFF21F@PGSMSX103.gar.corp.intel.com>

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?

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

  reply	other threads:[~2018-06-26  8:46 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 [this message]
2018-06-27  2:07                       ` Zhao1, Wei
2018-06-27 13:38                       ` Zhang, Qi Z
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=20180626084608.c4kfsc5ccnfwypgw@platinum \
    --to=olivier.matz@6wind.com \
    --cc=dev@dpdk.org \
    --cc=qi.z.zhang@intel.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).