diff mbox series

[bpf-next,08/29] bpf: Keep active attached trampoline in bpf_prog

Message ID 20211118112455.475349-9-jolsa@kernel.org (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series bpf: Add batch support for attaching trampolines | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next fail VM_Test
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count fail Series longer than 15 patches (and no cover letter)
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 12429 this patch: 12429
netdev/cc_maintainers warning 1 maintainers not CCed: kpsingh@kernel.org
netdev/build_clang success Errors and warnings before: 2101 this patch: 2101
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 11594 this patch: 11594
netdev/checkpatch warning WARNING: From:/Signed-off-by: email address mismatch: 'From: Jiri Olsa <jolsa@redhat.com>' != 'Signed-off-by: Jiri Olsa <jolsa@kernel.org>'
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jiri Olsa Nov. 18, 2021, 11:24 a.m. UTC 
Keeping active attached trampoline in bpf_prog so it can be used
in following changes to account for multiple functions attachments
in program.

As EXT programs are not going to be supported in multiple functions
attachment for now, I'm keeping them stored in link.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/linux/bpf.h  |  1 +
 kernel/bpf/syscall.c | 34 +++++++++++++++++++++++++++++-----
 2 files changed, 30 insertions(+), 5 deletions(-)

Comments

Andrii Nakryiko Nov. 24, 2021, 9:48 p.m. UTC | #1 
On Thu, Nov 18, 2021 at 3:25 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> Keeping active attached trampoline in bpf_prog so it can be used
> in following changes to account for multiple functions attachments
> in program.
>
> As EXT programs are not going to be supported in multiple functions
> attachment for now, I'm keeping them stored in link.

can the same EXT program be attached twice? If not, why can't you just
use the same prog->aux->trampoline instead of the if/else everywhere?

>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  include/linux/bpf.h  |  1 +
>  kernel/bpf/syscall.c | 34 +++++++++++++++++++++++++++++-----
>  2 files changed, 30 insertions(+), 5 deletions(-)
>
Jiri Olsa Nov. 28, 2021, 5:24 p.m. UTC | #2 
On Wed, Nov 24, 2021 at 01:48:09PM -0800, Andrii Nakryiko wrote:
> On Thu, Nov 18, 2021 at 3:25 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > Keeping active attached trampoline in bpf_prog so it can be used
> > in following changes to account for multiple functions attachments
> > in program.
> >
> > As EXT programs are not going to be supported in multiple functions
> > attachment for now, I'm keeping them stored in link.
> 
> can the same EXT program be attached twice? If not, why can't you just
> use the same prog->aux->trampoline instead of the if/else everywhere?

I recall that was my initial change, but it was clashing with
fentry/fexit programs because extensions are special

I'll re-check and try to make this generic

jirka

> 
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  include/linux/bpf.h  |  1 +
> >  kernel/bpf/syscall.c | 34 +++++++++++++++++++++++++++++-----
> >  2 files changed, 30 insertions(+), 5 deletions(-)
> >
>
diff mbox series

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 77c76e2fa9ff..c93c629b5725 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -872,6 +872,7 @@  struct bpf_prog_aux {
 	struct mutex dst_mutex; /* protects dst_* pointers below, *after* prog becomes visible */
 	struct bpf_prog *dst_prog;
 	struct bpf_trampoline *dst_trampoline;
+	struct bpf_trampoline *trampoline;
 	enum bpf_prog_type saved_dst_prog_type;
 	enum bpf_attach_type saved_dst_attach_type;
 	bool verifier_zext; /* Zero extensions has been inserted by verifier. */
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 50f96ea4452a..0df8b2f3d982 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2623,15 +2623,28 @@  struct bpf_tracing_link {
 	struct bpf_prog *tgt_prog;
 };
 
+static struct bpf_trampoline *link_trampoline(struct bpf_tracing_link *link)
+{
+	struct bpf_prog *prog = link->link.prog;
+
+	if (prog->type == BPF_PROG_TYPE_EXT)
+		return link->trampoline;
+	else
+		return prog->aux->trampoline;
+}
+
 static void bpf_tracing_link_release(struct bpf_link *link)
 {
 	struct bpf_tracing_link *tr_link =
 		container_of(link, struct bpf_tracing_link, link);
+	struct bpf_trampoline *tr = link_trampoline(tr_link);
+	struct bpf_prog *prog = link->prog;
 
-	WARN_ON_ONCE(bpf_trampoline_unlink_prog(link->prog,
-						tr_link->trampoline));
+	WARN_ON_ONCE(bpf_trampoline_unlink_prog(link->prog, tr));
 
-	bpf_trampoline_put(tr_link->trampoline);
+	if (prog->type != BPF_PROG_TYPE_EXT)
+		prog->aux->trampoline = NULL;
+	bpf_trampoline_put(tr);
 
 	/* tgt_prog is NULL if target is a kernel function */
 	if (tr_link->tgt_prog)
@@ -2662,9 +2675,10 @@  static int bpf_tracing_link_fill_link_info(const struct bpf_link *link,
 {
 	struct bpf_tracing_link *tr_link =
 		container_of(link, struct bpf_tracing_link, link);
+	struct bpf_trampoline *tr = link_trampoline(tr_link);
 
 	info->tracing.attach_type = tr_link->attach_type;
-	bpf_trampoline_unpack_key(tr_link->trampoline->key,
+	bpf_trampoline_unpack_key(tr->key,
 				  &info->tracing.target_obj_id,
 				  &info->tracing.target_btf_id);
 
@@ -2682,6 +2696,7 @@  static int bpf_tracing_prog_attach(struct bpf_prog *prog,
 				   int tgt_prog_fd,
 				   u32 btf_id)
 {
+	bool prog_extension = prog->type == BPF_PROG_TYPE_EXT;
 	struct bpf_link_primer link_primer;
 	struct bpf_prog *tgt_prog = NULL;
 	struct bpf_trampoline *tr = NULL;
@@ -2748,6 +2763,11 @@  static int bpf_tracing_prog_attach(struct bpf_prog *prog,
 
 	mutex_lock(&prog->aux->dst_mutex);
 
+	if (!prog_extension && prog->aux->trampoline) {
+		err = -EBUSY;
+		goto out_unlock;
+	}
+
 	/* There are a few possible cases here:
 	 *
 	 * - if prog->aux->dst_trampoline is set, the program was just loaded
@@ -2824,7 +2844,11 @@  static int bpf_tracing_prog_attach(struct bpf_prog *prog,
 	}
 
 	link->tgt_prog = tgt_prog;
-	link->trampoline = tr;
+
+	if (prog_extension)
+		link->trampoline = tr;
+	else
+		prog->aux->trampoline = tr;
 
 	/* Always clear the trampoline and target prog from prog->aux to make
 	 * sure the original attach destination is not kept alive after a