netdev - RE: XDP multi-buffer design discussion

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: <97dddd1c0f954ee1984032c3be412d06@EX13D11EUB003.ant.amazon.com>
Date:   Tue, 17 Dec 2019 15:44:44 +0000
From:   "Jubran, Samih" <sameehj@...zon.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>,
        "Jesper Dangaard Brouer" <brouer@...hat.com>,
        Luigi Rizzo <rizzo@....unipi.it>
CC:     "Machulsky, Zorik" <zorik@...zon.com>,
        Daniel Borkmann <borkmann@...earbox.net>,
        David Miller <davem@...emloft.net>,
        "Tzalik, Guy" <gtzalik@...zon.com>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        "Kiyanovski, Arthur" <akiyano@...zon.com>,
        Alexei Starovoitov <ast@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "David Ahern" <dsahern@...il.com>,
        "brouer@...hat.com" <brouer@...hat.com>
Subject: RE: XDP multi-buffer design discussion



> -----Original Message-----
> From: Toke Høiland-Jørgensen <toke@...hat.com>
> Sent: Tuesday, December 17, 2019 11:01 AM
> To: Jesper Dangaard Brouer <brouer@...hat.com>; Luigi Rizzo
> <rizzo@....unipi.it>
> Cc: Jubran, Samih <sameehj@...zon.com>; Machulsky, Zorik
> <zorik@...zon.com>; Daniel Borkmann <borkmann@...earbox.net>; David
> Miller <davem@...emloft.net>; Tzalik, Guy <gtzalik@...zon.com>; Ilias
> Apalodimas <ilias.apalodimas@...aro.org>; Kiyanovski, Arthur
> <akiyano@...zon.com>; Alexei Starovoitov <ast@...nel.org>;
> netdev@...r.kernel.org; David Ahern <dsahern@...il.com>;
> brouer@...hat.com
> Subject: Re: XDP multi-buffer design discussion
> 
> Jesper Dangaard Brouer <brouer@...hat.com> writes:
> 
> > On Mon, 16 Dec 2019 20:15:12 -0800
> > Luigi Rizzo <rizzo@....unipi.it> wrote:
> >
> >> On Mon, Dec 16, 2019 at 6:07 AM Jesper Dangaard Brouer
> >> <brouer@...hat.com> wrote:
> >> >
> >> >
> >> > See answers inlined below (please get an email client that support
> >> > inline replies... to interact with this community)
> >> >
> >> > On Sun, 15 Dec 2019 13:57:12 +0000
> >> > "Jubran, Samih" <sameehj@...zon.com> wrote:
> >> ...
> >> > > * Why should we provide the fragments to the bpf program if the
> >> > > program doesn't access them? If validating the length is what
> >> > > matters, we can provide only the full length info to the user
> >> > > with no issues.
> >> >
> >> > My Proposal#1 (in [base-doc]) is that XDP only get access to the
> >> > first-buffer.  People are welcome to challenge this choice.
> >> >
> >> > There are a several sub-questions and challenges hidden inside this
> >> > choice.
> >> >
> >> > As you hint, the total length... spawns some questions we should
> answer:
> >> >
> >> >  (1) is it relevant to the BPF program to know this, explain the use-case.
> >> >
> >> >  (2) if so, how does BPF prog access info (without slowdown
> >> > baseline)
> >>
> >> For some use cases, the bpf program could deduct the total length
> >> looking at the L3 header.
> >
> > Yes, that actually good insight.  I guess the BPF-program could also
> > use this to detect that it doesn't have access to the full-lineary
> > packet this way(?)
> >
> >> It won't work for XDP_TX response though.
> >
> > The XDP_TX case also need to be discussed/handled. IMHO need to
> > support XDP_TX for multi-buffer frames.  XDP_TX *can* be driver
> > specific, but most drivers choose to convert xdp_buff to xdp_frame,
> > which makes it possible to use/share part of the XDP_REDIRECT code from
> ndo_xdp_xmit.
> >
> > We also need to handle XDP_REDIRECT, which becomes challenging, as the
> > ndo_xdp_xmit functions of *all* drivers need to be updated (or
> > introduce a flag to handle this incrementally).
> 
> If we want to handle TX and REDIRECT (which I agree we do!), doesn't that
> imply that we have to structure the drivers so the XDP program isn't
> executed until we have the full packet (i.e., on the last segment)?
> 
> -Toke

Hi All,

Thank you for participating in this discussion, everything that was mentioned above is a good insight.
I'd like to sum it up with the following key points, please tell me if I'm missing something and share your opinions.

* The rationale to supply the frags are the following:
** XDP_PASS optimization: in case the verdict is XDP_PASS, creating the skb can save some work
** XDP_TX: when the verdict is XDP_TX, we need all the frags ready in order to send the packet
** The rx-loop of the driver must be modified, so why not supply information that already have been
   deducted and this should be great for the future in case XDP will be able to access the frags

* Since the XDP program won't be accessing the frags, we don't need to modify the xdp_convert_ctx_access() function, correct?

* We do need to add a way for the driver to indicate to the kernel that it supports multi buff. This is needed so we can reject programs that use bpf_xdp_adjust_tail()

- Sameeh