| Message ID | 20200110011559.1949913-1-yhs@fb.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: add bpf_send_signal_thread() helper | expand |
On Thu, Jan 9, 2020 at 5:16 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 > > Signed-off-by: Yonghong Song <yhs@fb.com> > --- > tools/include/uapi/linux/bpf.h | 18 +++++++++-- maybe do tools/uapi header sync in a first patch, along the original change? > .../selftests/bpf/prog_tests/send_signal.c | 30 ++++++++++++------- > .../bpf/progs/test_send_signal_kern.c | 9 ++++-- > 3 files changed, 42 insertions(+), 15 deletions(-) > [...] > 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..4a722024c32b 100644 > --- a/tools/testing/selftests/bpf/progs/test_send_signal_kern.c > +++ b/tools/testing/selftests/bpf/progs/test_send_signal_kern.c > @@ -23,6 +23,7 @@ int bpf_send_signal_test(void *ctx) > { > __u64 *info_val, *status_val; > __u32 key = 0, pid, sig; > + int use_signal_thread; > int ret; > > status_val = bpf_map_lookup_elem(&status_map, &key); > @@ -33,11 +34,15 @@ int bpf_send_signal_test(void *ctx) > if (!info_val || *info_val == 0) > return 0; > > - sig = *info_val >> 32; > + use_signal_thread = *info_val >> 48; > + sig = *info_val >> 32 & 0xFFFF; > pid = *info_val & 0xffffFFFF; Would you mind rewriting this test w/ BPF skeleton and global data? It would make it cleaner without all this masking stuff? > > if ((bpf_get_current_pid_tgid() >> 32) == pid) { > - ret = bpf_send_signal(sig); > + if (use_signal_thread) > + ret = bpf_send_signal_thread(sig); > + else > + ret = bpf_send_signal(sig); > if (ret == 0) > *status_val = 1; > } > -- > 2.17.1 >
On 1/13/20 5:19 PM, Andrii Nakryiko wrote: > On Thu, Jan 9, 2020 at 5:16 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 >> >> Signed-off-by: Yonghong Song <yhs@fb.com> >> --- >> tools/include/uapi/linux/bpf.h | 18 +++++++++-- > > maybe do tools/uapi header sync in a first patch, along the original change? Will do. > >> .../selftests/bpf/prog_tests/send_signal.c | 30 ++++++++++++------- >> .../bpf/progs/test_send_signal_kern.c | 9 ++++-- >> 3 files changed, 42 insertions(+), 15 deletions(-) >> > > [...] > >> 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..4a722024c32b 100644 >> --- a/tools/testing/selftests/bpf/progs/test_send_signal_kern.c >> +++ b/tools/testing/selftests/bpf/progs/test_send_signal_kern.c >> @@ -23,6 +23,7 @@ int bpf_send_signal_test(void *ctx) >> { >> __u64 *info_val, *status_val; >> __u32 key = 0, pid, sig; >> + int use_signal_thread; >> int ret; >> >> status_val = bpf_map_lookup_elem(&status_map, &key); >> @@ -33,11 +34,15 @@ int bpf_send_signal_test(void *ctx) >> if (!info_val || *info_val == 0) >> return 0; >> >> - sig = *info_val >> 32; >> + use_signal_thread = *info_val >> 48; >> + sig = *info_val >> 32 & 0xFFFF; >> pid = *info_val & 0xffffFFFF; > > Would you mind rewriting this test w/ BPF skeleton and global data? It > would make it cleaner without all this masking stuff? Previously I made the change to minimize the number of changed lines. But since you are mentioning rewriting here, I will do it in v2. > >> >> if ((bpf_get_current_pid_tgid() >> 32) == pid) { >> - ret = bpf_send_signal(sig); >> + if (use_signal_thread) >> + ret = bpf_send_signal_thread(sig); >> + else >> + ret = bpf_send_signal(sig); >> if (ret == 0) >> *status_val = 1; >> } >> -- >> 2.17.1 >>
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 52966e758fe5..3320f8bdfe7e 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -2714,7 +2714,7 @@ 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. * Return * 0 on success or successfully queued. * @@ -2850,6 +2850,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 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 +2981,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/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c index b607112c64e7..b12350832be3 100644 --- a/tools/testing/selftests/bpf/prog_tests/send_signal.c +++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c @@ -10,7 +10,8 @@ static void sigusr1_handler(int signum) static void test_send_signal_common(struct perf_event_attr *attr, int prog_type, - const char *test_name) + const char *test_name, + bool send_thread) { int err = -1, pmu_fd, prog_fd, info_map_fd, status_map_fd; const char *file = "./test_send_signal_kern.o"; @@ -110,7 +111,7 @@ static void test_send_signal_common(struct perf_event_attr *attr, /* trigger the bpf send_signal */ key = 0; - val = (((__u64)(SIGUSR1)) << 32) | pid; + val = (((__u64)send_thread) << 48) | (((__u64)(SIGUSR1)) << 32) | pid; bpf_map_update_elem(info_map_fd, &key, &val, 0); /* notify child that bpf program can send_signal now */ @@ -140,7 +141,7 @@ static void test_send_signal_common(struct perf_event_attr *attr, wait(NULL); } -static void test_send_signal_tracepoint(void) +static void test_send_signal_tracepoint(bool send_thread) { const char *id_path = "/sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/id"; struct perf_event_attr attr = { @@ -168,10 +169,11 @@ 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, BPF_PROG_TYPE_TRACEPOINT, "tracepoint", + send_thread); } -static void test_send_signal_perf(void) +static void test_send_signal_perf(bool send_thread) { struct perf_event_attr attr = { .sample_period = 1, @@ -180,10 +182,10 @@ static void test_send_signal_perf(void) }; test_send_signal_common(&attr, BPF_PROG_TYPE_PERF_EVENT, - "perf_sw_event"); + "perf_sw_event", send_thread); } -static void test_send_signal_nmi(void) +static void test_send_signal_nmi(bool send_thread) { struct perf_event_attr attr = { .sample_freq = 50, @@ -211,15 +213,21 @@ static void test_send_signal_nmi(void) } test_send_signal_common(&attr, BPF_PROG_TYPE_PERF_EVENT, - "perf_hw_event"); + "perf_hw_event", send_thread); } 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..4a722024c32b 100644 --- a/tools/testing/selftests/bpf/progs/test_send_signal_kern.c +++ b/tools/testing/selftests/bpf/progs/test_send_signal_kern.c @@ -23,6 +23,7 @@ int bpf_send_signal_test(void *ctx) { __u64 *info_val, *status_val; __u32 key = 0, pid, sig; + int use_signal_thread; int ret; status_val = bpf_map_lookup_elem(&status_map, &key); @@ -33,11 +34,15 @@ int bpf_send_signal_test(void *ctx) if (!info_val || *info_val == 0) return 0; - sig = *info_val >> 32; + use_signal_thread = *info_val >> 48; + sig = *info_val >> 32 & 0xFFFF; pid = *info_val & 0xffffFFFF; if ((bpf_get_current_pid_tgid() >> 32) == pid) { - ret = bpf_send_signal(sig); + if (use_signal_thread) + ret = bpf_send_signal_thread(sig); + else + ret = bpf_send_signal(sig); if (ret == 0) *status_val = 1; }
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 Signed-off-by: Yonghong Song <yhs@fb.com> --- tools/include/uapi/linux/bpf.h | 18 +++++++++-- .../selftests/bpf/prog_tests/send_signal.c | 30 ++++++++++++------- .../bpf/progs/test_send_signal_kern.c | 9 ++++-- 3 files changed, 42 insertions(+), 15 deletions(-)