From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 89246201 for ; Wed, 14 Dec 2016 08:26:02 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga104.jf.intel.com with ESMTP; 13 Dec 2016 23:26:01 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,345,1477983600"; d="scan'208";a="1081603463" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by fmsmga001.fm.intel.com with ESMTP; 13 Dec 2016 23:26:00 -0800 Date: Wed, 14 Dec 2016 15:27:50 +0800 From: Yuanhan Liu To: Olivier Matz Cc: dev@dpdk.org, maxime.coquelin@redhat.com, huawei.xie@intel.com, stephen@networkplumber.org Message-ID: <20161214072750.GK18991@yliu-dev.sh.intel.com> References: <1479977798-13417-1-git-send-email-olivier.matz@6wind.com> <1479977798-13417-6-git-send-email-olivier.matz@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1479977798-13417-6-git-send-email-olivier.matz@6wind.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH 5/5] net/virtio: fix Tso when mbuf is shared X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Dec 2016 07:26:03 -0000 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