* [PATCH bpf-next] bpftool: Fix missing pids during link show
@ 2024-03-11 21:41 Yonghong Song
2024-03-11 23:13 ` Andrii Nakryiko
0 siblings, 1 reply; 5+ messages in thread
From: Yonghong Song @ 2024-03-11 21:41 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau, Quentin Monnet
Current 'bpftool link' command does not show pids, e.g.,
$ tools/build/bpftool/bpftool link
...
4: tracing prog 23
prog_type lsm attach_type lsm_mac
target_obj_id 1 target_btf_id 31320
Hack the following change to enable normal libbpf debug output,
--- a/tools/bpf/bpftool/pids.c
+++ b/tools/bpf/bpftool/pids.c
@@ -121,9 +121,9 @@ int build_obj_refs_table(struct hashmap **map, enum bpf_obj_type type)
/* we don't want output polluted with libbpf errors if bpf_iter is not
* supported
*/
- default_print = libbpf_set_print(libbpf_print_none);
+ /* default_print = libbpf_set_print(libbpf_print_none); */
err = pid_iter_bpf__load(skel);
- libbpf_set_print(default_print);
+ /* libbpf_set_print(default_print); */
Rerun the above bpftool command:
$ tools/build/bpftool/bpftool link
libbpf: prog 'iter': BPF program load failed: Permission denied
libbpf: prog 'iter': -- BEGIN PROG LOAD LOG --
0: R1=ctx() R10=fp0
; struct task_struct *task = ctx->task; @ pid_iter.bpf.c:69
0: (79) r6 = *(u64 *)(r1 +8) ; R1=ctx() R6_w=ptr_or_null_task_struct(id=1)
; struct file *file = ctx->file; @ pid_iter.bpf.c:68
...
; struct bpf_link *link = (struct bpf_link *) file->private_data; @ pid_iter.bpf.c:103
80: (79) r3 = *(u64 *)(r8 +432) ; R3_w=scalar() R8=ptr_file()
; if (link->type == bpf_core_enum_value(enum bpf_link_type___local, @ pid_iter.bpf.c:105
81: (61) r1 = *(u32 *)(r3 +12)
R3 invalid mem access 'scalar'
processed 39 insns (limit 1000000) max_states_per_insn 0 total_states 3 peak_states 3 mark_read 2
-- END PROG LOAD LOG --
libbpf: prog 'iter': failed to load: -13
...
The 'file->private_data' returns a 'void' type and this caused subsequent 'link->type'
(insn #81) failed in verification.
To fix the issue, do a type cast from 'void *' to 'struct bpf_link *' for file->private_data
as in this patch, and the 'bpftool link' runs successfully with 'pids'.
$ tools/build/bpftool/bpftool link
...
4: tracing prog 23
prog_type lsm attach_type lsm_mac
target_obj_id 1 target_btf_id 31320
pids systemd(1)
Fixes: 44ba7b30e84f ("bpftool: Use a local copy of BPF_LINK_TYPE_PERF_EVENT in pid_iter.bpf.c")
Cc: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
tools/bpf/bpftool/skeleton/pid_iter.bpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
index 26004f0c5a6a..96ffcc4f0e67 100644
--- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
+++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
@@ -100,7 +100,7 @@ int iter(struct bpf_iter__task_file *ctx)
if (obj_type == BPF_OBJ_LINK &&
bpf_core_enum_value_exists(enum bpf_link_type___local,
BPF_LINK_TYPE_PERF_EVENT___local)) {
- struct bpf_link *link = (struct bpf_link *) file->private_data;
+ struct bpf_link *link = bpf_core_cast(file->private_data, struct bpf_link);
if (link->type == bpf_core_enum_value(enum bpf_link_type___local,
BPF_LINK_TYPE_PERF_EVENT___local)) {
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpftool: Fix missing pids during link show
2024-03-11 21:41 [PATCH bpf-next] bpftool: Fix missing pids during link show Yonghong Song
@ 2024-03-11 23:13 ` Andrii Nakryiko
2024-03-11 23:30 ` Yonghong Song
0 siblings, 1 reply; 5+ messages in thread
From: Andrii Nakryiko @ 2024-03-11 23:13 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Martin KaFai Lau, Quentin Monnet
On Mon, Mar 11, 2024 at 2:41 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
> Current 'bpftool link' command does not show pids, e.g.,
> $ tools/build/bpftool/bpftool link
> ...
> 4: tracing prog 23
> prog_type lsm attach_type lsm_mac
> target_obj_id 1 target_btf_id 31320
>
> Hack the following change to enable normal libbpf debug output,
> --- a/tools/bpf/bpftool/pids.c
> +++ b/tools/bpf/bpftool/pids.c
> @@ -121,9 +121,9 @@ int build_obj_refs_table(struct hashmap **map, enum bpf_obj_type type)
> /* we don't want output polluted with libbpf errors if bpf_iter is not
> * supported
> */
> - default_print = libbpf_set_print(libbpf_print_none);
> + /* default_print = libbpf_set_print(libbpf_print_none); */
> err = pid_iter_bpf__load(skel);
> - libbpf_set_print(default_print);
> + /* libbpf_set_print(default_print); */
>
> Rerun the above bpftool command:
> $ tools/build/bpftool/bpftool link
> libbpf: prog 'iter': BPF program load failed: Permission denied
> libbpf: prog 'iter': -- BEGIN PROG LOAD LOG --
> 0: R1=ctx() R10=fp0
> ; struct task_struct *task = ctx->task; @ pid_iter.bpf.c:69
> 0: (79) r6 = *(u64 *)(r1 +8) ; R1=ctx() R6_w=ptr_or_null_task_struct(id=1)
> ; struct file *file = ctx->file; @ pid_iter.bpf.c:68
> ...
> ; struct bpf_link *link = (struct bpf_link *) file->private_data; @ pid_iter.bpf.c:103
> 80: (79) r3 = *(u64 *)(r8 +432) ; R3_w=scalar() R8=ptr_file()
> ; if (link->type == bpf_core_enum_value(enum bpf_link_type___local, @ pid_iter.bpf.c:105
> 81: (61) r1 = *(u32 *)(r3 +12)
> R3 invalid mem access 'scalar'
> processed 39 insns (limit 1000000) max_states_per_insn 0 total_states 3 peak_states 3 mark_read 2
> -- END PROG LOAD LOG --
> libbpf: prog 'iter': failed to load: -13
> ...
>
> The 'file->private_data' returns a 'void' type and this caused subsequent 'link->type'
> (insn #81) failed in verification.
>
> To fix the issue, do a type cast from 'void *' to 'struct bpf_link *' for file->private_data
> as in this patch, and the 'bpftool link' runs successfully with 'pids'.
> $ tools/build/bpftool/bpftool link
> ...
> 4: tracing prog 23
> prog_type lsm attach_type lsm_mac
> target_obj_id 1 target_btf_id 31320
> pids systemd(1)
>
> Fixes: 44ba7b30e84f ("bpftool: Use a local copy of BPF_LINK_TYPE_PERF_EVENT in pid_iter.bpf.c")
> Cc: Quentin Monnet <quentin@isovalent.com>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
> tools/bpf/bpftool/skeleton/pid_iter.bpf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
> index 26004f0c5a6a..96ffcc4f0e67 100644
> --- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
> +++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
> @@ -100,7 +100,7 @@ int iter(struct bpf_iter__task_file *ctx)
> if (obj_type == BPF_OBJ_LINK &&
> bpf_core_enum_value_exists(enum bpf_link_type___local,
> BPF_LINK_TYPE_PERF_EVENT___local)) {
> - struct bpf_link *link = (struct bpf_link *) file->private_data;
> + struct bpf_link *link = bpf_core_cast(file->private_data, struct bpf_link);
bpf_core_cast() (i.e., bpf_rdonly_cast()) is newer feature, so relying
on it in bpftool will make PID printing not work on older kernels that
generally do support iter/task_file iterators.
>
> if (link->type == bpf_core_enum_value(enum bpf_link_type___local,
the problem is that we dropped BPF_CORE_READ() here and went with
`link->type`. Should we restore BPF_CORE_READ(link, type) here and
solve the problem, while also supporting this feature on wider set of
kernels? Quentin?
> BPF_LINK_TYPE_PERF_EVENT___local)) {
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpftool: Fix missing pids during link show
2024-03-11 23:13 ` Andrii Nakryiko
@ 2024-03-11 23:30 ` Yonghong Song
2024-03-12 14:00 ` Quentin Monnet
0 siblings, 1 reply; 5+ messages in thread
From: Yonghong Song @ 2024-03-11 23:30 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Martin KaFai Lau, Quentin Monnet
On 3/11/24 4:13 PM, Andrii Nakryiko wrote:
> On Mon, Mar 11, 2024 at 2:41 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>> Current 'bpftool link' command does not show pids, e.g.,
>> $ tools/build/bpftool/bpftool link
>> ...
>> 4: tracing prog 23
>> prog_type lsm attach_type lsm_mac
>> target_obj_id 1 target_btf_id 31320
>>
>> Hack the following change to enable normal libbpf debug output,
>> --- a/tools/bpf/bpftool/pids.c
>> +++ b/tools/bpf/bpftool/pids.c
>> @@ -121,9 +121,9 @@ int build_obj_refs_table(struct hashmap **map, enum bpf_obj_type type)
>> /* we don't want output polluted with libbpf errors if bpf_iter is not
>> * supported
>> */
>> - default_print = libbpf_set_print(libbpf_print_none);
>> + /* default_print = libbpf_set_print(libbpf_print_none); */
>> err = pid_iter_bpf__load(skel);
>> - libbpf_set_print(default_print);
>> + /* libbpf_set_print(default_print); */
>>
>> Rerun the above bpftool command:
>> $ tools/build/bpftool/bpftool link
>> libbpf: prog 'iter': BPF program load failed: Permission denied
>> libbpf: prog 'iter': -- BEGIN PROG LOAD LOG --
>> 0: R1=ctx() R10=fp0
>> ; struct task_struct *task = ctx->task; @ pid_iter.bpf.c:69
>> 0: (79) r6 = *(u64 *)(r1 +8) ; R1=ctx() R6_w=ptr_or_null_task_struct(id=1)
>> ; struct file *file = ctx->file; @ pid_iter.bpf.c:68
>> ...
>> ; struct bpf_link *link = (struct bpf_link *) file->private_data; @ pid_iter.bpf.c:103
>> 80: (79) r3 = *(u64 *)(r8 +432) ; R3_w=scalar() R8=ptr_file()
>> ; if (link->type == bpf_core_enum_value(enum bpf_link_type___local, @ pid_iter.bpf.c:105
>> 81: (61) r1 = *(u32 *)(r3 +12)
>> R3 invalid mem access 'scalar'
>> processed 39 insns (limit 1000000) max_states_per_insn 0 total_states 3 peak_states 3 mark_read 2
>> -- END PROG LOAD LOG --
>> libbpf: prog 'iter': failed to load: -13
>> ...
>>
>> The 'file->private_data' returns a 'void' type and this caused subsequent 'link->type'
>> (insn #81) failed in verification.
>>
>> To fix the issue, do a type cast from 'void *' to 'struct bpf_link *' for file->private_data
>> as in this patch, and the 'bpftool link' runs successfully with 'pids'.
>> $ tools/build/bpftool/bpftool link
>> ...
>> 4: tracing prog 23
>> prog_type lsm attach_type lsm_mac
>> target_obj_id 1 target_btf_id 31320
>> pids systemd(1)
>>
>> Fixes: 44ba7b30e84f ("bpftool: Use a local copy of BPF_LINK_TYPE_PERF_EVENT in pid_iter.bpf.c")
>> Cc: Quentin Monnet <quentin@isovalent.com>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>> tools/bpf/bpftool/skeleton/pid_iter.bpf.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
>> index 26004f0c5a6a..96ffcc4f0e67 100644
>> --- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
>> +++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
>> @@ -100,7 +100,7 @@ int iter(struct bpf_iter__task_file *ctx)
>> if (obj_type == BPF_OBJ_LINK &&
>> bpf_core_enum_value_exists(enum bpf_link_type___local,
>> BPF_LINK_TYPE_PERF_EVENT___local)) {
>> - struct bpf_link *link = (struct bpf_link *) file->private_data;
>> + struct bpf_link *link = bpf_core_cast(file->private_data, struct bpf_link);
> bpf_core_cast() (i.e., bpf_rdonly_cast()) is newer feature, so relying
> on it in bpftool will make PID printing not work on older kernels that
> generally do support iter/task_file iterators.
>
>> if (link->type == bpf_core_enum_value(enum bpf_link_type___local,
> the problem is that we dropped BPF_CORE_READ() here and went with
> `link->type`. Should we restore BPF_CORE_READ(link, type) here and
> solve the problem, while also supporting this feature on wider set of
> kernels? Quentin?
Good point. Using the latest fancy feature might not be the best way as old
kernels won't work and this is a strong argument. I will change
to use BPF_CORE_READ in v2.
>
>> BPF_LINK_TYPE_PERF_EVENT___local)) {
>> --
>> 2.43.0
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpftool: Fix missing pids during link show
2024-03-11 23:30 ` Yonghong Song
@ 2024-03-12 14:00 ` Quentin Monnet
2024-03-12 15:23 ` Yonghong Song
0 siblings, 1 reply; 5+ messages in thread
From: Quentin Monnet @ 2024-03-12 14:00 UTC (permalink / raw)
To: Yonghong Song, Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Martin KaFai Lau
2024-03-11 23:30 UTC+0000 ~ Yonghong Song <yonghong.song@linux.dev>
>
> On 3/11/24 4:13 PM, Andrii Nakryiko wrote:
>> On Mon, Mar 11, 2024 at 2:41 PM Yonghong Song
>> <yonghong.song@linux.dev> wrote:
>>> Current 'bpftool link' command does not show pids, e.g.,
>>> $ tools/build/bpftool/bpftool link
>>> ...
>>> 4: tracing prog 23
>>> prog_type lsm attach_type lsm_mac
>>> target_obj_id 1 target_btf_id 31320
>>>
>>> Hack the following change to enable normal libbpf debug output,
>>> --- a/tools/bpf/bpftool/pids.c
>>> +++ b/tools/bpf/bpftool/pids.c
>>> @@ -121,9 +121,9 @@ int build_obj_refs_table(struct hashmap **map,
>>> enum bpf_obj_type type)
>>> /* we don't want output polluted with libbpf errors if
>>> bpf_iter is not
>>> * supported
>>> */
>>> - default_print = libbpf_set_print(libbpf_print_none);
>>> + /* default_print = libbpf_set_print(libbpf_print_none); */
>>> err = pid_iter_bpf__load(skel);
>>> - libbpf_set_print(default_print);
>>> + /* libbpf_set_print(default_print); */
I'm taking note to make these logs available when users pass the --debug
flag (https://github.com/libbpf/bpftool/issues/137), there's no reason
to hide them in that case.
>>>
>>> Rerun the above bpftool command:
>>> $ tools/build/bpftool/bpftool link
>>> libbpf: prog 'iter': BPF program load failed: Permission denied
>>> libbpf: prog 'iter': -- BEGIN PROG LOAD LOG --
>>> 0: R1=ctx() R10=fp0
>>> ; struct task_struct *task = ctx->task; @ pid_iter.bpf.c:69
>>> 0: (79) r6 = *(u64 *)(r1 +8) ; R1=ctx()
>>> R6_w=ptr_or_null_task_struct(id=1)
>>> ; struct file *file = ctx->file; @ pid_iter.bpf.c:68
>>> ...
>>> ; struct bpf_link *link = (struct bpf_link *) file->private_data;
>>> @ pid_iter.bpf.c:103
>>> 80: (79) r3 = *(u64 *)(r8 +432) ; R3_w=scalar() R8=ptr_file()
>>> ; if (link->type == bpf_core_enum_value(enum
>>> bpf_link_type___local, @ pid_iter.bpf.c:105
>>> 81: (61) r1 = *(u32 *)(r3 +12)
>>> R3 invalid mem access 'scalar'
>>> processed 39 insns (limit 1000000) max_states_per_insn 0
>>> total_states 3 peak_states 3 mark_read 2
>>> -- END PROG LOAD LOG --
>>> libbpf: prog 'iter': failed to load: -13
>>> ...
>>>
>>> The 'file->private_data' returns a 'void' type and this caused
>>> subsequent 'link->type'
>>> (insn #81) failed in verification.
>>>
>>> To fix the issue, do a type cast from 'void *' to 'struct bpf_link *'
>>> for file->private_data
>>> as in this patch, and the 'bpftool link' runs successfully with 'pids'.
>>> $ tools/build/bpftool/bpftool link
>>> ...
>>> 4: tracing prog 23
>>> prog_type lsm attach_type lsm_mac
>>> target_obj_id 1 target_btf_id 31320
>>> pids systemd(1)
>>>
>>> Fixes: 44ba7b30e84f ("bpftool: Use a local copy of
>>> BPF_LINK_TYPE_PERF_EVENT in pid_iter.bpf.c")
>>> Cc: Quentin Monnet <quentin@isovalent.com>
>>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>>> ---
>>> tools/bpf/bpftool/skeleton/pid_iter.bpf.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
>>> b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
>>> index 26004f0c5a6a..96ffcc4f0e67 100644
>>> --- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
>>> +++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
>>> @@ -100,7 +100,7 @@ int iter(struct bpf_iter__task_file *ctx)
>>> if (obj_type == BPF_OBJ_LINK &&
>>> bpf_core_enum_value_exists(enum bpf_link_type___local,
>>>
>>> BPF_LINK_TYPE_PERF_EVENT___local)) {
>>> - struct bpf_link *link = (struct bpf_link *)
>>> file->private_data;
>>> + struct bpf_link *link =
>>> bpf_core_cast(file->private_data, struct bpf_link);
>> bpf_core_cast() (i.e., bpf_rdonly_cast()) is newer feature, so relying
>> on it in bpftool will make PID printing not work on older kernels that
>> generally do support iter/task_file iterators.
>>
>>> if (link->type == bpf_core_enum_value(enum
>>> bpf_link_type___local,
>> the problem is that we dropped BPF_CORE_READ() here and went with
>> `link->type`. Should we restore BPF_CORE_READ(link, type) here and
>> solve the problem, while also supporting this feature on wider set of
>> kernels? Quentin?
>
> Good point. Using the latest fancy feature might not be the best way as old
> kernels won't work and this is a strong argument. I will change
> to use BPF_CORE_READ in v2.
Agreed, the whole point of commit 44ba7b30e84f that introduced the issue
was to make bpftool work with older kernels. BPF_CORE_READ sounds like
the right approach, thank you!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpftool: Fix missing pids during link show
2024-03-12 14:00 ` Quentin Monnet
@ 2024-03-12 15:23 ` Yonghong Song
0 siblings, 0 replies; 5+ messages in thread
From: Yonghong Song @ 2024-03-12 15:23 UTC (permalink / raw)
To: Quentin Monnet, Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Martin KaFai Lau
On 3/12/24 7:00 AM, Quentin Monnet wrote:
> 2024-03-11 23:30 UTC+0000 ~ Yonghong Song <yonghong.song@linux.dev>
>> On 3/11/24 4:13 PM, Andrii Nakryiko wrote:
>>> On Mon, Mar 11, 2024 at 2:41 PM Yonghong Song
>>> <yonghong.song@linux.dev> wrote:
>>>> Current 'bpftool link' command does not show pids, e.g.,
>>>> $ tools/build/bpftool/bpftool link
>>>> ...
>>>> 4: tracing prog 23
>>>> prog_type lsm attach_type lsm_mac
>>>> target_obj_id 1 target_btf_id 31320
>>>>
>>>> Hack the following change to enable normal libbpf debug output,
>>>> --- a/tools/bpf/bpftool/pids.c
>>>> +++ b/tools/bpf/bpftool/pids.c
>>>> @@ -121,9 +121,9 @@ int build_obj_refs_table(struct hashmap **map,
>>>> enum bpf_obj_type type)
>>>> /* we don't want output polluted with libbpf errors if
>>>> bpf_iter is not
>>>> * supported
>>>> */
>>>> - default_print = libbpf_set_print(libbpf_print_none);
>>>> + /* default_print = libbpf_set_print(libbpf_print_none); */
>>>> err = pid_iter_bpf__load(skel);
>>>> - libbpf_set_print(default_print);
>>>> + /* libbpf_set_print(default_print); */
> I'm taking note to make these logs available when users pass the --debug
> flag (https://github.com/libbpf/bpftool/issues/137), there's no reason
> to hide them in that case.
Totally make sense as most kernels used in production should already have
bpf_link and bpf_iter support already.
[...]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-03-12 15:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-11 21:41 [PATCH bpf-next] bpftool: Fix missing pids during link show Yonghong Song
2024-03-11 23:13 ` Andrii Nakryiko
2024-03-11 23:30 ` Yonghong Song
2024-03-12 14:00 ` Quentin Monnet
2024-03-12 15:23 ` Yonghong Song
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox