bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH bpf-next 0/2] Priorities for bpf progs attached to the same tracepoint
@ 2022-04-03 16:07 Dmitrii Dolgov
  2022-04-03 16:07 ` [RFC PATCH bpf-next 1/2] bpf: tracing: Introduce prio field for bpf_prog Dmitrii Dolgov
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Dmitrii Dolgov @ 2022-04-03 16:07 UTC (permalink / raw)
  To: bpf, ast, andrii, yhs, songliubraving; +Cc: Dmitrii Dolgov

With growing number of various products and tools using BPF it could
easily happen that multiple BPF programs from different processes will
be attached to the same tracepoint. It seems that in such case there is
no way to specify a custom order in which those programs may want to be
executed -- it will depend on the order in which they were attached.

Consider an example when the BPF program A is attached to tracepoint T,
the BFP program B needs to be attached to the T as well and start
before/end after the A (e.g. to monitor the whole process of A +
tracepoint in some way).  If the program A could not be changed and is
attached before B, the order specified above will not be possible.

One way to address this in a limited, but less invasive way is to
utilize link options structure to pass the desired priority to
perf_event_set_bpf_prog, and add new bpf_prog into the bpf_prog_array
based on its value. This will allow to specify the priority value via
bpf_tracepoint_opts when attaching a new prog.

Does this make sense? There maybe a better way to achieve this, I would
be glad to hear any feedback on it.

Dmitrii Dolgov (2):
  bpf: tracing: Introduce prio field for bpf_prog
  libbpf: Allow setting bpf_prog priority

 drivers/media/rc/bpf-lirc.c    |  4 ++--
 include/linux/bpf.h            |  3 ++-
 include/linux/trace_events.h   |  7 ++++---
 include/uapi/linux/bpf.h       |  1 +
 kernel/bpf/core.c              | 19 +++++++++++++++++--
 kernel/bpf/syscall.c           |  3 ++-
 kernel/events/core.c           |  8 ++++----
 kernel/trace/bpf_trace.c       |  8 +++++---
 tools/include/uapi/linux/bpf.h |  1 +
 tools/lib/bpf/bpf.c            |  1 +
 tools/lib/bpf/bpf.h            |  1 +
 tools/lib/bpf/libbpf.c         |  4 +++-
 tools/lib/bpf/libbpf.h         |  6 ++++--
 13 files changed, 47 insertions(+), 19 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [RFC PATCH bpf-next 1/2] bpf: tracing: Introduce prio field for bpf_prog
  2022-04-03 16:07 [RFC PATCH bpf-next 0/2] Priorities for bpf progs attached to the same tracepoint Dmitrii Dolgov
@ 2022-04-03 16:07 ` Dmitrii Dolgov
  2022-04-03 16:07 ` [RFC PATCH bpf-next 2/2] libbpf: Allow setting bpf_prog priority Dmitrii Dolgov
  2022-04-04  0:17 ` [RFC PATCH bpf-next 0/2] Priorities for bpf progs attached to the same tracepoint Andrii Nakryiko
  2 siblings, 0 replies; 7+ messages in thread
From: Dmitrii Dolgov @ 2022-04-03 16:07 UTC (permalink / raw)
  To: bpf, ast, andrii, yhs, songliubraving; +Cc: Dmitrii Dolgov

Add prio field into bpf_prog_array_item. The field signals what is the
priority of the item in the array. Use it in bpf_prog_array_copy to
place the new item in the array according to the priority. The change
doesn't cover bpf_prog_array_update_at yet.

Signed-off-by: Dmitrii Dolgov <9erthalion6@gmail.com>
---
 drivers/media/rc/bpf-lirc.c    |  4 ++--
 include/linux/bpf.h            |  3 ++-
 include/linux/trace_events.h   |  7 ++++---
 include/uapi/linux/bpf.h       |  1 +
 kernel/bpf/core.c              | 19 +++++++++++++++++--
 kernel/bpf/syscall.c           |  2 +-
 kernel/events/core.c           |  8 ++++----
 kernel/trace/bpf_trace.c       |  8 +++++---
 tools/include/uapi/linux/bpf.h |  1 +
 9 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/drivers/media/rc/bpf-lirc.c b/drivers/media/rc/bpf-lirc.c
index 3eff08d7b8e5..b240149bd004 100644
--- a/drivers/media/rc/bpf-lirc.c
+++ b/drivers/media/rc/bpf-lirc.c
@@ -160,7 +160,7 @@ static int lirc_bpf_attach(struct rc_dev *rcdev, struct bpf_prog *prog)
 		goto unlock;
 	}
 
-	ret = bpf_prog_array_copy(old_array, NULL, prog, 0, &new_array);
+	ret = bpf_prog_array_copy(old_array, NULL, prog, 0, 0, &new_array);
 	if (ret < 0)
 		goto unlock;
 
@@ -193,7 +193,7 @@ static int lirc_bpf_detach(struct rc_dev *rcdev, struct bpf_prog *prog)
 	}
 
 	old_array = lirc_rcu_dereference(raw->progs);
-	ret = bpf_prog_array_copy(old_array, prog, NULL, 0, &new_array);
+	ret = bpf_prog_array_copy(old_array, prog, NULL, 0, 0, &new_array);
 	/*
 	 * Do not use bpf_prog_array_delete_safe() as we would end up
 	 * with a dummy entry in the array, and the we would free the
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index bdb5298735ce..f00ac9e5bfa2 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1235,6 +1235,7 @@ struct bpf_prog_array_item {
 	union {
 		struct bpf_cgroup_storage *cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE];
 		u64 bpf_cookie;
+		u32 prio;
 	};
 };
 
@@ -1274,7 +1275,7 @@ int bpf_prog_array_copy_info(struct bpf_prog_array *array,
 int bpf_prog_array_copy(struct bpf_prog_array *old_array,
 			struct bpf_prog *exclude_prog,
 			struct bpf_prog *include_prog,
-			u64 bpf_cookie,
+			u64 bpf_cookie, u32 prio,
 			struct bpf_prog_array **new_array);
 
 struct bpf_run_ctx {};
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index e6e95a9f07a5..06996f85def8 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -736,7 +736,7 @@ trace_trigger_soft_disabled(struct trace_event_file *file)
 
 #ifdef CONFIG_BPF_EVENTS
 unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx);
-int perf_event_attach_bpf_prog(struct perf_event *event, struct bpf_prog *prog, u64 bpf_cookie);
+int perf_event_attach_bpf_prog(struct perf_event *event, struct bpf_prog *prog, u64 bpf_cookie, u32 prio);
 void perf_event_detach_bpf_prog(struct perf_event *event);
 int perf_event_query_prog_array(struct perf_event *event, void __user *info);
 int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog);
@@ -754,7 +754,7 @@ static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *c
 }
 
 static inline int
-perf_event_attach_bpf_prog(struct perf_event *event, struct bpf_prog *prog, u64 bpf_cookie)
+perf_event_attach_bpf_prog(struct perf_event *event, struct bpf_prog *prog, u64 bpf_cookie, u32 prio)
 {
 	return -EOPNOTSUPP;
 }
@@ -871,7 +871,8 @@ extern void ftrace_profile_free_filter(struct perf_event *event);
 void perf_trace_buf_update(void *record, u16 type);
 void *perf_trace_buf_alloc(int size, struct pt_regs **regs, int *rctxp);
 
-int perf_event_set_bpf_prog(struct perf_event *event, struct bpf_prog *prog, u64 bpf_cookie);
+int perf_event_set_bpf_prog(struct perf_event *event, struct bpf_prog *prog,
+			u64 bpf_cookie, u32 prio);
 void perf_event_free_bpf_prog(struct perf_event *event);
 
 void bpf_trace_run1(struct bpf_prog *prog, u64 arg1);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index d14b10b85e51..10054c034518 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1481,6 +1481,7 @@ union bpf_attr {
 				 * accessible through bpf_get_attach_cookie() BPF helper
 				 */
 				__u64		bpf_cookie;
+				__u32		prio;
 			} perf_event;
 			struct {
 				__u32		flags;
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 13e9dbeeedf3..8a89cc69c74b 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2405,13 +2405,14 @@ int bpf_prog_array_update_at(struct bpf_prog_array *array, int index,
 int bpf_prog_array_copy(struct bpf_prog_array *old_array,
 			struct bpf_prog *exclude_prog,
 			struct bpf_prog *include_prog,
-			u64 bpf_cookie,
+			u64 bpf_cookie, u32 prio,
 			struct bpf_prog_array **new_array)
 {
 	int new_prog_cnt, carry_prog_cnt = 0;
 	struct bpf_prog_array_item *existing, *new;
 	struct bpf_prog_array *array;
 	bool found_exclude = false;
+	bool found_less_prio = false;
 
 	/* Figure out how many existing progs we need to carry over to
 	 * the new array.
@@ -2458,16 +2459,30 @@ int bpf_prog_array_copy(struct bpf_prog_array *old_array,
 			    existing->prog == &dummy_bpf_prog.prog)
 				continue;
 
+			if (include_prog && existing->prio <= prio) {
+				found_less_prio = true;
+
+				new->prog = include_prog;
+				new->prio = prio;
+				new->bpf_cookie = bpf_cookie;
+
+				new++;
+			}
+
 			new->prog = existing->prog;
 			new->bpf_cookie = existing->bpf_cookie;
+			new->prio = existing->prio;
 			new++;
 		}
 	}
-	if (include_prog) {
+
+	if (include_prog && !found_less_prio) {
 		new->prog = include_prog;
 		new->bpf_cookie = bpf_cookie;
+		new->prio = prio;
 		new++;
 	}
+
 	new->prog = NULL;
 	*new_array = array;
 	return 0;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index cdaa1152436a..72fb3d2c30a4 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3009,7 +3009,7 @@ static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *pro
 	}
 
 	event = perf_file->private_data;
-	err = perf_event_set_bpf_prog(event, prog, attr->link_create.perf_event.bpf_cookie);
+	err = perf_event_set_bpf_prog(event, prog, attr->link_create.perf_event.bpf_cookie, 0);
 	if (err) {
 		bpf_link_cleanup(&link_primer);
 		goto out_put_file;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index cfde994ce61c..283464c870f2 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5753,7 +5753,7 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
 		if (IS_ERR(prog))
 			return PTR_ERR(prog);
 
-		err = perf_event_set_bpf_prog(event, prog, 0);
+		err = perf_event_set_bpf_prog(event, prog, 0, 0);
 		if (err) {
 			bpf_prog_put(prog);
 			return err;
@@ -10172,7 +10172,7 @@ static inline bool perf_event_is_tracing(struct perf_event *event)
 }
 
 int perf_event_set_bpf_prog(struct perf_event *event, struct bpf_prog *prog,
-			    u64 bpf_cookie)
+			    u64 bpf_cookie, u32 prio)
 {
 	bool is_kprobe, is_tracepoint, is_syscall_tp;
 
@@ -10203,7 +10203,7 @@ int perf_event_set_bpf_prog(struct perf_event *event, struct bpf_prog *prog,
 			return -EACCES;
 	}
 
-	return perf_event_attach_bpf_prog(event, prog, bpf_cookie);
+	return perf_event_attach_bpf_prog(event, prog, bpf_cookie, prio);
 }
 
 void perf_event_free_bpf_prog(struct perf_event *event)
@@ -10226,7 +10226,7 @@ static void perf_event_free_filter(struct perf_event *event)
 }
 
 int perf_event_set_bpf_prog(struct perf_event *event, struct bpf_prog *prog,
-			    u64 bpf_cookie)
+			    u64 bpf_cookie, u32 prio)
 {
 	return -ENOENT;
 }
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 836f021cb609..be3b282f2909 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1877,7 +1877,7 @@ static DEFINE_MUTEX(bpf_event_mutex);
 
 int perf_event_attach_bpf_prog(struct perf_event *event,
 			       struct bpf_prog *prog,
-			       u64 bpf_cookie)
+			       u64 bpf_cookie, u32 prio)
 {
 	struct bpf_prog_array *old_array;
 	struct bpf_prog_array *new_array;
@@ -1904,7 +1904,9 @@ int perf_event_attach_bpf_prog(struct perf_event *event,
 		goto unlock;
 	}
 
-	ret = bpf_prog_array_copy(old_array, NULL, prog, bpf_cookie, &new_array);
+	ret = bpf_prog_array_copy(old_array, NULL, prog, bpf_cookie,
+										prio, &new_array);
+
 	if (ret < 0)
 		goto unlock;
 
@@ -1931,7 +1933,7 @@ void perf_event_detach_bpf_prog(struct perf_event *event)
 		goto unlock;
 
 	old_array = bpf_event_rcu_dereference(event->tp_event->prog_array);
-	ret = bpf_prog_array_copy(old_array, event->prog, NULL, 0, &new_array);
+	ret = bpf_prog_array_copy(old_array, event->prog, NULL, 0, 0, &new_array);
 	if (ret == -ENOENT)
 		goto unlock;
 	if (ret < 0) {
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index d14b10b85e51..10054c034518 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1481,6 +1481,7 @@ union bpf_attr {
 				 * accessible through bpf_get_attach_cookie() BPF helper
 				 */
 				__u64		bpf_cookie;
+				__u32		prio;
 			} perf_event;
 			struct {
 				__u32		flags;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH bpf-next 2/2] libbpf: Allow setting bpf_prog priority
  2022-04-03 16:07 [RFC PATCH bpf-next 0/2] Priorities for bpf progs attached to the same tracepoint Dmitrii Dolgov
  2022-04-03 16:07 ` [RFC PATCH bpf-next 1/2] bpf: tracing: Introduce prio field for bpf_prog Dmitrii Dolgov
@ 2022-04-03 16:07 ` Dmitrii Dolgov
  2022-04-04  0:17 ` [RFC PATCH bpf-next 0/2] Priorities for bpf progs attached to the same tracepoint Andrii Nakryiko
  2 siblings, 0 replies; 7+ messages in thread
From: Dmitrii Dolgov @ 2022-04-03 16:07 UTC (permalink / raw)
  To: bpf, ast, andrii, yhs, songliubraving; +Cc: Dmitrii Dolgov

Introduce prio field in opts structures (bpf_tracepoint_opts,
bpf_link_create_opts, bpf_perf_event_opts) to pass a priority value for
the bpf_prog when a new bpf link is created. This value will be further
used in perf_event_set_bpf_prog to specify in which order to execute
bpf_progs attached to the same tracepoint.

Signed-off-by: Dmitrii Dolgov <9erthalion6@gmail.com>
---
 kernel/bpf/syscall.c   | 3 ++-
 tools/lib/bpf/bpf.c    | 1 +
 tools/lib/bpf/bpf.h    | 1 +
 tools/lib/bpf/libbpf.c | 4 +++-
 tools/lib/bpf/libbpf.h | 6 ++++--
 5 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 72fb3d2c30a4..629852b35b21 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3009,7 +3009,8 @@ static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *pro
 	}
 
 	event = perf_file->private_data;
-	err = perf_event_set_bpf_prog(event, prog, attr->link_create.perf_event.bpf_cookie, 0);
+	err = perf_event_set_bpf_prog(event, prog, attr->link_create.perf_event.bpf_cookie,
+								  attr->link_create.perf_event.prio);
 	if (err) {
 		bpf_link_cleanup(&link_primer);
 		goto out_put_file;
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index cf27251adb92..029c9809bf9b 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -851,6 +851,7 @@ int bpf_link_create(int prog_fd, int target_fd,
 		break;
 	case BPF_PERF_EVENT:
 		attr.link_create.perf_event.bpf_cookie = OPTS_GET(opts, perf_event.bpf_cookie, 0);
+		attr.link_create.perf_event.prio = OPTS_GET(opts, perf_event.prio, 0);
 		if (!OPTS_ZEROED(opts, perf_event))
 			return libbpf_err(-EINVAL);
 		break;
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index f4b4afb6d4ba..9a8ec9081ba7 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -412,6 +412,7 @@ struct bpf_link_create_opts {
 	union {
 		struct {
 			__u64 bpf_cookie;
+			__u32 prio;
 		} perf_event;
 		struct {
 			__u32 flags;
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 809fe209cdcc..e09c00b53772 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -9912,7 +9912,8 @@ struct bpf_link *bpf_program__attach_perf_event_opts(const struct bpf_program *p
 
 	if (kernel_supports(prog->obj, FEAT_PERF_LINK)) {
 		DECLARE_LIBBPF_OPTS(bpf_link_create_opts, link_opts,
-			.perf_event.bpf_cookie = OPTS_GET(opts, bpf_cookie, 0));
+			.perf_event.bpf_cookie = OPTS_GET(opts, bpf_cookie, 0),
+			.perf_event.prio = OPTS_GET(opts, prio, 0));
 
 		link_fd = bpf_link_create(prog_fd, pfd, BPF_PERF_EVENT, &link_opts);
 		if (link_fd < 0) {
@@ -10663,6 +10664,7 @@ struct bpf_link *bpf_program__attach_tracepoint_opts(const struct bpf_program *p
 		return libbpf_err_ptr(-EINVAL);
 
 	pe_opts.bpf_cookie = OPTS_GET(opts, bpf_cookie, 0);
+	pe_opts.prio = OPTS_GET(opts, prio, 0);
 
 	pfd = perf_event_open_tracepoint(tp_category, tp_name);
 	if (pfd < 0) {
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 05dde85e19a6..30f1808a4b49 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -394,8 +394,9 @@ struct bpf_perf_event_opts {
 	size_t sz;
 	/* custom user-provided value fetchable through bpf_get_attach_cookie() */
 	__u64 bpf_cookie;
+	__u32 prio;
 };
-#define bpf_perf_event_opts__last_field bpf_cookie
+#define bpf_perf_event_opts__last_field prio
 
 LIBBPF_API struct bpf_link *
 bpf_program__attach_perf_event(const struct bpf_program *prog, int pfd);
@@ -508,8 +509,9 @@ struct bpf_tracepoint_opts {
 	size_t sz;
 	/* custom user-provided value fetchable through bpf_get_attach_cookie() */
 	__u64 bpf_cookie;
+	__u32 prio;
 };
-#define bpf_tracepoint_opts__last_field bpf_cookie
+#define bpf_tracepoint_opts__last_field prio
 
 LIBBPF_API struct bpf_link *
 bpf_program__attach_tracepoint(const struct bpf_program *prog,
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH bpf-next 0/2] Priorities for bpf progs attached to the same tracepoint
  2022-04-03 16:07 [RFC PATCH bpf-next 0/2] Priorities for bpf progs attached to the same tracepoint Dmitrii Dolgov
  2022-04-03 16:07 ` [RFC PATCH bpf-next 1/2] bpf: tracing: Introduce prio field for bpf_prog Dmitrii Dolgov
  2022-04-03 16:07 ` [RFC PATCH bpf-next 2/2] libbpf: Allow setting bpf_prog priority Dmitrii Dolgov
@ 2022-04-04  0:17 ` Andrii Nakryiko
  2022-04-04 15:29   ` Dmitry Dolgov
  2 siblings, 1 reply; 7+ messages in thread
From: Andrii Nakryiko @ 2022-04-04  0:17 UTC (permalink / raw)
  To: Dmitrii Dolgov
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Yonghong Song, Song Liu

On Sun, Apr 3, 2022 at 9:08 AM Dmitrii Dolgov <9erthalion6@gmail.com> wrote:
>
> With growing number of various products and tools using BPF it could
> easily happen that multiple BPF programs from different processes will
> be attached to the same tracepoint. It seems that in such case there is
> no way to specify a custom order in which those programs may want to be
> executed -- it will depend on the order in which they were attached.
>
> Consider an example when the BPF program A is attached to tracepoint T,
> the BFP program B needs to be attached to the T as well and start
> before/end after the A (e.g. to monitor the whole process of A +
> tracepoint in some way).  If the program A could not be changed and is
> attached before B, the order specified above will not be possible.
>
> One way to address this in a limited, but less invasive way is to
> utilize link options structure to pass the desired priority to
> perf_event_set_bpf_prog, and add new bpf_prog into the bpf_prog_array
> based on its value. This will allow to specify the priority value via
> bpf_tracepoint_opts when attaching a new prog.
>
> Does this make sense? There maybe a better way to achieve this, I would
> be glad to hear any feedback on it.

Not really. What's the real use case where you need to define relative
order of BPF programs on the same kprobe or tracepoint. Each of them
is supposed to be independent of each other and not depend on any
order of their execution. Further, given such tracing programs are
read-only relative to the kernel (they can't change kernel behavior),
the order is supposed to be irrelevant.

>
> Dmitrii Dolgov (2):
>   bpf: tracing: Introduce prio field for bpf_prog
>   libbpf: Allow setting bpf_prog priority
>
>  drivers/media/rc/bpf-lirc.c    |  4 ++--
>  include/linux/bpf.h            |  3 ++-
>  include/linux/trace_events.h   |  7 ++++---
>  include/uapi/linux/bpf.h       |  1 +
>  kernel/bpf/core.c              | 19 +++++++++++++++++--
>  kernel/bpf/syscall.c           |  3 ++-
>  kernel/events/core.c           |  8 ++++----
>  kernel/trace/bpf_trace.c       |  8 +++++---
>  tools/include/uapi/linux/bpf.h |  1 +
>  tools/lib/bpf/bpf.c            |  1 +
>  tools/lib/bpf/bpf.h            |  1 +
>  tools/lib/bpf/libbpf.c         |  4 +++-
>  tools/lib/bpf/libbpf.h         |  6 ++++--
>  13 files changed, 47 insertions(+), 19 deletions(-)
>
> --
> 2.32.0
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH bpf-next 0/2] Priorities for bpf progs attached to the same tracepoint
  2022-04-04  0:17 ` [RFC PATCH bpf-next 0/2] Priorities for bpf progs attached to the same tracepoint Andrii Nakryiko
@ 2022-04-04 15:29   ` Dmitry Dolgov
  2022-04-05 17:20     ` Alexei Starovoitov
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Dolgov @ 2022-04-04 15:29 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Yonghong Song, Song Liu

> On Sun, Apr 03, 2022 at 05:17:46PM -0700, Andrii Nakryiko wrote:
> On Sun, Apr 3, 2022 at 9:08 AM Dmitrii Dolgov <9erthalion6@gmail.com> wrote:
> >
> > With growing number of various products and tools using BPF it could
> > easily happen that multiple BPF programs from different processes will
> > be attached to the same tracepoint. It seems that in such case there is
> > no way to specify a custom order in which those programs may want to be
> > executed -- it will depend on the order in which they were attached.
> >
> > Consider an example when the BPF program A is attached to tracepoint T,
> > the BFP program B needs to be attached to the T as well and start
> > before/end after the A (e.g. to monitor the whole process of A +
> > tracepoint in some way).  If the program A could not be changed and is
> > attached before B, the order specified above will not be possible.
> >
> > One way to address this in a limited, but less invasive way is to
> > utilize link options structure to pass the desired priority to
> > perf_event_set_bpf_prog, and add new bpf_prog into the bpf_prog_array
> > based on its value. This will allow to specify the priority value via
> > bpf_tracepoint_opts when attaching a new prog.
> >
> > Does this make sense? There maybe a better way to achieve this, I would
> > be glad to hear any feedback on it.
>
> Not really. What's the real use case where you need to define relative
> order of BPF programs on the same kprobe or tracepoint. Each of them
> is supposed to be independent of each other and not depend on any
> order of their execution. Further, given such tracing programs are
> read-only relative to the kernel (they can't change kernel behavior),
> the order is supposed to be irrelevant.

The immediate trigger for this idea was inconvenience we faced, trying
to instrument one bpf prog with another. I guess the best practice in
such case would be to attach to fentry/fexit of the target bpf prog, but
from what I understand in this case there is no way to get information
about tracepoint arguments the target prog has received. Stepping back a
bit, what would be the best way of handling this?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH bpf-next 0/2] Priorities for bpf progs attached to the same tracepoint
  2022-04-04 15:29   ` Dmitry Dolgov
@ 2022-04-05 17:20     ` Alexei Starovoitov
  2022-04-06  8:12       ` Dmitry Dolgov
  0 siblings, 1 reply; 7+ messages in thread
From: Alexei Starovoitov @ 2022-04-05 17:20 UTC (permalink / raw)
  To: Dmitry Dolgov
  Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Andrii Nakryiko,
	Yonghong Song, Song Liu

On Mon, Apr 04, 2022 at 05:29:53PM +0200, Dmitry Dolgov wrote:
> > On Sun, Apr 03, 2022 at 05:17:46PM -0700, Andrii Nakryiko wrote:
> > On Sun, Apr 3, 2022 at 9:08 AM Dmitrii Dolgov <9erthalion6@gmail.com> wrote:
> > >
> > > With growing number of various products and tools using BPF it could
> > > easily happen that multiple BPF programs from different processes will
> > > be attached to the same tracepoint. It seems that in such case there is
> > > no way to specify a custom order in which those programs may want to be
> > > executed -- it will depend on the order in which they were attached.
> > >
> > > Consider an example when the BPF program A is attached to tracepoint T,
> > > the BFP program B needs to be attached to the T as well and start
> > > before/end after the A (e.g. to monitor the whole process of A +
> > > tracepoint in some way).  If the program A could not be changed and is
> > > attached before B, the order specified above will not be possible.
> > >
> > > One way to address this in a limited, but less invasive way is to
> > > utilize link options structure to pass the desired priority to
> > > perf_event_set_bpf_prog, and add new bpf_prog into the bpf_prog_array
> > > based on its value. This will allow to specify the priority value via
> > > bpf_tracepoint_opts when attaching a new prog.
> > >
> > > Does this make sense? There maybe a better way to achieve this, I would
> > > be glad to hear any feedback on it.
> >
> > Not really. What's the real use case where you need to define relative
> > order of BPF programs on the same kprobe or tracepoint. Each of them
> > is supposed to be independent of each other and not depend on any
> > order of their execution. Further, given such tracing programs are
> > read-only relative to the kernel (they can't change kernel behavior),
> > the order is supposed to be irrelevant.
> 
> The immediate trigger for this idea was inconvenience we faced, trying
> to instrument one bpf prog with another. I guess the best practice in
> such case would be to attach to fentry/fexit of the target bpf prog, 

yes. that's a recommended way.

> but
> from what I understand in this case there is no way to get information
> about tracepoint arguments the target prog has received. 

Not quite. fentry/fexit have access to the arguments of instrumented bpf prog.
See fexit_bpf2bpf.c
In case of tracepoint the fentry prog will see the same 'ctx' pointer as
bpf prog attached to a tp.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH bpf-next 0/2] Priorities for bpf progs attached to the same tracepoint
  2022-04-05 17:20     ` Alexei Starovoitov
@ 2022-04-06  8:12       ` Dmitry Dolgov
  0 siblings, 0 replies; 7+ messages in thread
From: Dmitry Dolgov @ 2022-04-06  8:12 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Andrii Nakryiko,
	Yonghong Song, Song Liu

> On Tue, Apr 05, 2022 at 10:20:17AM -0700, Alexei Starovoitov wrote:
> > The immediate trigger for this idea was inconvenience we faced, trying
> > to instrument one bpf prog with another. I guess the best practice in
> > such case would be to attach to fentry/fexit of the target bpf prog,
>
> yes. that's a recommended way.
>
> > but
> > from what I understand in this case there is no way to get information
> > about tracepoint arguments the target prog has received.
>
> Not quite. fentry/fexit have access to the arguments of instrumented bpf prog.
> See fexit_bpf2bpf.c
> In case of tracepoint the fentry prog will see the same 'ctx' pointer as
> bpf prog attached to a tp.

Thanks for the clarification, I'll take a look at it.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-04-06 12:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-03 16:07 [RFC PATCH bpf-next 0/2] Priorities for bpf progs attached to the same tracepoint Dmitrii Dolgov
2022-04-03 16:07 ` [RFC PATCH bpf-next 1/2] bpf: tracing: Introduce prio field for bpf_prog Dmitrii Dolgov
2022-04-03 16:07 ` [RFC PATCH bpf-next 2/2] libbpf: Allow setting bpf_prog priority Dmitrii Dolgov
2022-04-04  0:17 ` [RFC PATCH bpf-next 0/2] Priorities for bpf progs attached to the same tracepoint Andrii Nakryiko
2022-04-04 15:29   ` Dmitry Dolgov
2022-04-05 17:20     ` Alexei Starovoitov
2022-04-06  8:12       ` Dmitry Dolgov

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).