bpf/tracing: allow user space to query prog array on the same tp

PATCH Superseded Yonghong Song 2 participants 6 messages Dec 6, 2017

Subsystems (10)

Networking Network drivers Kselftest BPF +6 more subsystems

lore

Share & tools

lore

Yonghong Song

author

2017-12-06 06:31 UTC #0 lore raw

v2 cover bpf/tracing: allow user space to query prog array on the same tp

Commit e87c6bc3852b ("bpf: permit multiple bpf attachments for a single perf event") added support to attach multiple bpf programs to a single perf event. Given a perf event (kprobe, uprobe, or kernel tracepoint), the perf ioctl interface is used to query bpf programs attached to the same trace event.

There already exists a BPF_PROG_QUERY command for introspection currently used by cgroup+bpf. We did have an implementation for querying tracepoint+bpf through the same interface. However, it looks cleaner to use ioctl() style of api here, since attaching bpf prog to tracepoint/kuprobe is also done via ioctl.

Patch #1 had the core implementation and patch #2 added a test case in tools bpf selftests suite.

Changelogs:
v1-> v2:
- Rebase on top of net-next.
- Use existing bpf_prog_array_length function instead of implementing the same functionality in function bpf_prog_array_copy_info.

Yonghong Song (2):
bpf/tracing: allow user space to query prog array on the same tp bpf/tracing: add a bpf test for new ioctl query interface

 include/linux/bpf.h                           |   4 +
 include/uapi/linux/perf_event.h               |   6 +
 kernel/bpf/core.c                             |  21 ++++
 kernel/events/core.c                          |   3 +
 kernel/trace/bpf_trace.c                      |  23 ++++
 tools/include/uapi/linux/perf_event.h         |   6 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/test_progs.c      | 155 ++++++++++++++++++++++++++
 tools/testing/selftests/bpf/test_tracepoint.c |  26 +++++
 9 files changed, 245 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/test_tracepoint.c

Signature · 1 line

2.9.5

Yonghong Song

author

2017-12-06 06:31 UTC #1 lore raw

PATCH v2 1/2 bpf/tracing: allow user space to query prog array on the same tp

Commit e87c6bc3852b ("bpf: permit multiple bpf attachments for a single perf event") added support to attach multiple bpf programs to a single perf event.
Commit 2541517c32be ("tracing, perf: Implement BPF programs attached to kprobes") utilized the existing perf ioctl interface and added the command PERF_EVENT_IOC_SET_BPF to attach a bpf program to a tracepoint.

This patch adds a new ioctl
command, given a perf event fd, to query the bpf program array attached to the same perf tracepoint event.

The new uapi ioctl command:
PERF_EVENT_IOC_QUERY_BPF

The new uapi/linux/perf_event.h structure:

  struct perf_event_query_bpf {
       __u64	prog_ids;
       __u32	prog_cnt;

};

The usage:
struct perf_event_query_bpf query; query.prog_ids = (__u64)usr_prog_ids_buf; query.prog_cnt = usr_prog_ids_buf_len;
err = ioctl(pmu_efd, PERF_EVENT_IOC_QUERY_BPF, &query);

Signed-off-by Yonghong Song <[email protected]>

Acked-by Alexei Starovoitov <[email protected]>

---
 include/linux/bpf.h             |  4 ++++
 include/uapi/linux/perf_event.h |  6 ++++++
 kernel/bpf/core.c               | 21 +++++++++++++++++++++
 kernel/events/core.c            |  3 +++
 kernel/trace/bpf_trace.c        | 23 +++++++++++++++++++++++
 5 files changed, 57 insertions(+)

include/linux/bpf.h +4 -0

index e55e425..f812ac5 100644
@@ -254,6 +254,7 @@ typedef unsigned long (*bpf_ctx_copy_t)(void *dst, const void *src,
 
 u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
 		     void *ctx, u64 ctx_size, bpf_ctx_copy_t ctx_copy);
+int bpf_event_query_prog_array(struct perf_event *event, void __user *info);
 
 int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
 			  union bpf_attr __user *uattr);

@@ -285,6 +286,9 @@ int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
 
 void bpf_prog_array_delete_safe(struct bpf_prog_array __rcu *progs,
 				struct bpf_prog *old_prog);
+int bpf_prog_array_copy_info(struct bpf_prog_array __rcu *array,
+			     __u32 __user *prog_ids, u32 request_cnt,
+			     __u32 __user *prog_cnt);
 int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array,
 			struct bpf_prog *exclude_prog,
 			struct bpf_prog *include_prog,

include/uapi/linux/perf_event.h +6 -0

index b9a4953..fee0b43 100644
@@ -418,6 +418,11 @@ struct perf_event_attr {
 	__u16	__reserved_2;	/* align to __u64 */
 };
 
+struct perf_event_query_bpf {
+	__u64	prog_ids;
+	__u32	prog_cnt;
+};
+
 #define perf_flags(attr)	(*(&(attr)->read_format + 1))
 
 /*

@@ -433,6 +438,7 @@ struct perf_event_attr {
 #define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
 #define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
 #define PERF_EVENT_IOC_PAUSE_OUTPUT	_IOW('$', 9, __u32)
+#define PERF_EVENT_IOC_QUERY_BPF	_IOWR('$', 10, struct perf_event_query_bpf *)
 
 enum perf_event_ioc_flags {
 	PERF_IOC_FLAG_GROUP		= 1U << 0,

kernel/bpf/core.c +21 -0

index 86b50aa..35b427aa 100644
@@ -1462,6 +1462,8 @@ int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
 	rcu_read_lock();
 	prog = rcu_dereference(progs)->progs;
 	for (; *prog; prog++) {
+		if (*prog == &dummy_bpf_prog.prog)
+			continue;
 		id = (*prog)->aux->id;
 		if (copy_to_user(prog_ids + i, &id, sizeof(id))) {
 			rcu_read_unlock();

@@ -1545,6 +1547,25 @@ int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array,
 	return 0;
 }
 
+int bpf_prog_array_copy_info(struct bpf_prog_array __rcu *array,
+			     __u32 __user *prog_ids, u32 request_cnt,
+			     __u32 __user *prog_cnt)
+{
+	u32 cnt = 0;
+
+	if (array)
+		cnt = bpf_prog_array_length(array);
+
+	if (copy_to_user(prog_cnt, &cnt, sizeof(cnt)))
+		return -EFAULT;
+
+	/* return early if user requested only program count or nothing to copy */
+	if (!request_cnt || !prog_ids || !cnt)
+		return 0;
+
+	return bpf_prog_array_copy_to_user(array, prog_ids, request_cnt);
+}
+
 static void bpf_prog_free_deferred(struct work_struct *work)
 {
 	struct bpf_prog_aux *aux;

kernel/events/core.c +3 -0

index 16beab4..f10609e 100644
@@ -4723,6 +4723,9 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
 		rcu_read_unlock();
 		return 0;
 	}
+
+	case PERF_EVENT_IOC_QUERY_BPF:
+		return bpf_event_query_prog_array(event, (void __user *)arg);
 	default:
 		return -ENOTTY;
 	}

kernel/trace/bpf_trace.c +23 -0

index 0ce99c3..81eedb2 100644
@@ -820,3 +820,26 @@ void perf_event_detach_bpf_prog(struct perf_event *event)
 unlock:
 	mutex_unlock(&bpf_event_mutex);
 }
+
+int bpf_event_query_prog_array(struct perf_event *event, void __user *info)
+{
+	struct perf_event_query_bpf __user *uquery = info;
+	struct perf_event_query_bpf query = {};
+	int ret;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+	if (event->attr.type != PERF_TYPE_TRACEPOINT)
+		return -EINVAL;
+	if (copy_from_user(&query, uquery, sizeof(query)))
+		return -EFAULT;
+
+	mutex_lock(&bpf_event_mutex);
+	ret = bpf_prog_array_copy_info(event->tp_event->prog_array,
+				       u64_to_user_ptr(query.prog_ids),
+				       query.prog_cnt,
+				       &uquery->prog_cnt);
+	mutex_unlock(&bpf_event_mutex);
+
+	return ret;
+}

Signature · 1 line

2.9.5

Peter Zijlstra

maintainer

2017-12-06 11:56 UTC #2 lore raw

Quoted 21 lines · Yonghong Song

Commit e87c6bc3852b ("bpf: permit multiple bpf attachments for a single perf event") added support to attach multiple bpf programs to a single perf event.
Commit 2541517c32be ("tracing, perf: Implement BPF programs attached to kprobes") utilized the existing perf ioctl interface and added the command PERF_EVENT_IOC_SET_BPF to attach a bpf program to a tracepoint.
This patch adds a new ioctl
command, given a perf event fd, to query the bpf program array attached to the same perf tracepoint event.
The new uapi ioctl command:
PERF_EVENT_IOC_QUERY_BPF
The new uapi/linux/perf_event.h structure:
  struct perf_event_query_bpf {
       __u64	prog_ids;
       __u32	prog_cnt;
};

Quoted 8 lines

The usage:
struct perf_event_query_bpf query; query.prog_ids = (__u64)usr_prog_ids_buf; query.prog_cnt = usr_prog_ids_buf_len;
err = ioctl(pmu_efd, PERF_EVENT_IOC_QUERY_BPF, &query);
Signed-off-by Yonghong Song <[email protected]>
Acked-by Alexei Starovoitov <[email protected]>

Can you please fix that example to make it clear that prog_ids is in fact a pointer to an array of size prog_cnt. Ideally also describing what the type of array is.

In fact, would not something like:

	struct perf_event_query_bpf {
		__u32 len;
		__u32 __reserved;
		__u64 ids[0];
	};

be a much clearer interface?

Also, you forgot to tell us why we need this interface at all.

Peter Zijlstra

maintainer

2017-12-06 13:17 UTC #3 lore raw

Quoted 35 lines · Peter Zijlstra

Quoted 21 lines · Yonghong Song
Commit e87c6bc3852b ("bpf: permit multiple bpf attachments for a single perf event") added support to attach multiple bpf programs to a single perf event.
Commit 2541517c32be ("tracing, perf: Implement BPF programs attached to kprobes") utilized the existing perf ioctl interface and added the command PERF_EVENT_IOC_SET_BPF to attach a bpf program to a tracepoint.
This patch adds a new ioctl
command, given a perf event fd, to query the bpf program array attached to the same perf tracepoint event.
The new uapi ioctl command:
PERF_EVENT_IOC_QUERY_BPF
The new uapi/linux/perf_event.h structure:
  struct perf_event_query_bpf {
       __u64	prog_ids;
       __u32	prog_cnt;
};
Quoted 8 lines
The usage:
struct perf_event_query_bpf query; query.prog_ids = (__u64)usr_prog_ids_buf; query.prog_cnt = usr_prog_ids_buf_len;
err = ioctl(pmu_efd, PERF_EVENT_IOC_QUERY_BPF, &query);
Signed-off-by Yonghong Song <[email protected]>
Acked-by Alexei Starovoitov <[email protected]>
Can you please fix that example to make it clear that prog_ids is in fact a pointer to an array of size prog_cnt. Ideally also describing what the type of array is.

Quoted 5 lines

In fact, would not something like:
	struct perf_event_query_bpf {
		__u32 len;
		__u32 __reserved;

I suppose we could use this field to store the number of entries returned, retaining the len to indicate how large the structure is.

Quoted 6 lines

		__u64 ids[0];
	};
be a much clearer interface?
Also, you forgot to tell us why we need this interface at all.

Yonghong Song

author

2017-12-06 18:39 UTC #4 lore raw

Quoted 29 lines · Peter Zijlstra

Quoted 28 lines · Peter Zijlstra
Quoted 27 lines · Yonghong Song
Commit e87c6bc3852b ("bpf: permit multiple bpf attachments for a single perf event") added support to attach multiple bpf programs to a single perf event.
Commit 2541517c32be ("tracing, perf: Implement BPF programs attached to kprobes") utilized the existing perf ioctl interface and added the command PERF_EVENT_IOC_SET_BPF to attach a bpf program to a tracepoint.
This patch adds a new ioctl
command, given a perf event fd, to query the bpf program array attached to the same perf tracepoint event.
The new uapi ioctl command:
PERF_EVENT_IOC_QUERY_BPF
The new uapi/linux/perf_event.h structure:
   struct perf_event_query_bpf {
        __u64	prog_ids;
        __u32	prog_cnt;
   };
The usage:
   struct perf_event_query_bpf query;
   query.prog_ids = (__u64)usr_prog_ids_buf;
   query.prog_cnt = usr_prog_ids_buf_len;
   err = ioctl(pmu_efd, PERF_EVENT_IOC_QUERY_BPF, &query);

Quoted 6 lines

Quoted 6 lines
Quoted 2 lines
Signed-off-by Yonghong Song <[email protected]>
Acked-by Alexei Starovoitov <[email protected]>
Can you please fix that example to make it clear that prog_ids is in fact a pointer to an array of size prog_cnt. Ideally also describing what the type of array is.

Right. Will address this with more descriptions in the commit message and also add some comments in the perf_event.h.

Quoted 14 lines

Quoted 6 lines
In fact, would not something like:
	struct perf_event_query_bpf {
		__u32 len;
		__u32 __reserved;
I suppose we could use this field to store the number of entries returned, retaining the len to indicate how large the structure is.
Quoted 4 lines
		__u64 ids[0];
	};
be a much clearer interface?

Yes, this is clearer and may be consistent with perf interface.

FYI, my old interface is similar to the BPF query interface below:
struct { /* anonymous struct used by BPF_PROG_QUERY command */
__u32 target_fd; /* container object to
query */

                __u32           attach_type;
                __u32           query_flags;
                __u32           attach_flags;
                __aligned_u64   prog_ids;
                __u32           prog_cnt;
        } query;

Quoted 2 lines

Quoted 2 lines
Also, you forgot to tell us why we need this interface at all.

Right. Will add some descriptions for this.

Yonghong Song

author

2017-12-06 06:31 UTC #5 lore raw

PATCH v2 2/2 bpf/tracing: add a bpf test for new ioctl query interface

In reply to Yonghong Song

Added a subtest in test_progs. The tracepoint is sched/sched_switch. Multiple bpf programs are attached to this tracepoint and the query interface is exercised.

Signed-off-by Yonghong Song <[email protected]>

Acked-by Alexei Starovoitov <[email protected]>

---
 tools/include/uapi/linux/perf_event.h         |   6 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/test_progs.c      | 155 ++++++++++++++++++++++++++
 tools/testing/selftests/bpf/test_tracepoint.c |  26 +++++
 4 files changed, 188 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/test_tracepoint.c

tools/include/uapi/linux/perf_event.h +6 -0

index 362493a..8523db0 100644
@@ -418,6 +418,11 @@ struct perf_event_attr {
 	__u16	__reserved_2;	/* align to __u64 */
 };
 
+struct perf_event_query_bpf {
+	__u64	prog_ids;
+	__u32	prog_cnt;
+};
+
 #define perf_flags(attr)	(*(&(attr)->read_format + 1))
 
 /*

@@ -433,6 +438,7 @@ struct perf_event_attr {
 #define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
 #define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
 #define PERF_EVENT_IOC_PAUSE_OUTPUT	_IOW('$', 9, __u32)
+#define PERF_EVENT_IOC_QUERY_BPF	_IOWR('$', 10, struct perf_event_query_bpf *)
 
 enum perf_event_ioc_flags {
 	PERF_IOC_FLAG_GROUP		= 1U << 0,

tools/testing/selftests/bpf/Makefile +1 -1

index 2c9d8c6..255fb1f 100644
@@ -17,7 +17,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
 
 TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test_obj_id.o \
 	test_pkt_md_access.o test_xdp_redirect.o test_xdp_meta.o sockmap_parse_prog.o     \
-	sockmap_verdict_prog.o dev_cgroup.o sample_ret0.o
+	sockmap_verdict_prog.o dev_cgroup.o sample_ret0.o test_tracepoint.o
 
 TEST_PROGS := test_kmod.sh test_xdp_redirect.sh test_xdp_meta.sh \
 	test_offload.py

tools/testing/selftests/bpf/test_progs.c +155 -0

175 lines not on this page · raw message

tools/testing/selftests/bpf/test_tracepoint.c new file +26 -0

new file mode 100644
index 0000000..04bf084
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2017 Facebook
+
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+/* taken from /sys/kernel/debug/tracing/events/sched/sched_switch/format */
+struct sched_switch_args {
+	unsigned long long pad;
+	char prev_comm[16];
+	int prev_pid;
+	int prev_prio;
+	long long prev_state;
+	char next_comm[16];
+	int next_pid;
+	int next_prio;
+};
+
+SEC("tracepoint/sched/sched_switch")
+int oncpu(struct sched_switch_args *ctx)
+{
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1; /* ignored by tracepoints, required by libbpf.a */

Signature · 1 line

2.9.5