Re: [PATCH v2 bpf-next 3/4] libbpf: deprecate legacy BPF map definitions

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:

> On Thu, Jan 20, 2022 at 3:44 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> Andrii Nakryiko <andrii@kernel.org> writes:
>>
>> > Enact deprecation of legacy BPF map definition in SEC("maps") ([0]). For
>> > the definitions themselves introduce LIBBPF_STRICT_MAP_DEFINITIONS flag
>> > for libbpf strict mode. If it is set, error out on any struct
>> > bpf_map_def-based map definition. If not set, libbpf will print out
>> > a warning for each legacy BPF map to raise awareness that it goes
>> > away.
>>
>> We've touched upon this subject before, but I (still) don't think it's a
>> good idea to remove this support entirely: It makes it impossible to
>> write a loader that can handle both new and old BPF objects.
>>
>> So discourage the use of the old map definitions, sure, but please don't
>> make it completely impossible to load such objects.
>
> BTF-defined maps have been around for quite a long time now and only
> have benefits on top of the bpf_map_def way. The source code
> translation is also very straightforward. If someone didn't get around
> to update their BPF program in 2 years, I don't think we can do much
> about that.
>
> Maybe instead of trying to please everyone (especially those that
> refuse to do anything to their BPF programs), let's work together to
> nudge laggards to actually modernize their source code a little bit
> and gain some benefits from that along the way?

I'm completely fine with nudging people towards the newer features, and
I think the compile-time deprecation warning when someone is using the
old-style map definitions in their BPF programs is an excellent way to
do that. 

I'm also fine with libbpf *by default* refusing to load programs that
use the old-style map definitions, but if the code is removed completely
it becomes impossible to write general-purpose loaders that can handle
both old and new programs. The obvious example of such a loader is
iproute2, the loader in xdp-tools is another.

> It's the same thinking with stricter section names, and all the other
> backwards incompatible changes that libbpf 1.0 will do.

If the plan is to refuse entirely to load programs that use the older
section names, then I obviously have the same objection to that idea :)

> If you absolutely cannot afford to drop support for all the
> to-be-removed things from libbpf, you'll have to stick to 0.x libbpf
> version. I assume (it will be up to disto maintainers, I suppose)
> you'll have that option.

As in, you expect distributions to package up the old libbpf in a
separate package? Really?

But either way, that doesn't really help; it just makes it a choice
between supporting new or old programs. Can't very well link to two
versions of the same library...

I really don't get why you're so insistent on removing that code either;
it's not like it's code that has a lot of churn (by definition), nor is
it very much code in the first place. But if it's a question of
maintenance burden I'm happy to help maintain it; or we could find some
other way of letting applications hook into the ELF object parsing so
the code doesn't have to live inside libbpf proper if that's more to you
liking?

-Toke