netdev - Re: [PATCH bpf-next] selftests/bpf: convert bpf_iter_test_kern{3,4}.c to define own bpf_iter

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: <20200519210834.qwfrhq2ixgy6l3oy@ast-mbp.dhcp.thefacebook.com>
Date:   Tue, 19 May 2020 14:08:34 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Andrii Nakryiko <andriin@...com>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org, ast@...com,
        daniel@...earbox.net, andrii.nakryiko@...il.com, kernel-team@...com
Subject: Re: [PATCH bpf-next] selftests/bpf: convert
 bpf_iter_test_kern{3,4}.c to define own bpf_iter_meta

On Tue, May 19, 2020 at 12:23:41PM -0700, Andrii Nakryiko wrote:
> b9f4c01f3e0b ("selftest/bpf: Make bpf_iter selftest compilable against old vmlinux.h")
> missed the fact that bpf_iter_test_kern{3,4}.c are not just including
> bpf_iter_test_kern_common.h and need similar bpf_iter_meta re-definition
> explicitly.
> 
> Fixes: b9f4c01f3e0b ("selftest/bpf: Make bpf_iter selftest compilable against old vmlinux.h")
> Signed-off-by: Andrii Nakryiko <andriin@...com>
> ---
>  .../selftests/bpf/progs/bpf_iter_test_kern3.c     | 15 +++++++++++++++
>  .../selftests/bpf/progs/bpf_iter_test_kern4.c     | 15 +++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_test_kern3.c b/tools/testing/selftests/bpf/progs/bpf_iter_test_kern3.c
> index 636a00fa074d..13c2c90c835f 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_iter_test_kern3.c
> +++ b/tools/testing/selftests/bpf/progs/bpf_iter_test_kern3.c
> @@ -1,10 +1,25 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /* Copyright (c) 2020 Facebook */
> +#define bpf_iter_meta bpf_iter_meta___not_used
> +#define bpf_iter__task bpf_iter__task___not_used
>  #include "vmlinux.h"
> +#undef bpf_iter_meta
> +#undef bpf_iter__task
>  #include <bpf/bpf_helpers.h>
>  
>  char _license[] SEC("license") = "GPL";
>  
> +struct bpf_iter_meta {
> +	struct seq_file *seq;
> +	__u64 session_id;
> +	__u64 seq_num;
> +} __attribute__((preserve_access_index));
> +
> +struct bpf_iter__task {
> +	struct bpf_iter_meta *meta;
> +	struct task_struct *task;
> +} __attribute__((preserve_access_index));

Applied, but I was wondering whether all these structs can be placed
in a single header file like bpf_iters.h ?
struct bpf_iter_meta is common across all of them.
What if next iter patch changes the name in there?
We'd need to patch 10 tests? It's unstable api, so it's fine,
but considering the churn it seems common header would be good.
That .h would include struct bpf_iter__bpf_map, bpf_iter__task,
bpf_iter__task_file, etc
wdyt?