On Tue, Jan 15, 2019 at 4:49 AM Nicolas Dichtel
<nicolas.dich...@6wind.com> wrote:
>
> Le 14/01/2019 à 16:38, Willem de Bruijn a écrit :
> > On Mon, Jan 14, 2019 at 10:15 AM Willem de Bruijn
> > <willemdebruijn.ker...@gmail.com> wrote:
> [snip]
> >> It is wrong because for other devices l2 header length is not zero, so
> >> this incorrectly sets skb->network_header to the start of the link
> >> layer header on all those devices.
> Ok, thank you for the details.
>
> >>
> >> A one line variant of the above would be
> >>
> >> - if (len < reserve)
> >> + if (len < reserve + sizeof(struct ipv6hdr))
> >
> > and exclude the majority of devices with fixed hard header len. Those
> > require len to be >= reserve, so this workaround does not apply to
> > them:
> >
> > if (len < reserve + sizeof(struct ipv6hdr) &&
> > dev->min_header_len != dev->hard_header_len)
> >
> And what about:
> - if (len < reserve)
> + if (dev->min_header_len != dev->hard_header_len)
> skb_reset_network_header(skb);
> ?
As a fix for the regression introduced by cb9f1b783850 ("ip: validate
header length on virtual device xmit"), the test I proposed is a bit
more narrow by only applying this ugly workaround in cases where that
test might have started failing.
By limiting to short packets we also avoid in the common case reading
dev->min_header_len, which may not be cached (btw, we should use
reserve instead of dev->hard_header_len for the same reason).
But the length restriction does look rather arbitrary, so there is
something to say for your suggestion to apply this uniformly to
devices with variable length.
Note also the concurrent discussion in
http://patchwork.ozlabs.org/patch/1024489/ about extending header_ops
with a hard header parser which may return the exist header length.
That is only for net-next, but it will allow us to set
skb->network_header correctly even for these devices where
(dev->min_header_len != dev->hard_header_len).
