[PATCH bpf-next] libbpf: Let any two INT/UNION compatible if their names and sizes match

From: Feng Zhou <redacted>

commit:67c0496e87d193b8356d2af49ab95e8a1b954b3c(kernfs: convert
kernfs_node->id from union kernfs_node_id to u64).

The bpf program compiles on the kernel version after this commit and
then tries to run on the kernel before this commit, libbpf will report
an error. The reverse is also same.

libbpf: prog 'tcp_retransmit_synack_tp': relo #4: kind <byte_off> (0),
spec is [342] struct kernfs_node.id (0:9 @ offset 104)
libbpf: prog 'tcp_retransmit_synack_tp': relo #4: non-matching candidate
libbpf: prog 'tcp_retransmit_synack_tp': relo #4: non-matching candidate
libbpf: prog 'tcp_retransmit_synack_tp': relo #4: no matching targets
found

The type before this commit:
	union kernfs_node_id	id;
	union kernfs_node_id {
		struct {
			u32		ino;
			u32		generation;
		};
		u64			id;
	};

The type after this commit:
	u64 id;

We can find that the variable name and size have not changed except for
the type change.
So I added some judgment to let any two INT/UNION are compatible, if
their names and sizes match.

Reported-by: Chengming Zhou <redacted>
Tested-by: Chengming Zhou <redacted>
Signed-off-by: Feng Zhou <redacted>
---
 tools/lib/bpf/relo_core.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
index b5b8956a1be8..ff7f4e97bafb 100644
--- a/tools/lib/bpf/relo_core.c
+++ b/tools/lib/bpf/relo_core.c

@@ -294,6 +294,7 @@ static int bpf_core_parse_spec(const struct btf *btf,
  *   - any two FLOATs are always compatible;
  *   - for ARRAY, dimensionality is ignored, element types are checked for
  *     compatibility recursively;
+ *   - any two INT/UNION are compatible, if their names and sizes match;
  *   - everything else shouldn't be ever a target of relocation.
  * These rules are not set in stone and probably will be adjusted as we get
  * more experience with using BPF CO-RE relocations.

@@ -313,8 +314,14 @@ static int bpf_core_fields_are_compat(const struct btf *local_btf,
 
 	if (btf_is_composite(local_type) && btf_is_composite(targ_type))
 		return 1;
-	if (btf_kind(local_type) != btf_kind(targ_type))
-		return 0;
+	if (btf_kind(local_type) != btf_kind(targ_type)) {
+		if (local_type->size == targ_type->size &&
+		    (btf_is_union(local_type) || btf_is_union(targ_type)) &&
+		    (btf_is_int(local_type) || btf_is_int(targ_type)))
+			return 1;
+		else
+			return 0;
+	}
 
 	switch (btf_kind(local_type)) {
 	case BTF_KIND_PTR:

@@ -384,11 +391,17 @@ static int bpf_core_match_member(const struct btf *local_btf,
 	targ_type = skip_mods_and_typedefs(targ_btf, targ_id, &targ_id);
 	if (!targ_type)
 		return -EINVAL;
-	if (!btf_is_composite(targ_type))
-		return 0;
 
 	local_id = local_acc->type_id;
 	local_type = btf__type_by_id(local_btf, local_id);
+	if (!btf_is_composite(targ_type)) {
+		if (local_type->size == targ_type->size &&
+		    btf_is_union(local_type) && btf_is_int(targ_type))
+			return 1;
+		else
+			return 0;
+	}
+
 	local_member = btf_members(local_type) + local_acc->idx;
 	local_name = btf__name_by_offset(local_btf, local_member->name_off);
 

-- 
2.11.0

On Tue, Nov 30, 2021 at 7:55 PM Feng zhou [off-list ref] wrote:

From: Feng Zhou <redacted>

commit:67c0496e87d193b8356d2af49ab95e8a1b954b3c(kernfs: convert
kernfs_node->id from union kernfs_node_id to u64).

The bpf program compiles on the kernel version after this commit and
then tries to run on the kernel before this commit, libbpf will report
an error. The reverse is also same.

libbpf: prog 'tcp_retransmit_synack_tp': relo #4: kind <byte_off> (0),
spec is [342] struct kernfs_node.id (0:9 @ offset 104)
libbpf: prog 'tcp_retransmit_synack_tp': relo #4: non-matching candidate
libbpf: prog 'tcp_retransmit_synack_tp': relo #4: non-matching candidate
libbpf: prog 'tcp_retransmit_synack_tp': relo #4: no matching targets
found

The type before this commit:
        union kernfs_node_id    id;
        union kernfs_node_id {
                struct {
                        u32             ino;
                        u32             generation;
                };
                u64                     id;
        };

The type after this commit:
        u64 id;

We can find that the variable name and size have not changed except for
the type change.
So I added some judgment to let any two INT/UNION are compatible, if
their names and sizes match.

Reported-by: Chengming Zhou <redacted>
Tested-by: Chengming Zhou <redacted>
Signed-off-by: Feng Zhou <redacted>
---

This should be handled by application, not by hacking libbpf's CO-RE
relocation logic. See [0] for how this should be done with existing
BPF CO-RE mechanisms.

  [0] https://nakryiko.com/posts/bpf-core-reference-guide/#handling-incompatible-field-and-type-changes

在 2021/12/1 下午12:17, Andrii Nakryiko 写道:

On Tue, Nov 30, 2021 at 7:55 PM Feng zhou [off-list ref] wrote:

quoted

From: Feng Zhou <redacted>

commit:67c0496e87d193b8356d2af49ab95e8a1b954b3c(kernfs: convert
kernfs_node->id from union kernfs_node_id to u64).

The bpf program compiles on the kernel version after this commit and
then tries to run on the kernel before this commit, libbpf will report
an error. The reverse is also same.

libbpf: prog 'tcp_retransmit_synack_tp': relo #4: kind <byte_off> (0),
spec is [342] struct kernfs_node.id (0:9 @ offset 104)
libbpf: prog 'tcp_retransmit_synack_tp': relo #4: non-matching candidate
libbpf: prog 'tcp_retransmit_synack_tp': relo #4: non-matching candidate
libbpf: prog 'tcp_retransmit_synack_tp': relo #4: no matching targets
found

The type before this commit:
         union kernfs_node_id    id;
         union kernfs_node_id {
                 struct {
                         u32             ino;
                         u32             generation;
                 };
                 u64                     id;
         };

The type after this commit:
         u64 id;

We can find that the variable name and size have not changed except for
the type change.
So I added some judgment to let any two INT/UNION are compatible, if
their names and sizes match.

Reported-by: Chengming Zhou <redacted>
Tested-by: Chengming Zhou <redacted>
Signed-off-by: Feng Zhou <redacted>
---

This should be handled by application, not by hacking libbpf's CO-RE
relocation logic. See [0] for how this should be done with existing
BPF CO-RE mechanisms.

   [0] https://nakryiko.com/posts/bpf-core-reference-guide/#handling-incompatible-field-and-type-changes

This is very useful to me, thank you very much.

quoted

  tools/lib/bpf/relo_core.c | 21 +++++++++++++++++----
  1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
index b5b8956a1be8..ff7f4e97bafb 100644
--- a/tools/lib/bpf/relo_core.c
+++ b/tools/lib/bpf/relo_core.c

@@ -294,6 +294,7 @@ static int bpf_core_parse_spec(const struct btf *btf,
   *   - any two FLOATs are always compatible;
   *   - for ARRAY, dimensionality is ignored, element types are checked for
   *     compatibility recursively;
+ *   - any two INT/UNION are compatible, if their names and sizes match;
   *   - everything else shouldn't be ever a target of relocation.
   * These rules are not set in stone and probably will be adjusted as we get
   * more experience with using BPF CO-RE relocations.

@@ -313,8 +314,14 @@ static int bpf_core_fields_are_compat(const struct btf *local_btf,

         if (btf_is_composite(local_type) && btf_is_composite(targ_type))
                 return 1;
-       if (btf_kind(local_type) != btf_kind(targ_type))
-               return 0;
+       if (btf_kind(local_type) != btf_kind(targ_type)) {
+               if (local_type->size == targ_type->size &&
+                   (btf_is_union(local_type) || btf_is_union(targ_type)) &&
+                   (btf_is_int(local_type) || btf_is_int(targ_type)))
+                       return 1;
+               else
+                       return 0;
+       }

         switch (btf_kind(local_type)) {
         case BTF_KIND_PTR:

@@ -384,11 +391,17 @@ static int bpf_core_match_member(const struct btf *local_btf,
         targ_type = skip_mods_and_typedefs(targ_btf, targ_id, &targ_id);
         if (!targ_type)
                 return -EINVAL;
-       if (!btf_is_composite(targ_type))
-               return 0;

         local_id = local_acc->type_id;
         local_type = btf__type_by_id(local_btf, local_id);
+       if (!btf_is_composite(targ_type)) {
+               if (local_type->size == targ_type->size &&
+                   btf_is_union(local_type) && btf_is_int(targ_type))
+                       return 1;
+               else
+                       return 0;
+       }
+
         local_member = btf_members(local_type) + local_acc->idx;
         local_name = btf__name_by_offset(local_btf, local_member->name_off);

--

2.11.0