From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Andrii Nakryiko <andriin@fb.com>, bpf <bpf@vger.kernel.org>,
Networking <netdev@vger.kernel.org>,
Alexei Starovoitov <ast@fb.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH bpf-next 2/3] libbpf: add bpf_link pinning/unpinning
Date: Mon, 02 Mar 2020 22:45:50 +0100 [thread overview]
Message-ID: <87o8tesa5t.fsf@toke.dk> (raw)
In-Reply-To: <CAEf4BzakHPjOHcyf=idh+kMUVhk0jr=Zqd2px8vfxU5N1MV3Rg@mail.gmail.com>
Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
> On Mon, Mar 2, 2020 at 2:17 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> Andrii Nakryiko <andriin@fb.com> writes:
>>
>> > With bpf_link abstraction supported by kernel explicitly, add
>> > pinning/unpinning API for links. Also allow to create (open) bpf_link from BPF
>> > FS file.
>> >
>> > This API allows to have an "ephemeral" FD-based BPF links (like raw tracepoint
>> > or fexit/freplace attachments) surviving user process exit, by pinning them in
>> > a BPF FS, which is an important use case for long-running BPF programs.
>> >
>> > As part of this, expose underlying FD for bpf_link. While legacy bpf_link's
>> > might not have a FD associated with them (which will be expressed as
>> > a bpf_link with fd=-1), kernel's abstraction is based around FD-based usage,
>> > so match it closely. This, subsequently, allows to have a generic
>> > pinning/unpinning API for generalized bpf_link. For some types of bpf_links
>> > kernel might not support pinning, in which case bpf_link__pin() will return
>> > error.
>> >
>> > With FD being part of generic bpf_link, also get rid of bpf_link_fd in favor
>> > of using vanialla bpf_link.
>> >
>> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>> > ---
>> > tools/lib/bpf/libbpf.c | 131 +++++++++++++++++++++++++++++++--------
>> > tools/lib/bpf/libbpf.h | 5 ++
>> > tools/lib/bpf/libbpf.map | 5 ++
>> > 3 files changed, 114 insertions(+), 27 deletions(-)
>> >
>> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> > index 996162801f7a..f8c4042e5855 100644
>> > --- a/tools/lib/bpf/libbpf.c
>> > +++ b/tools/lib/bpf/libbpf.c
>> > @@ -6931,6 +6931,8 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
>> > struct bpf_link {
>> > int (*detach)(struct bpf_link *link);
>> > int (*destroy)(struct bpf_link *link);
>> > + char *pin_path; /* NULL, if not pinned */
>> > + int fd; /* hook FD, -1 if not applicable */
>> > bool disconnected;
>> > };
>> >
>> > @@ -6960,26 +6962,109 @@ int bpf_link__destroy(struct bpf_link *link)
>> > err = link->detach(link);
>> > if (link->destroy)
>> > link->destroy(link);
>> > + if (link->pin_path)
>> > + free(link->pin_path);
>>
>> This will still detach the link even if it's pinned, won't it? What's
>
> No, this will just free pin_path string memory.
I meant the containing function; i.e., link->detach() call above will
close the fd.
>> the expectation, that the calling application just won't call
>> bpf_link__destroy() if it pins the link? But then it will leak memory?
>> Or is it just that __destroy() will close the fd, but if it's pinned the
>> kernel won't actually detach anything? In that case, it seems like the
>> function name becomes somewhat misleading?
>
> Yes, the latter, it will close its own FD, but if someone else has
> open other FD against the same bpf_link (due to pinning or if you
> shared FD with child process, etc), then kernel will keep it.
> bpf_link__destroy() is more of a "sever the link my process has" or
> "destroy my local link". Maybe not ideal name, but should be close
> enough, I think.
Hmm, yeah, OK, I guess I can live with it ;)
-Toke
next prev parent reply other threads:[~2020-03-02 21:45 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-28 22:39 [PATCH bpf-next 0/3] Introduce pinnable bpf_link kernel abstraction Andrii Nakryiko
2020-02-28 22:39 ` [PATCH bpf-next 1/3] bpf: introduce pinnable bpf_link abstraction Andrii Nakryiko
2020-03-02 10:13 ` Toke Høiland-Jørgensen
2020-03-02 18:06 ` Andrii Nakryiko
2020-03-02 21:40 ` Toke Høiland-Jørgensen
2020-03-02 23:37 ` Andrii Nakryiko
2020-03-03 2:50 ` Alexei Starovoitov
2020-03-03 4:18 ` Andrii Nakryiko
2020-02-28 22:39 ` [PATCH bpf-next 2/3] libbpf: add bpf_link pinning/unpinning Andrii Nakryiko
2020-03-02 10:16 ` Toke Høiland-Jørgensen
2020-03-02 18:09 ` Andrii Nakryiko
2020-03-02 21:45 ` Toke Høiland-Jørgensen [this message]
2020-02-28 22:39 ` [PATCH bpf-next 3/3] selftests/bpf: add link pinning selftests Andrii Nakryiko
2020-03-02 10:11 ` [PATCH bpf-next 0/3] Introduce pinnable bpf_link kernel abstraction Toke Høiland-Jørgensen
2020-03-02 18:05 ` Andrii Nakryiko
2020-03-02 22:24 ` Toke Høiland-Jørgensen
2020-03-02 23:35 ` Andrii Nakryiko
2020-03-03 8:12 ` Toke Høiland-Jørgensen
2020-03-03 8:12 ` Daniel Borkmann
2020-03-03 15:46 ` Alexei Starovoitov
2020-03-03 19:23 ` Daniel Borkmann
2020-03-03 19:46 ` Andrii Nakryiko
2020-03-03 20:24 ` Toke Høiland-Jørgensen
2020-03-03 20:53 ` Daniel Borkmann
2020-03-03 22:01 ` Alexei Starovoitov
2020-03-03 22:27 ` Toke Høiland-Jørgensen
2020-03-04 4:36 ` Alexei Starovoitov
2020-03-04 7:47 ` Toke Høiland-Jørgensen
2020-03-04 15:47 ` Alexei Starovoitov
2020-03-05 10:37 ` Toke Høiland-Jørgensen
2020-03-05 16:34 ` Alexei Starovoitov
2020-03-05 22:34 ` Daniel Borkmann
2020-03-05 22:50 ` Alexei Starovoitov
2020-03-05 23:42 ` Daniel Borkmann
2020-03-06 8:31 ` Toke Høiland-Jørgensen
2020-03-06 10:25 ` Daniel Borkmann
2020-03-06 10:42 ` Toke Høiland-Jørgensen
2020-03-06 18:09 ` David Ahern
2020-03-04 19:41 ` Jakub Kicinski
2020-03-04 20:45 ` Alexei Starovoitov
2020-03-04 21:24 ` Jakub Kicinski
2020-03-05 1:07 ` Alexei Starovoitov
2020-03-05 8:16 ` Jakub Kicinski
2020-03-05 11:05 ` Toke Høiland-Jørgensen
2020-03-05 18:13 ` Jakub Kicinski
2020-03-09 11:41 ` Toke Høiland-Jørgensen
2020-03-09 18:50 ` Jakub Kicinski
2020-03-10 12:22 ` Toke Høiland-Jørgensen
2020-03-05 16:39 ` Alexei Starovoitov
2020-03-03 22:40 ` Jakub Kicinski
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=87o8tesa5t.fsf@toke.dk \
--to=toke@redhat.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andriin@fb.com \
--cc=ast@fb.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=netdev@vger.kernel.org \
/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).