netdev - Re: [PATCH bpf-next 0/9] xdp: Support multiple programs on a single interface through chain calls

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: <87tv8pnd9c.fsf@toke.dk>
Date:   Fri, 04 Oct 2019 10:09:03 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     John Fastabend <john.fastabend@...il.com>,
        Edward Cree <ecree@...arflare.com>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     Song Liu <songliubraving@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>, Martin Lau <kafai@...com>,
        Yonghong Song <yhs@...com>,
        Marek Majkowski <marek@...udflare.com>,
        Lorenz Bauer <lmb@...udflare.com>,
        David Miller <davem@...emloft.net>,
        "netdev\@vger.kernel.org" <netdev@...r.kernel.org>,
        "bpf\@vger.kernel.org" <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next 0/9] xdp: Support multiple programs on a single interface through chain calls

John Fastabend <john.fastabend@...il.com> writes:

> Edward Cree wrote:
>> On 03/10/2019 15:33, Toke Høiland-Jørgensen wrote:
>> > In all cases, the sysadmin can't (or doesn't want to) modify any of the
>> > XDP programs. In fact, they may just be installed as pre-compiled .so
>> > BPF files on his system. So he needs to be able to configure the call
>> > chain of different programs without modifying the eBPF program source
>> > code.
>> Perhaps I'm being dumb, but can't we solve this if we make linking work?
>> I.e. myIDS.so has ids_main() function, myFirewall.so has firewall()
>>  function, and sysadmin writes a little XDP prog to call these:
>> 
>> int main(struct xdp_md *ctx)
>> {
>>         int rc = firewall(ctx), rc2;
>> 
>>         switch(rc) {
>>         case XDP_DROP:
>>         case XDP_ABORTED:
>>         default:
>>                 return rc;
>>         case XDP_PASS:
>>                 return ids_main(ctx);
>>         case XDP_TX:
>>         case XDP_REDIRECT:
>>                 rc2 = ids_main(ctx);
>>                 if (rc2 == XDP_PASS)
>>                         return rc;
>>                 return rc2;
>>         }
>> }
>> 
>> Now he compiles this and links it against those .so files, giving him
>>  a new object file which he can then install.
>> 
>> (One problem which does spring to mind is that the .so files may very
>>  inconsiderately both name their entry points main(), which makes
>>  linking against both of them rather challenging.  But I think that
>>  can be worked around with a sufficiently clever linker).
>
> I agree but the same could be done today if ids_main and firewall
> were inline functions. Admin can write their little program like above
> and just '#include firewall', '#include ids'. Then you don't need
> linking although it does make things nicer.

(Replying to both you and Edward here, as what you suggested in your
other email is basically the same).

Yes, you are right that we could technically do this entirely in
userspace with a sufficiently smart loader. This was even in the
presentation at LPC (see slide 34: "Implementation option #1: userspace
only").

The reason we want to add kernel support is that it solves several
important problems:

- Centralisation: To do this entirely in userspace, everything has to go
  through the same loader that builds the "dispatcher program".

- Enforcement: Related to the point above - if an XDP-enabled
  application loads its own XDP program without going through the
  central loader, we can't load another program after that.

- State inspection: If userspace builds and loads a single dispatch
  program, it is difficult to go from that back to the call sequence
  graph.

- Dynamic changes: While the dispatch program can be rebuilt from a
  config file, it becomes difficult to do dynamic changes to the
  sequence (such as temporarily attaching xdpdump to the XDP_DROP action
  of this already-loaded program while debugging).

Having the mechanism be in-kernel makes solving these problems a lot
easier, because the kernel can be responsible for state management, and
it can enforce the chain call execution logic.

The fact that Lorenz et al are interested in this feature (even though
they are essentially already doing what you suggested, by having a
centralised daemon to manage all XDP programs), tells me that having
kernel support for this is the right thing to do.

-Toke