bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
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>,
	"Toke Høiland-Jørgensen" <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>
Subject: Re: [RFC PATCH] bpftool btf: Add prefix option to dump C
Date: Tue, 21 Jul 2020 23:57:48 -0700	[thread overview]
Message-ID: <CAEf4BzaBYaFJ3eUinS9nHeykJ0xEbZpwLts33ZDp1PT=bkyjww@mail.gmail.com> (raw)
In-Reply-To: <20200722054314.2103880-1-irogers@google.com>

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

[...]

  reply	other threads:[~2020-07-22  6:58 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-22  5:43 [RFC PATCH] bpftool btf: Add prefix option to dump C Ian Rogers
2020-07-22  6:57 ` Andrii Nakryiko [this message]
2020-08-01  1:47   ` Ian Rogers
2020-08-01  3:47     ` Andrii Nakryiko
2020-08-06 17:58       ` Andrii Nakryiko
2020-08-06 19:42         ` Ian Rogers
2020-08-07  0:29           ` Andrii Nakryiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAEf4BzaBYaFJ3eUinS9nHeykJ0xEbZpwLts33ZDp1PT=bkyjww@mail.gmail.com' \
    --to=andrii.nakryiko@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=irogers@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kafai@fb.com \
    --cc=kpsingh@chromium.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=quentin@isovalent.com \
    --cc=sdf@google.com \
    --cc=songliubraving@fb.com \
    --cc=toke@redhat.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).