Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200722054314.2103880-1-irogers@google.com>
In-Reply-To: <20200722054314.2103880-1-irogers@google.com>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Tue, 21 Jul 2020 23:57:48 -0700
Message-ID: <CAEf4BzaBYaFJ3eUinS9nHeykJ0xEbZpwLts33ZDp1PT=bkyjww@mail.gmail.com>
Subject: Re: [RFC PATCH] bpftool btf: Add prefix option to dump C
To:     Ian Rogers <irogers@google.com>
Cc:     Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        Andrii Nakryiko <andriin@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@chromium.org>,
        Quentin Monnet <quentin@isovalent.com>,
        Jakub Kicinski <kuba@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
        =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= <toke@redhat.com>,
        Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        Stanislav Fomichev <sdf@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Jul 21, 2020 at 10:44 PM Ian Rogers <irogers@google.com> wrote:
>
> When bpftool dumps types and enum members into a header file for
> inclusion the names match those in the original source. If the same
> header file needs to be included in the original source and the bpf
> program, the names of structs, unions, typedefs and enum members will
> have naming collisions.

vmlinux.h is not really intended to be used from user-space, because
it's incompatible with pretty much any other header that declares any
type. Ideally we should make this better, but that might require some
compiler support. We've been discussing with Yonghong extending Clang
with a compile-time check for whether some type is defined or not,
which would allow to guard every type and only declare it
conditionally, if it's missing. But that's just an idea at this point.

Regardless, vmlinux.h is also very much Clang-specific, and shouldn't
work well with GCC. Could you elaborate on the specifics of the use
case you have in mind? That could help me see what might be the right
solution. Thanks!

>
> To avoid these collisions an approach is to redeclare the header file
> types and enum members, which leads to duplication and possible
> inconsistencies. Another approach is to use preprocessor macros
> to rename conflicting names, but this can be cumbersome if there are
> many conflicts.
>
> This patch adds a prefix option for the dumped names. Use of this option
> can avoid name conflicts and compile time errors.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  .../bpf/bpftool/Documentation/bpftool-btf.rst |  7 ++++++-
>  tools/bpf/bpftool/btf.c                       | 18 ++++++++++++++---
>  tools/lib/bpf/btf.h                           |  1 +
>  tools/lib/bpf/btf_dump.c                      | 20 +++++++++++++------
>  4 files changed, 36 insertions(+), 10 deletions(-)
>

[...]

> diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> index 491c7b41ffdc..fea4baab00bd 100644
> --- a/tools/lib/bpf/btf.h
> +++ b/tools/lib/bpf/btf.h
> @@ -117,6 +117,7 @@ struct btf_dump;
>
>  struct btf_dump_opts {
>         void *ctx;
> +       const char *name_prefix;
>  };

BTW, we can't do that, this breaks ABI. btf_dump_opts were added
before we understood the problem of backward/forward  compatibility of
libbpf APIs, unfortunately.

>
>  typedef void (*btf_dump_printf_fn_t)(void *ctx, const char *fmt, va_list args);
> diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> index e1c344504cae..baf2b4d82e1e 100644
> --- a/tools/lib/bpf/btf_dump.c
> +++ b/tools/lib/bpf/btf_dump.c

[...]