From: Jakub Kicinski <jakub.kicinski@netronome.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>,
Stanislav Fomichev <sdf@fomichev.me>,
Andrii Nakryiko <andriin@fb.com>,
LKML <linux-kernel@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
Networking <netdev@vger.kernel.org>,
Alexei Starovoitov <ast@fb.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH bpf-next 11/15] bpftool: add skeleton codegen command
Date: Thu, 12 Dec 2019 13:59:58 -0800 [thread overview]
Message-ID: <20191212135958.2970f188@cakuba.netronome.com> (raw)
In-Reply-To: <20191212212759.mhzrlqj5brcyfwgb@ast-mbp.dhcp.thefacebook.com>
On Thu, 12 Dec 2019 13:28:00 -0800, Alexei Starovoitov wrote:
> On Thu, Dec 12, 2019 at 12:21:15PM -0800, Jakub Kicinski wrote:
> > > > It can be a separate tool like
> > > > libbpf-skel-gen or libbpf-c-skel or something, distributed with libbpf.
> > > > That way you can actually soften the backward compat. In case people
> > > > become dependent on it they can carry that little tool on their own.
> > >
> > > Jakub,
> > >
> > > Could you please consider Andrii's reply to your comment from two days ago:
> > > https://lore.kernel.org/bpf/CAEf4BzbeZbmCTOOo2uQXjm0GL0WDu7aLN6fdUk18Nv2g0kfwVg@mail.gmail.com/
> > > "we are trying to make users lives easier by having major distributions
> > > distribute bpftool and libbpf properly. Adding extra binaries to
> > > distribute around doesn't seem to be easing any of users pains."
> >
> > Last time we argued I heard how GH makes libbpf packaging easier.
> > Only to have that dis-proven once the people in Europe who do distro
> > packaging woke up:
> >
> > https://lkml.org/lkml/2019/12/5/101
> > https://lkml.org/lkml/2019/12/5/312
>
> I think you missed the point of these two comments. It was about packaging
> bpftool and libbpf together. Regardless how bpftool is packaged. I still
> strongly suggest to use github/libbpf to package libbpf. It's something that is
> actually tested whereas libbpf in the kernel tree has unit test coverage only.
I disagree.
> > > My opinion is the following.
> > > bpftool is necessary to write bpf programs already. It's necessary to produce
> > > vmlinux.h for bpf programs to include it. It's part of build process. I can
> > > relate to Stan's complains that he needs to update clang and pahole. He missed
> > > the fact that he needs to update bpftool too if he wants to use all features of
> > > CO-RE. Same thing for skeleton generation. If people need to run the latest
> > > selftest/bpf on the latest kernel they need to upgrade to the latest clang,
> > > pahole, libbpf, bpftool. Nothing new here.
> >
> > They have to update libbpf, so why can't the code gen tool be part of
> > libbpf?
>
> I'm not sure why two answers were not enough.
> No idea how to answer this question differently for the third time.
I'm just presenting what I consider to be a cleaner solution.
> > > Backwards compat is the same concern for skeleton generation and for vmlinux.h
> > > generation. Obviously no one wants to introduce something that will keep
> > > changing. Is vmlinux.h generation stable? I like to believe so. Same with
> > > skeleton. I wouldn't want to see it changing, but in both cases such chance
> > > exists.
> >
> > vmlinux.h is pretty stable, there isn't much wiggle room there.
>
> Do you have experience working with vmlinux.h? I bet the answer is no.
> While we have and identified few things that needs improvement.
> They require vmlinux.h to be generated differently.
>
> > It's more of a conversion tool, if you will.
> >
> > Skeleton OTOH is supposed to make people's lives easier, so it's a
> > completely different beast. It should be malleable so that users can
> > improve and hack on it. Baking it into as system tool is counter
> > productive. Users should be able to grab the skel tool single-file
> > source and adjust for their project's needs. Distributing your own copy
> > of bpftool because you want to adjust skel is a heavy lift.
>
> Adjust generator for their custom needs? essentially fork it for
> private use? I'd rather prevent such possibility.
> When people start using it I'd prefer they come back to this mailing
> list with patches than do 'easy fork'.
>
> > > Now consider if vmlinux.h and skeleton generation is split out of bpftool into
> > > new tool. Effectively it would mean a fork of bpftool. Two binaries doing bpf
> > > elf file processing without clear distinction between them is going to be very
> > > confusing.
> >
> > To be clear I'm suggesting skel gen is a separate tool, vmlinux and
> > Quentin's header gen work on the running system, they are not pure
> > build env tools.
>
> You meant to say Andrii's header generator that is based on Quentin's man page
> generator. Its output bpf_helper_defs.h makes sense as a part of libbpf
> package. The generator script itself doesn't need to be included with any package.
> bpftool vmlinux gen consumes vmlinux elf files and is a part of the build.
> bpftool skeleton gen consumes bpf elf files and is a part of the same build.
I said what I meant to say tools/bpf/bpftool/feature.c
next prev parent reply other threads:[~2019-12-12 22:00 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-10 1:14 [PATCH bpf-next 00/15] Add code-generated BPF object skeleton support Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 01/15] libbpf: don't require root for bpf_object__open() Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 02/15] libbpf: add generic bpf_program__attach() Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 03/15] libbpf: move non-public APIs from libbpf.h to libbpf_internal.h Andrii Nakryiko
2019-12-10 1:33 ` Jakub Kicinski
2019-12-10 17:04 ` Andrii Nakryiko
2019-12-10 18:17 ` Jakub Kicinski
2019-12-10 18:47 ` Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 04/15] libbpf: add BPF_EMBED_OBJ macro for embedding BPF .o files Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 05/15] libbpf: expose field/var declaration emitting API internally Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 06/15] libbpf: expose BPF program's function name Andrii Nakryiko
2019-12-11 19:38 ` [Potential Spoof] " Martin Lau
2019-12-11 19:54 ` Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 07/15] libbpf: refactor global data map initialization Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 08/15] libbpf: postpone BTF ID finding for TRACING programs to load phase Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 09/15] libbpf: reduce log level of supported section names dump Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 10/15] libbpf: add experimental BPF object skeleton support Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 11/15] bpftool: add skeleton codegen command Andrii Nakryiko
2019-12-10 1:57 ` Jakub Kicinski
2019-12-10 17:11 ` Andrii Nakryiko
2019-12-10 18:05 ` Jakub Kicinski
2019-12-10 18:56 ` Andrii Nakryiko
2019-12-10 21:44 ` Stanislav Fomichev
2019-12-10 22:33 ` Andrii Nakryiko
2019-12-10 22:59 ` Stanislav Fomichev
2019-12-11 7:07 ` Andrii Nakryiko
2019-12-11 17:24 ` Stanislav Fomichev
2019-12-11 18:26 ` Andrii Nakryiko
2019-12-11 19:15 ` Stanislav Fomichev
2019-12-11 19:41 ` Andrii Nakryiko
2019-12-11 20:09 ` Stanislav Fomichev
2019-12-12 0:50 ` Andrii Nakryiko
2019-12-12 2:57 ` Stanislav Fomichev
2019-12-12 7:27 ` Andrii Nakryiko
2019-12-12 16:29 ` Stanislav Fomichev
2019-12-12 16:53 ` Andrii Nakryiko
2019-12-12 18:43 ` Jakub Kicinski
2019-12-12 18:58 ` Stanislav Fomichev
2019-12-12 19:23 ` Jakub Kicinski
2019-12-12 19:54 ` Alexei Starovoitov
2019-12-12 20:21 ` Jakub Kicinski
2019-12-12 21:28 ` Alexei Starovoitov
2019-12-12 21:59 ` Jakub Kicinski [this message]
2019-12-13 6:48 ` Andrii Nakryiko
2019-12-13 17:47 ` Jakub Kicinski
2019-12-12 21:45 ` Stanislav Fomichev
2019-12-13 6:23 ` Andrii Nakryiko
2019-12-11 22:50 ` [Potential Spoof] " Martin Lau
2019-12-16 14:16 ` Daniel Borkmann
2019-12-16 18:53 ` Andrii Nakryiko
2019-12-17 13:59 ` Daniel Borkmann
2019-12-17 15:45 ` Alexei Starovoitov
2019-12-10 1:14 ` [PATCH bpf-next 12/15] selftests/bpf: add BPF skeletons selftests and convert attach_probe.c Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 13/15] selftests/bpf: convert few more selftest to skeletons Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 14/15] selftests/bpf: add test validating data section to struct convertion layout Andrii Nakryiko
2019-12-10 1:14 ` [PATCH bpf-next 15/15] bpftool: add `gen skeleton` BASH completions Andrii Nakryiko
2019-12-11 22:55 ` [PATCH bpf-next 00/15] Add code-generated BPF object skeleton support Martin Lau
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=20191212135958.2970f188@cakuba.netronome.com \
--to=jakub.kicinski@netronome.com \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andriin@fb.com \
--cc=ast@fb.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=sdf@fomichev.me \
/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