Message-ID: <CAADnVQKDOWz7fW0kxGEeLtMJLf7J5v9Un=uDXKmwhkweoVQ3Lw@mail.gmail.com>
Date:   Sun, 28 Mar 2021 18:16:45 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Andrii Nakryiko <andrii@...nel.org>, 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 3/3] selftests/bpf: allow compiling BPF objects
 without BTF

On Fri, Mar 26, 2021 at 9:44 AM Andrii Nakryiko
<andrii.nakryiko@...il.com> wrote:
> > Because they double the maintenance cost now and double the support forever.
> > We never needed to worry about skeleton without BTF and now it would be
> > a thing ? So all tests realistically need to be doubled: with and without BTF.
>
> 2x? Realistically?.. No, I wouldn't say so. :) Extra test or a few
> might be warranted, but doubling the amount of testing is a huge
> exaggeration.

It's not only doubling the test coverage, but it would double the development
effort to keep everything w/BTF and w/o BTF functional.
So 2x is far from exaggeration.

> > Even more so for static linking. If one .o has BTF and another doesn't
> > what linker suppose to do? Keep it, but the linked BTF will sort of cover
> > both .o-s, but line info in some funcs will be missing.
> > All these weird combinations would need to be tested.
>
> BPF static linker already supports that mode, btw.

Are you considering one-liner commit 78b226d48106 ("libbpf: Skip BTF
fixup if object file has no BTF")
as support for static linking w/o BTF?
Jiri's other email pointed out another place in libbpf that breaks w/o BTF.
The only thing the commit 78b226d48106 achieved is it closed
the one case of static linker crashing w/o BTF.
Does linker do anything sensible when a mix of .o with and without BTF
is given? No.
It happens not to crash w/o BTF. That's about it.

> And yes, it
> shouldn't crash the kernel. And you don't need a skeleton or static
> linker to do that to the kernel, so I don't know how that is a new
> mode of operation.
>
> > The sensible thing to do would be to reject skel generation without BTF
> > and reject linking without BTF. The user most likely forgot -g during
> > compilation of bpf prog. The bpftool should give the helpful message
> > in such case. Whether it's generating skel or linking. Silently proceeding
> > and generating half-featured skeleton is not what user wanted.
>
> Sure, a warning message makes sense. Outright disabling this - not so
> much. I still can't get why I can't get BPF skeleton and static
> linking without BTF, if I really want to. Both are useful without BTF.

What are the cases that would benefit?
So far skeleton was used by tracing progs only. Those that need CO-RE.
Which means they must have BTF.
Networking progs didn't need CO-RE and no one bothered adopting
skeleton because of that.
Are you implying that a BTF-less skeleton will be useful for
networking progs? What for?
So far I see only downsides from increasing the amount of work needed
to support skel and static linking w/o BTF. The extra 100% or just 1%
would be equally taxing, since it's extra work for the foreseeable future and
compounded 1% will add up.

Looking at it from another angle... the skeleton generation existed
for more than
a year now and it implicitly required BTF. That requirement was not
written down.
Now you're proposing to support skeleton gen w/o BTF when not a single user
requested it and not providing any advantages of such support while
ignoring the long term maintenance of such effort.

Another angle... I did git grep in selftests that use skeleton.
Only a handful of tests use it as *skel*__open_and_load() w/o global data
(which would still work w/o BTF)
and only because we forcefully converted them when skel was introduced.
bcc/libbpf-tools/* need skel with BTF.
I couldn't find a _single_ case where people use
skeleton and don't need BTF driven parts of it.

> So I don't know, it's the third different argument I'm addressing
without any conclusion on the previous two.

So far you haven't addressed the first question:
Who is asking for a BTF-less skeleton? What for? What features are requested?
I've seen none of such requests.

BTF is not a debug info of the BPF program.
If it was then I would agree that compiling with and without -g shouldn't
make a difference.
But BTF is not a debug-info. It's type and much more description of the program
that is mandatory for the verification of the program.
btf_id-based attach, CO-RE, trampoline, calling kernel funcs, etc all
require BTF.
Not because it's convenient to use BTF, but because assembly doesn't
have enough information.
There is no C, C++, Rust equivalent of BTF. There is none in the user
space world.
Traditional languages translate a language into assembly code and cpus
execute it.
Static analyzers use high level languages to understand the intent.
BPF workflow translates a language into assembly and BTF, so that the verifier
can see the intent. It could happen that BPF will gain something else beyond BTF
and it will become a mandatory part of the workflow. Just like BTF is today.
At that point all new features will be supported with that new
"annotation" only,
not because of "subjective personal preferences", but because that is
a fundamental
program description.

Powered by blists - more mailing lists