bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v2 0/2] bpf: add bpf_send_signal_thread() helper
@ 2020-01-14 21:11 Yonghong Song
  2020-01-14 21:11 ` [PATCH bpf-next v2 1/2] " Yonghong Song
  2020-01-14 21:11 ` [PATCH bpf-next v2 2/2] tools/bpf: add self tests for bpf_send_signal_thread() Yonghong Song
  0 siblings, 2 replies; 4+ messages in thread
From: Yonghong Song @ 2020-01-14 21:11 UTC (permalink / raw)
  To: bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team

Commit 8b401f9ed244 ("bpf: implement bpf_send_signal() helper")
added helper bpf_send_signal() which permits bpf program to
send a signal to the current process. The signal may send to
any thread of the process.

This patch implemented a new helper bpf_send_signal_thread()
to send a signal to the thread corresponding to the kernel current task.
This helper can simplify user space code if the thread context of
bpf sending signal is needed in user space. Please see Patch #1 for
details of use case and kernel implementation.

Patch #2 added some bpf self tests for the new helper.

Changelogs:
  v1 -> v2:
    - More description for the difference between bpf_send_signal()
      and bpf_send_signal_thread() in the uapi header bpf.h.
    - Use skeleton and mmap for send_signal test.

Yonghong Song (2):
  bpf: add bpf_send_signal_thread() helper
  tools/bpf: add self tests for bpf_send_signal_thread()

 include/uapi/linux/bpf.h                      |  19 ++-
 kernel/trace/bpf_trace.c                      |  27 ++++-
 tools/include/uapi/linux/bpf.h                |  19 ++-
 .../selftests/bpf/prog_tests/send_signal.c    | 111 ++++++++++--------
 .../bpf/progs/test_send_signal_kern.c         |  51 ++++----
 5 files changed, 145 insertions(+), 82 deletions(-)

-- 
2.17.1


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

* [PATCH bpf-next v2 1/2] bpf: add bpf_send_signal_thread() helper
  2020-01-14 21:11 [PATCH bpf-next v2 0/2] bpf: add bpf_send_signal_thread() helper Yonghong Song
@ 2020-01-14 21:11 ` Yonghong Song
  2020-01-14 21:11 ` [PATCH bpf-next v2 2/2] tools/bpf: add self tests for bpf_send_signal_thread() Yonghong Song
  1 sibling, 0 replies; 4+ messages in thread
From: Yonghong Song @ 2020-01-14 21:11 UTC (permalink / raw)
  To: bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team

Commit 8b401f9ed244 ("bpf: implement bpf_send_signal() helper")
added helper bpf_send_signal() which permits bpf program to
send a signal to the current process. The signal may be
delivered to any threads in the process.

We found a use case where sending the signal to the current
thread is more preferable.
  - A bpf program will collect the stack trace and then
    send signal to the user application.
  - The user application will add some thread specific
    information to the just collected stack trace for
    later analysis.

If bpf_send_signal() is used, user application will need
to check whether the thread receiving the signal matches
the thread collecting the stack by checking thread id.
If not, it will need to send signal to another thread
through pthread_kill().

This patch proposed a new helper bpf_send_signal_thread(),
which sends the signal to the thread corresponding to
the current kernel task. This way, user space is guaranteed that
bpf_program execution context and user space signal handling
context are the same thread.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 include/uapi/linux/bpf.h       | 19 +++++++++++++++++--
 kernel/trace/bpf_trace.c       | 27 ++++++++++++++++++++++++---
 tools/include/uapi/linux/bpf.h | 19 +++++++++++++++++--
 3 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 52966e758fe5..a88837d4dd18 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2714,7 +2714,8 @@ union bpf_attr {
  *
  * int bpf_send_signal(u32 sig)
  *	Description
- *		Send signal *sig* to the current task.
+ *		Send signal *sig* to the process of the current task.
+ *		The signal may be delivered to any of this process's threads.
  *	Return
  *		0 on success or successfully queued.
  *
@@ -2850,6 +2851,19 @@ union bpf_attr {
  *	Return
  *		0 on success, or a negative error in case of failure.
  *
+ * int bpf_send_signal_thread(u32 sig)
+ *	Description
+ *		Send signal *sig* to the thread corresponding to the current task.
+ *	Return
+ *		0 on success or successfully queued.
+ *
+ *		**-EBUSY** if work queue under nmi is full.
+ *
+ *		**-EINVAL** if *sig* is invalid.
+ *
+ *		**-EPERM** if no permission to send the *sig*.
+ *
+ *		**-EAGAIN** if bpf program can try again.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2968,7 +2982,8 @@ union bpf_attr {
 	FN(probe_read_kernel),		\
 	FN(probe_read_user_str),	\
 	FN(probe_read_kernel_str),	\
-	FN(tcp_send_ack),
+	FN(tcp_send_ack),		\
+	FN(send_signal_thread),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index e5ef4ae9edb5..19e793aa441a 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -703,6 +703,7 @@ struct send_signal_irq_work {
 	struct irq_work irq_work;
 	struct task_struct *task;
 	u32 sig;
+	enum pid_type type;
 };
 
 static DEFINE_PER_CPU(struct send_signal_irq_work, send_signal_work);
@@ -712,10 +713,10 @@ static void do_bpf_send_signal(struct irq_work *entry)
 	struct send_signal_irq_work *work;
 
 	work = container_of(entry, struct send_signal_irq_work, irq_work);
-	group_send_sig_info(work->sig, SEND_SIG_PRIV, work->task, PIDTYPE_TGID);
+	group_send_sig_info(work->sig, SEND_SIG_PRIV, work->task, work->type);
 }
 
-BPF_CALL_1(bpf_send_signal, u32, sig)
+static int bpf_send_signal_common(u32 sig, enum pid_type type)
 {
 	struct send_signal_irq_work *work = NULL;
 
@@ -748,11 +749,17 @@ BPF_CALL_1(bpf_send_signal, u32, sig)
 		 */
 		work->task = current;
 		work->sig = sig;
+		work->type = type;
 		irq_work_queue(&work->irq_work);
 		return 0;
 	}
 
-	return group_send_sig_info(sig, SEND_SIG_PRIV, current, PIDTYPE_TGID);
+	return group_send_sig_info(sig, SEND_SIG_PRIV, current, type);
+}
+
+BPF_CALL_1(bpf_send_signal, u32, sig)
+{
+	return bpf_send_signal_common(sig, PIDTYPE_TGID);
 }
 
 static const struct bpf_func_proto bpf_send_signal_proto = {
@@ -762,6 +769,18 @@ static const struct bpf_func_proto bpf_send_signal_proto = {
 	.arg1_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_1(bpf_send_signal_thread, u32, sig)
+{
+	return bpf_send_signal_common(sig, PIDTYPE_PID);
+}
+
+static const struct bpf_func_proto bpf_send_signal_thread_proto = {
+	.func		= bpf_send_signal_thread,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_ANYTHING,
+};
+
 static const struct bpf_func_proto *
 tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
@@ -822,6 +841,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 #endif
 	case BPF_FUNC_send_signal:
 		return &bpf_send_signal_proto;
+	case BPF_FUNC_send_signal_thread:
+		return &bpf_send_signal_thread_proto;
 	default:
 		return NULL;
 	}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 52966e758fe5..a88837d4dd18 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2714,7 +2714,8 @@ union bpf_attr {
  *
  * int bpf_send_signal(u32 sig)
  *	Description
- *		Send signal *sig* to the current task.
+ *		Send signal *sig* to the process of the current task.
+ *		The signal may be delivered to any of this process's threads.
  *	Return
  *		0 on success or successfully queued.
  *
@@ -2850,6 +2851,19 @@ union bpf_attr {
  *	Return
  *		0 on success, or a negative error in case of failure.
  *
+ * int bpf_send_signal_thread(u32 sig)
+ *	Description
+ *		Send signal *sig* to the thread corresponding to the current task.
+ *	Return
+ *		0 on success or successfully queued.
+ *
+ *		**-EBUSY** if work queue under nmi is full.
+ *
+ *		**-EINVAL** if *sig* is invalid.
+ *
+ *		**-EPERM** if no permission to send the *sig*.
+ *
+ *		**-EAGAIN** if bpf program can try again.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2968,7 +2982,8 @@ union bpf_attr {
 	FN(probe_read_kernel),		\
 	FN(probe_read_user_str),	\
 	FN(probe_read_kernel_str),	\
-	FN(tcp_send_ack),
+	FN(tcp_send_ack),		\
+	FN(send_signal_thread),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
-- 
2.17.1


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

* [PATCH bpf-next v2 2/2] tools/bpf: add self tests for bpf_send_signal_thread()
  2020-01-14 21:11 [PATCH bpf-next v2 0/2] bpf: add bpf_send_signal_thread() helper Yonghong Song
  2020-01-14 21:11 ` [PATCH bpf-next v2 1/2] " Yonghong Song
@ 2020-01-14 21:11 ` Yonghong Song
  2020-01-15  0:07   ` Andrii Nakryiko
  1 sibling, 1 reply; 4+ messages in thread
From: Yonghong Song @ 2020-01-14 21:11 UTC (permalink / raw)
  To: bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team

The test_progs send_signal() is amended to test
bpf_send_signal_thread() as well.

   $ ./test_progs -n 40
   #40/1 send_signal_tracepoint:OK
   #40/2 send_signal_perf:OK
   #40/3 send_signal_nmi:OK
   #40/4 send_signal_tracepoint_thread:OK
   #40/5 send_signal_perf_thread:OK
   #40/6 send_signal_nmi_thread:OK
   #40 send_signal:OK
   Summary: 1/6 PASSED, 0 SKIPPED, 0 FAILED

Also took this opportunity to rewrite the send_signal test
using skeleton framework and array mmap to make code
simpler and more readable.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 .../selftests/bpf/prog_tests/send_signal.c    | 111 ++++++++++--------
 .../bpf/progs/test_send_signal_kern.c         |  51 ++++----
 2 files changed, 87 insertions(+), 75 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
index b607112c64e7..14ec9cf218ed 100644
--- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
+++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <test_progs.h>
+#include <sys/mman.h>
+#include "test_send_signal_kern.skel.h"
 
 static volatile int sigusr1_received = 0;
 
@@ -8,18 +10,26 @@ static void sigusr1_handler(int signum)
 	sigusr1_received++;
 }
 
+static size_t roundup_page(size_t sz)
+{
+	long page_size = sysconf(_SC_PAGE_SIZE);
+	return (sz + page_size - 1) / page_size * page_size;
+}
+
 static void test_send_signal_common(struct perf_event_attr *attr,
-				    int prog_type,
+				    bool is_tp, bool signal_thread,
 				    const char *test_name)
 {
-	int err = -1, pmu_fd, prog_fd, info_map_fd, status_map_fd;
-	const char *file = "./test_send_signal_kern.o";
-	struct bpf_object *obj = NULL;
+	const size_t bss_sz = roundup_page(sizeof(struct test_send_signal_kern__bss));
+	struct test_send_signal_kern__bss *bss_data;
+	struct test_send_signal_kern *skel;
 	int pipe_c2p[2], pipe_p2c[2];
-	__u32 key = 0, duration = 0;
+	struct bpf_map *bss_map;
+	void *bss_mmaped = NULL;
+	int err = -1, pmu_fd;
+	__u32 duration = 0;
 	char buf[256];
 	pid_t pid;
-	__u64 val;
 
 	if (CHECK(pipe(pipe_c2p), test_name,
 		  "pipe pipe_c2p error: %s\n", strerror(errno)))
@@ -73,45 +83,50 @@ static void test_send_signal_common(struct perf_event_attr *attr,
 	close(pipe_c2p[1]); /* close write */
 	close(pipe_p2c[0]); /* close read */
 
-	err = bpf_prog_load(file, prog_type, &obj, &prog_fd);
-	if (CHECK(err < 0, test_name, "bpf_prog_load error: %s\n",
-		  strerror(errno)))
-		goto prog_load_failure;
+	skel = test_send_signal_kern__open_and_load();
+	if (CHECK(!skel, "skel_open_and_load", "skeleton open_and_load failed\n"))
+		goto skel_open_load_failure;
+
+	bss_map = skel->maps.bss;
+	bss_mmaped = mmap(NULL, bss_sz, PROT_READ | PROT_WRITE, MAP_SHARED,
+			  bpf_map__fd(bss_map), 0);
+	if (CHECK(bss_mmaped == MAP_FAILED, "bss_mmap",
+		  ".bss mmap failed: %d\n", errno)) {
+		bss_mmaped = NULL;
+		goto destroy_skel;
+	}
+
+	bss_data = bss_mmaped;
 
 	pmu_fd = syscall(__NR_perf_event_open, attr, pid, -1,
 			 -1 /* group id */, 0 /* flags */);
 	if (CHECK(pmu_fd < 0, test_name, "perf_event_open error: %s\n",
 		  strerror(errno))) {
 		err = -1;
-		goto close_prog;
+		goto destroy_skel;
 	}
 
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
-	if (CHECK(err < 0, test_name, "ioctl perf_event_ioc_enable error: %s\n",
-		  strerror(errno)))
-		goto disable_pmu;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
-	if (CHECK(err < 0, test_name, "ioctl perf_event_ioc_set_bpf error: %s\n",
-		  strerror(errno)))
-		goto disable_pmu;
-
-	err = -1;
-	info_map_fd = bpf_object__find_map_fd_by_name(obj, "info_map");
-	if (CHECK(info_map_fd < 0, test_name, "find map %s error\n", "info_map"))
-		goto disable_pmu;
-
-	status_map_fd = bpf_object__find_map_fd_by_name(obj, "status_map");
-	if (CHECK(status_map_fd < 0, test_name, "find map %s error\n", "status_map"))
-		goto disable_pmu;
+	if (is_tp) {
+		skel->links.send_signal_tp =
+			bpf_program__attach_perf_event(skel->progs.send_signal_tp, pmu_fd);
+		if (CHECK(IS_ERR(skel->links.send_signal_tp), "attach_perf_event",
+			"err %ld\n", PTR_ERR(skel->links.send_signal_tp)))
+			goto disable_pmu;
+	} else {
+		skel->links.send_signal_perf =
+			bpf_program__attach_perf_event(skel->progs.send_signal_perf, pmu_fd);
+		if (CHECK(IS_ERR(skel->links.send_signal_perf), "attach_perf_event",
+			"err %ld\n", PTR_ERR(skel->links.send_signal_perf)))
+			goto disable_pmu;
+	}
 
 	/* wait until child signal handler installed */
 	read(pipe_c2p[0], buf, 1);
 
 	/* trigger the bpf send_signal */
-	key = 0;
-	val = (((__u64)(SIGUSR1)) << 32) | pid;
-	bpf_map_update_elem(info_map_fd, &key, &val, 0);
+	bss_data->pid = pid;
+	bss_data->sig = SIGUSR1;
+	bss_data->signal_thread = signal_thread;
 
 	/* notify child that bpf program can send_signal now */
 	write(pipe_p2c[1], buf, 1);
@@ -132,15 +147,15 @@ static void test_send_signal_common(struct perf_event_attr *attr,
 
 disable_pmu:
 	close(pmu_fd);
-close_prog:
-	bpf_object__close(obj);
-prog_load_failure:
+destroy_skel:
+	test_send_signal_kern__destroy(skel);
+skel_open_load_failure:
 	close(pipe_c2p[0]);
 	close(pipe_p2c[1]);
 	wait(NULL);
 }
 
-static void test_send_signal_tracepoint(void)
+static void test_send_signal_tracepoint(bool signal_thread)
 {
 	const char *id_path = "/sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/id";
 	struct perf_event_attr attr = {
@@ -168,10 +183,10 @@ static void test_send_signal_tracepoint(void)
 
 	attr.config = strtol(buf, NULL, 0);
 
-	test_send_signal_common(&attr, BPF_PROG_TYPE_TRACEPOINT, "tracepoint");
+	test_send_signal_common(&attr, true, signal_thread, "tracepoint");
 }
 
-static void test_send_signal_perf(void)
+static void test_send_signal_perf(bool signal_thread)
 {
 	struct perf_event_attr attr = {
 		.sample_period = 1,
@@ -179,11 +194,10 @@ static void test_send_signal_perf(void)
 		.config = PERF_COUNT_SW_CPU_CLOCK,
 	};
 
-	test_send_signal_common(&attr, BPF_PROG_TYPE_PERF_EVENT,
-				"perf_sw_event");
+	test_send_signal_common(&attr, false, signal_thread, "perf_sw_event");
 }
 
-static void test_send_signal_nmi(void)
+static void test_send_signal_nmi(bool signal_thread)
 {
 	struct perf_event_attr attr = {
 		.sample_freq = 50,
@@ -210,16 +224,21 @@ static void test_send_signal_nmi(void)
 		close(pmu_fd);
 	}
 
-	test_send_signal_common(&attr, BPF_PROG_TYPE_PERF_EVENT,
-				"perf_hw_event");
+	test_send_signal_common(&attr, false, signal_thread, "perf_hw_event");
 }
 
 void test_send_signal(void)
 {
 	if (test__start_subtest("send_signal_tracepoint"))
-		test_send_signal_tracepoint();
+		test_send_signal_tracepoint(false);
 	if (test__start_subtest("send_signal_perf"))
-		test_send_signal_perf();
+		test_send_signal_perf(false);
 	if (test__start_subtest("send_signal_nmi"))
-		test_send_signal_nmi();
+		test_send_signal_nmi(false);
+	if (test__start_subtest("send_signal_tracepoint_thread"))
+		test_send_signal_tracepoint(true);
+	if (test__start_subtest("send_signal_perf_thread"))
+		test_send_signal_perf(true);
+	if (test__start_subtest("send_signal_nmi_thread"))
+		test_send_signal_nmi(true);
 }
diff --git a/tools/testing/selftests/bpf/progs/test_send_signal_kern.c b/tools/testing/selftests/bpf/progs/test_send_signal_kern.c
index 0e6be01157e6..726733f9f20b 100644
--- a/tools/testing/selftests/bpf/progs/test_send_signal_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_send_signal_kern.c
@@ -4,44 +4,37 @@
 #include <linux/version.h>
 #include "bpf_helpers.h"
 
-struct {
-	__uint(type, BPF_MAP_TYPE_ARRAY);
-	__uint(max_entries, 1);
-	__type(key, __u32);
-	__type(value, __u64);
-} info_map SEC(".maps");
-
-struct {
-	__uint(type, BPF_MAP_TYPE_ARRAY);
-	__uint(max_entries, 1);
-	__type(key, __u32);
-	__type(value, __u64);
-} status_map SEC(".maps");
-
-SEC("send_signal_demo")
-int bpf_send_signal_test(void *ctx)
+__u32 sig = 0, pid = 0, status = 0, signal_thread = 0;
+
+static __always_inline int bpf_send_signal_test(void *ctx)
 {
-	__u64 *info_val, *status_val;
-	__u32 key = 0, pid, sig;
 	int ret;
 
-	status_val = bpf_map_lookup_elem(&status_map, &key);
-	if (!status_val || *status_val != 0)
-		return 0;
-
-	info_val = bpf_map_lookup_elem(&info_map, &key);
-	if (!info_val || *info_val == 0)
+	if (status != 0 || sig == 0 || pid == 0)
 		return 0;
 
-	sig = *info_val >> 32;
-	pid = *info_val & 0xffffFFFF;
-
 	if ((bpf_get_current_pid_tgid() >> 32) == pid) {
-		ret = bpf_send_signal(sig);
+		if (signal_thread)
+			ret = bpf_send_signal_thread(sig);
+		else
+			ret = bpf_send_signal(sig);
 		if (ret == 0)
-			*status_val = 1;
+			status = 1;
 	}
 
 	return 0;
 }
+
+SEC("tracepoint/syscalls/sys_enter_nanosleep")
+int send_signal_tp(void *ctx)
+{
+	return bpf_send_signal_test(ctx);
+}
+
+SEC("perf_event")
+int send_signal_perf(void *ctx)
+{
+	return bpf_send_signal_test(ctx);
+}
+
 char __license[] SEC("license") = "GPL";
-- 
2.17.1


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

* Re: [PATCH bpf-next v2 2/2] tools/bpf: add self tests for bpf_send_signal_thread()
  2020-01-14 21:11 ` [PATCH bpf-next v2 2/2] tools/bpf: add self tests for bpf_send_signal_thread() Yonghong Song
@ 2020-01-15  0:07   ` Andrii Nakryiko
  0 siblings, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2020-01-15  0:07 UTC (permalink / raw)
  To: Yonghong Song; +Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team

On Tue, Jan 14, 2020 at 1:12 PM Yonghong Song <yhs@fb.com> wrote:
>
> The test_progs send_signal() is amended to test
> bpf_send_signal_thread() as well.
>
>    $ ./test_progs -n 40
>    #40/1 send_signal_tracepoint:OK
>    #40/2 send_signal_perf:OK
>    #40/3 send_signal_nmi:OK
>    #40/4 send_signal_tracepoint_thread:OK
>    #40/5 send_signal_perf_thread:OK
>    #40/6 send_signal_nmi_thread:OK
>    #40 send_signal:OK
>    Summary: 1/6 PASSED, 0 SKIPPED, 0 FAILED
>
> Also took this opportunity to rewrite the send_signal test
> using skeleton framework and array mmap to make code
> simpler and more readable.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---

thanks for converting, see some stuff you don't need to do with skeleton below

>  .../selftests/bpf/prog_tests/send_signal.c    | 111 ++++++++++--------
>  .../bpf/progs/test_send_signal_kern.c         |  51 ++++----
>  2 files changed, 87 insertions(+), 75 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
> index b607112c64e7..14ec9cf218ed 100644
> --- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
> +++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
> @@ -1,5 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #include <test_progs.h>
> +#include <sys/mman.h>
> +#include "test_send_signal_kern.skel.h"
>
>  static volatile int sigusr1_received = 0;
>
> @@ -8,18 +10,26 @@ static void sigusr1_handler(int signum)
>         sigusr1_received++;
>  }
>
> +static size_t roundup_page(size_t sz)
> +{
> +       long page_size = sysconf(_SC_PAGE_SIZE);
> +       return (sz + page_size - 1) / page_size * page_size;
> +}
> +
>  static void test_send_signal_common(struct perf_event_attr *attr,
> -                                   int prog_type,
> +                                   bool is_tp, bool signal_thread,
>                                     const char *test_name)
>  {
> -       int err = -1, pmu_fd, prog_fd, info_map_fd, status_map_fd;
> -       const char *file = "./test_send_signal_kern.o";
> -       struct bpf_object *obj = NULL;
> +       const size_t bss_sz = roundup_page(sizeof(struct test_send_signal_kern__bss));
> +       struct test_send_signal_kern__bss *bss_data;
> +       struct test_send_signal_kern *skel;
>         int pipe_c2p[2], pipe_p2c[2];
> -       __u32 key = 0, duration = 0;
> +       struct bpf_map *bss_map;
> +       void *bss_mmaped = NULL;
> +       int err = -1, pmu_fd;
> +       __u32 duration = 0;
>         char buf[256];
>         pid_t pid;
> -       __u64 val;
>
>         if (CHECK(pipe(pipe_c2p), test_name,
>                   "pipe pipe_c2p error: %s\n", strerror(errno)))
> @@ -73,45 +83,50 @@ static void test_send_signal_common(struct perf_event_attr *attr,
>         close(pipe_c2p[1]); /* close write */
>         close(pipe_p2c[0]); /* close read */
>
> -       err = bpf_prog_load(file, prog_type, &obj, &prog_fd);
> -       if (CHECK(err < 0, test_name, "bpf_prog_load error: %s\n",
> -                 strerror(errno)))
> -               goto prog_load_failure;
> +       skel = test_send_signal_kern__open_and_load();
> +       if (CHECK(!skel, "skel_open_and_load", "skeleton open_and_load failed\n"))
> +               goto skel_open_load_failure;
> +
> +       bss_map = skel->maps.bss;
> +       bss_mmaped = mmap(NULL, bss_sz, PROT_READ | PROT_WRITE, MAP_SHARED,
> +                         bpf_map__fd(bss_map), 0);
> +       if (CHECK(bss_mmaped == MAP_FAILED, "bss_mmap",
> +                 ".bss mmap failed: %d\n", errno)) {
> +               bss_mmaped = NULL;
> +               goto destroy_skel;
> +       }
> +
> +       bss_data = bss_mmaped;

You don't need to mmap() manually, it's all done automatically as part
of loading skeleton. Just use skel->bss->{pid,sig,signal_thread}
directly.

>
>         pmu_fd = syscall(__NR_perf_event_open, attr, pid, -1,
>                          -1 /* group id */, 0 /* flags */);
>         if (CHECK(pmu_fd < 0, test_name, "perf_event_open error: %s\n",
>                   strerror(errno))) {
>                 err = -1;
> -               goto close_prog;
> +               goto destroy_skel;
>         }
>
> -       err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
> -       if (CHECK(err < 0, test_name, "ioctl perf_event_ioc_enable error: %s\n",
> -                 strerror(errno)))
> -               goto disable_pmu;
> -
> -       err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
> -       if (CHECK(err < 0, test_name, "ioctl perf_event_ioc_set_bpf error: %s\n",
> -                 strerror(errno)))
> -               goto disable_pmu;
> -
> -       err = -1;
> -       info_map_fd = bpf_object__find_map_fd_by_name(obj, "info_map");
> -       if (CHECK(info_map_fd < 0, test_name, "find map %s error\n", "info_map"))
> -               goto disable_pmu;
> -
> -       status_map_fd = bpf_object__find_map_fd_by_name(obj, "status_map");
> -       if (CHECK(status_map_fd < 0, test_name, "find map %s error\n", "status_map"))
> -               goto disable_pmu;
> +       if (is_tp) {
> +               skel->links.send_signal_tp =
> +                       bpf_program__attach_perf_event(skel->progs.send_signal_tp, pmu_fd);
> +               if (CHECK(IS_ERR(skel->links.send_signal_tp), "attach_perf_event",
> +                       "err %ld\n", PTR_ERR(skel->links.send_signal_tp)))
> +                       goto disable_pmu;

tracepoint handler should be loaded automatically, so here you'll be
leaking already create bpf_link and creating a new one.

> +       } else {
> +               skel->links.send_signal_perf =
> +                       bpf_program__attach_perf_event(skel->progs.send_signal_perf, pmu_fd);
> +               if (CHECK(IS_ERR(skel->links.send_signal_perf), "attach_perf_event",
> +                       "err %ld\n", PTR_ERR(skel->links.send_signal_perf)))
> +                       goto disable_pmu;
> +       }
>
>         /* wait until child signal handler installed */
>         read(pipe_c2p[0], buf, 1);
>
>         /* trigger the bpf send_signal */
> -       key = 0;
> -       val = (((__u64)(SIGUSR1)) << 32) | pid;
> -       bpf_map_update_elem(info_map_fd, &key, &val, 0);
> +       bss_data->pid = pid;
> +       bss_data->sig = SIGUSR1;
> +       bss_data->signal_thread = signal_thread;
>
>         /* notify child that bpf program can send_signal now */
>         write(pipe_p2c[1], buf, 1);
> @@ -132,15 +147,15 @@ static void test_send_signal_common(struct perf_event_attr *attr,
>
>  disable_pmu:
>         close(pmu_fd);
> -close_prog:
> -       bpf_object__close(obj);
> -prog_load_failure:
> +destroy_skel:
> +       test_send_signal_kern__destroy(skel);
> +skel_open_load_failure:
>         close(pipe_c2p[0]);
>         close(pipe_p2c[1]);
>         wait(NULL);
>  }
>
> -static void test_send_signal_tracepoint(void)
> +static void test_send_signal_tracepoint(bool signal_thread)
>  {
>         const char *id_path = "/sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/id";

You probably don't need this as well, because tracepoint BPF programs
should be auto-attached.

>         struct perf_event_attr attr = {
> @@ -168,10 +183,10 @@ static void test_send_signal_tracepoint(void)
>
>         attr.config = strtol(buf, NULL, 0);
>
> -       test_send_signal_common(&attr, BPF_PROG_TYPE_TRACEPOINT, "tracepoint");
> +       test_send_signal_common(&attr, true, signal_thread, "tracepoint");
>  }
>

[...]

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

end of thread, other threads:[~2020-01-15  0:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 21:11 [PATCH bpf-next v2 0/2] bpf: add bpf_send_signal_thread() helper Yonghong Song
2020-01-14 21:11 ` [PATCH bpf-next v2 1/2] " Yonghong Song
2020-01-14 21:11 ` [PATCH bpf-next v2 2/2] tools/bpf: add self tests for bpf_send_signal_thread() Yonghong Song
2020-01-15  0:07   ` Andrii Nakryiko

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