diff mbox series

[v1,6/7] perf trace: 5sec fix libbpf 1.0+ compatibility

Message ID 20221103045437.163510-7-irogers@google.com
State New
Headers
Series Fix perf trace libbpf 1.0+ compatibility |

Commit Message

Ian Rogers Nov. 3, 2022, 4:54 a.m. UTC 
  Avoid use of tools/perf/include/bpf/bpf.h and use the more regular BPF
headers.

Note, on testing the probe was unable to attach and the program failed.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/examples/bpf/5sec.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Arnaldo Carvalho de Melo Nov. 3, 2022, 3:36 p.m. UTC | #1 
Em Wed, Nov 02, 2022 at 09:54:36PM -0700, Ian Rogers escreveu:
> Avoid use of tools/perf/include/bpf/bpf.h and use the more regular BPF
> headers.
> 
> Note, on testing the probe was unable to attach and the program failed.

Humm, trying to test:

[root@quaco ~]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/5sec.c |& head -15
In file included from /home/acme/git/perf/tools/perf/examples/bpf/5sec.c:42:
In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/bpf.h:9:
In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/workqueue.h:9:
In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/timer.h:6:
In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/ktime.h:24:
In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/time.h:6:
In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/math64.h:6:
In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/math.h:6:
/lib/modules/5.19.16-200.fc36.x86_64/build/./arch/x86/include/asm/div64.h:85:28: error: invalid output constraint '=a' in asm
        asm ("mulq %2; divq %3" : "=a" (q)
                                  ^
In file included from /home/acme/git/perf/tools/perf/examples/bpf/5sec.c:42:
In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/bpf.h:9:
In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/workqueue.h:9:
In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/timer.h:6:
[root@quaco ~]#

So I go and try to remove that <linux/bpf.h>:

[acme@quaco perf]$ git diff
diff --git a/tools/perf/examples/bpf/5sec.c b/tools/perf/examples/bpf/5sec.c
index 3bd7fc17631f0440..e0d5525c6a1374ae 100644
--- a/tools/perf/examples/bpf/5sec.c
+++ b/tools/perf/examples/bpf/5sec.c
@@ -39,7 +39,6 @@
    Copyright (C) 2018 Red Hat, Inc., Arnaldo Carvalho de Melo <acme@redhat.com>
 */

-#include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>

 #define NSEC_PER_SEC   1000000000L
[acme@quaco perf]$

[root@quaco ~]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
/home/acme/git/perf/tools/perf/examples/bpf/5sec.c:42:10: fatal error: 'bpf/bpf_helpers.h' file not found
#include <bpf/bpf_helpers.h>
         ^~~~~~~~~~~~~~~~~~~
1 error generated.
ERROR:	unable to compile /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
Hint:	Check error message shown above.
Hint:	You can also pre-compile it into .o using:
     		clang -target bpf -O2 -c /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
     	with proper -I and -D options.
event syntax error: '/home/acme/git/perf/tools/perf/examples/bpf/5sec.c'
                     \___ Failed to load /home/acme/git/perf/tools/perf/examples/bpf/5sec.c from source: Error when compiling BPF scriptlet

(add -v to see detail)
Run 'perf list' for a list of valid events

 Usage: perf trace [<options>] [<command>]
    or: perf trace [<options>] -- <command> [<options>]
    or: perf trace record [<options>] [<command>]
    or: perf trace record [<options>] -- <command> [<options>]

    -e, --event <event>   event/syscall selector. use 'perf list' to list available events
[root@quaco ~]#

It is not even finding it, in this machine I have libbpf 0.7.0, so there
is a /usr/include/bpf/bpf_helpers.h, but probably that isn't in the
include path set up to build the tools/perf/examples/bpf/ files, perhaps
it should use:

-Itools/lib/  so that it gets tools/lib/bpf_helpers.h?

Trying to get this tested...

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/examples/bpf/5sec.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/examples/bpf/5sec.c b/tools/perf/examples/bpf/5sec.c
> index e6b6181c6dc6..f22312c64713 100644
> --- a/tools/perf/examples/bpf/5sec.c
> +++ b/tools/perf/examples/bpf/5sec.c
> @@ -39,13 +39,15 @@
>     Copyright (C) 2018 Red Hat, Inc., Arnaldo Carvalho de Melo <acme@redhat.com>
>  */
>  
> -#include <bpf.h>
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
>  
>  #define NSEC_PER_SEC	1000000000L
>  
> -int probe(hrtimer_nanosleep, rqtp)(void *ctx, int err, long long sec)
> +SEC("hrtimer_nanosleep=hrtimer_nanosleep rqtp")
> +int hrtimer_nanosleep(void *ctx, int err, long long sec)
>  {
>  	return sec / NSEC_PER_SEC == 5ULL;
>  }
> +char _license[] SEC("license") = "GPL";
>  
> -license(GPL);
> -- 
> 2.38.1.273.g43a17bfeac-goog
Arnaldo Carvalho de Melo Nov. 3, 2022, 3:39 p.m. UTC | #2 
Em Thu, Nov 03, 2022 at 12:36:36PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Nov 02, 2022 at 09:54:36PM -0700, Ian Rogers escreveu:
> > Avoid use of tools/perf/include/bpf/bpf.h and use the more regular BPF
> > headers.
> > 
> > Note, on testing the probe was unable to attach and the program failed.
> 
> Humm, trying to test:
> 
> [root@quaco ~]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/5sec.c |& head -15
> In file included from /home/acme/git/perf/tools/perf/examples/bpf/5sec.c:42:
> In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/bpf.h:9:
> In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/workqueue.h:9:
> In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/timer.h:6:
> In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/ktime.h:24:
> In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/time.h:6:
> In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/math64.h:6:
> In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/math.h:6:
> /lib/modules/5.19.16-200.fc36.x86_64/build/./arch/x86/include/asm/div64.h:85:28: error: invalid output constraint '=a' in asm
>         asm ("mulq %2; divq %3" : "=a" (q)
>                                   ^
> In file included from /home/acme/git/perf/tools/perf/examples/bpf/5sec.c:42:
> In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/bpf.h:9:
> In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/workqueue.h:9:
> In file included from /lib/modules/5.19.16-200.fc36.x86_64/build/./include/linux/timer.h:6:
> [root@quaco ~]#
> 
> So I go and try to remove that <linux/bpf.h>:
> 
> [acme@quaco perf]$ git diff
> diff --git a/tools/perf/examples/bpf/5sec.c b/tools/perf/examples/bpf/5sec.c
> index 3bd7fc17631f0440..e0d5525c6a1374ae 100644
> --- a/tools/perf/examples/bpf/5sec.c
> +++ b/tools/perf/examples/bpf/5sec.c
> @@ -39,7 +39,6 @@
>     Copyright (C) 2018 Red Hat, Inc., Arnaldo Carvalho de Melo <acme@redhat.com>
>  */
> 
> -#include <linux/bpf.h>
>  #include <bpf/bpf_helpers.h>
> 
>  #define NSEC_PER_SEC   1000000000L
> [acme@quaco perf]$
> 
> [root@quaco ~]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
> /home/acme/git/perf/tools/perf/examples/bpf/5sec.c:42:10: fatal error: 'bpf/bpf_helpers.h' file not found
> #include <bpf/bpf_helpers.h>
>          ^~~~~~~~~~~~~~~~~~~
> 1 error generated.
> ERROR:	unable to compile /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
> Hint:	Check error message shown above.
> Hint:	You can also pre-compile it into .o using:
>      		clang -target bpf -O2 -c /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
>      	with proper -I and -D options.
> event syntax error: '/home/acme/git/perf/tools/perf/examples/bpf/5sec.c'
>                      \___ Failed to load /home/acme/git/perf/tools/perf/examples/bpf/5sec.c from source: Error when compiling BPF scriptlet
> 
> (add -v to see detail)
> Run 'perf list' for a list of valid events
> 
>  Usage: perf trace [<options>] [<command>]
>     or: perf trace [<options>] -- <command> [<options>]
>     or: perf trace record [<options>] [<command>]
>     or: perf trace record [<options>] -- <command> [<options>]
> 
>     -e, --event <event>   event/syscall selector. use 'perf list' to list available events
> [root@quaco ~]#
> 
> It is not even finding it, in this machine I have libbpf 0.7.0, so there
> is a /usr/include/bpf/bpf_helpers.h, but probably that isn't in the
> include path set up to build the tools/perf/examples/bpf/ files, perhaps
> it should use:
> 
> -Itools/lib/  so that it gets tools/lib/bpf_helpers.h?
> 
> Trying to get this tested...

Running with -v:

llvm compiling command : /usr/lib64/ccache/clang -D__KERNEL__ -D__NR_CPUS__=8 -DLINUX_VERSION_CODE=0x51310 -g -I/home/acme/lib/perf/include/bpf -nostdinc -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h  -Wno-unused-value -Wno-pointer-sign -working-directory /lib/modules/5.19.16-200.fc36.x86_64/build -c /home/acme/git/perf/tools/perf/examples/bpf/5sec.c -target bpf  -g -O2 -o -
/home/acme/git/perf/tools/perf/examples/bpf/5sec.c:42:10: fatal error: 'bpf/bpf_helpers.h' file not found

There is still that -I/home/acme/lib/perf/include/bpf, I'll remove it
from the include path and try to replace it with the libbpf path...

- Arnaldo
Arnaldo Carvalho de Melo Nov. 3, 2022, 3:52 p.m. UTC | #3 
Em Thu, Nov 03, 2022 at 12:39:59PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Nov 03, 2022 at 12:36:36PM -0300, Arnaldo Carvalho de Melo escreveu:
> > [root@quaco ~]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
> > /home/acme/git/perf/tools/perf/examples/bpf/5sec.c:42:10: fatal error: 'bpf/bpf_helpers.h' file not found
> > #include <bpf/bpf_helpers.h>
> >          ^~~~~~~~~~~~~~~~~~~
> > 1 error generated.
> > ERROR:	unable to compile /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
> > Hint:	Check error message shown above.
> > Hint:	You can also pre-compile it into .o using:
> >      		clang -target bpf -O2 -c /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
> >      	with proper -I and -D options.
> > event syntax error: '/home/acme/git/perf/tools/perf/examples/bpf/5sec.c'
> >                      \___ Failed to load /home/acme/git/perf/tools/perf/examples/bpf/5sec.c from source: Error when compiling BPF scriptlet
> > 
> > (add -v to see detail)
> > Run 'perf list' for a list of valid events
> > 
> >  Usage: perf trace [<options>] [<command>]
> >     or: perf trace [<options>] -- <command> [<options>]
> >     or: perf trace record [<options>] [<command>]
> >     or: perf trace record [<options>] -- <command> [<options>]
> > 
> >     -e, --event <event>   event/syscall selector. use 'perf list' to list available events
> > [root@quaco ~]#
> > 
> > It is not even finding it, in this machine I have libbpf 0.7.0, so there
> > is a /usr/include/bpf/bpf_helpers.h, but probably that isn't in the
> > include path set up to build the tools/perf/examples/bpf/ files, perhaps
> > it should use:
> > 
> > -Itools/lib/  so that it gets tools/lib/bpf_helpers.h?
> > 
> > Trying to get this tested...
> 
> Running with -v:
> 
> llvm compiling command : /usr/lib64/ccache/clang -D__KERNEL__ -D__NR_CPUS__=8 -DLINUX_VERSION_CODE=0x51310 -g -I/home/acme/lib/perf/include/bpf -nostdinc -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h  -Wno-unused-value -Wno-pointer-sign -working-directory /lib/modules/5.19.16-200.fc36.x86_64/build -c /home/acme/git/perf/tools/perf/examples/bpf/5sec.c -target bpf  -g -O2 -o -
> /home/acme/git/perf/tools/perf/examples/bpf/5sec.c:42:10: fatal error: 'bpf/bpf_helpers.h' file not found
> 
> There is still that -I/home/acme/lib/perf/include/bpf, I'll remove it
> from the include path and try to replace it with the libbpf path...

Ok, works with the patch below, that needs some more renaming from "perf_" to
"libbpf_", etc:

[root@quaco ~]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/5sec.c  sleep 5
     0.000 sleep/160828 perf_bpf_probe:hrtimer_nanosleep(__probe_ip: -1474734416, rqtp: 5000000000)
[root@quaco ~]#

Since I have:

[root@quaco ~]# cat ~/.perfconfig
[llvm]
	dump-obj = true
	clang-opt = -g
#

I end up with:

[root@quaco ~]# ls -la /home/acme/git/perf/tools/perf/examples/bpf/5sec.o
-rw-r--r--. 1 root root 3696 Nov  3 12:47 /home/acme/git/perf/tools/perf/examples/bpf/5sec.o
[root@quaco ~]# file /home/acme/git/perf/tools/perf/examples/bpf/5sec.o
/home/acme/git/perf/tools/perf/examples/bpf/5sec.o: ELF 64-bit LSB relocatable, eBPF, version 1 (SYSV), with debug_info, not stripped
[root@quaco ~]#

and can test with the pre-built .o eBPF bytecode + capped backtrace:

[root@quaco ~]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/5sec.o/max-stack=6/  sleep 5
     0.000 sleep/161037 perf_bpf_probe:hrtimer_nanosleep(__probe_ip: -1474734416, rqtp: 5000000000)
                                       hrtimer_nanosleep ([kernel.kallsyms])
                                       common_nsleep ([kernel.kallsyms])
                                       __x64_sys_clock_nanosleep ([kernel.kallsyms])
                                       do_syscall_64 ([kernel.kallsyms])
                                       entry_SYSCALL_64_after_hwframe ([kernel.kallsyms])
                                       __GI___clock_nanosleep (/usr/lib64/libc.so.6)
[root@quaco ~]#

I'll test the other examples with these changes after I drive Pedro to
school and get back to the office.

- Arnaldo

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index d3d3c13a9f25b55c..067a6e56eeacc9fc 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -1239,7 +1239,7 @@ includedir = $(abspath $(prefix)/$(includedir_relative))
 mandir = share/man
 infodir = share/info
 perfexecdir = libexec/perf-core
-perf_include_dir = lib/perf/include
+perf_include_dir = /usr/include
 perf_examples_dir = lib/perf/examples
 sharedir = $(prefix)/share
 template_dir = share/perf-core/templates
diff --git a/tools/perf/util/llvm-utils.c b/tools/perf/util/llvm-utils.c
index 2dc7970074196ca8..a5cac85783d8711f 100644
--- a/tools/perf/util/llvm-utils.c
+++ b/tools/perf/util/llvm-utils.c
@@ -495,7 +495,7 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf,
 
 	snprintf(linux_version_code_str, sizeof(linux_version_code_str),
 		 "0x%x", kernel_version);
-	if (asprintf(&perf_bpf_include_opts, "-I%s/bpf", perf_include_dir) < 0)
+	if (asprintf(&perf_bpf_include_opts, "-I%s/", perf_include_dir) < 0)
 		goto errout;
 	force_set_env("NR_CPUS", nr_cpus_avail_str);
 	force_set_env("LINUX_VERSION_CODE", linux_version_code_str);
Ian Rogers Nov. 3, 2022, 4:04 p.m. UTC | #4 
On Thu, Nov 3, 2022 at 8:52 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Thu, Nov 03, 2022 at 12:39:59PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Nov 03, 2022 at 12:36:36PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > [root@quaco ~]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
> > > /home/acme/git/perf/tools/perf/examples/bpf/5sec.c:42:10: fatal error: 'bpf/bpf_helpers.h' file not found
> > > #include <bpf/bpf_helpers.h>
> > >          ^~~~~~~~~~~~~~~~~~~
> > > 1 error generated.
> > > ERROR:      unable to compile /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
> > > Hint:       Check error message shown above.
> > > Hint:       You can also pre-compile it into .o using:
> > >                     clang -target bpf -O2 -c /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
> > >             with proper -I and -D options.
> > > event syntax error: '/home/acme/git/perf/tools/perf/examples/bpf/5sec.c'
> > >                      \___ Failed to load /home/acme/git/perf/tools/perf/examples/bpf/5sec.c from source: Error when compiling BPF scriptlet
> > >
> > > (add -v to see detail)
> > > Run 'perf list' for a list of valid events
> > >
> > >  Usage: perf trace [<options>] [<command>]
> > >     or: perf trace [<options>] -- <command> [<options>]
> > >     or: perf trace record [<options>] [<command>]
> > >     or: perf trace record [<options>] -- <command> [<options>]
> > >
> > >     -e, --event <event>   event/syscall selector. use 'perf list' to list available events
> > > [root@quaco ~]#
> > >
> > > It is not even finding it, in this machine I have libbpf 0.7.0, so there
> > > is a /usr/include/bpf/bpf_helpers.h, but probably that isn't in the
> > > include path set up to build the tools/perf/examples/bpf/ files, perhaps
> > > it should use:
> > >
> > > -Itools/lib/  so that it gets tools/lib/bpf_helpers.h?
> > >
> > > Trying to get this tested...
> >
> > Running with -v:
> >
> > llvm compiling command : /usr/lib64/ccache/clang -D__KERNEL__ -D__NR_CPUS__=8 -DLINUX_VERSION_CODE=0x51310 -g -I/home/acme/lib/perf/include/bpf -nostdinc -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h  -Wno-unused-value -Wno-pointer-sign -working-directory /lib/modules/5.19.16-200.fc36.x86_64/build -c /home/acme/git/perf/tools/perf/examples/bpf/5sec.c -target bpf  -g -O2 -o -
> > /home/acme/git/perf/tools/perf/examples/bpf/5sec.c:42:10: fatal error: 'bpf/bpf_helpers.h' file not found
> >
> > There is still that -I/home/acme/lib/perf/include/bpf, I'll remove it
> > from the include path and try to replace it with the libbpf path...
>
> Ok, works with the patch below, that needs some more renaming from "perf_" to
> "libbpf_", etc:
>
> [root@quaco ~]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/5sec.c  sleep 5
>      0.000 sleep/160828 perf_bpf_probe:hrtimer_nanosleep(__probe_ip: -1474734416, rqtp: 5000000000)
> [root@quaco ~]#
>
> Since I have:
>
> [root@quaco ~]# cat ~/.perfconfig
> [llvm]
>         dump-obj = true
>         clang-opt = -g
> #
>
> I end up with:
>
> [root@quaco ~]# ls -la /home/acme/git/perf/tools/perf/examples/bpf/5sec.o
> -rw-r--r--. 1 root root 3696 Nov  3 12:47 /home/acme/git/perf/tools/perf/examples/bpf/5sec.o
> [root@quaco ~]# file /home/acme/git/perf/tools/perf/examples/bpf/5sec.o
> /home/acme/git/perf/tools/perf/examples/bpf/5sec.o: ELF 64-bit LSB relocatable, eBPF, version 1 (SYSV), with debug_info, not stripped
> [root@quaco ~]#
>
> and can test with the pre-built .o eBPF bytecode + capped backtrace:
>
> [root@quaco ~]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/5sec.o/max-stack=6/  sleep 5
>      0.000 sleep/161037 perf_bpf_probe:hrtimer_nanosleep(__probe_ip: -1474734416, rqtp: 5000000000)
>                                        hrtimer_nanosleep ([kernel.kallsyms])
>                                        common_nsleep ([kernel.kallsyms])
>                                        __x64_sys_clock_nanosleep ([kernel.kallsyms])
>                                        do_syscall_64 ([kernel.kallsyms])
>                                        entry_SYSCALL_64_after_hwframe ([kernel.kallsyms])
>                                        __GI___clock_nanosleep (/usr/lib64/libc.so.6)
> [root@quaco ~]#
>
> I'll test the other examples with these changes after I drive Pedro to
> school and get back to the office.

Thanks, I was somewhat coding in the dark with this as I was using a
bpf object file and clearly there's been some attention missed for a
while in these code paths. I couldn't get the hello variant to do
anything with the openat tracing it was set up for, for example, the
change puts the call on the sys_enter raw syscall. I suspect you
remember how these things should be and I'm happy to roll a v2, have
you fix it, etc.

Fwiw, two things I'd like to see further here is somehow the augmented
code to be the default (with a BPF skeleton possibly) and an ability
for perf_event_open tracing to dump the perf_event_attr. I'm unclear
on the advantages of having different augmenters. Perhaps we could
just drop support for different augmenters, use BPF skeletons for the
raw syscall version and keep the BPF logic in parse events for adding
filters - the process of doing augmentation isn't clear to me even
after having produced these changes. There is quite a bit of plumbing
necessary to remove the notion that the BPF object comes from event
parsing and I suspect to achieve this in the current code there is
some overhead that the skeleton could remove - like the empty
sys_enter functions I added.

Anyway, fixing libbpf 1.0+ and removing a header file called bpf.h
were what I was after achieving here :-)

Thanks,
Ian


> - Arnaldo
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index d3d3c13a9f25b55c..067a6e56eeacc9fc 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -1239,7 +1239,7 @@ includedir = $(abspath $(prefix)/$(includedir_relative))
>  mandir = share/man
>  infodir = share/info
>  perfexecdir = libexec/perf-core
> -perf_include_dir = lib/perf/include
> +perf_include_dir = /usr/include
>  perf_examples_dir = lib/perf/examples
>  sharedir = $(prefix)/share
>  template_dir = share/perf-core/templates
> diff --git a/tools/perf/util/llvm-utils.c b/tools/perf/util/llvm-utils.c
> index 2dc7970074196ca8..a5cac85783d8711f 100644
> --- a/tools/perf/util/llvm-utils.c
> +++ b/tools/perf/util/llvm-utils.c
> @@ -495,7 +495,7 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf,
>
>         snprintf(linux_version_code_str, sizeof(linux_version_code_str),
>                  "0x%x", kernel_version);
> -       if (asprintf(&perf_bpf_include_opts, "-I%s/bpf", perf_include_dir) < 0)
> +       if (asprintf(&perf_bpf_include_opts, "-I%s/", perf_include_dir) < 0)
>                 goto errout;
>         force_set_env("NR_CPUS", nr_cpus_avail_str);
>         force_set_env("LINUX_VERSION_CODE", linux_version_code_str);
Arnaldo Carvalho de Melo Nov. 3, 2022, 7:54 p.m. UTC | #5 
Em Thu, Nov 03, 2022 at 09:04:49AM -0700, Ian Rogers escreveu:
> On Thu, Nov 3, 2022 at 8:52 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > Em Thu, Nov 03, 2022 at 12:39:59PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Thu, Nov 03, 2022 at 12:36:36PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > [root@quaco ~]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
> > > > /home/acme/git/perf/tools/perf/examples/bpf/5sec.c:42:10: fatal error: 'bpf/bpf_helpers.h' file not found
> > > > #include <bpf/bpf_helpers.h>
> > > >          ^~~~~~~~~~~~~~~~~~~
> > > > 1 error generated.
> > > > ERROR:      unable to compile /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
> > > > Hint:       Check error message shown above.
> > > > Hint:       You can also pre-compile it into .o using:
> > > >                     clang -target bpf -O2 -c /home/acme/git/perf/tools/perf/examples/bpf/5sec.c
> > > >             with proper -I and -D options.
> > > > event syntax error: '/home/acme/git/perf/tools/perf/examples/bpf/5sec.c'
> > > >                      \___ Failed to load /home/acme/git/perf/tools/perf/examples/bpf/5sec.c from source: Error when compiling BPF scriptlet
> > > >
> > > > (add -v to see detail)
> > > > Run 'perf list' for a list of valid events
> > > >
> > > >  Usage: perf trace [<options>] [<command>]
> > > >     or: perf trace [<options>] -- <command> [<options>]
> > > >     or: perf trace record [<options>] [<command>]
> > > >     or: perf trace record [<options>] -- <command> [<options>]
> > > >
> > > >     -e, --event <event>   event/syscall selector. use 'perf list' to list available events
> > > > [root@quaco ~]#
> > > >
> > > > It is not even finding it, in this machine I have libbpf 0.7.0, so there
> > > > is a /usr/include/bpf/bpf_helpers.h, but probably that isn't in the
> > > > include path set up to build the tools/perf/examples/bpf/ files, perhaps
> > > > it should use:
> > > >
> > > > -Itools/lib/  so that it gets tools/lib/bpf_helpers.h?
> > > >
> > > > Trying to get this tested...
> > >
> > > Running with -v:
> > >
> > > llvm compiling command : /usr/lib64/ccache/clang -D__KERNEL__ -D__NR_CPUS__=8 -DLINUX_VERSION_CODE=0x51310 -g -I/home/acme/lib/perf/include/bpf -nostdinc -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h  -Wno-unused-value -Wno-pointer-sign -working-directory /lib/modules/5.19.16-200.fc36.x86_64/build -c /home/acme/git/perf/tools/perf/examples/bpf/5sec.c -target bpf  -g -O2 -o -
> > > /home/acme/git/perf/tools/perf/examples/bpf/5sec.c:42:10: fatal error: 'bpf/bpf_helpers.h' file not found
> > >
> > > There is still that -I/home/acme/lib/perf/include/bpf, I'll remove it
> > > from the include path and try to replace it with the libbpf path...
> >
> > Ok, works with the patch below, that needs some more renaming from "perf_" to
> > "libbpf_", etc:
> >
> > [root@quaco ~]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/5sec.c  sleep 5
> >      0.000 sleep/160828 perf_bpf_probe:hrtimer_nanosleep(__probe_ip: -1474734416, rqtp: 5000000000)
> > [root@quaco ~]#
> >
> > Since I have:
> >
> > [root@quaco ~]# cat ~/.perfconfig
> > [llvm]
> >         dump-obj = true
> >         clang-opt = -g
> > #

> > I end up with:

> > [root@quaco ~]# ls -la /home/acme/git/perf/tools/perf/examples/bpf/5sec.o
> > -rw-r--r--. 1 root root 3696 Nov  3 12:47 /home/acme/git/perf/tools/perf/examples/bpf/5sec.o
> > [root@quaco ~]# file /home/acme/git/perf/tools/perf/examples/bpf/5sec.o
> > /home/acme/git/perf/tools/perf/examples/bpf/5sec.o: ELF 64-bit LSB relocatable, eBPF, version 1 (SYSV), with debug_info, not stripped
> > [root@quaco ~]#

> > and can test with the pre-built .o eBPF bytecode + capped backtrace:

> > [root@quaco ~]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/5sec.o/max-stack=6/  sleep 5
> >      0.000 sleep/161037 perf_bpf_probe:hrtimer_nanosleep(__probe_ip: -1474734416, rqtp: 5000000000)
> >                                        hrtimer_nanosleep ([kernel.kallsyms])
> >                                        common_nsleep ([kernel.kallsyms])
> >                                        __x64_sys_clock_nanosleep ([kernel.kallsyms])
> >                                        do_syscall_64 ([kernel.kallsyms])
> >                                        entry_SYSCALL_64_after_hwframe ([kernel.kallsyms])
> >                                        __GI___clock_nanosleep (/usr/lib64/libc.so.6)
> > [root@quaco ~]#

> > I'll test the other examples with these changes after I drive Pedro to
> > school and get back to the office.

> Thanks, I was somewhat coding in the dark with this as I was using a
> bpf object file and clearly there's been some attention missed for a
> while in these code paths.

Sure, the way libbpf provided its services has been changing.

> I couldn't get the hello variant to do anything with the openat
> tracing it was set up for, for example, the change puts the call on
> the sys_enter raw syscall. I suspect you remember how these things
> should be and I'm happy to roll a v2, have you fix it, etc.

So, with my change:

[root@quaco ~]# perf trace -e ~acme/git/perf/tools/perf/examples/bpf/hello.c --call-graph=dwarf sleep 0.09
BFD: DWARF error: could not find variable specification at offset 0x6ed3
BFD: DWARF error: could not find variable specification at offset 0x3ca3
BFD: DWARF error: could not find variable specification at offset 0x1b8f
BFD: DWARF error: could not find variable specification at offset 0x3853
BFD: DWARF error: could not find variable specification at offset 0x71eb
     0.000 sleep/167762 __bpf_stdout__(Hello, world)
                                       syscall_trace_enter.constprop.0 ([kernel.kallsyms])
                                       syscall_trace_enter.constprop.0 ([kernel.kallsyms])
                                       do_syscall_64 ([kernel.kallsyms])
                                       entry_SYSCALL_64_after_hwframe ([kernel.kallsyms])
                                       __brk (/usr/lib64/ld-linux-x86-64.so.2)
                                       _dl_sysdep_start (/usr/lib64/ld-linux-x86-64.so.2)
                                       _dl_start_final (inlined)
                                       _dl_start (/usr/lib64/ld-linux-x86-64.so.2)
                                       _start (/usr/lib64/ld-linux-x86-64.so.2)
BFD: DWARF error: could not find variable specification at offset 0x3e33
BFD: DWARF error: could not find variable specification at offset 0x4eef
BFD: DWARF error: could not find variable specification at offset 0x6edf
     0.021 sleep/167762 __bpf_stdout__(Hello, world)
                                       syscall_trace_enter.constprop.0 ([kernel.kallsyms])
                                       syscall_trace_enter.constprop.0 ([kernel.kallsyms])
                                       do_syscall_64 ([kernel.kallsyms])
                                       entry_SYSCALL_64_after_hwframe ([kernel.kallsyms])
                                       get_cet_status (inlined)
                                       init_cpu_features (inlined)
                                       dl_platform_init (inlined)
                                       _dl_sysdep_start (/usr/lib64/ld-linux-x86-64.so.2)
                                       _dl_start_final (inlined)
                                       _dl_start (/usr/lib64/ld-linux-x86-64.so.2)
                                       _start (/usr/lib64/ld-linux-x86-64.so.2)
     0.040 sleep/167762 __bpf_stdout__(Hello, world)
                                       syscall_trace_enter.constprop.0 ([kernel.kallsyms])
                                       syscall_trace_enter.constprop.0 ([kernel.kallsyms])
                                       do_syscall_64 ([kernel.kallsyms])
                                       entry_SYSCALL_64_after_hwframe ([kernel.kallsyms])
                                       __access (inlined)
                                       dl_main (/usr/lib64/ld-linux-x86-64.so.2)
                                       _dl_sysdep_start (/usr/lib64/ld-linux-x86-64.so.2)
                                       _dl_start_final (inlined)
                                       _dl_start (/usr/lib64/ld-linux-x86-64.so.2)
                                       _start (/usr/lib64/ld-linux-x86-64.so.2)
<SNIP>

Those BFD warnings need some investigation.
 
> Fwiw, two things I'd like to see further here is somehow the augmented
> code to be the default (with a BPF skeleton possibly)

Yeah, that was/is an experiment in getting perf and bpf together, one
that predates BPF skeletons, that is now the way of doing things for
perf+bpf (just do a 'ls tools/perf/util/bpf_skel/'), doing it with BPF
skeletons, which your patchkit paves the way to, delights me 8-)

> and an ability for perf_event_open tracing to dump the
> perf_event_attr. I'm unclear on the advantages of having different
> augmenters.

So, no advantage in having different augmenters besides the
experimentation before BPF skeletons, and as I wasn't the one working on
the BPF + perf glueing that gave birth to tools/lib/bpf/, I was just
trying to exercise what Wang Nan was doing, however, in hindsight,
brittle that initial interface was, but experimenting we went.

> Perhaps we could just drop support for different
> augmenters, use BPF skeletons for the raw syscall version and keep the
> BPF logic in parse events for adding filters - the process of doing
> augmentation isn't clear to me even after having produced these
> changes.

Basically you add extra data to existing tracepoints, with
per-tracepoint handlers in 'perf trace' consuming that extra data.

Mostly collecting pointer contents, the canonical one is for pathnames.

Then 'perf trace' tries to be smart and reuses "augmenters" (copies of
pointer contents) on all the syscalls that consume the same type.

> There is quite a bit of plumbing necessary to remove the
> notion that the BPF object comes from event parsing

No need for that, that concept may stay, I think, being able to attach
a .c file to some point in the kernel and then provide a trace_printk()
like interface (that bpf-output codebase) seems interesting.

The augmenter is something of a protocol that 'perf trace' can consume,
but then other 'perf script' can as well consume and do extra stuff we
don´t envision now with whatever BPF programs add to the perf ring
buffer (perf.data files).

> and I suspect to achieve this in the current code there is some
> overhead that the skeleton could remove - like the empty sys_enter
> functions I added.

I didn't look at those yet, unsure if they are needed/
 
> Anyway, fixing libbpf 1.0+ and removing a header file called bpf.h
> were what I was after achieving here :-)

Thanks for moving the needle here.

Providing extra test results:

[root@quaco perf]# perf trace -e tools/perf/examples/bpf/empty.c
^C[root@quaco perf]#

Perfect, if I instead change it to not unconditionally filter the thing
is being hooked: all syscalls..

[root@quaco perf]# perf trace -e tools/perf/examples/bpf/empty.c/max-stack=3/ --max-events 3
     0.000 pipewire/2290 raw_syscalls:sys_enter(args: 140270486487156)
                                       syscall_trace_enter.constprop.0 ([kernel.kallsyms])
                                       syscall_trace_enter.constprop.0 ([kernel.kallsyms])
                                       do_syscall_64 ([kernel.kallsyms])
     0.011 pipewire/2290 raw_syscalls:sys_enter(id: 16, args: 140270486487324)
                                       syscall_trace_enter.constprop.0 ([kernel.kallsyms])
                                       syscall_trace_enter.constprop.0 ([kernel.kallsyms])
                                       do_syscall_64 ([kernel.kallsyms])
     0.022 pipewire/2290 raw_syscalls:sys_enter(id: 1, args: 140270486487492)
                                       syscall_trace_enter.constprop.0 ([kernel.kallsyms])
                                       syscall_trace_enter.constprop.0 ([kernel.kallsyms])
                                       do_syscall_64 ([kernel.kallsyms])
[root@quaco perf]#

And for the most "sophisticated" of those examples, that should be made
the default mode for 'perf trace': 

[root@quaco perf]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.c,open*,connect* --max-events 5
/home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.c:20:10: fatal error: 'stdbool.h' file not found
#include <stdbool.h>
         ^~~~~~~~~~~
1 error generated.
ERROR:	unable to compile /home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.c
Hint:	Check error message shown above.
Hint:	You can also pre-compile it into .o using:
     		clang -target bpf -O2 -c /home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.c
     	with proper -I and -D options.
event syntax error: '/home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.c'
                     \___ Failed to load /home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.c from source: Error when compiling BPF scriptlet

(add -v to see detail)
Run 'perf list' for a list of valid events

 Usage: perf trace [<options>] [<command>]
    or: perf trace [<options>] -- <command> [<options>]
    or: perf trace record [<options>] [<command>]
    or: perf trace record [<options>] -- <command> [<options>]

    -e, --event <event>   event/syscall selector. use 'perf list' to list available events
[root@quaco perf]#

Not a good start:

[root@quaco perf]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.c,string --max-events 6
     0.000 ( 0.019 ms): systemd-oomd/972 openat(dfd: CWD, filename: "/proc/meminfo", flags: RDONLY|CLOEXEC)    = 12
     0.026 ( 0.003 ms): systemd-oomd/972 newfstatat(dfd: 12, filename: "", statbuf: 0x7ffd2bc73cc0, flag: 4096) = 0
18446744073709.520 ( 0.044 ms): cgroupify/36478 openat(dfd: 4, filename: ".", flags: RDONLY|CLOEXEC|DIRECTORY|NONBLOCK) = 5
     0.015 ( 0.004 ms): cgroupify/36478 newfstatat(dfd: 5, filename: "", statbuf: 0x7fffca5b4130, flag: 4096) = 0
     0.075 ( 0.011 ms): cgroupify/36478 openat(dfd: 4, filename: "59457/cgroup.procs")                        = 5
     0.105 ( 0.009 ms): cgroupify/36478 openat(dfd: 4, filename: "37855/cgroup.procs")                        = 5
[root@quaco perf]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.c,string --max-events 10
     0.000 ( 0.065 ms): thermald/1144 openat(dfd: CWD, filename: "/sys/class/powercap/intel-rapl/intel-rapl:0/intel-rapl:0:2/energy_uj") = 13
     0.105 ( 0.008 ms): thermald/1144 openat(dfd: CWD, filename: "/sys/class/powercap/intel-rapl/intel-rapl:0/energy_uj") = 13
     0.125 ( 0.028 ms): thermald/1144 openat(dfd: CWD, filename: "/sys/class/thermal/thermal_zone3/temp")   = 13
     1.079 ( 0.022 ms): thermald/1144 openat(dfd: CWD, filename: "/sys/bus/platform/devices/INT3400:00/odvp6") = 13
     1.113 ( 0.009 ms): thermald/1144 newfstatat(dfd: CWD, filename: "/var/run/thermald/debug_mode", statbuf: 0x7f0e733fb790) = -1 ENOENT (No such file or directory)
     1.127 ( 0.007 ms): thermald/1144 openat(dfd: CWD, filename: "/sys/bus/platform/devices/INT3400:00/odvp18") = 13
     1.141 ( 0.003 ms): thermald/1144 newfstatat(dfd: CWD, filename: "/var/run/thermald/debug_mode", statbuf: 0x7f0e733fb790) = -1 ENOENT (No such file or directory)
     1.147 ( 0.006 ms): thermald/1144 openat(dfd: CWD, filename: "/sys/bus/platform/devices/INT3400:00/odvp18") = 13
     1.159 ( 0.003 ms): thermald/1144 newfstatat(dfd: CWD, filename: "/var/run/thermald/debug_mode", statbuf: 0x7f0e733fb790) = -1 ENOENT (No such file or directory)
     1.165 ( 0.024 ms): thermald/1144 openat(dfd: CWD, filename: "/sys/bus/platform/devices/INT3400:00/odvp18") = 13
[root@quaco perf]#

This after sidestepping the sockaddr and stddef cases and making
'empty.c' become 'everything.c' by making the hook on
raw_syscalls:sys_enter return 1, i.e. don't filter this out, please:

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index d3d3c13a9f25b55c..067a6e56eeacc9fc 100644
diff --git a/tools/perf/examples/bpf/augmented_raw_syscalls.c b/tools/perf/examples/bpf/augmented_raw_syscalls.c
index 13c72fd602c307e4..1d2894fa1083b81a 100644
--- a/tools/perf/examples/bpf/augmented_raw_syscalls.c
+++ b/tools/perf/examples/bpf/augmented_raw_syscalls.c
@@ -17,8 +17,9 @@
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 #include <linux/limits.h>
-#include <stdbool.h>
-#include <sys/socket.h>
+
+typedef char bool;
+typedef int pid_t;
 
 /* bpf-output associated map */
 struct __augmented_syscalls__ {
@@ -100,7 +101,7 @@ struct augmented_args_payload {
 		struct {
 			struct augmented_arg arg, arg2;
 		};
-		struct sockaddr_storage saddr;
+	//	struct sockaddr_storage saddr;
 	};
 };
 
@@ -157,6 +158,7 @@ int syscall_unaugmented(struct syscall_enter_args *args)
 	return 1;
 }
 
+#if 0
 /*
  * These will be tail_called from SEC("raw_syscalls:sys_enter"), so will find in
  * augmented_args_tmp what was read by that raw_syscalls:sys_enter and go
@@ -200,6 +202,7 @@ int sys_enter_sendto(struct syscall_enter_args *args)
 
 	return augmented__output(args, augmented_args, len + socklen);
 }
+#endif
 
 SEC("!syscalls:sys_enter_open")
 int sys_enter_open(struct syscall_enter_args *args)
diff --git a/tools/perf/examples/bpf/empty.c b/tools/perf/examples/bpf/empty.c
index 3e296c0c53d7d8e2..e4872c48a484f218 100644
--- a/tools/perf/examples/bpf/empty.c
+++ b/tools/perf/examples/bpf/empty.c
@@ -7,6 +7,6 @@ struct syscall_enter_args;
 SEC("raw_syscalls:sys_enter")
 int sys_enter(struct syscall_enter_args *args)
 {
-	return 0;
+	return 1;
 }
 char _license[] SEC("license") = "GPL";
Arnaldo Carvalho de Melo Nov. 3, 2022, 9:35 p.m. UTC | #6 
With this:

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index d3d3c13a9f25b55c..067a6e56eeacc9fc 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -1239,7 +1239,7 @@ includedir = $(abspath $(prefix)/$(includedir_relative))
 mandir = share/man
 infodir = share/info
 perfexecdir = libexec/perf-core
-perf_include_dir = lib/perf/include
+perf_include_dir = /usr/include
 perf_examples_dir = lib/perf/examples
 sharedir = $(prefix)/share
 template_dir = share/perf-core/templates
diff --git a/tools/perf/examples/bpf/augmented_raw_syscalls.c b/tools/perf/examples/bpf/augmented_raw_syscalls.c
index 13c72fd602c307e4..98a2731c011339ba 100644
--- a/tools/perf/examples/bpf/augmented_raw_syscalls.c
+++ b/tools/perf/examples/bpf/augmented_raw_syscalls.c
@@ -17,8 +17,9 @@
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 #include <linux/limits.h>
-#include <stdbool.h>
-#include <sys/socket.h>
+
+typedef char bool;
+typedef int pid_t;
 
 /* bpf-output associated map */
 struct __augmented_syscalls__ {
@@ -94,6 +95,30 @@ struct pids_filtered {
 	__uint(max_entries, 64);
 } pids_filtered SEC(".maps");
 
+/*
+ * Desired design of maximum size and alignment (see RFC2553)
+ */
+#define _K_SS_MAXSIZE   128     /* Implementation specific max size */
+
+typedef unsigned short sa_family_t;
+
+/*
+ * The definition uses anonymous union and struct in order to control the
+ * default alignment.
+ */
+struct sockaddr_storage {
+        union {
+                struct {
+                        sa_family_t    ss_family; /* address family */
+                        /* Following field(s) are implementation specific */
+                        char __data[_K_SS_MAXSIZE - sizeof(unsigned short)];
+                                /* space to achieve desired size, */
+                                /* _SS_MAXSIZE value minus size of ss_family */
+                };
+                void *__align; /* implementation specific desired alignment */
+        };
+};
+
 struct augmented_args_payload {
        struct syscall_enter_args args;
        union {
diff --git a/tools/perf/examples/bpf/empty.c b/tools/perf/examples/bpf/empty.c
index 3e296c0c53d7d8e2..e4872c48a484f218 100644
--- a/tools/perf/examples/bpf/empty.c
+++ b/tools/perf/examples/bpf/empty.c
@@ -7,6 +7,6 @@ struct syscall_enter_args;
 SEC("raw_syscalls:sys_enter")
 int sys_enter(struct syscall_enter_args *args)
 {
-	return 0;
+	return 1;
 }
 char _license[] SEC("license") = "GPL";
diff --git a/tools/perf/util/llvm-utils.c b/tools/perf/util/llvm-utils.c
index 2dc7970074196ca8..a5cac85783d8711f 100644
--- a/tools/perf/util/llvm-utils.c
+++ b/tools/perf/util/llvm-utils.c
@@ -495,7 +495,7 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf,
 
 	snprintf(linux_version_code_str, sizeof(linux_version_code_str),
 		 "0x%x", kernel_version);
-	if (asprintf(&perf_bpf_include_opts, "-I%s/bpf", perf_include_dir) < 0)
+	if (asprintf(&perf_bpf_include_opts, "-I%s/", perf_include_dir) < 0)
 		goto errout;
 	force_set_env("NR_CPUS", nr_cpus_avail_str);
 	force_set_env("LINUX_VERSION_CODE", linux_version_code_str);


The connect calls gets served, tomorrow, if you don't beat me I'll apply
the series after adding these minimal changes so that we have this
working with libbpf 1.0 and then we can move from there, with a switch
to a BPF skel, simplest things first, then deal with faults at pointer
payload copy, which is another avenue, AFAIK with solutions already.

Thanks,

- Arnaldo

[root@quaco perf]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.c,connect* --max-events 10
     0.000 ( 0.074 ms): fetchmail/175578 connect(fd: 3, uservaddr: { .family: LOCAL, path: /run/systemd/resolve/io.systemd.Resolve }, addrlen: 42) = 0
     0.397 ( 0.011 ms): systemd-resolv/973 connect(fd: 23, uservaddr: { .family: INET, port: 53, addr: 127.0.0.1 }, addrlen: 16) = 0
     0.532 ( 0.006 ms): systemd-resolv/973 connect(fd: 24, uservaddr: { .family: INET, port: 53, addr: 127.0.0.1 }, addrlen: 16) = 0
     0.910 ( 0.007 ms): systemd-resolv/973 connect(fd: 23, uservaddr: { .family: INET, port: 53, addr: 127.0.0.1 }, addrlen: 16) = 0
     0.962 ( 0.003 ms): systemd-resolv/973 connect(fd: 24, uservaddr: { .family: INET, port: 53, addr: 127.0.0.1 }, addrlen: 16) = 0
     1.337 ( 0.007 ms): fetchmail/175578 connect(fd: 3, uservaddr: { .family: INET, port: 0, addr: 67.195.176.151 }, addrlen: 16) = 0
     1.348 ( 0.014 ms): fetchmail/175578 connect(fd: 3, uservaddr: { .family: UNSPEC }, addrlen: 16)           = 0
     1.363 ( 0.003 ms): fetchmail/175578 connect(fd: 3, uservaddr: { .family: INET, port: 0, addr: 76.13.33.33 }, addrlen: 16) = 0
     1.527 ( 0.014 ms): fetchmail/175578 connect(fd: 3, uservaddr: { .family: LOCAL, path: /run/systemd/resolve/io.systemd.Resolve }, addrlen: 42) = 0
     1.726 ( 0.008 ms): systemd-resolv/973 connect(fd: 23, uservaddr: { .family: INET, port: 53, addr: 127.0.0.1 }, addrlen: 16) = 0
[root@quaco perf]#
Ian Rogers Nov. 3, 2022, 10:01 p.m. UTC | #7 
On Thu, Nov 3, 2022 at 2:35 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> With this:
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index d3d3c13a9f25b55c..067a6e56eeacc9fc 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -1239,7 +1239,7 @@ includedir = $(abspath $(prefix)/$(includedir_relative))
>  mandir = share/man
>  infodir = share/info
>  perfexecdir = libexec/perf-core
> -perf_include_dir = lib/perf/include
> +perf_include_dir = /usr/include
>  perf_examples_dir = lib/perf/examples
>  sharedir = $(prefix)/share
>  template_dir = share/perf-core/templates
> diff --git a/tools/perf/examples/bpf/augmented_raw_syscalls.c b/tools/perf/examples/bpf/augmented_raw_syscalls.c
> index 13c72fd602c307e4..98a2731c011339ba 100644
> --- a/tools/perf/examples/bpf/augmented_raw_syscalls.c
> +++ b/tools/perf/examples/bpf/augmented_raw_syscalls.c
> @@ -17,8 +17,9 @@
>  #include <linux/bpf.h>
>  #include <bpf/bpf_helpers.h>
>  #include <linux/limits.h>
> -#include <stdbool.h>
> -#include <sys/socket.h>
> +
> +typedef char bool;
> +typedef int pid_t;
>
>  /* bpf-output associated map */
>  struct __augmented_syscalls__ {
> @@ -94,6 +95,30 @@ struct pids_filtered {
>         __uint(max_entries, 64);
>  } pids_filtered SEC(".maps");
>
> +/*
> + * Desired design of maximum size and alignment (see RFC2553)
> + */
> +#define _K_SS_MAXSIZE   128     /* Implementation specific max size */
> +
> +typedef unsigned short sa_family_t;
> +
> +/*
> + * The definition uses anonymous union and struct in order to control the
> + * default alignment.
> + */
> +struct sockaddr_storage {
> +        union {
> +                struct {
> +                        sa_family_t    ss_family; /* address family */
> +                        /* Following field(s) are implementation specific */
> +                        char __data[_K_SS_MAXSIZE - sizeof(unsigned short)];
> +                                /* space to achieve desired size, */
> +                                /* _SS_MAXSIZE value minus size of ss_family */
> +                };
> +                void *__align; /* implementation specific desired alignment */
> +        };
> +};
> +
>  struct augmented_args_payload {
>         struct syscall_enter_args args;
>         union {
> diff --git a/tools/perf/examples/bpf/empty.c b/tools/perf/examples/bpf/empty.c
> index 3e296c0c53d7d8e2..e4872c48a484f218 100644
> --- a/tools/perf/examples/bpf/empty.c
> +++ b/tools/perf/examples/bpf/empty.c
> @@ -7,6 +7,6 @@ struct syscall_enter_args;
>  SEC("raw_syscalls:sys_enter")
>  int sys_enter(struct syscall_enter_args *args)
>  {
> -       return 0;
> +       return 1;
>  }
>  char _license[] SEC("license") = "GPL";
> diff --git a/tools/perf/util/llvm-utils.c b/tools/perf/util/llvm-utils.c
> index 2dc7970074196ca8..a5cac85783d8711f 100644
> --- a/tools/perf/util/llvm-utils.c
> +++ b/tools/perf/util/llvm-utils.c
> @@ -495,7 +495,7 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf,
>
>         snprintf(linux_version_code_str, sizeof(linux_version_code_str),
>                  "0x%x", kernel_version);
> -       if (asprintf(&perf_bpf_include_opts, "-I%s/bpf", perf_include_dir) < 0)
> +       if (asprintf(&perf_bpf_include_opts, "-I%s/", perf_include_dir) < 0)
>                 goto errout;
>         force_set_env("NR_CPUS", nr_cpus_avail_str);
>         force_set_env("LINUX_VERSION_CODE", linux_version_code_str);
>
>
> The connect calls gets served, tomorrow, if you don't beat me I'll apply
> the series after adding these minimal changes so that we have this
> working with libbpf 1.0 and then we can move from there, with a switch
> to a BPF skel, simplest things first, then deal with faults at pointer
> payload copy, which is another avenue, AFAIK with solutions already.

So I was trying to be clean and not redefine too much. My clang
command line was:

clang -target bpf -O2 -g -c -I/usr/include/x86_64-linux-gnu
-D__NR_CPUS__=16 -D__x86_64__=1

It'd be nice to just drop the need for __NR_CPUS__ and have it be
dynamic, the skeleton approach would require this. Not sure how to
workaround the x86 define and path :-/ Perhaps send out your changes
for review and I can look at and test them.

Thanks,
Ian

> Thanks,
>
> - Arnaldo
>
> [root@quaco perf]# perf trace -e /home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.c,connect* --max-events 10
>      0.000 ( 0.074 ms): fetchmail/175578 connect(fd: 3, uservaddr: { .family: LOCAL, path: /run/systemd/resolve/io.systemd.Resolve }, addrlen: 42) = 0
>      0.397 ( 0.011 ms): systemd-resolv/973 connect(fd: 23, uservaddr: { .family: INET, port: 53, addr: 127.0.0.1 }, addrlen: 16) = 0
>      0.532 ( 0.006 ms): systemd-resolv/973 connect(fd: 24, uservaddr: { .family: INET, port: 53, addr: 127.0.0.1 }, addrlen: 16) = 0
>      0.910 ( 0.007 ms): systemd-resolv/973 connect(fd: 23, uservaddr: { .family: INET, port: 53, addr: 127.0.0.1 }, addrlen: 16) = 0
>      0.962 ( 0.003 ms): systemd-resolv/973 connect(fd: 24, uservaddr: { .family: INET, port: 53, addr: 127.0.0.1 }, addrlen: 16) = 0
>      1.337 ( 0.007 ms): fetchmail/175578 connect(fd: 3, uservaddr: { .family: INET, port: 0, addr: 67.195.176.151 }, addrlen: 16) = 0
>      1.348 ( 0.014 ms): fetchmail/175578 connect(fd: 3, uservaddr: { .family: UNSPEC }, addrlen: 16)           = 0
>      1.363 ( 0.003 ms): fetchmail/175578 connect(fd: 3, uservaddr: { .family: INET, port: 0, addr: 76.13.33.33 }, addrlen: 16) = 0
>      1.527 ( 0.014 ms): fetchmail/175578 connect(fd: 3, uservaddr: { .family: LOCAL, path: /run/systemd/resolve/io.systemd.Resolve }, addrlen: 42) = 0
>      1.726 ( 0.008 ms): systemd-resolv/973 connect(fd: 23, uservaddr: { .family: INET, port: 53, addr: 127.0.0.1 }, addrlen: 16) = 0
> [root@quaco perf]#
Arnaldo Carvalho de Melo Nov. 4, 2022, 12:21 a.m. UTC | #8 
Em Thu, Nov 03, 2022 at 03:01:21PM -0700, Ian Rogers escreveu:
> On Thu, Nov 3, 2022 at 2:35 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >
> > With this:
> >
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index d3d3c13a9f25b55c..067a6e56eeacc9fc 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -1239,7 +1239,7 @@ includedir = $(abspath $(prefix)/$(includedir_relative))
> >  mandir = share/man
> >  infodir = share/info
> >  perfexecdir = libexec/perf-core
> > -perf_include_dir = lib/perf/include
> > +perf_include_dir = /usr/include
> >  perf_examples_dir = lib/perf/examples
> >  sharedir = $(prefix)/share
> >  template_dir = share/perf-core/templates
> > diff --git a/tools/perf/examples/bpf/augmented_raw_syscalls.c b/tools/perf/examples/bpf/augmented_raw_syscalls.c
> > index 13c72fd602c307e4..98a2731c011339ba 100644
> > --- a/tools/perf/examples/bpf/augmented_raw_syscalls.c
> > +++ b/tools/perf/examples/bpf/augmented_raw_syscalls.c
> > @@ -17,8 +17,9 @@
> >  #include <linux/bpf.h>
> >  #include <bpf/bpf_helpers.h>
> >  #include <linux/limits.h>
> > -#include <stdbool.h>
> > -#include <sys/socket.h>
> > +
> > +typedef char bool;
> > +typedef int pid_t;
> >
> >  /* bpf-output associated map */
> >  struct __augmented_syscalls__ {
> > @@ -94,6 +95,30 @@ struct pids_filtered {
> >         __uint(max_entries, 64);
> >  } pids_filtered SEC(".maps");
> >
> > +/*
> > + * Desired design of maximum size and alignment (see RFC2553)
> > + */
> > +#define _K_SS_MAXSIZE   128     /* Implementation specific max size */
> > +
> > +typedef unsigned short sa_family_t;
> > +
> > +/*
> > + * The definition uses anonymous union and struct in order to control the
> > + * default alignment.
> > + */
> > +struct sockaddr_storage {
> > +        union {
> > +                struct {
> > +                        sa_family_t    ss_family; /* address family */
> > +                        /* Following field(s) are implementation specific */
> > +                        char __data[_K_SS_MAXSIZE - sizeof(unsigned short)];
> > +                                /* space to achieve desired size, */
> > +                                /* _SS_MAXSIZE value minus size of ss_family */
> > +                };
> > +                void *__align; /* implementation specific desired alignment */
> > +        };
> > +};
> > +
> >  struct augmented_args_payload {
> >         struct syscall_enter_args args;
> >         union {
> > diff --git a/tools/perf/examples/bpf/empty.c b/tools/perf/examples/bpf/empty.c
> > index 3e296c0c53d7d8e2..e4872c48a484f218 100644
> > --- a/tools/perf/examples/bpf/empty.c
> > +++ b/tools/perf/examples/bpf/empty.c
> > @@ -7,6 +7,6 @@ struct syscall_enter_args;
> >  SEC("raw_syscalls:sys_enter")
> >  int sys_enter(struct syscall_enter_args *args)
> >  {
> > -       return 0;
> > +       return 1;
> >  }
> >  char _license[] SEC("license") = "GPL";
> > diff --git a/tools/perf/util/llvm-utils.c b/tools/perf/util/llvm-utils.c
> > index 2dc7970074196ca8..a5cac85783d8711f 100644
> > --- a/tools/perf/util/llvm-utils.c
> > +++ b/tools/perf/util/llvm-utils.c
> > @@ -495,7 +495,7 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf,
> >
> >         snprintf(linux_version_code_str, sizeof(linux_version_code_str),
> >                  "0x%x", kernel_version);
> > -       if (asprintf(&perf_bpf_include_opts, "-I%s/bpf", perf_include_dir) < 0)
> > +       if (asprintf(&perf_bpf_include_opts, "-I%s/", perf_include_dir) < 0)
> >                 goto errout;
> >         force_set_env("NR_CPUS", nr_cpus_avail_str);
> >         force_set_env("LINUX_VERSION_CODE", linux_version_code_str);
> >
> >
> > The connect calls gets served, tomorrow, if you don't beat me I'll apply
> > the series after adding these minimal changes so that we have this
> > working with libbpf 1.0 and then we can move from there, with a switch
> > to a BPF skel, simplest things first, then deal with faults at pointer
> > payload copy, which is another avenue, AFAIK with solutions already.
> 
> So I was trying to be clean and not redefine too much. My clang
> command line was:
> 
> clang -target bpf -O2 -g -c -I/usr/include/x86_64-linux-gnu
> -D__NR_CPUS__=16 -D__x86_64__=1

Sure, I haven't even checked why that is needed, maybe its not anymore.

I just tried the first hunch about the header files, those other
variables maybe were needed long ago, maybe its just more stuff to trow
out in the direction of doing it the modern way, BPF skels.
 
> It'd be nice to just drop the need for __NR_CPUS__ and have it be
> dynamic, the skeleton approach would require this. Not sure how to
> workaround the x86 define and path :-/ Perhaps send out your changes
> for review and I can look at and test them.

Done deal, early tomorrow I'll send it and wait for your review.

- Arnaldo
Arnaldo Carvalho de Melo Nov. 4, 2022, 1:04 a.m. UTC | #9 
Em Thu, Nov 03, 2022 at 09:21:06PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Nov 03, 2022 at 03:01:21PM -0700, Ian Rogers escreveu:
> > On Thu, Nov 3, 2022 at 2:35 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > >
> > > With this:
> > >
> > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > > index d3d3c13a9f25b55c..067a6e56eeacc9fc 100644
> > > --- a/tools/perf/Makefile.config
> > > +++ b/tools/perf/Makefile.config
> > > @@ -1239,7 +1239,7 @@ includedir = $(abspath $(prefix)/$(includedir_relative))
> > >  mandir = share/man
> > >  infodir = share/info
> > >  perfexecdir = libexec/perf-core
> > > -perf_include_dir = lib/perf/include
> > > +perf_include_dir = /usr/include
> > >  perf_examples_dir = lib/perf/examples
> > >  sharedir = $(prefix)/share
> > >  template_dir = share/perf-core/templates
> > > diff --git a/tools/perf/examples/bpf/augmented_raw_syscalls.c b/tools/perf/examples/bpf/augmented_raw_syscalls.c
> > > index 13c72fd602c307e4..98a2731c011339ba 100644
> > > --- a/tools/perf/examples/bpf/augmented_raw_syscalls.c
> > > +++ b/tools/perf/examples/bpf/augmented_raw_syscalls.c
> > > @@ -17,8 +17,9 @@
> > >  #include <linux/bpf.h>
> > >  #include <bpf/bpf_helpers.h>
> > >  #include <linux/limits.h>
> > > -#include <stdbool.h>
> > > -#include <sys/socket.h>
> > > +
> > > +typedef char bool;
> > > +typedef int pid_t;
> > >
> > >  /* bpf-output associated map */
> > >  struct __augmented_syscalls__ {
> > > @@ -94,6 +95,30 @@ struct pids_filtered {
> > >         __uint(max_entries, 64);
> > >  } pids_filtered SEC(".maps");
> > >
> > > +/*
> > > + * Desired design of maximum size and alignment (see RFC2553)
> > > + */
> > > +#define _K_SS_MAXSIZE   128     /* Implementation specific max size */
> > > +
> > > +typedef unsigned short sa_family_t;
> > > +
> > > +/*
> > > + * The definition uses anonymous union and struct in order to control the
> > > + * default alignment.
> > > + */
> > > +struct sockaddr_storage {
> > > +        union {
> > > +                struct {
> > > +                        sa_family_t    ss_family; /* address family */
> > > +                        /* Following field(s) are implementation specific */
> > > +                        char __data[_K_SS_MAXSIZE - sizeof(unsigned short)];
> > > +                                /* space to achieve desired size, */
> > > +                                /* _SS_MAXSIZE value minus size of ss_family */
> > > +                };
> > > +                void *__align; /* implementation specific desired alignment */
> > > +        };
> > > +};
> > > +
> > >  struct augmented_args_payload {
> > >         struct syscall_enter_args args;
> > >         union {
> > > diff --git a/tools/perf/examples/bpf/empty.c b/tools/perf/examples/bpf/empty.c
> > > index 3e296c0c53d7d8e2..e4872c48a484f218 100644
> > > --- a/tools/perf/examples/bpf/empty.c
> > > +++ b/tools/perf/examples/bpf/empty.c
> > > @@ -7,6 +7,6 @@ struct syscall_enter_args;
> > >  SEC("raw_syscalls:sys_enter")
> > >  int sys_enter(struct syscall_enter_args *args)
> > >  {
> > > -       return 0;
> > > +       return 1;
> > >  }
> > >  char _license[] SEC("license") = "GPL";
> > > diff --git a/tools/perf/util/llvm-utils.c b/tools/perf/util/llvm-utils.c
> > > index 2dc7970074196ca8..a5cac85783d8711f 100644
> > > --- a/tools/perf/util/llvm-utils.c
> > > +++ b/tools/perf/util/llvm-utils.c
> > > @@ -495,7 +495,7 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf,
> > >
> > >         snprintf(linux_version_code_str, sizeof(linux_version_code_str),
> > >                  "0x%x", kernel_version);
> > > -       if (asprintf(&perf_bpf_include_opts, "-I%s/bpf", perf_include_dir) < 0)
> > > +       if (asprintf(&perf_bpf_include_opts, "-I%s/", perf_include_dir) < 0)
> > >                 goto errout;
> > >         force_set_env("NR_CPUS", nr_cpus_avail_str);
> > >         force_set_env("LINUX_VERSION_CODE", linux_version_code_str);
> > >
> > >
> > > The connect calls gets served, tomorrow, if you don't beat me I'll apply
> > > the series after adding these minimal changes so that we have this
> > > working with libbpf 1.0 and then we can move from there, with a switch
> > > to a BPF skel, simplest things first, then deal with faults at pointer
> > > payload copy, which is another avenue, AFAIK with solutions already.
> > 
> > So I was trying to be clean and not redefine too much. My clang
> > command line was:
> > 
> > clang -target bpf -O2 -g -c -I/usr/include/x86_64-linux-gnu
> > -D__NR_CPUS__=16 -D__x86_64__=1
> 
> Sure, I haven't even checked why that is needed, maybe its not anymore.
> 
> I just tried the first hunch about the header files, those other
> variables maybe were needed long ago, maybe its just more stuff to trow
> out in the direction of doing it the modern way, BPF skels.
>  
> > It'd be nice to just drop the need for __NR_CPUS__ and have it be
> > dynamic, the skeleton approach would require this. Not sure how to
> > workaround the x86 define and path :-/ Perhaps send out your changes
> > for review and I can look at and test them.
> 
> Done deal, early tomorrow I'll send it and wait for your review.

Did it now, please take a look at my tmp.perf/core branch.

Tomorrow I'll add the Tested-by tags.

- Arnaldo
diff mbox series

Patch

diff --git a/tools/perf/examples/bpf/5sec.c b/tools/perf/examples/bpf/5sec.c
index e6b6181c6dc6..f22312c64713 100644
--- a/tools/perf/examples/bpf/5sec.c
+++ b/tools/perf/examples/bpf/5sec.c
@@ -39,13 +39,15 @@ 
    Copyright (C) 2018 Red Hat, Inc., Arnaldo Carvalho de Melo <acme@redhat.com>
 */
 
-#include <bpf.h>
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
 
 #define NSEC_PER_SEC	1000000000L
 
-int probe(hrtimer_nanosleep, rqtp)(void *ctx, int err, long long sec)
+SEC("hrtimer_nanosleep=hrtimer_nanosleep rqtp")
+int hrtimer_nanosleep(void *ctx, int err, long long sec)
 {
 	return sec / NSEC_PER_SEC == 5ULL;
 }
+char _license[] SEC("license") = "GPL";
 
-license(GPL);