netdev - Re: XDP multi-buffer incl. jumbo-frames (Was: [RFC V1 net-next 1/1] net: ena: implement XDP drop support)

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]

Date:   Fri, 28 Jun 2019 10:46:23 +0200
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     Toke Høiland-Jørgensen <toke@...hat.com>,
        "Machulsky, Zorik" <zorik@...zon.com>,
        "Jubran, Samih" <sameehj@...zon.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Woodhouse, David" <dwmw@...zon.co.uk>,
        "Matushevsky, Alexander" <matua@...zon.com>,
        "Bshara, Saeed" <saeedb@...zon.com>,
        "Wilson, Matt" <msw@...zon.com>,
        "Liguori, Anthony" <aliguori@...zon.com>,
        "Bshara, Nafea" <nafea@...zon.com>,
        "Tzalik, Guy" <gtzalik@...zon.com>,
        "Belgazal, Netanel" <netanel@...zon.com>,
        "Saidi, Ali" <alisaidi@...zon.com>,
        "Herrenschmidt, Benjamin" <benh@...zon.com>,
        "Kiyanovski, Arthur" <akiyano@...zon.com>,
        Daniel Borkmann <borkmann@...earbox.net>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        "xdp-newbies@...r.kernel.org" <xdp-newbies@...r.kernel.org>,
        brouer@...hat.com
Subject: Re: XDP multi-buffer incl. jumbo-frames (Was: [RFC V1 net-next 1/1]
 net: ena: implement XDP drop support)


On Wed, 26 Jun 2019 11:20:45 -0400 Willem de Bruijn <willemdebruijn.kernel@...il.com> wrote:
> On Wed, Jun 26, 2019 at 11:01 AM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
> > Jesper Dangaard Brouer <brouer@...hat.com> writes:
> > > On Wed, 26 Jun 2019 13:52:16 +0200
> > > Toke Høiland-Jørgensen <toke@...hat.com> wrote:
> > >  
> > >> Jesper Dangaard Brouer <brouer@...hat.com> writes:
> > >>  
[...]
> > >
> > > You touch upon some interesting complications already:
> > >
> > > 1. It is valuable for XDP bpf_prog to know "full" length?
> > >    (if so, then we need to extend xdp ctx with info)  
> >
> > Valuable, quite likely. A hard requirement, probably not (for all use
> > cases).  
> 
> Agreed.
> 
> One common validation use would be to drop any packets whose header
> length disagrees with the actual packet length.

That is a good point.

Added a section "XDP access to full packet length?" to capture this:
- https://github.com/xdp-project/xdp-project/commit/da5b84264b85b0d
- https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org#xdp-access-to-full-packet-length


> > >  But if we need to know the full length, when the first-buffer is
> > >  processed. Then realize that this affect the drivers RX-loop, because
> > >  then we need to "collect" all the buffers before we can know the
> > >  length (although some HW provide this in first descriptor).
> > >
> > >  We likely have to change drivers RX-loop anyhow, as XDP_TX and
> > >  XDP_REDIRECT will also need to "collect" all buffers before the packet
> > >  can be forwarded. (Although this could potentially happen later in
> > >  driver loop when it meet/find the End-Of-Packet descriptor bit).  
> 
> Yes, this might be quite a bit of refactoring of device driver code.
> 
> Should we move forward with some initial constraints, e.g., no
> XDP_REDIRECT, no "full" length and no bpf_xdp_adjust_tail?

I generally like this...

If not adding "full" length. Maybe we could add an indication to
XDP-developer, that his is a multi-buffer/multi-segment packet, such
that header length validation code against packet length (data_end-data)
is not possible.  This is user visible, so we would have to keep it
forever... I'm leaning towards adding "full" length from beginning.

> That already allows many useful programs.
>
> As long as we don't arrive at a design that cannot be extended with
> those features later.

That is the important part...

 
> > > 2. Can we even allow helper bpf_xdp_adjust_tail() ?
[...]
> >  
> > >  Perhaps it is better to let bpf_xdp_adjust_tail() fail runtime?  
> >
> > If we do disallow it, I think I'd lean towards failing the call at
> > runtime...  
> 
> Disagree. I'd rather have a program fail at load if it depends on
> multi-frag support while the (driver) implementation does not yet
> support it.

I usually agree that we should fail the program, early at load time.
For XDP we are unfortunately missing some knobs to do this, see[1].

Specifically for bpf_xdp_adjust_tail(), it might be better to fail
runtime.  Because, the driver might have enabled TSO for TCP packets,
while your XDP use-case is for adjusting UDP-packets (and do XDP level
replies), which will never see multi-buffer packets.  If we fail use of
bpf_xdp_adjust_tail(), then you would have to disable TSO to allow
loading your XDP-prog, hurting the other TSO-TCP use-case.


[1] http://vger.kernel.org/netconf2019_files/xdp-feature-detection.pdf
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer