Message-ID: <CAEf4Bzas1UPV_eDOJAu_rsMFegQrjNbPpovCmyjaa=cnDq1jxA@mail.gmail.com>
Date:   Wed, 30 Sep 2020 15:47:35 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH bpf-next 0/4] libbpf: add raw BTF type dumping

On Wed, Sep 30, 2020 at 2:29 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Wed, Sep 30, 2020 at 11:22:50AM -0700, Andrii Nakryiko wrote:
> >
> > If you are saying it should emit it in Go format, Rust format, or
> > other language-specific way, then sure,
>
> Yes. that's what I'm saying. cloudflare and cilium are favoring golang.
> Hopefully they can adopt skeleton when it's generated in golang.
> It would probably mean some support from libbpf and vmlinux.go
> Which means BTF dumping in golang.

Yes, if they were to adopt the skeleton approach, we'd need some sort
of BTF-to-Go struct dumping. But as for vmlinux.h, keep in mind that
that thing is supposed to be only included from the BPF side, which so
far is always pure C (apart from RedBPF approach of compiling Rust
code into BPF code). I don't think we want to have BPF-side code
written in Go?

>
> > 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.
>
> sure. that's fine.
>
> > 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).
>
> yes. New or reusing api doesn't matter much.
> The question is what dumpers libbpf provides.
>
> > 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.
>
> why not. If libbpf does more than one there is no reason to restrict.

just extra code and maintenance burden without clear benefits, that's
the only reason

>
> >
> > > 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.
>
> Ok, but it's easy to add dumping of these extra data into vmlinux.h
> They can come inside /* */ or as 'extern'.
> So C output can be complete and suitable for selftest's strcmp.

yeah, comments might work to "augment" vmlinux.h. There is still the
question of output type ordering, it's not always a single unique
ordering, which makes it harder to use for testing arbitrary BTFs. I
was very careful with existing BTF dump tests to ensure the order of
types is unique, but as a general case that's not true.

E.g., these two are equivalent:

struct a;

struct b { struct a *a; };

struct a { struct b *b; };

And:

struct b;

struct a { struct b *b; };

struct b { struct a *a; };

>
> > 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.
>
> Applied 1 and 4. I was waiting to patchwork bot to notice this partial

thanks!

> application, but looks like it's not that smart... yet.

software, maybe some day :)

Powered by blists - more mailing lists