netdev - Re: [PATCH v9 bpf-next 01/14] net: skbuff: add data_len field to skb_shared

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKgT0Ue1HKMpsBtoW=js2oMRAhcqSrAfTTmPC8Wc97G6=TiaZg@mail.gmail.com>
Date:   Tue, 29 Jun 2021 11:18:38 -0700
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Lorenzo Bianconi <lorenzo.bianconi@...hat.com>,
        Lorenzo Bianconi <lorenzo@...nel.org>,
        bpf <bpf@...r.kernel.org>, Netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, shayagr@...zon.com,
        "Jubran, Samih" <sameehj@...zon.com>,
        John Fastabend <john.fastabend@...il.com>,
        David Ahern <dsahern@...nel.org>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Eelco Chaudron <echaudro@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Saeed Mahameed <saeed@...nel.org>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        Tirthendu <tirthendu.sarkar@...el.com>
Subject: Re: [PATCH v9 bpf-next 01/14] net: skbuff: add data_len field to skb_shared_info

On Tue, Jun 29, 2021 at 10:08 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Tue, 29 Jun 2021 14:44:56 +0200 Lorenzo Bianconi wrote:
> > > On Mon, Jun 14, 2021 at 5:50 AM Lorenzo Bianconi <lorenzo@...nel.org> wrote:
> > > >
> > > > data_len field will be used for paged frame len for xdp_buff/xdp_frame.
> > > > This is a preliminary patch to properly support xdp-multibuff
> > > >
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
> > > > ---
> > > >  include/linux/skbuff.h | 5 ++++-
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > > > index dbf820a50a39..332ec56c200d 100644
> > > > --- a/include/linux/skbuff.h
> > > > +++ b/include/linux/skbuff.h
> > > > @@ -522,7 +522,10 @@ struct skb_shared_info {
> > > >         struct sk_buff  *frag_list;
> > > >         struct skb_shared_hwtstamps hwtstamps;
> > > >         unsigned int    gso_type;
> > > > -       u32             tskey;
> > > > +       union {
> > > > +               u32     tskey;
> > > > +               u32     data_len;
> > > > +       };
> > > >
> > >
> > > Rather than use the tskey field why not repurpose the gso_size field?
> > > I would think in the XDP paths that the gso fields would be unused
> > > since LRO and HW_GRO would be incompatible with XDP anyway.
> >
> > ack, I agree. I will fix it in v10.
>
> Why is XDP mb incompatible with LRO? I thought that was one of the use
> cases (mentioned by Willem IIRC).

XDP is meant to be a per packet operation with support for TX and
REDIRECT, and LRO isn't routable. So we could put together a large LRO
frame but we wouldn't be able to break it apart again. If we allow
that then we are going to need a ton more exception handling added to
the XDP paths.

As far as GSO it would require setting many more fields in order to
actually make it offloadable by any hardware. My preference would be
to make use of gso_segs and gso_size to store the truesize and datalen
of the pages. That way we keep all of the data fields used in the
shared info in the first 8 bytes assuming we don't end up having to
actually use multiple buffers.