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 5/6] libbpf: add support for bpf_link-based cgroup attachment
Date: Mon, 23 Mar 2020 20:31:03 +0100 [thread overview]
Message-ID: <87blom3m2w.fsf@toke.dk> (raw)
In-Reply-To: <CAEf4Bzbt7-A+2dH0kSAM11mjwX+ZDV8JBhJZDArAU=Q9+y79mw@mail.gmail.com>
Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
> On Mon, Mar 23, 2020 at 4:02 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> Andrii Nakryiko <andriin@fb.com> writes:
>>
>> > Add bpf_program__attach_cgroup(), which uses BPF_LINK_CREATE subcommand to
>> > create an FD-based kernel bpf_link. Also add low-level bpf_link_create() API.
>> >
>> > If expected_attach_type is not specified explicitly with
>> > bpf_program__set_expected_attach_type(), libbpf will try to determine proper
>> > attach type from BPF program's section definition.
>> >
>> > Also add support for bpf_link's underlying BPF program replacement:
>> > - unconditional through high-level bpf_link__update_program() API;
>> > - cmpxchg-like with specifying expected current BPF program through
>> > low-level bpf_link_update() API.
>> >
>> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>> > ---
>> > tools/include/uapi/linux/bpf.h | 12 +++++++++
>> > tools/lib/bpf/bpf.c | 34 +++++++++++++++++++++++++
>> > tools/lib/bpf/bpf.h | 19 ++++++++++++++
>> > tools/lib/bpf/libbpf.c | 46 ++++++++++++++++++++++++++++++++++
>> > tools/lib/bpf/libbpf.h | 8 +++++-
>> > tools/lib/bpf/libbpf.map | 4 +++
>> > 6 files changed, 122 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
>> > index fad9f79bb8f1..fa944093f9fc 100644
>> > --- a/tools/include/uapi/linux/bpf.h
>> > +++ b/tools/include/uapi/linux/bpf.h
>> > @@ -112,6 +112,7 @@ enum bpf_cmd {
>> > BPF_MAP_UPDATE_BATCH,
>> > BPF_MAP_DELETE_BATCH,
>> > BPF_LINK_CREATE,
>> > + BPF_LINK_UPDATE,
>> > };
>> >
>> > enum bpf_map_type {
>> > @@ -574,6 +575,17 @@ union bpf_attr {
>> > __u32 target_fd; /* object to attach to */
>> > __u32 attach_type; /* attach type */
>> > } link_create;
>> > +
>> > + struct { /* struct used by BPF_LINK_UPDATE command */
>> > + __u32 link_fd; /* link fd */
>> > + /* new program fd to update link with */
>> > + __u32 new_prog_fd;
>> > + __u32 flags; /* extra flags */
>> > + /* expected link's program fd; is specified only if
>> > + * BPF_F_REPLACE flag is set in flags */
>> > + __u32 old_prog_fd;
>> > + } link_update;
>> > +
>> > } __attribute__((aligned(8)));
>> >
>> > /* The description below is an attempt at providing documentation to eBPF
>> > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>> > index c6dafe563176..35c34fc81bd0 100644
>> > --- a/tools/lib/bpf/bpf.c
>> > +++ b/tools/lib/bpf/bpf.c
>> > @@ -584,6 +584,40 @@ int bpf_prog_detach2(int prog_fd, int target_fd, enum bpf_attach_type type)
>> > return sys_bpf(BPF_PROG_DETACH, &attr, sizeof(attr));
>> > }
>> >
>> > +int bpf_link_create(int prog_fd, int target_fd,
>> > + enum bpf_attach_type attach_type,
>> > + const struct bpf_link_create_opts *opts)
>> > +{
>> > + union bpf_attr attr;
>> > +
>> > + if (!OPTS_VALID(opts, bpf_link_create_opts))
>> > + return -EINVAL;
>> > +
>> > + memset(&attr, 0, sizeof(attr));
>> > + attr.link_create.prog_fd = prog_fd;
>> > + attr.link_create.target_fd = target_fd;
>> > + attr.link_create.attach_type = attach_type;
>> > +
>> > + return sys_bpf(BPF_LINK_CREATE, &attr, sizeof(attr));
>> > +}
>> > +
>> > +int bpf_link_update(int link_fd, int new_prog_fd,
>> > + const struct bpf_link_update_opts *opts)
>> > +{
>> > + union bpf_attr attr;
>> > +
>> > + if (!OPTS_VALID(opts, bpf_link_update_opts))
>> > + return -EINVAL;
>> > +
>> > + memset(&attr, 0, sizeof(attr));
>> > + attr.link_update.link_fd = link_fd;
>> > + attr.link_update.new_prog_fd = new_prog_fd;
>> > + attr.link_update.flags = OPTS_GET(opts, flags, 0);
>> > + attr.link_update.old_prog_fd = OPTS_GET(opts, old_prog_fd, 0);
>> > +
>> > + return sys_bpf(BPF_LINK_UPDATE, &attr, sizeof(attr));
>> > +}
>> > +
>> > int bpf_prog_query(int target_fd, enum bpf_attach_type type, __u32 query_flags,
>> > __u32 *attach_flags, __u32 *prog_ids, __u32 *prog_cnt)
>> > {
>> > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
>> > index b976e77316cc..46d47afdd887 100644
>> > --- a/tools/lib/bpf/bpf.h
>> > +++ b/tools/lib/bpf/bpf.h
>> > @@ -168,6 +168,25 @@ LIBBPF_API int bpf_prog_detach(int attachable_fd, enum bpf_attach_type type);
>> > LIBBPF_API int bpf_prog_detach2(int prog_fd, int attachable_fd,
>> > enum bpf_attach_type type);
>> >
>> > +struct bpf_link_create_opts {
>> > + size_t sz; /* size of this struct for forward/backward compatibility */
>> > +};
>> > +#define bpf_link_create_opts__last_field sz
>> > +
>> > +LIBBPF_API int bpf_link_create(int prog_fd, int target_fd,
>> > + enum bpf_attach_type attach_type,
>> > + const struct bpf_link_create_opts *opts);
>> > +
>> > +struct bpf_link_update_opts {
>> > + size_t sz; /* size of this struct for forward/backward compatibility */
>> > + __u32 flags; /* extra flags */
>> > + __u32 old_prog_fd; /* expected old program FD */
>> > +};
>> > +#define bpf_link_update_opts__last_field old_prog_fd
>> > +
>> > +LIBBPF_API int bpf_link_update(int link_fd, int new_prog_fd,
>> > + const struct bpf_link_update_opts *opts);
>> > +
>> > struct bpf_prog_test_run_attr {
>> > int prog_fd;
>> > int repeat;
>> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> > index 085e41f9b68e..8b23c70033d3 100644
>> > --- a/tools/lib/bpf/libbpf.c
>> > +++ b/tools/lib/bpf/libbpf.c
>> > @@ -6951,6 +6951,12 @@ struct bpf_link {
>> > bool disconnected;
>> > };
>> >
>> > +/* Replace link's underlying BPF program with the new one */
>> > +int bpf_link__update_program(struct bpf_link *link, struct bpf_program *prog)
>> > +{
>> > + return bpf_link_update(bpf_link__fd(link), bpf_program__fd(prog), NULL);
>> > +}
>>
>> I would expect bpf_link to keep track of the previous program and
>> automatically fill it in with this operation. I.e., it should be
>> possible to do something like:
>>
>> link = bpf_link__open("/sys/fs/bpf/my_link");
>> prog = bpf_link__get_prog(link);
>
> I don't think libbpf is able to construct struct bpf_program from link
> info. It can get program FD, of course, but struct bpf_program is much
> more than that and not sure kernel has all the necessary info. Some
> parts of bpf_program is coming from ELF file, which is gone by this
> time.
Hmm, sure, maybe, but it could still get enough information (such as the
prog fd, and everything returned by GET_PROG_INFO) for userspace could
do something meaningful with the result. So that would turn the above
into bpf_link__get_prog_fd(), and struct bpf_link would contain the fd
of the currently-attached program so it can be supplied in any future
replacement calls.
-Toke
next prev parent reply other threads:[~2020-03-23 19:31 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-20 20:36 [PATCH bpf-next 0/6] Add support for cgroup bpf_link Andrii Nakryiko
2020-03-20 20:36 ` [PATCH bpf-next 1/6] bpf: factor out cgroup storages operations Andrii Nakryiko
2020-03-20 20:36 ` [PATCH bpf-next 2/6] bpf: factor out attach_type to prog_type mapping for attach/detach Andrii Nakryiko
2020-03-20 20:36 ` [PATCH bpf-next 3/6] bpf: implement bpf_link-based cgroup BPF program attachment Andrii Nakryiko
2020-03-20 21:33 ` Stanislav Fomichev
2020-03-20 21:47 ` Andrii Nakryiko
2020-03-20 23:46 ` [Potential Spoof] " Andrey Ignatov
2020-03-21 0:19 ` Andrii Nakryiko
2020-03-23 18:03 ` Andrii Nakryiko
2020-03-20 20:36 ` [PATCH bpf-next 4/6] bpf: implement bpf_prog replacement for an active bpf_cgroup_link Andrii Nakryiko
2020-03-21 0:33 ` kbuild test robot
2020-03-21 0:33 ` kbuild test robot
2020-03-21 0:58 ` Andrii Nakryiko
2020-03-21 1:28 ` kbuild test robot
2020-03-21 1:28 ` kbuild test robot
2020-03-23 10:58 ` Toke Høiland-Jørgensen
2020-03-23 17:55 ` Andrii Nakryiko
2020-03-23 19:29 ` Toke Høiland-Jørgensen
2020-03-20 20:36 ` [PATCH bpf-next 5/6] libbpf: add support for bpf_link-based cgroup attachment Andrii Nakryiko
2020-03-23 11:02 ` Toke Høiland-Jørgensen
2020-03-23 17:58 ` Andrii Nakryiko
2020-03-23 19:31 ` Toke Høiland-Jørgensen [this message]
2020-03-24 23:05 ` Andrii Nakryiko
2020-03-20 20:36 ` [PATCH bpf-next 6/6] selftests/bpf: test FD-based " Andrii Nakryiko
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=87blom3m2w.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.