From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Tobin C. Harding" Subject: Re: [PATCH v2 bpf-next 2/2] bpf: add selftest for stackmap with build_id in NMI context Date: Thu, 3 May 2018 17:19:02 +1000 Message-ID: <20180503071902.GP3791@eros> References: <20180502232030.3788284-1-songliubraving@fb.com> <20180502232030.3788284-3-songliubraving@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, kernel-team@fb.com, qinteng@fb.com To: Song Liu Return-path: Received: from out1-smtp.messagingengine.com ([66.111.4.25]:48757 "EHLO out1-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751076AbeECHTH (ORCPT ); Thu, 3 May 2018 03:19:07 -0400 Content-Disposition: inline In-Reply-To: <20180502232030.3788284-3-songliubraving@fb.com> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, May 02, 2018 at 04:20:30PM -0700, Song Liu wrote: > This new test captures stackmap with build_id with hardware event > PERF_COUNT_HW_CPU_CYCLES. > > Because we only support one ips-to-build_id lookup per cpu in NMI > context, stack_amap will not be able to do the lookup in this test. stack_map ? > Therefore, we didn't do compare_stack_ips(), as it will alwasy fail. > > urandom_read.c is extended to run configurable cycles so that it can be > caught by the perf event. > > Signed-off-by: Song Liu > --- > tools/testing/selftests/bpf/test_progs.c | 137 +++++++++++++++++++++++++++++ > tools/testing/selftests/bpf/urandom_read.c | 10 ++- > 2 files changed, 145 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c > index aa336f0..00bb08c 100644 > --- a/tools/testing/selftests/bpf/test_progs.c > +++ b/tools/testing/selftests/bpf/test_progs.c > @@ -1272,6 +1272,142 @@ static void test_stacktrace_build_id(void) > return; > } > > +static void test_stacktrace_build_id_nmi(void) > +{ > + int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd; > + const char *file = "./test_stacktrace_build_id.o"; > + int err, pmu_fd, prog_fd; > + struct perf_event_attr attr = { > + .sample_freq = 5000, > + .freq = 1, > + .type = PERF_TYPE_HARDWARE, > + .config = PERF_COUNT_HW_CPU_CYCLES, > + }; > + __u32 key, previous_key, val, duration = 0; > + struct bpf_object *obj; > + char buf[256]; > + int i, j; > + struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH]; > + int build_id_matches = 0; > + > + err = bpf_prog_load(file, BPF_PROG_TYPE_PERF_EVENT, &obj, &prog_fd); > + if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno)) > + goto out; perhaps: return; > + pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, > + 0 /* cpu 0 */, -1 /* group id */, > + 0 /* flags */); > + if (CHECK(pmu_fd < 0, "perf_event_open", > + "err %d errno %d. Does the test host support PERF_COUNT_HW_CPU_CYCLES?\n", > + pmu_fd, errno)) > + goto close_prog; > + > + err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0); > + if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n", > + err, errno)) > + goto close_pmu; > + > + err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd); > + if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n", > + err, errno)) > + goto disable_pmu; > + > + /* find map fds */ > + control_map_fd = bpf_find_map(__func__, obj, "control_map"); > + if (CHECK(control_map_fd < 0, "bpf_find_map control_map", > + "err %d errno %d\n", err, errno)) > + goto disable_pmu; > + > + stackid_hmap_fd = bpf_find_map(__func__, obj, "stackid_hmap"); > + if (CHECK(stackid_hmap_fd < 0, "bpf_find_map stackid_hmap", > + "err %d errno %d\n", err, errno)) > + goto disable_pmu; > + > + stackmap_fd = bpf_find_map(__func__, obj, "stackmap"); > + if (CHECK(stackmap_fd < 0, "bpf_find_map stackmap", "err %d errno %d\n", > + err, errno)) > + goto disable_pmu; > + > + stack_amap_fd = bpf_find_map(__func__, obj, "stack_amap"); > + if (CHECK(stack_amap_fd < 0, "bpf_find_map stack_amap", > + "err %d errno %d\n", err, errno)) > + goto disable_pmu; > + > + assert(system("dd if=/dev/urandom of=/dev/zero count=4 2> /dev/null") > + == 0); > + assert(system("taskset 0x1 ./urandom_read 100000") == 0); > + /* disable stack trace collection */ > + key = 0; > + val = 1; > + bpf_map_update_elem(control_map_fd, &key, &val, 0); > + > + /* for every element in stackid_hmap, we can find a corresponding one > + * in stackmap, and vise versa. > + */ > + err = compare_map_keys(stackid_hmap_fd, stackmap_fd); > + if (CHECK(err, "compare_map_keys stackid_hmap vs. stackmap", > + "err %d errno %d\n", err, errno)) > + goto disable_pmu; > + > + err = compare_map_keys(stackmap_fd, stackid_hmap_fd); > + if (CHECK(err, "compare_map_keys stackmap vs. stackid_hmap", > + "err %d errno %d\n", err, errno)) > + goto disable_pmu; > + > + err = extract_build_id(buf, 256); > + > + if (CHECK(err, "get build_id with readelf", > + "err %d errno %d\n", err, errno)) > + goto disable_pmu; > + > + err = bpf_map_get_next_key(stackmap_fd, NULL, &key); > + if (CHECK(err, "get_next_key from stackmap", > + "err %d, errno %d\n", err, errno)) > + goto disable_pmu; > + > + do { > + char build_id[64]; > + > + err = bpf_map_lookup_elem(stackmap_fd, &key, id_offs); > + if (CHECK(err, "lookup_elem from stackmap", > + "err %d, errno %d\n", err, errno)) > + goto disable_pmu; > + for (i = 0; i < PERF_MAX_STACK_DEPTH; ++i) > + if (id_offs[i].status == BPF_STACK_BUILD_ID_VALID && > + id_offs[i].offset != 0) { > + for (j = 0; j < 20; ++j) > + sprintf(build_id + 2 * j, "%02x", > + id_offs[i].build_id[j] & 0xff); > + if (strstr(buf, build_id) != NULL) > + build_id_matches = 1; > + } > + previous_key = key; > + } while (bpf_map_get_next_key(stackmap_fd, &previous_key, &key) == 0); > + > + if (CHECK(build_id_matches < 1, "build id match", > + "Didn't find expected build ID from the map\n")) > + goto disable_pmu; > + > + /* > + * We intentionally skip compare_stack_ips(). This is because we > + * only support one in_nmi() ips-to-build_id translation per cpu > + * at any time, thus stack_amap here will always fallback to > + * BPF_STACK_BUILD_ID_IP; > + */ > + > +disable_pmu: > + ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE); > + > +close_pmu: > + close(pmu_fd); > + > +close_prog: > + bpf_object__close(obj); > + > +out: > + return; > +} No real need for label 'out' right? We can just return directly and remove the last three lines of this function. Hope this helps, Tobin.