Martynas Pumputis <m@xxxxxxxxx> writes:
> On 7/8/21 12:58 AM, Andrii Nakryiko wrote:
>> On Tue, Jul 6, 2021 at 10:24 AM Martynas Pumputis <m@xxxxxxxxx> wrote:
>>>
>>> When loading a BPF program with a pinned map, the loader checks whether
>>> the pinned map can be reused, i.e. their properties match. To derive
>>> such of the pinned map, the loader invokes BPF_OBJ_GET_INFO_BY_FD and
>>> then does the comparison.
>>>
>>> Unfortunately, on < 4.12 kernels the BPF_OBJ_GET_INFO_BY_FD is not
>>> available, so loading the program fails with the following error:
>>>
>>> libbpf: failed to get map info for map FD 5: Invalid argument
>>> libbpf: couldn't reuse pinned map at
>>> '/sys/fs/bpf/tc/globals/cilium_call_policy': parameter
>>> mismatch"
>>> libbpf: map 'cilium_call_policy': error reusing pinned map
>>> libbpf: map 'cilium_call_policy': failed to create:
>>> Invalid argument(-22)
>>> libbpf: failed to load object 'bpf_overlay.o'
>>>
>>> To fix this, probe the kernel for BPF_OBJ_GET_INFO_BY_FD support. If it
>>> doesn't support, then fallback to derivation of the map properties via
>>> /proc/$PID/fdinfo/$MAP_FD.
>>>
>>> Signed-off-by: Martynas Pumputis <m@xxxxxxxxx>
>>> ---
>>> tools/lib/bpf/libbpf.c | 103 +++++++++++++++++++++++++++++++++++++++++++++------
>>> 1 file changed, 92 insertions(+), 11 deletions(-)
>>>
>>
>> [...]
>>
>>> @@ -4406,10 +4478,19 @@ static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd)
>>>
>>> map_info_len = sizeof(map_info);
>>>
>>> - if (bpf_obj_get_info_by_fd(map_fd, &map_info, &map_info_len)) {
>>> - pr_warn("failed to get map info for map FD %d: %s\n",
>>> - map_fd, libbpf_strerror_r(errno, msg, sizeof(msg)));
>>> - return false;
>>> + if (kernel_supports(obj, FEAT_OBJ_GET_INFO_BY_FD)) {
>>
>> why not just try bpf_obj_get_info_by_fd() first, and if it fails
>> always fallback to bpf_get_map_info_from_fdinfo(). No need to do
>> feature detection. This will cut down on the amount of code without
>> any regression in behavior. More so, it will actually now be
>> consistent and good behavior in case of bpf_map__reuse_fd() where we
>> don't have obj. WDYT?
>
> I was thinking about it, but then decided to use the kernel probing
> instead. The reasoning was the following:
>
> 1) For programs with many pinned maps we would issue many failing
> BPF_OBJ_GET_INFO_BY_FD calls (instead of a single one) which might
> hinder the performance.
"Might" hinder the performance? Did you measure this? We're usually
talking (at most) dozens of maps per object file, not thousands; also,
this would only be incurred if the initial call actually fails (i.e., on
old kernels).
> 2) A canonical way in libbpf to detect features is via kernel_supports()
> and friends, so I didn't want to diverge there.
>
> Re bpf_map__reuse_fd(), if we are OK to break the API before libbpf
> v1.0, then we could extend bpf_map__reuse_fd() to accept the obj.
> However, this would break some consumers of the lib, e.g., iproute2 [1].
IMO, this does not sound like something worth breaking the API
compatibility for...
-Toke
