Date:   Fri, 24 Sep 2021 10:55:27 -0700
From:   Zvi Effron <zeffron@...tgames.com>
To:     Lorenz Bauer <lmb@...udflare.com>
Cc:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Lorenzo Bianconi <lbianconi@...hat.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        John Fastabend <john.fastabend@...il.com>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: Redux: Backwards compatibility for XDP multi-buff

On Fri, Sep 24, 2021 at 3:19 AM Lorenz Bauer <lmb@...udflare.com> wrote:
>
> On Thu, 23 Sept 2021 at 13:59, Toke Høiland-Jørgensen <toke@...hat.com> wrote:
> >
> > I don't think it has to be quite that bleak :)
> >
> > Specifically, there is no reason to block mb-aware programs from loading
> > even when the multi-buffer mode is disabled. So a migration plan would
> > look something like:
>
> ...
>
> > 2. Start porting all your XDP programs to make them mb-aware, and switch
> > their program type as you do. In many cases this is just a matter of
> > checking that the programs don't care about packet length. [...]
>
> Porting is only easy if we are guaranteed that the first PAGE_SIZE
> bytes (or whatever the current limit is) are available via ->data
> without trickery. Otherwise we have to convert all direct packet
> access to the new API, whatever that ends up being. It seemed to me
> like you were saying there is no such guarantee, and it could be
> driver dependent, which is the worst possible outcome imo. This is the
> status quo for TC classifiers, which is a great source of hard to
> diagnose bugs.
>
> > 3. Once all your programs have been ported and marked as such, flip the
> > sysctl. This will make the system start refusing to load any XDP
> > programs that are not mb-aware.
>
> By this you mean reboot the system and early in boot change the
> sysctl? That could work I guess.
>

I don't think a reboot is required. Just an unload of all
mb-unaware XDP programs to allow adjusting the sysctl via normal sysctl
changing methods (i.e. sysctl -w). Even if rebooting, it should just need an
entry in /etc/sysctl.conf, nothing necessarily early in the boot process. The
sysctl would only need to be set before the first load of an mn-unaware XDP
program, which, if everything is ported correctly, would never happen.

> > > 2. Add a compatibility shim for mb-unaware programs receiving an mb frame.
> > >
> > > We'd still need a way to indicate "MB-OK", but it could be a piece of
> > > metadata on a bpf_prog. Whatever code dispatches to an XDP program
> > > would have to include a prologue that linearises the xdp_buff if
> > > necessary which implies allocating memory. I don't know how hard it is
> > > to implement this.
> >
> > I think it would be somewhat non-trivial, and more importantly would
> > absolutely slaughter performance. And if you're using XDP, presumably
> > you care about that, so I'm not sure we're doing anyone any favours by
> > implementing such a compatibility layer?
>
> I see your point: having a single non-mb-aware program trash
> performance is bad for marketing. Better to not let people bump the
> MTU in that case.
>
> > > 3. Make non-linearity invisible to the BPF program
> > >
> > > Something I've wished for often is that I didn't have to deal with
> > > nonlinearity at all, based on my experience with cls_redirect [2].
> > > It's really hard to write a BPF program that handles non-linear skb,
> > > especially when you have to call adjust_head, etc. which invalidates
> > > packet buffers. This is probably impossible, but maybe someone has a
> > > crazy idea? :)
> >
> > With the other helpers that we started discussing, I don't think you
> > have to? I.e., with an xdp_load_bytes() or an xdp_data_pointer()-type
> > helper that works across fragment boundaries I think you'd be fine, no?
>
> I'll take a look!
>
> Lorenz
>
> --
> Lorenz Bauer | Systems Engineer
> 6th Floor, County Hall/The Riverside Building, SE1 7PB, UK
>
> www.cloudflare.com

Powered by blists - more mailing lists