From: Feng Zhou <[email protected]>
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 <[email protected]>
Tested-by: Chengming Zhou <[email protected]>
Signed-off-by: Feng Zhou <[email protected]>
---
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 <[email protected]> wrote:
>
> From: Feng Zhou <[email protected]>
>
> 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 <[email protected]>
> Tested-by: Chengming Zhou <[email protected]>
> Signed-off-by: Feng Zhou <[email protected]>
> ---
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
> 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
>
在 2021/12/1 下午12:17, Andrii Nakryiko 写道:
> On Tue, Nov 30, 2021 at 7:55 PM Feng zhou <[email protected]> wrote:
>> From: Feng Zhou <[email protected]>
>>
>> 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 <[email protected]>
>> Tested-by: Chengming Zhou <[email protected]>
>> Signed-off-by: Feng Zhou <[email protected]>
>> ---
> 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.
>> 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
>>