Re: [PATCH v4] netdev attribute to control xdpgeneric skb linearization

[Luigi Rizzo]

On Fri, Feb 28, 2020 at 4:30 AM Jesper Dangaard Brouer
<jbrouer@xxxxxxxxxx> wrote:
>
> On Fri, 28 Feb 2020 02:54:35 -0800
> Luigi Rizzo <lrizzo@xxxxxxxxxx> wrote:
>
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index dbbfff123196..c539489d3166 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4520,9 +4520,12 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> >       /* XDP packets must be linear and must have sufficient headroom
> >        * of XDP_PACKET_HEADROOM bytes. This is the guarantee that also
> >        * native XDP provides, thus we need to do it here as well.
> > +      * For non shared skbs, xdpgeneric_linearize controls linearization.
> >        */
> > -     if (skb_cloned(skb) || skb_is_nonlinear(skb) ||
> > -         skb_headroom(skb) < XDP_PACKET_HEADROOM) {
> > +     if (skb_cloned(skb) ||
> > +         (skb->dev->xdpgeneric_linearize &&
> > +          (skb_is_nonlinear(skb) ||
> > +           skb_headroom(skb) < XDP_PACKET_HEADROOM))) {
> >               int hroom = XDP_PACKET_HEADROOM - skb_headroom(skb);
> >               int troom = skb->tail + skb->data_len - skb->end;
> >
>
> Have you checked that calling bpf_xdp_adjust_tail() is not breaking anything?

It won't leak memory or cause crashes if that is what you mean.
Of course if there are more segments the effect won't be the desired one,
as it will chop off the tail of the first segment.

But this is an opt-in feature and requires the same permissions needed to load
an xdp program, so I expect it to be used consciously.

It would be nice if we had a flag in the xdp_buff to communicate that
the packet is
incomplete, but there isn't a way that I can see.

cheers
luigi