From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier MATZ Subject: Re: [PATCH 5/5] net/virtio: fix Tso when mbuf is shared Date: Tue, 24 Jan 2017 11:51:20 +0100 Message-ID: <20170124115120.2c2667f4@glumotte.dev.6wind.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> <20170118050348.GO10293@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Olivier Matz , dev@dpdk.org, maxime.coquelin@redhat.com, huawei.xie@intel.com, stephen@networkplumber.org, "Tan, Jianfeng" , Tomasz Kulasek , Thomas Monjalon , Konstantin Ananyev To: Yuanhan Liu Return-path: Received: from mail-wm0-f54.google.com (mail-wm0-f54.google.com [74.125.82.54]) by dpdk.org (Postfix) with ESMTP id 18E4F2A5B for ; Tue, 24 Jan 2017 11:51:28 +0100 (CET) Received: by mail-wm0-f54.google.com with SMTP id c206so202460822wme.0 for ; Tue, 24 Jan 2017 02:51:28 -0800 (PST) In-Reply-To: <20170118050348.GO10293@yliu-dev.sh.intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, 18 Jan 2017 13:03:48 +0800, Yuanhan Liu wrote: > 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? Yes sounds good. I'll check how a tx_prepare() could be implemented for virtio, in order to fix this issue. In the meanwhile, for the 17.02, I think it could be good to highlight the problem in the known issues, what do you think? Thanks Olivier