netdev - Re: [PATCH v6 bpf-next 3/3] bpf: add selftest for BPF_ENABLE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <290a94fb-f3ee-227c-ffa0-66629ce8327a@fb.com>
Date:   Tue, 28 Apr 2020 17:33:54 -0700
From:   Yonghong Song <yhs@...com>
To:     Song Liu <songliubraving@...com>, <bpf@...r.kernel.org>,
        <netdev@...r.kernel.org>
CC:     <ast@...nel.org>, <daniel@...earbox.net>, <kernel-team@...com>
Subject: Re: [PATCH v6 bpf-next 3/3] bpf: add selftest for BPF_ENABLE_STATS



On 4/28/20 5:29 PM, Song Liu wrote:
> Add test for  BPF_ENABLE_STATS, which should enable run_time_ns stats.
> 
> ~/selftests/bpf# ./test_progs -t enable_stats  -v
> test_enable_stats:PASS:skel_open_and_load 0 nsec
> test_enable_stats:PASS:get_stats_fd 0 nsec
> test_enable_stats:PASS:attach_raw_tp 0 nsec
> test_enable_stats:PASS:get_prog_info 0 nsec
> test_enable_stats:PASS:check_stats_enabled 0 nsec
> Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
> 
> Signed-off-by: Song Liu <songliubraving@...com>
> ---
>   .../selftests/bpf/prog_tests/enable_stats.c   | 45 +++++++++++++++++++
>   .../selftests/bpf/progs/test_enable_stats.c   | 21 +++++++++
>   2 files changed, 66 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/enable_stats.c
>   create mode 100644 tools/testing/selftests/bpf/progs/test_enable_stats.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/enable_stats.c b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
> new file mode 100644
> index 000000000000..987fc743ab75
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include <sys/mman.h>
> +#include "test_enable_stats.skel.h"
> +
> +void test_enable_stats(void)
> +{
> +	struct test_enable_stats *skel;
> +	struct bpf_prog_info info = {};
> +	__u32 info_len = sizeof(info);
> +	int stats_fd, err, prog_fd;
> +	int duration = 0;
> +
> +	skel = test_enable_stats__open_and_load();
> +
> +	if (CHECK(!skel, "skel_open_and_load", "skeleton open/load failed\n"))
> +		return;
> +
> +	stats_fd = bpf_enable_stats(BPF_STATS_RUNTIME_CNT);
> +
> +	if (CHECK(stats_fd < 0, "get_stats_fd", "failed %d\n", errno))
> +		goto cleanup;
> +
> +	err = test_enable_stats__attach(skel);
> +
> +	if (CHECK(err, "attach_raw_tp", "err %d\n", err))
> +		goto cleanup;
> +
> +	usleep(1000);
> +
> +	prog_fd = bpf_program__fd(skel->progs.test_enable_stats);
> +	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> +
> +	if (CHECK(err, "get_prog_info",
> +		  "failed to get bpf_prog_info for fd %d\n", prog_fd))
> +		goto cleanup;
> +
> +	CHECK(info.run_time_ns == 0, "check_stats_enabled",
> +	      "failed to enable run_time_ns stats\n");
> +
> +cleanup:
> +	test_enable_stats__destroy(skel);
> +	if (stats_fd >= 0)
> +		close(stats_fd);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_enable_stats.c b/tools/testing/selftests/bpf/progs/test_enable_stats.c
> new file mode 100644
> index 000000000000..aa9626330a9e
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_enable_stats.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2020 Facebook
> +
> +#include <linux/bpf.h>
> +#include <stdint.h>
> +#include <bpf/bpf_helpers.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +static int val = 1;
> +
> +SEC("raw_tracepoint/sys_enter")
> +int test_enable_stats(void *ctx)
> +{
> +	__u32 key = 0;
> +	__u64 *val;

The above two declarations (key/val) are not needed,
esp. "val" is shadowing.
Maybe the maintainer can fix it up before merging
if there is no other changes for this patch set.

> +
> +	val += 1;
> +
> +	return 0;
> +}
>