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. 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].
Anyway, if you think that we can ignore 1) and 2), then I'm happy to change. Also, I'm going to submit to bpf-next.
[1]: https://github.com/shemminger/iproute2/blob/v5.11.0/lib/bpf_libbpf.c#L98
+ 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; + }[...]
