From: Yongseok Koh <yskoh@mellanox.com>
To: "Nélio Laranjeiro" <nelio.laranjeiro@6wind.com>
Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"stable@dpdk.org" <stable@dpdk.org>,
Shahaf Shuler <shahafs@mellanox.com>
Subject: Re: [dpdk-stable] [PATCH] net/mlx5: fix calculating TSO inline size
Date: Tue, 12 Sep 2017 18:34:05 +0000 [thread overview]
Message-ID: <30C13A14-CB69-45B0-9DC8-EBE8048098B7@mellanox.com> (raw)
In-Reply-To: <20170912072411.GO14138@autoinstall.dev.6wind.com>
> On Sep 12, 2017, at 12:24 AM, Nélio Laranjeiro <nelio.laranjeiro@6wind.com> wrote:
>
> On Mon, Sep 11, 2017 at 03:17:44PM -0700, Yongseok Koh wrote:
>> On Mon, Sep 04, 2017 at 04:01:08PM +0200, Nélio Laranjeiro wrote:
>>> Hi Yongseok,
>>>
>>> Please see some comments below,
>>>
>>> On Thu, Aug 31, 2017 at 09:27:06AM -0700, Yongseok Koh wrote:
>>>> Tx descriptor for TSO embeds packet header to be replicated. If Tx inline
>>>> is enabled, there could be additional packet data inlined with 4B inline
>>>> header ahead. And between the header and additional inlined packet data,
>>>> there may be padding to make the inline part aligned to
>>>> MLX5_WQE_DWORD_SIZE. In calculating the total size of inlined data, the
>>>> size of inline header and padding is missing.
>>>>
>>>> Fixes: 3f13f8c23a7c ("net/mlx5: support hardware TSO")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
>>>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>>>> ---
>>>> drivers/net/mlx5/mlx5_rxtx.c | 25 +++++++++----------------
>>>> 1 file changed, 9 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
>>>> index fe9e7eac0..f89fa40b5 100644
>>>> --- a/drivers/net/mlx5/mlx5_rxtx.c
>>>> +++ b/drivers/net/mlx5/mlx5_rxtx.c
>>>> @@ -524,19 +521,20 @@ mlx5_tx_burst(void *dpdk_txq, struct rte_mbuf **pkts, uint16_t pkts_n)
>>>> }
>>>> /* Inline if enough room. */
>>>> if (inline_en || tso) {
>>>> + uint32_t inl;
>>>> uintptr_t end = (uintptr_t)
>>>> (((uintptr_t)txq->wqes) +
>>>> (1 << txq->wqe_n) * MLX5_WQE_SIZE);
>>>> unsigned int inline_room = max_inline *
>>>> RTE_CACHE_LINE_SIZE -
>>>> - (pkt_inline_sz - 2);
>>>> + (pkt_inline_sz - 2) -
>>>> + !!tso * sizeof(inl);
>>>
>>> Is not it dangerous to assume inl will always be 4 bytes long? Why not
>>> writing the real value instead?
>> That was for readability of the code and uint32_t will be always 4bytes. But for
>> better readability, it should be 'inline_header' instead of 'inl' though. I'm
>> also okay with using "4" as well. Which way do you prefer?
>
> I agree on both, I was not clear enough to explain my thought, if for some
> reason the inl moves from uint32_t to uint16_t without touching the sizeof
> later, it will cause an issue.
I tried to change the sizeof but I found that there are more "sizeof(inl)" in
the following lines. Changing all the sizeof would be beyond the scope of this
patch. So, how about leaving it as is for consistency?
Thanks
Yongseok
next prev parent reply other threads:[~2017-09-12 18:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-31 16:27 Yongseok Koh
2017-09-04 14:01 ` Nélio Laranjeiro
2017-09-11 22:17 ` Yongseok Koh
2017-09-12 7:24 ` Nélio Laranjeiro
2017-09-12 18:34 ` Yongseok Koh [this message]
2017-09-13 5:05 ` Shahaf Shuler
2017-09-13 7:26 ` Nélio Laranjeiro
2017-09-15 8:39 ` Ferruh Yigit
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=30C13A14-CB69-45B0-9DC8-EBE8048098B7@mellanox.com \
--to=yskoh@mellanox.com \
--cc=adrien.mazarguil@6wind.com \
--cc=dev@dpdk.org \
--cc=nelio.laranjeiro@6wind.com \
--cc=shahafs@mellanox.com \
--cc=stable@dpdk.org \
/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).