On Fri, Mar 18, 2016 at 4:25 PM, Alexander Duyck <adu...@mirantis.com> wrote:
> 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.
> 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!
