From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Edward Cree <ecree@solarflare.com>,
John Fastabend <john.fastabend@gmail.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Alexei Starovoitov <ast@kernel.org>,
Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
Yonghong Song <yhs@fb.com>,
Marek Majkowski <marek@cloudflare.com>,
Lorenz Bauer <lmb@cloudflare.com>,
Alan Maguire <alan.maguire@oracle.com>,
Jesper Dangaard Brouer <brouer@redhat.com>,
David Miller <davem@davemloft.net>,
netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: static and dynamic linking. Was: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF
Date: Thu, 14 Nov 2019 16:41:01 +0100 [thread overview]
Message-ID: <87y2wimpo2.fsf@toke.dk> (raw)
In-Reply-To: <20191112195223.cp5kcmkko54dsfbg@ast-mbp.dhcp.thefacebook.com>
Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
[...]
> Back to your question of how fw2 will get loaded.. I'm thinking the following:
> 1. Static linking:
> obj = bpf_object__open("rootlet.o", "fw1.o", "fw2.o");
> // libbpf adjusts call offsets and links into single loadable bpf_object
> bpf_object__load(obj);
> bpf_set_link_xdp_fd()
> No kernel changes are necessary to support program chaining via static linking.
>
> 2. Dynamic linking:
> // assuming libxdp.so manages eth0
> rootlet_fd = get_xdp_fd(eth0);
> subprog_btf_id = libbpf_find_prog_btf_id("name_of_placeholder", roolet_fd);
> // ^ this function is in patch 16/18 of trampoline
> attr.attach_prog_fd = roolet_fd;
> attr.attach_btf_id = subprog_btf_id;
> // pair (prog_fd, btf_id) needs to be specified at load time
> obj = bpf_object__open("fw2.o", attr);
> bpf_object__load(obj);
> prog = bpf_object__find_program_by_title(obj);
> link = bpf_program__replace(prog); // similar to bpf_program__attach_trace()
> // no extra arguments during 'replace'.
> // Target (prog_fd, btf_id) already known to the kernel and verified
OK, this makes sense.
>> So the two component programs would still exist as kernel objects,
>> right?
>
> yes. Both fw1.o and fw2.o will be loaded and running instead of placeholders.
>
>> And the trampolines would keep individual stats for each one (if
>> BPF stats are enabled)?
>
> In case of dynamic linking both fw1.o and fw2.o will be seen as individual
> programs from 'bpftool p s' point of view. And both will have
> individual stats.
Right, this is important, and I think it's where my skepticism about
static linking comes from. With static linking, each XDP program will be
"reduced" to a subprog instead of a full stand-alone program. Which
means that its execution will be different depending on whether it is
just attached directly to an interface, or if it's been linked with a
rootlet before loading.
I'll admit I don't know enough about how subprograms actually work to
know if it's a *meaningful* difference, so I guess I'll go play around
with it. If nothing else, experimenting with static linking can be a way
to hash out the semantics until dynamic linking lands.
>> Could userspace also extract the prog IDs being
>> referenced by the "glue" proglet?
>
> Not sure I follow. Both fw1.o and fw2.o will have their own prog ids.
> fw1_prog->aux->linked_prog == rootlet_prog
> fw2_prog->aux->linked_prog == rootlet_prog
> Unloading and detaching fw1.o will make kernel to switch back to placeholder
> subprog in roolet_prog. I believe roolet_prog should not keep a list of progs
> that attached to it (or replaced its subprogs) to avoid circular
> dependency.
Well I did mean the link in the other direction. But thinking about it
some more, I don't think it really matters. The important bit is that
userspace can answer the question "given that rootlet ID X is currently
attached on eth0, which two program IDs Y and Z will actually run on
that interface?". And if there's a link in the other direction, it could
just iterate over all loaded programs in the kernel to find them, so
that is OK; as long as we can also tell in which "slot" in the rootlet a
given program is currently attached.
> Due to that detaching roolet_prog from netdev will stop the flow of
> packets into fw1.o, but refcnt of rootlet_prog will not go to zero, so
> it will stay in memory until both fw1.o and fw2.o detach from
> rootlet.o.
OK, that is probably fine. I think we should teach most utilities to
deal with this anyway; in particular, iproute2 should know about
multi-progs (i.e., link against libxdp).
>> What about attaching a third program? Would that work by recursion (as
>> above, but with the old proglet as old_fd), or should the library build
>> a whole new sequence from the component programs?
>
> This choice is up to libxdp.so. It can have a number of placeholders
> ready to be replaced by new progs. Or it can re-generate rootlet.o
> every time new fwX.o comes along. Short term I would start development
> with auto-generated roolet.o and static linking done by libbpf
> while the policy and roolet are done by libxdp.so, since this work
> doesn't depend on any kernel changes. Long term auto-generation
> can stay in libxdp.so if it turns out to be sufficient.
Yes, as I said above this sounds like at least it's a start.
>> Finally, what happens if someone where to try to attach a retprobe to
>> one of the component programs? Could it be possible to do that even
>> while program is being run from proglet dispatch? That way we can still
>> debug an individual XDP program even though it's run as part of a chain.
>
> Right. The fentry/fexit tracing is orthogonal to static/dynamic linking.
> It will be available for all prog types after trampoline patches land.
> See fexit_bpf2bpf.c example in the last 18/18 patch.
> We will be able to debug XDP program regardless whether it's a rootlet
> or a subprogram. Doesn't matter whether linking was static or dynamic.
OK, that's great, and certainly resolved one point of skepticism :)
> With fentry/fexit we will be able to do different stats too.
> Right now bpf program stats are limited to cycles and I resisted a lot
> of pressure to add more hard coded stats. With fentry/fexit we can
> collect arbitrary counters per program. Like number of L1-cache misses
> or number of TLB misses in a given XDP prog.
Yeah, that makes a lot of sense, of course. Great!
>> Sounds reasonable. Any reason libxdp.so couldn't be part of libbpf?
>
> libxdp.so is a policy specifier while libbpf is a tool. It makes more
> sense for them to be separate. libbpf has strong api compatibility
> guarantees. While I don't think anyone knows at this point how libxdp
> api should look and it will take some time for it to mature.
Well, we'd want libxdp to have the same strong API guarantees,
eventually. Which would be a reason to just include it in libbpf. But
sure, I wasn't suggesting to do this from the get-go; we can start out
with something separate and decide later when/if it makes sense to
integrate. As long as libbpf can do the heavy lifting on the actual
linking that is fine with me.
-Toke
next prev parent reply other threads:[~2019-11-14 15:41 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-07 17:20 [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through chain calls Toke Høiland-Jørgensen
2019-10-07 17:20 ` [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other Toke Høiland-Jørgensen
2019-10-07 20:42 ` Alexei Starovoitov
2019-10-08 8:07 ` Toke Høiland-Jørgensen
2019-10-09 1:51 ` Alexei Starovoitov
2019-10-09 8:03 ` Toke Høiland-Jørgensen
2019-10-10 4:41 ` Alexei Starovoitov
2019-10-14 12:35 ` Toke Høiland-Jørgensen
2019-10-14 17:08 ` John Fastabend
2019-10-14 18:48 ` Toke Høiland-Jørgensen
2019-10-15 16:30 ` Edward Cree
2019-10-15 16:42 ` Toke Høiland-Jørgensen
2019-10-15 18:33 ` Edward Cree
2019-10-17 12:11 ` Toke Høiland-Jørgensen
2019-10-22 17:27 ` Edward Cree
2019-10-22 18:07 ` Toke Høiland-Jørgensen
2019-11-12 2:51 ` static and dynamic linking. Was: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF Alexei Starovoitov
2019-11-12 16:20 ` Toke Høiland-Jørgensen
2019-11-12 19:52 ` Alexei Starovoitov
2019-11-12 21:25 ` Edward Cree
2019-11-12 23:18 ` Alexei Starovoitov
2019-11-13 18:30 ` Edward Cree
2019-11-13 18:51 ` Andrii Nakryiko
2019-11-15 2:13 ` Alexei Starovoitov
2019-11-15 16:56 ` John Fastabend
2019-11-12 23:25 ` John Fastabend
2019-11-13 0:21 ` Alexei Starovoitov
2019-11-13 5:33 ` John Fastabend
2019-11-15 1:50 ` Alexei Starovoitov
2019-11-15 16:39 ` John Fastabend
2019-11-14 15:41 ` Toke Høiland-Jørgensen [this message]
2019-11-12 16:32 ` Edward Cree
2019-11-15 11:48 ` Lorenz Bauer
2019-11-15 23:02 ` Alexei Starovoitov
2019-11-18 13:29 ` Lorenz Bauer
2019-10-21 23:51 ` [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other Edward Cree
2019-10-16 2:28 ` Alexei Starovoitov
2019-10-16 8:27 ` Jesper Dangaard Brouer
2019-10-16 10:35 ` Daniel Borkmann
2019-10-16 11:16 ` Toke Høiland-Jørgensen
2019-10-16 13:51 ` Toke Høiland-Jørgensen
2019-10-19 20:09 ` bpf indirect calls Alexei Starovoitov
2019-10-20 10:58 ` Toke Høiland-Jørgensen
2019-10-25 16:30 ` Alexei Starovoitov
2019-10-27 12:15 ` Toke Høiland-Jørgensen
2023-09-27 13:27 ` Matt Bobrowski
2023-09-29 21:06 ` Alexei Starovoitov
2023-10-02 18:50 ` Barret Rhoden
2023-10-06 9:36 ` Matt Bobrowski
2023-10-06 18:49 ` Alexei Starovoitov
2023-10-19 12:28 ` Matt Bobrowski
2019-10-09 10:19 ` [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other Jesper Dangaard Brouer
2019-10-09 17:57 ` Alexei Starovoitov
2019-10-07 17:20 ` [PATCH bpf-next v3 2/5] bpf: Add support for setting chain call sequence for programs Toke Høiland-Jørgensen
2019-10-07 20:38 ` Daniel Borkmann
2019-10-08 8:09 ` Toke Høiland-Jørgensen
2019-10-07 17:20 ` [PATCH bpf-next v3 3/5] tools: Update bpf.h header for program chain calls Toke Høiland-Jørgensen
2019-10-07 17:20 ` [PATCH bpf-next v3 4/5] libbpf: Add syscall wrappers for BPF_PROG_CHAIN_* commands Toke Høiland-Jørgensen
2019-10-07 17:20 ` [PATCH bpf-next v3 5/5] selftests: Add tests for XDP chain calls Toke Høiland-Jørgensen
2019-10-07 18:58 ` [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through " John Fastabend
2019-10-08 8:42 ` 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=87y2wimpo2.fsf@toke.dk \
--to=toke@redhat.com \
--cc=alan.maguire@oracle.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).