On Tue, Jan 14, 2020 at 1:12 PM Yonghong Song <yhs@xxxxxx> 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@xxxxxx>
> ---
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");
> }
>
[...]
