On Fri, May 29, 2020 at 8:27 PM David Miller <da...@davemloft.net> wrote:
>
> From: Willem de Bruijn <willemdebruijn.ker...@gmail.com>
> Date: Thu, 28 May 2020 13:05:32 -0400
>
> > Temporarily pull ETH_HLEN to make control flow the same for frags and
> > not frags. Then push the header just before calling napi_gro_frags.
> ...
> > case IFF_TAP:
> > - if (!frags)
> > - skb->protocol = eth_type_trans(skb, tun->dev);
> > + if (frags && !pskb_may_pull(skb, ETH_HLEN)) {
> > + err = -ENOMEM;
> > + goto drop;
> > + }
> > + skb->protocol = eth_type_trans(skb, tun->dev);
> ...
> > /* Exercise flow dissector code path. */
> > - u32 headlen = eth_get_headlen(tun->dev, skb->data,
> > - skb_headlen(skb));
> > + skb_push(skb, ETH_HLEN);
> > + headlen = eth_get_headlen(tun->dev, skb->data,
> > + skb_headlen(skb));
>
> I hate to be a stickler on wording in the commit message, but the
> change is not really "pulling" the ethernet header from the SKB.
>
> Instead it is invoking pskb_may_pull() which just makes sure the
> header is there in the linear SKB data area.
>
> Can you please refine this description and resubmit?
Of course. How is this
"
Ensure the link layer header lies in linear as eth_type_trans pulls
ETH_HLEN. Then take the same code paths for frags as for not frags.
Push the link layer header back just before calling napi_gro_frags.
By pulling up to ETH_HLEN from frag0 into linear, this disables the
frag0 optimization in the special case when IFF_NAPI_FRAGS is
called with zero length iov[0] (and thus empty skb->linear).
"
Seemed good to add the extra clarification. I don't see a reasonable way
to avoid that consequence, especially as I cannot restore the first skb frag
(iov[1]) if it was exactly ETH_HLEN bytes and thus freed by __skb_pull_tail.
