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

From: David Ahern <dsahern@gmail.com>
To: "Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Andrii Nakryiko" <andrii.nakryiko@gmail.com>
Cc: Andrii Nakryiko <andrii@kernel.org>, bpf <bpf@vger.kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Kernel Team <kernel-team@fb.com>,
	David Ahern <dsahern@kernel.org>,
	Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [PATCH v2 bpf-next 3/4] libbpf: deprecate legacy BPF map definitions
Date: Fri, 21 Jan 2022 15:04:01 -0700	[thread overview]
Message-ID: <e58aabf9-af55-8eac-1047-b18801141d80@gmail.com> (raw)
In-Reply-To: <87lez87rbm.fsf@toke.dk>

On 1/21/22 1:43 PM, Toke Høiland-Jørgensen wrote:
> 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.
> 

I agree with Toke's response.

2 years is a very small amount of time when it comes to OS and kernel
versions. Many companies base products on enterprise distributions and
run them for 10+ years. During that time there will be needs to update
some components - like kernel version or some tool but that is done with
the least amount of churn possible. Every update has the potential to
bring in unknown behavior changes. Requiring updates to entire tool
chains, multiple tool sets and libraries to accommodate some deprecation
will only hinder being able to update anything.

Further, programs (e.g., debugging as just one example) can and will
need to be used across many OS and kernel versions.