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

[Arnaldo Carvalho de Melo]

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@xxxxxxxxxx> 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