Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Jesse Gross <jesse@kernel.org>
Date: 2016-03-28 05:36:30
On Fri, Mar 18, 2016 at 4:25 PM, Alexander Duyck [off-list ref] wrote:
quoted hunk ↗ jump to hunk
diff --git a/net/core/dev.c b/net/core/dev.c index edb7179bc051..666cf427898b 100644 --- a/net/core/dev.c +++ b/net/core/dev.c@@ -2711,6 +2711,19 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
[...]
+ /* Only report GSO partial support if it will enable us to
+ * support segmentation on this frame without needing additional
+ * work.
+ */
+ if (features & NETIF_F_GSO_PARTIAL) {
+ netdev_features_t partial_features;
+ struct net_device *dev = skb->dev;
+
+ partial_features = dev->features & dev->gso_partial_features;
+ if (!skb_gso_ok(skb, features | partial_features))
+ features &= ~NETIF_F_GSO_PARTIAL;I think we need to add NETIF_F_GSO_ROBUST into the skb_gso_ok() check - otherwise packets coming from VMs fail this test and we lose GSO partial. It's totally safe to expose this feature, since we'll compute gso_segs anyways.
quoted hunk ↗ jump to hunk
diff --git a/net/core/skbuff.c b/net/core/skbuff.c index f044f970f1a6..bdcba77e164c 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c@@ -3281,6 +3291,15 @@ perform_csum_check: */ segs->prev = tail; + /* Update GSO info on first skb in partial sequence. */ + if (partial_segs) { + skb_shinfo(segs)->gso_size = mss / partial_segs;
One small thing: this gso_size is the same as the original MSS, right? It seems like we could trivially stick it in a local variable and avoid the extra division.
+ skb_shinfo(segs)->gso_segs = partial_segs; + skb_shinfo(segs)->gso_type = skb_shinfo(head_skb)->gso_type | + SKB_GSO_PARTIAL;
Since we're computing the gso_segs ourselves, it might be nice to strip out SKB_GSO_DODGY when we set the type. I just wanted to say that this is really nice work - I was expecting it to turn out to be really messy and unmaintainable but this is very clean. Thanks!