From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb Date: Mon, 30 Jul 2012 15:50:26 -0700 Message-ID: <20120730155026.7460a9a6@nehalam.linuxnetplumber.net> References: <1343668602.2667.6.camel@bwh-desktop.uk.solarflarecom.com> <1343669507.21269.33.camel@edumazet-glaptop> <1343676952.2667.26.camel@bwh-desktop.uk.solarflarecom.com> <20120730.144632.478408817308488569.davem@davemloft.net> <1343686857.2667.60.camel@bwh-desktop.uk.solarflarecom.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: David Miller , , , To: Ben Hutchings Return-path: Received: from mail.vyatta.com ([76.74.103.46]:39015 "EHLO mail.vyatta.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751075Ab2G3Wuw (ORCPT ); Mon, 30 Jul 2012 18:50:52 -0400 In-Reply-To: <1343686857.2667.60.camel@bwh-desktop.uk.solarflarecom.com> Sender: netdev-owner@vger.kernel.org List-ID: On Mon, 30 Jul 2012 23:20:57 +0100 Ben Hutchings wrote: > On Mon, 2012-07-30 at 14:46 -0700, David Miller wrote: > > From: Ben Hutchings > > Date: Mon, 30 Jul 2012 20:35:52 +0100 > > > > > On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote: > > >> Or you could introduce a new wk->sk_gso_max_segments, that your sfc > > >> driver sets to whatever limit ? > > > > > > Yes, that's another option. > > > > This is how I want this handled. > > How should that be applied in the GRO-forwarding case? > > Ben. > Why not make max_frags a property of the device? Something like the following untested idea: diff --git a/net/core/dev.c b/net/core/dev.c index 0ebaea1..bfb005b 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -2159,14 +2159,16 @@ EXPORT_SYMBOL(netif_skb_features); * at least one of fragments is in highmem and device does not * support DMA from it. */ -static inline int skb_needs_linearize(struct sk_buff *skb, - int features) +static inline bool skb_needs_linearize(struct sk_buff *skb, + int features, unsigned int maxfrags) { - return skb_is_nonlinear(skb) && - ((skb_has_frag_list(skb) && - !(features & NETIF_F_FRAGLIST)) || - (skb_shinfo(skb)->nr_frags && - !(features & NETIF_F_SG))); + if (!skb_is_nonlinear(skb)) + return false; + + if (skb_has_frag_list(skb)) + return !(features & NETIF_F_FRAGLIST); + else + return skb_shinfo(skb)->nr_frags > maxfrags; } int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev, @@ -2206,7 +2208,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev, if (skb->next) goto gso; } else { - if (skb_needs_linearize(skb, features) && + if (skb_needs_linearize(skb, features, dev->max_frags) && __skb_linearize(skb)) goto out_kfree_skb; @@ -5544,6 +5546,20 @@ int register_netdevice(struct net_device *dev) dev->features |= NETIF_F_SOFT_FEATURES; dev->wanted_features = dev->features & dev->hw_features; + if (dev->max_frags > 0) { + if (!(features & NETIF_F_SG)) { + netdev_dbg(dev, + "Resetting max fragments since no NETIF_F_SG\n"); + dev->max_frags = 0; + } + } else { + /* If device has not set maximum number of fragments + * then assume it can take any number of them + */ + if (features & NETIF_F_SG) + dev->max_frags = MAX_SKB_FRAGS; + } + /* Turn on no cache copy if HW is doing checksum */ if (!(dev->flags & IFF_LOOPBACK)) { dev->hw_features |= NETIF_F_NOCACHE_COPY;