From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [PATCH 0/2]  virtio fix false offload claims
Date: Wed, 23 Aug 2017 08:31:35 -0700
Message-ID: <20170823083135.67193f91@xeon-e3>
References: <20170707195250.22259-1-stephen@networkplumber.org>
 <20170708031222.GA11626@yliu-home>
 <20170823093025.bhjwbygfyz4ifd6r@glumotte.dev.6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: Yuanhan Liu <yliu@fridaylinux.org>, maxime.coquelin@redhat.com,
 dev@dpdk.org, stable@dpdk.org
To: Olivier MATZ <olivier.matz@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-pf0-f182.google.com (mail-pf0-f182.google.com
 [209.85.192.182]) by dpdk.org (Postfix) with ESMTP id 3868C7D4A
 for <dev@dpdk.org>; Wed, 23 Aug 2017 17:31:47 +0200 (CEST)
Received: by mail-pf0-f182.google.com with SMTP id k3so1524012pfc.0
 for <dev@dpdk.org>; Wed, 23 Aug 2017 08:31:47 -0700 (PDT)
In-Reply-To: <20170823093025.bhjwbygfyz4ifd6r@glumotte.dev.6wind.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Wed, 23 Aug 2017 11:30:26 +0200
Olivier MATZ <olivier.matz@6wind.com> wrote:

> Hello,
> 
> On Sat, Jul 08, 2017 at 11:12:22AM +0800, Yuanhan Liu wrote:
> > On Fri, Jul 07, 2017 at 12:52:48PM -0700, Stephen Hemminger wrote:  
> > > While doing code for Hyper-V, noticed that the virtio driver was
> > > confused about receive versus transmit offloads.  The virtio
> > > checksum offload is L4 (TCP/UDP) only, not IPv4. Also, TSO
> > > and LRO are not the same.
> > > 
> > > This may break some program that was assuming it was getting offloads
> > > that it wasn't.  
> > 
> > Applied to dpdk-next-virtio.
> > 
> > And I think they should be backported to stable releases, thus,
> > 
> >     Cc: stable@dpdk.org
> > 
> > Thanks.
> > 
> > 	--yliu  
> > > 
> > > Stephen Hemminger (2):
> > >   virtio: don't falsely claim to do IP checksum
> > >   virtio: don't claim to support LRO
> > > 
> > >  drivers/net/virtio/virtio_ethdev.c | 30 +++++-------------------------
> > >  1 file changed, 5 insertions(+), 25 deletions(-)
> > > 
> > > -- 
> > > 2.11.0  
> 
> I think these 2 commits break the virtio offload, which can be tested as
> described in this test plan:
> http://dpdk.org/ml/archives/dev/2016-October/048092.html
> 
> First, about checksum: the description of rxmode->hw_ip_checksum is:
> 
>      hw_ip_checksum   : 1, /**< IP/UDP/TCP checksum offload enable. */
> 
> So, while I agree the name is not well chosen, it is valid to set it
> for virtio to enable L4 checksum.
> 
> Then about LRO: setting rxmode->enable_lro is a way to tell the host that the
> guest is ok to receive tso packets. From the guest point of view, it is like
> enabling lro on a physical driver. Again, it is valid and useful to do this.
> 
> Before removing these features, it would have been nice to have a quick look at
> the commits that introduced them.

I am ok with keeping LRO as long as the documentation changed.  And virtio
driver did some enforcement. 

For checksums, the hw_ip_checksum flag either needs to be more fine grain (IP, UDP, TCP)
which would be best, or virtio would have to check IP checksum in software.