From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 8D4C0100F for ; Wed, 18 Jan 2017 06:01:39 +0100 (CET) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga101.jf.intel.com with ESMTP; 17 Jan 2017 21:01:38 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,247,1477983600"; d="scan'208";a="54281735" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by orsmga005.jf.intel.com with ESMTP; 17 Jan 2017 21:01:36 -0800 Date: Wed, 18 Jan 2017 13:03:48 +0800 From: Yuanhan Liu To: Olivier Matz Cc: dev@dpdk.org, maxime.coquelin@redhat.com, huawei.xie@intel.com, stephen@networkplumber.org, "Tan, Jianfeng" , Tomasz Kulasek , Thomas Monjalon , Konstantin Ananyev Message-ID: <20170118050348.GO10293@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> <20161214072750.GK18991@yliu-dev.sh.intel.com> <20170109184625.7884290a@platinum> <20170116064819.GL9770@yliu-dev.sh.intel.com> <20170117121825.3db48369@platinum> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170117121825.3db48369@platinum> 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, 18 Jan 2017 05:01:40 -0000 On Tue, Jan 17, 2017 at 12:18:25PM +0100, Olivier Matz wrote: > > I hope I could have time to dig this further, since, honestly, I don't > > quite like this patch: it makes things un-maintainable. > > Well, I'm not that proud of the patch, but that's the best solution > I've found. Nevertheless saying it makes things un-maintainable looks a > bit excessive to me :) Aha... really sorry about that! But honestly, I'd say again, it makes thing more complex, just for fixing a corner and rare issue. I'd try to avoid that. > > The option of reallocating a mbuf, copy and fix network headers in it > looks even more complex to me (that was my first approach). > > > Besides that, I think we have similar issue with nic drivers. See the > > rte_net_intel_cksum_flags_prepare() function introduced at commit > > 4fb7e803eb1a ("ethdev: add Tx preparation"). > > Yes, that was discussed a bit. See [1] and the subsequent mails. > http://dpdk.org/ml/archives/dev/2016-December/051014.html Thanks for the info, and I'm pretty Okay with that. > My opinion is that tx_burst() should not change the mbuf data, it's > always been like this. For Intel NICs, there is no issue since the DPDK > API is derived from Intel NICs API, so there is no fix to do in the > mbuf data. > > For tx_prepare(), it's explicitly said that it can update the data. > If tx_prepare() becomes mandatory, it will naturally fix this issue > without modifying the driver, because the phdr csum calculation will be > done in tx_prepare(). > > An alternative is to mark this as a known issue for now, and wait until > tx_prepare() is mandatory. I see no reason to wait. Though my understanding is it may not be a mandatory so far, but user is supposed to calculate the pseudo-header checksum by themself before. Now they have one more option: tx_prepare. That means, in either way, user has to do some extra works to make TSO work (either by themself or call tx_prepare). So I don't think it'd be an issue? --yliu