bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: "Yonghong Song" <yhs@meta.com>,
	"Toke Høiland-Jørgensen" <toke@kernel.org>,
	"Alexei Starovoitov" <alexei.starovoitov@gmail.com>,
	"Alan Maguire" <alan.maguire@oracle.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Arnaldo Carvalho de Melo" <acme@kernel.org>,
	"Martin KaFai Lau" <martin.lau@linux.dev>,
	"Song Liu" <song@kernel.org>, "Yonghong Song" <yhs@fb.com>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"KP Singh" <kpsingh@kernel.org>,
	"Stanislav Fomichev" <sdf@google.com>,
	"Hao Luo" <haoluo@google.com>, "Jiri Olsa" <jolsa@kernel.org>,
	"Quentin Monnet" <quentin@isovalent.com>,
	"Mykola Lysenko" <mykolal@fb.com>, bpf <bpf@vger.kernel.org>
Subject: Re: [RFC bpf-next 1/8] btf: add kind metadata encoding to UAPI
Date: Thu, 08 Jun 2023 01:05:21 +0300	[thread overview]
Message-ID: <24c2b0295ea9b8a3f3fc256e2d7bf004a046ebb1.camel@gmail.com> (raw)
In-Reply-To: <CAEf4BzY9_EiBqM74Gce9-Z5O+uubCY=CUejXr79hDY6SbOvTOg@mail.gmail.com>

On Wed, 2023-06-07 at 14:47 -0700, Andrii Nakryiko wrote:
> On Wed, Jun 7, 2023 at 9:14 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Wed, 2023-06-07 at 08:29 -0700, Yonghong Song wrote:
> > > 
> > > On 6/7/23 4:55 AM, Eduard Zingerman wrote:
> > > > On Tue, 2023-06-06 at 13:30 +0200, Toke Høiland-Jørgensen wrote:
> > > > [...]
> > > > > 
> > > > > As for bumping the version number, I don't think it's a good idea to
> > > > > deliberately break compatibility this way unless it's absolutely
> > > > > necessary. With "absolutely necessary" meaning "things will break in
> > > > > subtle ways in any case, so it's better to make the breakage obvious".
> > > > > But it libbpf is not checking the version field anyway, that becomes
> > > > > kind of a moot point, as bumping it doesn't really gain us anything,
> > > > > then...
> > > > 
> > > > It seems to me that in terms of backward compatibility, the ability to
> > > > specify the size for each kind entry is more valuable than the
> > > > capability to add new BTF kinds:
> > > > - The former allows for extending kind records in
> > > >    a backward-compatible manner, such as adding a function address to
> > > >    BTF_KIND_FUNC.
> > > 
> > > Eduard, the new proposal is to add new kind, e.g., BTF_KIND_KFUNC, which
> > > will have an 'address' field. BTF_KIND_KFUNC is for kernel functions.
> > > So we will not have size compatibility issue for BTF_KIND_FUNC.
> > 
> > Well, actually this might be a way to avoid BTF_KIND_KFUNC :)
> > What I wanted to say is that any use of this feature leads to
> > incompatibility with current BTF parsers, as either size of existing
> > kinds would be changed or a new kind with unknown size would be added.
> > It seems to me that this warrants version bump (or some other way to
> > signal existing parsers that format is incompatible).
> 
> It is probably too late to have existing KINDs changing their size. If
> this layout metadata was mandatory from the very beginning, then we
> could have relied on it for determining new extra fields for
> BTF_KIND_FUNC.
> 
> As things stand right now, new BTF_KIND_KFUNC is both a signal of
> newer format (for kernel-side BTF; nothing changes for BPF object file
> BTFs, which is great side-effect making backwards compat pain
> smaller), and is a simpler and safer way to add extra information.
> 
> > 
> > > 
> > > > - The latter is much more fragile. Types refer to each other,
> > > >    compatibility is already lost once a new "unknown" tag is introduced
> > > >    in a type chain.
> > > > 
> > > > However, changing the size of existing BTF kinds is itself a
> > > > backward-incompatible change. Therefore, a version bump may be
> > > > warranted in this regard.
> > 
> 
> See above and previous emails. Not having to bump version means we can
> start emitting this layout info from Clang and pahole with no extra
> opt-in flags, and not worry about breaking existing tools and apps.
> This is great, so let's not ruin that property :)

I'm not sure I understand how this would help:
- If no new kinds are added, absence or presence of metadata section
  does not matter. Old parsers would ignore it, new parsers would work
  as old parsers, so there is no added value in generating metadata.
- As soon as new kind is added old parsers are broken.

What am I missing?

  reply	other threads:[~2023-06-07 22:05 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-31 20:19 [RFC bpf-next 0/8] bpf: support BTF kind metadata to separate Alan Maguire
2023-05-31 20:19 ` [RFC bpf-next 1/8] btf: add kind metadata encoding to UAPI Alan Maguire
2023-06-01  3:53   ` Alexei Starovoitov
2023-06-01 10:36     ` Alan Maguire
2023-06-01 16:53       ` Alexei Starovoitov
2023-06-02 16:32         ` Andrii Nakryiko
2023-06-02 16:34           ` Andrii Nakryiko
2023-06-02 18:11           ` Alexei Starovoitov
2023-06-02 20:33             ` Andrii Nakryiko
2023-06-05 16:14               ` Alexei Starovoitov
2023-06-05 22:38                 ` Andrii Nakryiko
2023-06-06  2:46                   ` Alexei Starovoitov
2023-06-06 11:30                     ` Toke Høiland-Jørgensen
2023-06-07 11:55                       ` Eduard Zingerman
2023-06-07 15:29                         ` Yonghong Song
2023-06-07 16:14                           ` Eduard Zingerman
2023-06-07 21:47                             ` Andrii Nakryiko
2023-06-07 22:05                               ` Eduard Zingerman [this message]
2023-06-07 22:34                                 ` Andrii Nakryiko
2023-06-06 16:50                     ` Andrii Nakryiko
2023-06-07  1:16                       ` Alexei Starovoitov
2023-06-07 21:43                         ` Andrii Nakryiko
2023-05-31 20:19 ` [RFC bpf-next 2/8] libbpf: support handling of metadata section in BTF Alan Maguire
2023-06-05 11:01   ` Jiri Olsa
2023-06-05 21:40     ` Andrii Nakryiko
2023-05-31 20:19 ` [RFC bpf-next 3/8] libbpf: use metadata to compute an unknown kind size Alan Maguire
2023-05-31 20:19 ` [RFC bpf-next 4/8] btf: support kernel parsing of BTF with metadata, use it to parse BTF with unknown kinds Alan Maguire
2023-06-07 19:51   ` Eduard Zingerman
2023-05-31 20:19 ` [RFC bpf-next 5/8] libbpf: add metadata encoding support Alan Maguire
2023-05-31 20:19 ` [RFC bpf-next 6/8] btf: generate metadata for vmlinux/module BTF Alan Maguire
2023-05-31 20:19 ` [RFC bpf-next 7/8] bpftool: add BTF dump "format meta" to dump header/metadata Alan Maguire
2023-06-01 16:33   ` Quentin Monnet
2023-06-02 16:57   ` Andrii Nakryiko
2023-05-31 20:19 ` [RFC bpf-next 8/8] selftests/bpf: test kind encoding/decoding Alan Maguire
2023-05-31 20:19 ` [RFC dwarves] dwarves: encode BTF metadata if --btf_gen_meta is set Alan Maguire

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=24c2b0295ea9b8a3f3fc256e2d7bf004a046ebb1.camel@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=acme@kernel.org \
    --cc=alan.maguire@oracle.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=mykolal@fb.com \
    --cc=quentin@isovalent.com \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=toke@kernel.org \
    --cc=yhs@fb.com \
    --cc=yhs@meta.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).