Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Stephen Hemminger <hidden>
Date: 2012-07-30 22:50:52
Subsystem:
networking [general], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
On Mon, 30 Jul 2012 23:20:57 +0100 Ben Hutchings [off-list ref] wrote:
On Mon, 2012-07-30 at 14:46 -0700, David Miller wrote:quoted
From: Ben Hutchings <redacted> Date: Mon, 30 Jul 2012 20:35:52 +0100quoted
On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote:quoted
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;