netdev - Re: [PATCH v4 bpf-next 03/14] bpf: Add BTF_ID_LIST/BTF

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: <CAEf4BzbceMFA60=7Jp7oC9x-gMvhKQtWmuhV3ncVKUhHHDzugA@mail.gmail.com>
Date:   Fri, 26 Jun 2020 14:32:48 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Jiri Olsa <jolsa@...nel.org>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Martin KaFai Lau <kafai@...com>,
        David Miller <davem@...hat.com>,
        John Fastabend <john.fastabend@...il.com>,
        Wenbo Zhang <ethercflow@...il.com>,
        KP Singh <kpsingh@...omium.org>,
        Andrii Nakryiko <andriin@...com>,
        Brendan Gregg <bgregg@...flix.com>,
        Florent Revest <revest@...omium.org>,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH v4 bpf-next 03/14] bpf: Add BTF_ID_LIST/BTF_ID macros

On Thu, Jun 25, 2020 at 3:13 PM Jiri Olsa <jolsa@...nel.org> wrote:
>
> Adding support to generate .BTF_ids section that will hold BTF
> ID lists for verifier.
>
> Adding macros that will help to define lists of BTF ID values
> placed in .BTF_ids section. They are initially filled with zeros
> (during compilation) and resolved later during the linking phase
> by resolve_btfids tool.
>
> Following defines list of one BTF ID value:
>
>   BTF_ID_LIST(bpf_skb_output_btf_ids)
>   BTF_ID(struct, sk_buff)
>
> It also defines following variable to access the list:
>
>   extern int bpf_skb_output_btf_ids[];
>
> Suggested-by: Andrii Nakryiko <andrii.nakryiko@...il.com>
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> ---

Looks good, with few nits below.

Acked-by: Andrii Nakryiko <andriin@...com>

>  include/asm-generic/vmlinux.lds.h |  4 ++
>  include/linux/btf_ids.h           | 69 +++++++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+)
>  create mode 100644 include/linux/btf_ids.h
>

[...]

> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> new file mode 100644
> index 000000000000..f7f9dc4d9a9f
> --- /dev/null
> +++ b/include/linux/btf_ids.h
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _LINUX_BTF_IDS_H
> +#define _LINUX_BTF_IDS_H 1

this "1", is it necessary? I think it's always just `#define HEADER_GUARD`?

> +
> +#include <linux/compiler.h> /* for __PASTE */
> +

[...]

> +#define __BTF_ID_LIST(name)                            \
> +asm(                                                   \
> +".pushsection " BTF_IDS_SECTION ",\"a\";       \n"     \
> +".local " #name ";                             \n"     \
> +#name ":;                                      \n"     \
> +".popsection;                                  \n");   \
> +
> +#define BTF_ID_LIST(name)                              \
> +__BTF_ID_LIST(name)                                    \
> +extern int name[];

nit: extern u32 (or __u32) perhaps?

> +
> +#endif
> --
> 2.25.4
>