From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: dev@dpdk.org, maxime.coquelin@redhat.com, huawei.xie@intel.com,
stephen@networkplumber.org
Subject: Re: [dpdk-dev] [PATCH 5/5] net/virtio: fix Tso when mbuf is shared
Date: Wed, 14 Dec 2016 15:27:50 +0800 [thread overview]
Message-ID: <20161214072750.GK18991@yliu-dev.sh.intel.com> (raw)
In-Reply-To: <1479977798-13417-6-git-send-email-olivier.matz@6wind.com>
Hi Olivier,
Firstly sorry for late response!
On Thu, Nov 24, 2016 at 09:56:38AM +0100, Olivier Matz wrote:
> With virtio, doing tso requires to modify the network
> packet data:
I thought more about it this time, and I'm wondering why it's needed.
> - the dpdk API requires to set the l4 checksum to an
> Intel-Nic-like pseudo header checksum that does
> not include the ip length
If the packet is for a NIC pmd driver in the end, then the NIC driver
(or application) would handle the checksum correctly. You could check
the tx_prep patchset for example.
> - the virtio peer expects that the l4 checksum is
> a standard pseudo header checksum.
For this case, the checksum is then not needed: we could assume the data
between virtio to virtio transmission on the same host is always valid,
that checksum validation is unnecessary.
So, in either case, it doesn't seem to me we have to generate the checksum
here. Or am I miss something?
OTOH, even if it does, I still see some issues (see below).
> /* TCP Segmentation Offload */
> if (cookie->ol_flags & PKT_TX_TCP_SEG) {
> - virtio_tso_fix_cksum(cookie);
> + offset = virtio_tso_fix_cksum(cookie,
> + RTE_PTR_ADD(hdr, start_dp[hdr_idx].len),
> + VIRTIO_MAX_HDR_SZ);
> + if (offset > 0) {
> + RTE_ASSERT(can_push != 0);
I think it's (can_push == 0) ?
> + start_dp[hdr_idx].len += offset;
Actually, there is an assumption if you do this, that the backend driver
must have to support ANY_LAYOUT. Otherwise, it won't work: the driver
would expect the header and packet data is totally separated into two
desc buffers.
Though the assumption is most likely true in nowadays, I don't think
it's a guarantee.
--yliu
next prev parent reply other threads:[~2016-12-14 7:26 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-24 8:56 [dpdk-dev] [PATCH 0/5] virtio/mbuf: fix virtio tso with shared mbufs Olivier Matz
2016-11-24 8:56 ` [dpdk-dev] [PATCH 1/5] mbuf: remove const attribute in mbuf read function Olivier Matz
2016-11-24 8:56 ` [dpdk-dev] [PATCH 2/5] mbuf: new helper to check if a mbuf is shared Olivier Matz
2016-11-24 8:56 ` [dpdk-dev] [PATCH 3/5] mbuf: new helper to write data in a mbuf chain Olivier Matz
2016-11-24 8:56 ` [dpdk-dev] [PATCH 4/5] mbuf: new helper to copy data from a mbuf Olivier Matz
2016-11-24 8:56 ` [dpdk-dev] [PATCH 5/5] net/virtio: fix Tso when mbuf is shared Olivier Matz
2016-12-14 7:27 ` Yuanhan Liu [this message]
2017-01-09 17:46 ` Olivier Matz
2017-01-16 6:48 ` Yuanhan Liu
2017-01-17 11:18 ` Olivier Matz
2017-01-18 5:03 ` Yuanhan Liu
2017-01-24 10:51 ` Olivier MATZ
2017-01-28 12:32 ` Yuanhan Liu
2017-01-09 17:59 ` Stephen Hemminger
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=20161214072750.GK18991@yliu-dev.sh.intel.com \
--to=yuanhan.liu@linux.intel.com \
--cc=dev@dpdk.org \
--cc=huawei.xie@intel.com \
--cc=maxime.coquelin@redhat.com \
--cc=olivier.matz@6wind.com \
--cc=stephen@networkplumber.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).