Re: [PATCH net-next v1] gso: Support partial splitting at the frag_list pointer

[Steffen Klassert <steffen.klassert@secunet.com>]

On Thu, Aug 25, 2016 at 09:02:33AM -0700, Alexander Duyck wrote:
> On Thu, Aug 25, 2016 at 4:00 AM, Steffen Klassert
> > +
> > +                       skb_walk_frags(head_skb, segs) {
> > +                               len -= segs->len;
> > +                       }
> 
> One thought I had is that you may want to add a check here to make
> certain that the skb frags have a headlen of 0.  If they don't you
> would probably need to bail.

Why do you think we need this check? Is it because the assumption that
all excluding the last skb have the same size might be wrong in this
case?

> >
> > +       gso_partial = !!(skb_shinfo(segs)->gso_type & SKB_GSO_PARTIAL);
> > +
> >         outer_hlen = skb_tnl_header_len(skb);
> >         udp_offset = outer_hlen - tnl_hlen;
> >         skb = segs;
> > @@ -117,7 +119,7 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> >                  * will be using a length value equal to only one MSS sized
> >                  * segment instead of the entire frame.
> >                  */
> > -               if (skb_is_gso(skb)) {
> > +               if (skb_is_gso(skb) && gso_partial) {
> >                         uh->len = htons(skb_shinfo(skb)->gso_size +
> >                                         SKB_GSO_CB(skb)->data_offset +
> >                                         skb->head - (unsigned char *)uh);
> 
> It might be worth while to reverse these checks.  You can probably
> save a few cycles if you check for gso_partial first and then
> skb_is_gso instead of the current ordering.

Maybe we can even remove the skb_is_gso check, the gso_partial
check should be sufficient.