libbpf: fix reuse of pinned map on older kernel

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 <[email protected]>

Quoted 22 lines · Martynas Pumputis

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.

Quoted 1 line

Signed-off-by Martynas Pumputis <[email protected]>

The code looks good to me. Except a checkpatch CHECK:

CHECK: Comparison to NULL could be written "!obj"
#96: FILE: tools/lib/bpf/libbpf.c:3943:
+ if (obj == NULL || kernel_supports(obj, FEAT_OBJ_GET_INFO_BY_FD))

Also, I think this should target bpf-next tree?

Thanks,
Song

Quoted 23 lines · Song Liu

Quoted 22 lines · Martynas Pumputis

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.

Quoted 7 lines

Quoted 1 line

Signed-off-by Martynas Pumputis <[email protected]>

The code looks good to me. Except a checkpatch CHECK:

CHECK: Comparison to NULL could be written "!obj"
#96: FILE: tools/lib/bpf/libbpf.c:3943:
+ if (obj == NULL || kernel_supports(obj, FEAT_OBJ_GET_INFO_BY_FD))

Thanks for the review. I will send v2 with the fix.

Quoted 1 line

Quoted 1 line

Also, I think this should target bpf-next tree?

Considering that libbpf is supported on older kernels, w/o this patch it is impossible to use it on < 4.12 kernels for programs with pinned maps. Therefore, I think that this is a fix and thus it should target the bpf tree instead.

Quoted 5 lines

Thanks,
Song

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 <[email protected]>

---
 tools/lib/bpf/libbpf.c | 103 +++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 92 insertions(+), 11 deletions(-)

tools/lib/bpf/libbpf.c +6 -0

+               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;
+               }

Quoted 27 lines · Martynas Pumputis

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 <[email protected]>

---
  tools/lib/bpf/libbpf.c | 103 +++++++++++++++++++++++++++++++++++++++++++++------
  1 file changed, 92 insertions(+), 11 deletions(-)

[...]

Quoted 4 lines

tools/lib/bpf/libbpf.c +0 -0

@@ -4406,10 +4478,19 @@ static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd)
 
         map_info_len = sizeof(map_info);
 

tools/lib/bpf/libbpf.c static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd) · line 4481

Quoted 5 lines

tools/lib/bpf/libbpf.c +1 -4

-       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?

Quoted 8 lines

tools/lib/bpf/libbpf.c +6 -0

+               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;
+               }

[...]

Quoted 35 lines · Andrii Nakryiko

Quoted 27 lines · Martynas Pumputis

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 <[email protected]>

---
  tools/lib/bpf/libbpf.c | 103 +++++++++++++++++++++++++++++++++++++++++++++------
  1 file changed, 92 insertions(+), 11 deletions(-)

[...]

Quoted 4 lines

tools/lib/bpf/libbpf.c +0 -0

@@ -4406,10 +4478,19 @@ static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd)
 
         map_info_len = sizeof(map_info);
 

Quoted 12 lines

tools/lib/bpf/libbpf.c static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd) · line 4481

Quoted 5 lines

tools/lib/bpf/libbpf.c +1 -4

-       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].

Quoted 35 lines · Andrii Nakryiko

Quoted 27 lines · Martynas Pumputis

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 <[email protected]>

---
  tools/lib/bpf/libbpf.c | 103 +++++++++++++++++++++++++++++++++++++++++++++------
  1 file changed, 92 insertions(+), 11 deletions(-)

[...]

Quoted 4 lines

tools/lib/bpf/libbpf.c +0 -0

@@ -4406,10 +4478,19 @@ static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd)
 
         map_info_len = sizeof(map_info);
 

Quoted 12 lines

tools/lib/bpf/libbpf.c static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd) · line 4481

Quoted 5 lines

tools/lib/bpf/libbpf.c +1 -4

-       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

Quoted 11 lines

Quoted 8 lines

tools/lib/bpf/libbpf.c +6 -0

+               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;
+               }

[...]