All of
 help / color / mirror / Atom feed
From: Andrii Nakryiko <>
To: Alexei Starovoitov <>
Cc: Andrii Nakryiko <>, bpf <>,
	Networking <>,
	Alexei Starovoitov <>,
	Daniel Borkmann <>,
	Kernel Team <>
Subject: Re: [PATCH bpf-next 0/4] libbpf: add raw BTF type dumping
Date: Wed, 30 Sep 2020 11:22:50 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Tue, Sep 29, 2020 at 8:18 PM Alexei Starovoitov
<> wrote:
> On Tue, Sep 29, 2020 at 05:44:48PM -0700, Andrii Nakryiko wrote:
> > On Tue, Sep 29, 2020 at 5:03 PM Alexei Starovoitov
> > <> wrote:
> > >
> > > On Tue, Sep 29, 2020 at 04:28:39PM -0700, Andrii Nakryiko wrote:
> > > > Add btf_dump__dump_type_raw() API that emits human-readable low-level BTF type
> > > > information, same as bpftool output. bpftool is not switched to this API
> > > > because bpftool still needs to perform all the same BTF type processing logic
> > > > to do JSON output, so benefits are pretty much zero.
> > >
> > > If the only existing user cannot actually use such api it speaks heavily
> > > against adding such api to libbpf. Comparing strings in tests is nice, but
> > > could be done with C output just as well.
> >
> > It certainly can, it just won't save much code, because bpftool would
> > still need to have a big switch over BTF type kinds to do JSON output.
> So you're saying that most of the dump_btf_type() in bpftool/btf.c will stay as-is.
> Only 'if (json_output)' will become unconditional? Hmm.


> I know you don't want json in libbpf, but I think it's the point of
> making a call on such things. Either libbpf gets to dump both
> json and text dump_btf_type()-like output or it stays with C only.

Right, I don't think JSON belongs in libbpf. But I fail to see why
this is the point where we need to make such a decision.

> Doing C and this text and not doing json is inconsistent.

Inconsistent with what? I've never found bpftool's raw BTF dump in
JSON format useful. At all. Saying raw BTF dump is useful and
consistent (?) only if it's both human-readable text and JSON makes no
sense to me. Libbpf doesn't have to re-implement entire bpftool

> Either libbpf can print btf in many different ways or it stays with C.
> 2nd format is not special in any way.

I don't understand your point. With my patch it now can dump it as
valid C language definition or as a textual low-level BTF

If you are saying it should emit it in Go format, Rust format, or
other language-specific way, then sure, maybe, but it sure won't
re-use C-specific logic of btf_dump__dump_type() as is, because it is
C language specific. For Go there would be different logic, just as
for any other language. And someone will have to implement it (and
there would need to be a compelling use case for that, of course). And
it will be a different API, or at least a generic API with some enum
specifying "format" (which is the same thing, really, but inferior for
customizability reasons).

But JSON is different from that. It's just a more machine-friendly
output of textual low-level BTF dump. It could have been BSON or YAML,
but I hope you don't suggest to emit in those formats as well.

> I don't think that text and json formats bring much value comparing to C,
> so I would be fine with C only.

Noted. I disagree and find it very useful all the time, it's pretty
much the only way I look at BTF. C output is not complete: it doesn't
show functions, data sections and variables. It's not a replacement
for raw BTF dump. I don't even consider it as a different "format".
It's an entirely different and complementary (not alternative) view
(interpretation) of BTF.

> But if we allow 2nd format we should
> do json at the same time too to save bpftool the hassle.

There is no hassle for bpftool, code is written and working. Libbpf's
goal is not to minimize bpftool code either. So I hear you, but I
don't think about this the same way.

> And in the future we should allow 4th and 5th formats.

Ok, but there is no contradiction with what I'm doing here.

Regardless, feel free to drop patches #2 and #3, but patch #1 fixes
real issue, so would be nice to land it anyways. Patch #4 adds test
for changes in patch #1. Let me know if you want me to respin with
just those 2 patches.

  reply	other threads:[~2020-09-30 18:23 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-29 23:28 [PATCH bpf-next 0/4] libbpf: add raw BTF type dumping Andrii Nakryiko
2020-09-29 23:28 ` [PATCH bpf-next 1/4] libbpf: make btf_dump work with modifiable BTF Andrii Nakryiko
2020-09-29 23:28 ` [PATCH bpf-next 2/4] libbpf: add raw dumping of BTF types Andrii Nakryiko
2020-09-29 23:28 ` [PATCH bpf-next 3/4] selftests/bpf: add checking of raw type dump in BTF writer APIs selftests Andrii Nakryiko
2020-09-29 23:28 ` [PATCH bpf-next 4/4] selftests/bpf: test "incremental" btf_dump in C format Andrii Nakryiko
2020-09-30  0:03 ` [PATCH bpf-next 0/4] libbpf: add raw BTF type dumping Alexei Starovoitov
2020-09-30  0:44   ` Andrii Nakryiko
2020-09-30  3:18     ` Alexei Starovoitov
2020-09-30 18:22       ` Andrii Nakryiko [this message]
2020-09-30 21:29         ` Alexei Starovoitov
2020-09-30 22:47           ` 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:

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

  git send-email \ \ \ \ \ \ \ \ \ \

* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.