Date:   Thu, 3 May 2018 17:19:02 +1000
From:   "Tobin C. Harding" <tobin@...orbit.com>
To:     Song Liu <songliubraving@...com>
Cc:     netdev@...r.kernel.org, kernel-team@...com, qinteng@...com
Subject: Re: [PATCH v2 bpf-next 2/2] bpf: add selftest for stackmap with
 build_id in NMI context

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 <songliubraving@...com>
> ---
>  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.

Powered by blists - more mailing lists