bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: John Fastabend <john.fastabend@gmail.com>,
	Edward Cree <ecree@solarflare.com>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Song Liu <songliubraving@fb.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>, Martin Lau <kafai@fb.com>,
	Yonghong Song <yhs@fb.com>,
	Marek Majkowski <marek@cloudflare.com>,
	Lorenz Bauer <lmb@cloudflare.com>,
	David Miller <davem@davemloft.net>,
	"netdev\@vger.kernel.org" <netdev@vger.kernel.org>,
	"bpf\@vger.kernel.org" <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next 0/9] xdp: Support multiple programs on a single interface through chain calls
Date: Fri, 04 Oct 2019 10:09:03 +0200	[thread overview]
Message-ID: <87tv8pnd9c.fsf@toke.dk> (raw)
In-Reply-To: <5d964d8ccfd90_55732aec43fe05c47b@john-XPS-13-9370.notmuch>

John Fastabend <john.fastabend@gmail.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

  reply	other threads:[~2019-10-04  8:09 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02 13:30 [PATCH bpf-next 0/9] xdp: Support multiple programs on a single interface through chain calls Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 1/9] hashtab: Add new bpf_map_fd_put_value op Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 2/9] xdp: Add new xdp_chain_map type for specifying XDP call sequences Toke Høiland-Jørgensen
2019-10-02 15:50   ` Lorenz Bauer
2019-10-02 18:25     ` Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 3/9] xdp: Support setting and getting device chain map Toke Høiland-Jørgensen
2019-10-02 15:50   ` Lorenz Bauer
2019-10-02 18:32     ` Toke Høiland-Jørgensen
2019-10-02 18:07   ` kbuild test robot
2019-10-02 18:29   ` kbuild test robot
2019-10-02 13:30 ` [PATCH bpf-next 4/9] xdp: Implement chain call logic to support multiple programs on one interface Toke Høiland-Jørgensen
2019-10-02 17:33   ` kbuild test robot
2019-10-02 17:53   ` kbuild test robot
2019-10-02 13:30 ` [PATCH bpf-next 5/9] tools/include/uapi: Add XDP chain map definitions Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 6/9] tools/libbpf_probes: Add support for xdp_chain map type Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 7/9] bpftool: Add definitions " Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 8/9] libbpf: Add support for setting and getting XDP chain maps Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 9/9] selftests: Add tests for XDP chain calls Toke Høiland-Jørgensen
2019-10-02 15:10 ` [PATCH bpf-next 0/9] xdp: Support multiple programs on a single interface through " Alan Maguire
2019-10-02 15:33   ` Toke Høiland-Jørgensen
2019-10-02 16:34     ` John Fastabend
2019-10-02 18:33       ` Toke Høiland-Jørgensen
2019-10-02 20:34         ` John Fastabend
2019-10-03  7:48           ` Toke Høiland-Jørgensen
2019-10-03 10:09             ` Jesper Dangaard Brouer
2019-10-03 19:45               ` John Fastabend
2019-10-02 16:35 ` Lorenz Bauer
2019-10-02 18:54   ` Toke Høiland-Jørgensen
2019-10-02 16:43 ` John Fastabend
2019-10-02 19:09   ` Toke Høiland-Jørgensen
2019-10-02 19:15   ` Daniel Borkmann
2019-10-02 19:29     ` Toke Høiland-Jørgensen
2019-10-02 19:46     ` Alexei Starovoitov
2019-10-03  7:58       ` Toke Høiland-Jørgensen
2019-10-02 18:38 ` Song Liu
2019-10-02 18:54   ` Song Liu
2019-10-02 19:25     ` Toke Høiland-Jørgensen
2019-10-03  8:53       ` Jesper Dangaard Brouer
2019-10-03 14:03         ` Alexei Starovoitov
2019-10-03 14:33           ` Toke Høiland-Jørgensen
2019-10-03 14:53             ` Edward Cree
2019-10-03 18:49               ` Jesper Dangaard Brouer
2019-10-03 19:35               ` John Fastabend
2019-10-04  8:09                 ` Toke Høiland-Jørgensen [this message]
2019-10-04 10:34                   ` Edward Cree
2019-10-04 15:58                     ` Lorenz Bauer
2019-10-07 16:43                       ` Edward Cree
2019-10-07 17:12                         ` Lorenz Bauer
2019-10-07 19:21                           ` Edward Cree
2019-10-07 21:01                         ` Alexei Starovoitov
2019-10-02 19:23   ` Toke Høiland-Jørgensen
2019-10-02 19:49     ` Song Liu
2019-10-03  7:59       ` Toke Høiland-Jørgensen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87tv8pnd9c.fsf@toke.dk \
    --to=toke@redhat.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=ecree@solarflare.com \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=lmb@cloudflare.com \
    --cc=marek@cloudflare.com \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).