diff mbox series

[bpf-next,v2] bpftool: Fix missing pids during link show

Message ID 20240312023249.3776718-1-yonghong.song@linux.dev (mailing list archive)
State Accepted
Commit fe879bb42f8a6513ed18e9d22efb99cb35590201
Delegated to: BPF
Headers show
Series [bpf-next,v2] bpftool: Fix missing pids during link show | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-VM_Test-43 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-44 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-47 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-46 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-45 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-PR fail PR summary
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 9 maintainers not CCed: alobakin@pm.me martin.lau@linux.dev kpsingh@kernel.org haoluo@google.com song@kernel.org sdf@google.com jolsa@kernel.org john.fastabend@gmail.com eddyz87@gmail.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch fail CHECK: Alignment should match open parenthesis ERROR: Avoid using diff content in the commit message - patch(1) might not work WARNING: line length of 96 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-18 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 fail Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 fail Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_maps, false, 360) / test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18

Commit Message

Yonghong Song March 12, 2024, 2:32 a.m. UTC 
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, restore the previous BPF_CORE_READ so old kernels can also work.
With this patch, 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(-)

Comments

Quentin Monnet March 12, 2024, 2:01 p.m. UTC | #1 
2024-03-12 02:32 UTC+0000 ~ Yonghong Song <yonghong.song@linux.dev>
> 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, restore the previous BPF_CORE_READ so old kernels can also work.
> With this patch, 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..1eb756f8d02e 100644
> --- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
> +++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
> @@ -102,7 +102,7 @@ int iter(struct bpf_iter__task_file *ctx)
>  				       BPF_LINK_TYPE_PERF_EVENT___local)) {
>  		struct bpf_link *link = (struct bpf_link *) file->private_data;
>  
> -		if (link->type == bpf_core_enum_value(enum bpf_link_type___local,
> +		if (BPF_CORE_READ(link, type) == bpf_core_enum_value(enum bpf_link_type___local,
>  						      BPF_LINK_TYPE_PERF_EVENT___local)) {

Looks good, thank you! Could you please just realign the
BPF_LINK_TYPE_PERF_EVENT___local argument on the next line properly?

With that:

Tested-by: Quentin Monnet <quentin@isovalent.com>
Reviewed-by: Quentin Monnet <quentin@isovalent.com>
Yonghong Song March 12, 2024, 3:24 p.m. UTC | #2 
On 3/12/24 7:01 AM, Quentin Monnet wrote:
> 2024-03-12 02:32 UTC+0000 ~ Yonghong Song <yonghong.song@linux.dev>
>> 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, restore the previous BPF_CORE_READ so old kernels can also work.
>> With this patch, 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..1eb756f8d02e 100644
>> --- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
>> +++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
>> @@ -102,7 +102,7 @@ int iter(struct bpf_iter__task_file *ctx)
>>   				       BPF_LINK_TYPE_PERF_EVENT___local)) {
>>   		struct bpf_link *link = (struct bpf_link *) file->private_data;
>>   
>> -		if (link->type == bpf_core_enum_value(enum bpf_link_type___local,
>> +		if (BPF_CORE_READ(link, type) == bpf_core_enum_value(enum bpf_link_type___local,
>>   						      BPF_LINK_TYPE_PERF_EVENT___local)) {
> Looks good, thank you! Could you please just realign the
> BPF_LINK_TYPE_PERF_EVENT___local argument on the next line properly?

Will do. thanks.

>
> With that:
>
> Tested-by: Quentin Monnet <quentin@isovalent.com>
> Reviewed-by: Quentin Monnet <quentin@isovalent.com>
Andrii Nakryiko March 12, 2024, 11:11 p.m. UTC | #3 
On Tue, Mar 12, 2024 at 8:24 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
> On 3/12/24 7:01 AM, Quentin Monnet wrote:
> > 2024-03-12 02:32 UTC+0000 ~ Yonghong Song <yonghong.song@linux.dev>
> >> 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, restore the previous BPF_CORE_READ so old kernels can also work.
> >> With this patch, 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..1eb756f8d02e 100644
> >> --- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
> >> +++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
> >> @@ -102,7 +102,7 @@ int iter(struct bpf_iter__task_file *ctx)
> >>                                     BPF_LINK_TYPE_PERF_EVENT___local)) {
> >>              struct bpf_link *link = (struct bpf_link *) file->private_data;
> >>
> >> -            if (link->type == bpf_core_enum_value(enum bpf_link_type___local,
> >> +            if (BPF_CORE_READ(link, type) == bpf_core_enum_value(enum bpf_link_type___local,
> >>                                                    BPF_LINK_TYPE_PERF_EVENT___local)) {
> > Looks good, thank you! Could you please just realign the
> > BPF_LINK_TYPE_PERF_EVENT___local argument on the next line properly?
>
> Will do. thanks.
>

We can fix it up while applying, so don't send another revision. We
are waiting for trees to converge, so applying patches will be delayed
a bit.

> >
> > With that:
> >
> > Tested-by: Quentin Monnet <quentin@isovalent.com>
> > Reviewed-by: Quentin Monnet <quentin@isovalent.com>
Yonghong Song March 13, 2024, 5:01 a.m. UTC | #4 
On 3/12/24 4:11 PM, Andrii Nakryiko wrote:
> On Tue, Mar 12, 2024 at 8:24 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>> On 3/12/24 7:01 AM, Quentin Monnet wrote:
>>> 2024-03-12 02:32 UTC+0000 ~ Yonghong Song <yonghong.song@linux.dev>
>>>> 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, restore the previous BPF_CORE_READ so old kernels can also work.
>>>> With this patch, 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..1eb756f8d02e 100644
>>>> --- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
>>>> +++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
>>>> @@ -102,7 +102,7 @@ int iter(struct bpf_iter__task_file *ctx)
>>>>                                      BPF_LINK_TYPE_PERF_EVENT___local)) {
>>>>               struct bpf_link *link = (struct bpf_link *) file->private_data;
>>>>
>>>> -            if (link->type == bpf_core_enum_value(enum bpf_link_type___local,
>>>> +            if (BPF_CORE_READ(link, type) == bpf_core_enum_value(enum bpf_link_type___local,
>>>>                                                     BPF_LINK_TYPE_PERF_EVENT___local)) {
>>> Looks good, thank you! Could you please just realign the
>>> BPF_LINK_TYPE_PERF_EVENT___local argument on the next line properly?
>> Will do. thanks.
>>
> We can fix it up while applying, so don't send another revision. We
> are waiting for trees to converge, so applying patches will be delayed
> a bit.

Thanks!

>
>>> With that:
>>>
>>> Tested-by: Quentin Monnet <quentin@isovalent.com>
>>> Reviewed-by: Quentin Monnet <quentin@isovalent.com>
patchwork-bot+netdevbpf@kernel.org March 14, 2024, 8:40 p.m. UTC | #5 
Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Mon, 11 Mar 2024 19:32:49 -0700 you 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
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2] bpftool: Fix missing pids during link show
    https://git.kernel.org/bpf/bpf-next/c/fe879bb42f8a

You are awesome, thank you!
diff mbox series

Patch

diff --git a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
index 26004f0c5a6a..1eb756f8d02e 100644
--- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
+++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
@@ -102,7 +102,7 @@  int iter(struct bpf_iter__task_file *ctx)
 				       BPF_LINK_TYPE_PERF_EVENT___local)) {
 		struct bpf_link *link = (struct bpf_link *) file->private_data;
 
-		if (link->type == bpf_core_enum_value(enum bpf_link_type___local,
+		if (BPF_CORE_READ(link, type) == bpf_core_enum_value(enum bpf_link_type___local,
 						      BPF_LINK_TYPE_PERF_EVENT___local)) {
 			e.has_bpf_cookie = true;
 			e.bpf_cookie = get_bpf_cookie(link);