* [PATCH bpf-next] libbpf: Let any two INT/UNION compatible if their names and sizes match
@ 2021-12-01 3:54 Feng zhou
2021-12-01 4:17 ` Andrii Nakryiko
0 siblings, 1 reply; 3+ messages in thread
From: Feng zhou @ 2021-12-01 3:54 UTC (permalink / raw)
To: ast, daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
kpsingh
Cc: netdev, bpf, linux-kernel, duanxiongchun, songmuchun,
zhouchengming, zhoufeng.zf
From: Feng Zhou <zhoufeng.zf@bytedance.com>
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 <zhouchengming@bytedance.com>
Tested-by: Chengming Zhou <zhouchengming@bytedance.com>
Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH bpf-next] libbpf: Let any two INT/UNION compatible if their names and sizes match
2021-12-01 3:54 [PATCH bpf-next] libbpf: Let any two INT/UNION compatible if their names and sizes match Feng zhou
@ 2021-12-01 4:17 ` Andrii Nakryiko
2021-12-01 8:54 ` [External] " Feng Zhou
0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2021-12-01 4:17 UTC (permalink / raw)
To: Feng zhou
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, Networking,
bpf, open list, duanxiongchun, Muchun Song, zhouchengming
On Tue, Nov 30, 2021 at 7:55 PM Feng zhou <zhoufeng.zf@bytedance.com> wrote:
>
> From: Feng Zhou <zhoufeng.zf@bytedance.com>
>
> 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 <zhouchengming@bytedance.com>
> Tested-by: Chengming Zhou <zhouchengming@bytedance.com>
> Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
> ---
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [External] Re: [PATCH bpf-next] libbpf: Let any two INT/UNION compatible if their names and sizes match
2021-12-01 4:17 ` Andrii Nakryiko
@ 2021-12-01 8:54 ` Feng Zhou
0 siblings, 0 replies; 3+ messages in thread
From: Feng Zhou @ 2021-12-01 8:54 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, Networking,
bpf, open list, duanxiongchun, Muchun Song, zhouchengming
在 2021/12/1 下午12:17, Andrii Nakryiko 写道:
> On Tue, Nov 30, 2021 at 7:55 PM Feng zhou <zhoufeng.zf@bytedance.com> wrote:
>> From: Feng Zhou <zhoufeng.zf@bytedance.com>
>>
>> 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 <zhouchengming@bytedance.com>
>> Tested-by: Chengming Zhou <zhouchengming@bytedance.com>
>> Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
>> ---
> 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
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-12-01 8:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-01 3:54 [PATCH bpf-next] libbpf: Let any two INT/UNION compatible if their names and sizes match Feng zhou
2021-12-01 4:17 ` Andrii Nakryiko
2021-12-01 8:54 ` [External] " Feng Zhou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox