From patchwork Tue Apr 5 18:31:38 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Ben Hutchings X-Patchwork-Id: 89918 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 9FC1BB6F95 for ; Wed, 6 Apr 2011 04:31:45 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754632Ab1DESbl (ORCPT ); Tue, 5 Apr 2011 14:31:41 -0400 Received: from mail.solarflare.com ([216.237.3.220]:18666 "EHLO exchange.solarflare.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753994Ab1DESbl convert rfc822-to-8bit (ORCPT ); Tue, 5 Apr 2011 14:31:41 -0400 Received: from [10.17.20.137] ([10.17.20.137]) by exchange.solarflare.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Tue, 5 Apr 2011 11:31:40 -0700 Subject: Re: NETIF_F_TSO vs NETIF_F_TSO{6,_ECN} From: Ben Hutchings To: =?UTF-8?Q?Micha=C5=82_Miros=C5=82aw?= Cc: netdev , Herbert Xu In-Reply-To: <20110405180337.GA16977@rere.qmqm.pl> References: <1302018602.2932.22.camel@bwh-desktop> <20110405180337.GA16977@rere.qmqm.pl> Organization: Solarflare Communications Date: Tue, 05 Apr 2011 19:31:38 +0100 Message-ID: <1302028298.2932.76.camel@bwh-desktop> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 (2.32.1-1.fc14) X-OriginalArrivalTime: 05 Apr 2011 18:31:40.0719 (UTC) FILETIME=[B4C343F0:01CBF3BF] X-TM-AS-Product-Ver: SMEX-8.0.0.1181-6.500.1024-18054.005 X-TM-AS-Result: No--34.269100-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Tue, 2011-04-05 at 20:03 +0200, Michał Mirosław wrote: > On Tue, Apr 05, 2011 at 04:50:02PM +0100, Ben Hutchings wrote: > > According to the commit that introduced NETIF_F_TSO6 > > (f83ef8c0b58dac17211a4c0b6df0e2b1bd6637b1): > > > > This patch will introduce a new flag NETIF_F_TSO6 which will be used > > to check whether device supports TSO over IPv6. If device support TSO > > over IPv6 then we don't clear of NETIF_F_TSO and which will make the > > TCP layer to create TSO packets. Any device supporting TSO over IPv6 > > will set NETIF_F_TSO6 flag in "dev->features" along with NETIF_F_TSO. > > > > In case when user disables TSO using ethtool, NETIF_F_TSO will get > > cleared from "dev->features". So even if we have NETIF_F_TSO6 we don't > > get TSO packets created by TCP layer. > > > > So I think we need to either: > > 1. Disallow toggling NETIF_F_TSO6 (following the previous rule) > > 2. Disable NETIF_F_TSO6 when NETIF_F_TSO is disabled > > > > The same presumably applies to NETIF_F_TSO_ECN. > > There seems to be no such dependency in the networking code. I.e. TSO6 > should just work with TSO4 disabled. *sigh* So it seems the commit message was wrong... and it should have included a change like this: --- Now that we're relying on these checks for dynamic changes to features, this is pretty important. Ben. --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5203,9 +5203,9 @@ u32 netdev_fix_features(struct net_device *dev, u32 features) } /* TSO requires that SG is present as well. */ - if ((features & NETIF_F_TSO) && !(features & NETIF_F_SG)) { - netdev_info(dev, "Dropping NETIF_F_TSO since no SG feature.\n"); - features &= ~NETIF_F_TSO; + if ((features & NETIF_F_ALL_TSO) && !(features & NETIF_F_SG)) { + netdev_info(dev, "Dropping TSO since no SG feature.\n"); + features &= ~NETIF_F_ALL_TSO; } /* Software GSO depends on SG. */