diff mbox series

libbpf: Fix up generation of bpf_helper_defs.h

Message ID 20191126151045.GB19483@kernel.org
State Accepted
Delegated to: BPF Maintainers
Headers show
Series libbpf: Fix up generation of bpf_helper_defs.h | expand

Commit Message

Arnaldo Carvalho de Melo Nov. 26, 2019, 3:10 p.m. UTC
Hi guys,

   While merging perf/core with mainline I found the problem below for
which I'm adding this patch to my perf/core branch, that soon will go
Ingo's way, etc. Please let me know if you think this should be handled
some other way,

Thanks,

- Arnaldo

commit 94b2e22463f592d2161eb491ddb0b4659e2a91b4
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Tue Nov 26 11:46:08 2019 -0300

    libbpf: Fix up generation of bpf_helper_defs.h
    
    Building perf as a detached tarball, i.e. by using one of:
    
      $ make help | grep perf
        perf-tar-src-pkg    - Build perf-5.4.0.tar source tarball
        perf-targz-src-pkg  - Build perf-5.4.0.tar.gz source tarball
        perf-tarbz2-src-pkg - Build perf-5.4.0.tar.bz2 source tarball
        perf-tarxz-src-pkg  - Build perf-5.4.0.tar.xz source tarball
      $
    
    And then trying to build the resulting tarball, which is the first thing
    that running:
    
      $ make -C tools/perf build-test
    
    does, ends up with these two problems:
    
      make[3]: *** No rule to make target '/tmp/tmp.zq13cHILGB/perf-5.3.0/include/uapi/linux/bpf.h', needed by 'bpf_helper_defs.h'.  Stop.
      make[3]: *** Waiting for unfinished jobs....
      make[2]: *** [Makefile.perf:757: /tmp/tmp.zq13cHILGB/perf-5.3.0/tools/lib/bpf/libbpf.a] Error 2
      make[2]: *** Waiting for unfinished jobs....
    
    Because $(srcdir) points to the /tmp/tmp.zq13cHILGB/perf-5.3.0 directory
    and we need '/tools/ after that variable, and after fixing this then we
    get to another problem:
    
      /bin/sh: /home/acme/git/perf/tools/scripts/bpf_helpers_doc.py: No such file or directory
      make[3]: *** [Makefile:184: bpf_helper_defs.h] Error 127
      make[3]: *** Deleting file 'bpf_helper_defs.h'
        LD       /tmp/build/perf/libapi-in.o
      make[2]: *** [Makefile.perf:778: /tmp/build/perf/libbpf.a] Error 2
      make[2]: *** Waiting for unfinished jobs....
    
    Because this requires something outside the tools/ directories that gets
    collected into perf's detached tarballs, to fix it just add it to
    tools/perf/MANIFEST, which this patch does, now it works for that case
    and also for all these other cases after doing a:
    
      $ make -C tools clean
    
    On a kernel sources directory:
    
      $ make -C tools/bpf/bpftool/
      make: Entering directory '/home/acme/git/perf/tools/bpf/bpftool'
    
      Auto-detecting system features:
      ...                        libbfd: [ on  ]
      ...        disassembler-four-args: [ on  ]
      ...                          zlib: [ on  ]
    
        CC       map_perf_ring.o
      <SNIP>
        CC       disasm.o
      make[1]: Entering directory '/home/acme/git/perf/tools/lib/bpf'
    
      Auto-detecting system features:
      ...                        libelf: [ on  ]
      ...                           bpf: [ on  ]
    
        MKDIR    staticobjs/
        CC       staticobjs/libbpf.o
      <SNIP>
        LD       staticobjs/libbpf-in.o
        LINK     libbpf.a
      make[1]: Leaving directory '/home/acme/git/perf/tools/lib/bpf'
        LINK     bpftool
      make: Leaving directory '/home/acme/git/perf/tools/bpf/bpftool'
      $
    
      $ make -C tools/perf
      <SNIP>
      Auto-detecting system features:
      ...                         dwarf: [ on  ]
      ...            dwarf_getlocations: [ on  ]
      ...                         glibc: [ on  ]
      ...                          gtk2: [ on  ]
      ...                      libaudit: [ on  ]
      ...                        libbfd: [ on  ]
      ...                        libcap: [ on  ]
      ...                        libelf: [ on  ]
      ...                       libnuma: [ on  ]
      ...        numa_num_possible_cpus: [ on  ]
      ...                       libperl: [ on  ]
      ...                     libpython: [ on  ]
      ...                     libcrypto: [ on  ]
      ...                     libunwind: [ on  ]
      ...            libdw-dwarf-unwind: [ on  ]
      ...                          zlib: [ on  ]
      ...                          lzma: [ on  ]
      ...                     get_cpuid: [ on  ]
      ...                           bpf: [ on  ]
      ...                        libaio: [ on  ]
      ...                       libzstd: [ on  ]
      ...        disassembler-four-args: [ on  ]
    
        GEN      common-cmds.h
        CC       exec-cmd.o
        <SNIP>
        CC       util/pmu.o
        CC       util/pmu-flex.o
        LD       util/perf-in.o
        LD       perf-in.o
        LINK     perf
      make: Leaving directory '/home/acme/git/perf/tools/perf'
      $
    
      $ make -C tools/lib/bpf
      make: Entering directory '/home/acme/git/perf/tools/lib/bpf'
    
      Auto-detecting system features:
      ...                        libelf: [ on  ]
      ...                           bpf: [ on  ]
    
        HOSTCC   fixdep.o
        HOSTLD   fixdep-in.o
        LINK     fixdep
      Parsed description of 117 helper function(s)
        MKDIR    staticobjs/
        CC       staticobjs/libbpf.o
        CC       staticobjs/bpf.o
        CC       staticobjs/nlattr.o
        CC       staticobjs/btf.o
        CC       staticobjs/libbpf_errno.o
        CC       staticobjs/str_error.o
        CC       staticobjs/netlink.o
        CC       staticobjs/bpf_prog_linfo.o
        CC       staticobjs/libbpf_probes.o
        CC       staticobjs/xsk.o
        CC       staticobjs/hashmap.o
        CC       staticobjs/btf_dump.o
        LD       staticobjs/libbpf-in.o
        LINK     libbpf.a
        MKDIR    sharedobjs/
        CC       sharedobjs/libbpf.o
        CC       sharedobjs/bpf.o
        CC       sharedobjs/nlattr.o
        CC       sharedobjs/btf.o
        CC       sharedobjs/libbpf_errno.o
        CC       sharedobjs/str_error.o
        CC       sharedobjs/netlink.o
        CC       sharedobjs/bpf_prog_linfo.o
        CC       sharedobjs/libbpf_probes.o
        CC       sharedobjs/xsk.o
        CC       sharedobjs/hashmap.o
        CC       sharedobjs/btf_dump.o
        LD       sharedobjs/libbpf-in.o
        LINK     libbpf.so.0.0.6
        GEN      libbpf.pc
        LINK     test_libbpf
      make: Leaving directory '/home/acme/git/perf/tools/lib/bpf'
      $
    
    Fixes: e01a75c15969 ("libbpf: Move bpf_{helpers, helper_defs, endian, tracing}.h into libbpf")
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Alexei Starovoitov <ast@kernel.org>
    Cc: Andrii Nakryiko <andriin@fb.com>
    Cc: Daniel Borkmann <daniel@iogearbox.net>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Martin KaFai Lau <kafai@fb.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Link: https://lkml.kernel.org/n/tip-4pnkg2vmdvq5u6eivc887wen@git.kernel.org
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Comments

Arnaldo Carvalho de Melo Nov. 26, 2019, 3:48 p.m. UTC | #1
Em Tue, Nov 26, 2019 at 12:10:45PM -0300, Arnaldo Carvalho de Melo escreveu:
> Hi guys,
> 
>    While merging perf/core with mainline I found the problem below for
> which I'm adding this patch to my perf/core branch, that soon will go
> Ingo's way, etc. Please let me know if you think this should be handled
> some other way,

This is still not enough, fails building in a container where all we
have is the tarball contents, will try to fix later.

- Arnaldo
 
> Thanks,
> 
> - Arnaldo
> 
> commit 94b2e22463f592d2161eb491ddb0b4659e2a91b4
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Tue Nov 26 11:46:08 2019 -0300
> 
>     libbpf: Fix up generation of bpf_helper_defs.h
>     
>     Building perf as a detached tarball, i.e. by using one of:
>     
>       $ make help | grep perf
>         perf-tar-src-pkg    - Build perf-5.4.0.tar source tarball
>         perf-targz-src-pkg  - Build perf-5.4.0.tar.gz source tarball
>         perf-tarbz2-src-pkg - Build perf-5.4.0.tar.bz2 source tarball
>         perf-tarxz-src-pkg  - Build perf-5.4.0.tar.xz source tarball
>       $
>     
>     And then trying to build the resulting tarball, which is the first thing
>     that running:
>     
>       $ make -C tools/perf build-test
>     
>     does, ends up with these two problems:
>     
>       make[3]: *** No rule to make target '/tmp/tmp.zq13cHILGB/perf-5.3.0/include/uapi/linux/bpf.h', needed by 'bpf_helper_defs.h'.  Stop.
>       make[3]: *** Waiting for unfinished jobs....
>       make[2]: *** [Makefile.perf:757: /tmp/tmp.zq13cHILGB/perf-5.3.0/tools/lib/bpf/libbpf.a] Error 2
>       make[2]: *** Waiting for unfinished jobs....
>     
>     Because $(srcdir) points to the /tmp/tmp.zq13cHILGB/perf-5.3.0 directory
>     and we need '/tools/ after that variable, and after fixing this then we
>     get to another problem:
>     
>       /bin/sh: /home/acme/git/perf/tools/scripts/bpf_helpers_doc.py: No such file or directory
>       make[3]: *** [Makefile:184: bpf_helper_defs.h] Error 127
>       make[3]: *** Deleting file 'bpf_helper_defs.h'
>         LD       /tmp/build/perf/libapi-in.o
>       make[2]: *** [Makefile.perf:778: /tmp/build/perf/libbpf.a] Error 2
>       make[2]: *** Waiting for unfinished jobs....
>     
>     Because this requires something outside the tools/ directories that gets
>     collected into perf's detached tarballs, to fix it just add it to
>     tools/perf/MANIFEST, which this patch does, now it works for that case
>     and also for all these other cases after doing a:
>     
>       $ make -C tools clean
>     
>     On a kernel sources directory:
>     
>       $ make -C tools/bpf/bpftool/
>       make: Entering directory '/home/acme/git/perf/tools/bpf/bpftool'
>     
>       Auto-detecting system features:
>       ...                        libbfd: [ on  ]
>       ...        disassembler-four-args: [ on  ]
>       ...                          zlib: [ on  ]
>     
>         CC       map_perf_ring.o
>       <SNIP>
>         CC       disasm.o
>       make[1]: Entering directory '/home/acme/git/perf/tools/lib/bpf'
>     
>       Auto-detecting system features:
>       ...                        libelf: [ on  ]
>       ...                           bpf: [ on  ]
>     
>         MKDIR    staticobjs/
>         CC       staticobjs/libbpf.o
>       <SNIP>
>         LD       staticobjs/libbpf-in.o
>         LINK     libbpf.a
>       make[1]: Leaving directory '/home/acme/git/perf/tools/lib/bpf'
>         LINK     bpftool
>       make: Leaving directory '/home/acme/git/perf/tools/bpf/bpftool'
>       $
>     
>       $ make -C tools/perf
>       <SNIP>
>       Auto-detecting system features:
>       ...                         dwarf: [ on  ]
>       ...            dwarf_getlocations: [ on  ]
>       ...                         glibc: [ on  ]
>       ...                          gtk2: [ on  ]
>       ...                      libaudit: [ on  ]
>       ...                        libbfd: [ on  ]
>       ...                        libcap: [ on  ]
>       ...                        libelf: [ on  ]
>       ...                       libnuma: [ on  ]
>       ...        numa_num_possible_cpus: [ on  ]
>       ...                       libperl: [ on  ]
>       ...                     libpython: [ on  ]
>       ...                     libcrypto: [ on  ]
>       ...                     libunwind: [ on  ]
>       ...            libdw-dwarf-unwind: [ on  ]
>       ...                          zlib: [ on  ]
>       ...                          lzma: [ on  ]
>       ...                     get_cpuid: [ on  ]
>       ...                           bpf: [ on  ]
>       ...                        libaio: [ on  ]
>       ...                       libzstd: [ on  ]
>       ...        disassembler-four-args: [ on  ]
>     
>         GEN      common-cmds.h
>         CC       exec-cmd.o
>         <SNIP>
>         CC       util/pmu.o
>         CC       util/pmu-flex.o
>         LD       util/perf-in.o
>         LD       perf-in.o
>         LINK     perf
>       make: Leaving directory '/home/acme/git/perf/tools/perf'
>       $
>     
>       $ make -C tools/lib/bpf
>       make: Entering directory '/home/acme/git/perf/tools/lib/bpf'
>     
>       Auto-detecting system features:
>       ...                        libelf: [ on  ]
>       ...                           bpf: [ on  ]
>     
>         HOSTCC   fixdep.o
>         HOSTLD   fixdep-in.o
>         LINK     fixdep
>       Parsed description of 117 helper function(s)
>         MKDIR    staticobjs/
>         CC       staticobjs/libbpf.o
>         CC       staticobjs/bpf.o
>         CC       staticobjs/nlattr.o
>         CC       staticobjs/btf.o
>         CC       staticobjs/libbpf_errno.o
>         CC       staticobjs/str_error.o
>         CC       staticobjs/netlink.o
>         CC       staticobjs/bpf_prog_linfo.o
>         CC       staticobjs/libbpf_probes.o
>         CC       staticobjs/xsk.o
>         CC       staticobjs/hashmap.o
>         CC       staticobjs/btf_dump.o
>         LD       staticobjs/libbpf-in.o
>         LINK     libbpf.a
>         MKDIR    sharedobjs/
>         CC       sharedobjs/libbpf.o
>         CC       sharedobjs/bpf.o
>         CC       sharedobjs/nlattr.o
>         CC       sharedobjs/btf.o
>         CC       sharedobjs/libbpf_errno.o
>         CC       sharedobjs/str_error.o
>         CC       sharedobjs/netlink.o
>         CC       sharedobjs/bpf_prog_linfo.o
>         CC       sharedobjs/libbpf_probes.o
>         CC       sharedobjs/xsk.o
>         CC       sharedobjs/hashmap.o
>         CC       sharedobjs/btf_dump.o
>         LD       sharedobjs/libbpf-in.o
>         LINK     libbpf.so.0.0.6
>         GEN      libbpf.pc
>         LINK     test_libbpf
>       make: Leaving directory '/home/acme/git/perf/tools/lib/bpf'
>       $
>     
>     Fixes: e01a75c15969 ("libbpf: Move bpf_{helpers, helper_defs, endian, tracing}.h into libbpf")
>     Cc: Adrian Hunter <adrian.hunter@intel.com>
>     Cc: Alexei Starovoitov <ast@kernel.org>
>     Cc: Andrii Nakryiko <andriin@fb.com>
>     Cc: Daniel Borkmann <daniel@iogearbox.net>
>     Cc: Jiri Olsa <jolsa@kernel.org>
>     Cc: Martin KaFai Lau <kafai@fb.com>
>     Cc: Namhyung Kim <namhyung@kernel.org>
>     Link: https://lkml.kernel.org/n/tip-4pnkg2vmdvq5u6eivc887wen@git.kernel.org
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 99425d0be6ff..8ec6bc4e5e46 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -180,9 +180,9 @@ $(BPF_IN_SHARED): force elfdep bpfdep bpf_helper_defs.h
>  $(BPF_IN_STATIC): force elfdep bpfdep bpf_helper_defs.h
>  	$(Q)$(MAKE) $(build)=libbpf OUTPUT=$(STATIC_OBJDIR)
>  
> -bpf_helper_defs.h: $(srctree)/include/uapi/linux/bpf.h
> +bpf_helper_defs.h: $(srctree)/tools/include/uapi/linux/bpf.h
>  	$(Q)$(srctree)/scripts/bpf_helpers_doc.py --header 		\
> -		--file $(srctree)/include/uapi/linux/bpf.h > bpf_helper_defs.h
> +		--file $(srctree)/tools/include/uapi/linux/bpf.h > bpf_helper_defs.h
>  
>  $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
>  
> diff --git a/tools/perf/MANIFEST b/tools/perf/MANIFEST
> index 70f1ff4e2eb4..4934edb5adfd 100644
> --- a/tools/perf/MANIFEST
> +++ b/tools/perf/MANIFEST
> @@ -19,3 +19,4 @@ tools/lib/bitmap.c
>  tools/lib/str_error_r.c
>  tools/lib/vsprintf.c
>  tools/lib/zalloc.c
> +scripts/bpf_helpers_doc.py
Toke Høiland-Jørgensen Nov. 26, 2019, 4:38 p.m. UTC | #2
Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:

> Em Tue, Nov 26, 2019 at 12:10:45PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Hi guys,
>> 
>>    While merging perf/core with mainline I found the problem below for
>> which I'm adding this patch to my perf/core branch, that soon will go
>> Ingo's way, etc. Please let me know if you think this should be handled
>> some other way,
>
> This is still not enough, fails building in a container where all we
> have is the tarball contents, will try to fix later.

Wouldn't the right thing to do not be to just run the script, and then
put the generated bpf_helper_defs.h into the tarball?

-Toke
Alexei Starovoitov Nov. 26, 2019, 4:53 p.m. UTC | #3
On Tue, Nov 26, 2019 at 7:10 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Hi guys,
>
>    While merging perf/core with mainline I found the problem below for
> which I'm adding this patch to my perf/core branch, that soon will go
> Ingo's way, etc. Please let me know if you think this should be handled
> some other way,
>
> Thanks,
>
> - Arnaldo
>
> commit 94b2e22463f592d2161eb491ddb0b4659e2a91b4
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Tue Nov 26 11:46:08 2019 -0300
>
>     libbpf: Fix up generation of bpf_helper_defs.h
>
>     Building perf as a detached tarball, i.e. by using one of:
>
>       $ make help | grep perf
>         perf-tar-src-pkg    - Build perf-5.4.0.tar source tarball
>         perf-targz-src-pkg  - Build perf-5.4.0.tar.gz source tarball
>         perf-tarbz2-src-pkg - Build perf-5.4.0.tar.bz2 source tarball
>         perf-tarxz-src-pkg  - Build perf-5.4.0.tar.xz source tarball
>       $
>
>     And then trying to build the resulting tarball, which is the first thing
>     that running:
>
>       $ make -C tools/perf build-test
>
>     does, ends up with these two problems:
>
>       make[3]: *** No rule to make target '/tmp/tmp.zq13cHILGB/perf-5.3.0/include/uapi/linux/bpf.h', needed by 'bpf_helper_defs.h'.  Stop.
>       make[3]: *** Waiting for unfinished jobs....
>       make[2]: *** [Makefile.perf:757: /tmp/tmp.zq13cHILGB/perf-5.3.0/tools/lib/bpf/libbpf.a] Error 2
>       make[2]: *** Waiting for unfinished jobs....
>
>     Because $(srcdir) points to the /tmp/tmp.zq13cHILGB/perf-5.3.0 directory
>     and we need '/tools/ after that variable, and after fixing this then we
>     get to another problem:
>
>       /bin/sh: /home/acme/git/perf/tools/scripts/bpf_helpers_doc.py: No such file or directory
>       make[3]: *** [Makefile:184: bpf_helper_defs.h] Error 127
>       make[3]: *** Deleting file 'bpf_helper_defs.h'
>         LD       /tmp/build/perf/libapi-in.o
>       make[2]: *** [Makefile.perf:778: /tmp/build/perf/libbpf.a] Error 2
>       make[2]: *** Waiting for unfinished jobs....
>
>     Because this requires something outside the tools/ directories that gets
>     collected into perf's detached tarballs, to fix it just add it to
>     tools/perf/MANIFEST, which this patch does, now it works for that case
>     and also for all these other cases after doing a:
>
>       $ make -C tools clean
>
>     On a kernel sources directory:
>
>       $ make -C tools/bpf/bpftool/
>       make: Entering directory '/home/acme/git/perf/tools/bpf/bpftool'
>
>       Auto-detecting system features:
>       ...                        libbfd: [ on  ]
>       ...        disassembler-four-args: [ on  ]
>       ...                          zlib: [ on  ]
>
>         CC       map_perf_ring.o
>       <SNIP>
>         CC       disasm.o
>       make[1]: Entering directory '/home/acme/git/perf/tools/lib/bpf'
>
>       Auto-detecting system features:
>       ...                        libelf: [ on  ]
>       ...                           bpf: [ on  ]
>
>         MKDIR    staticobjs/
>         CC       staticobjs/libbpf.o
>       <SNIP>
>         LD       staticobjs/libbpf-in.o
>         LINK     libbpf.a
>       make[1]: Leaving directory '/home/acme/git/perf/tools/lib/bpf'
>         LINK     bpftool
>       make: Leaving directory '/home/acme/git/perf/tools/bpf/bpftool'
>       $
>
>       $ make -C tools/perf
>       <SNIP>
>       Auto-detecting system features:
>       ...                         dwarf: [ on  ]
>       ...            dwarf_getlocations: [ on  ]
>       ...                         glibc: [ on  ]
>       ...                          gtk2: [ on  ]
>       ...                      libaudit: [ on  ]
>       ...                        libbfd: [ on  ]
>       ...                        libcap: [ on  ]
>       ...                        libelf: [ on  ]
>       ...                       libnuma: [ on  ]
>       ...        numa_num_possible_cpus: [ on  ]
>       ...                       libperl: [ on  ]
>       ...                     libpython: [ on  ]
>       ...                     libcrypto: [ on  ]
>       ...                     libunwind: [ on  ]
>       ...            libdw-dwarf-unwind: [ on  ]
>       ...                          zlib: [ on  ]
>       ...                          lzma: [ on  ]
>       ...                     get_cpuid: [ on  ]
>       ...                           bpf: [ on  ]
>       ...                        libaio: [ on  ]
>       ...                       libzstd: [ on  ]
>       ...        disassembler-four-args: [ on  ]
>
>         GEN      common-cmds.h
>         CC       exec-cmd.o
>         <SNIP>
>         CC       util/pmu.o
>         CC       util/pmu-flex.o
>         LD       util/perf-in.o
>         LD       perf-in.o
>         LINK     perf
>       make: Leaving directory '/home/acme/git/perf/tools/perf'
>       $
>
>       $ make -C tools/lib/bpf
>       make: Entering directory '/home/acme/git/perf/tools/lib/bpf'
>
>       Auto-detecting system features:
>       ...                        libelf: [ on  ]
>       ...                           bpf: [ on  ]
>
>         HOSTCC   fixdep.o
>         HOSTLD   fixdep-in.o
>         LINK     fixdep
>       Parsed description of 117 helper function(s)
>         MKDIR    staticobjs/
>         CC       staticobjs/libbpf.o
>         CC       staticobjs/bpf.o
>         CC       staticobjs/nlattr.o
>         CC       staticobjs/btf.o
>         CC       staticobjs/libbpf_errno.o
>         CC       staticobjs/str_error.o
>         CC       staticobjs/netlink.o
>         CC       staticobjs/bpf_prog_linfo.o
>         CC       staticobjs/libbpf_probes.o
>         CC       staticobjs/xsk.o
>         CC       staticobjs/hashmap.o
>         CC       staticobjs/btf_dump.o
>         LD       staticobjs/libbpf-in.o
>         LINK     libbpf.a
>         MKDIR    sharedobjs/
>         CC       sharedobjs/libbpf.o
>         CC       sharedobjs/bpf.o
>         CC       sharedobjs/nlattr.o
>         CC       sharedobjs/btf.o
>         CC       sharedobjs/libbpf_errno.o
>         CC       sharedobjs/str_error.o
>         CC       sharedobjs/netlink.o
>         CC       sharedobjs/bpf_prog_linfo.o
>         CC       sharedobjs/libbpf_probes.o
>         CC       sharedobjs/xsk.o
>         CC       sharedobjs/hashmap.o
>         CC       sharedobjs/btf_dump.o
>         LD       sharedobjs/libbpf-in.o
>         LINK     libbpf.so.0.0.6
>         GEN      libbpf.pc
>         LINK     test_libbpf
>       make: Leaving directory '/home/acme/git/perf/tools/lib/bpf'
>       $
>
>     Fixes: e01a75c15969 ("libbpf: Move bpf_{helpers, helper_defs, endian, tracing}.h into libbpf")
>     Cc: Adrian Hunter <adrian.hunter@intel.com>
>     Cc: Alexei Starovoitov <ast@kernel.org>
>     Cc: Andrii Nakryiko <andriin@fb.com>
>     Cc: Daniel Borkmann <daniel@iogearbox.net>
>     Cc: Jiri Olsa <jolsa@kernel.org>
>     Cc: Martin KaFai Lau <kafai@fb.com>
>     Cc: Namhyung Kim <namhyung@kernel.org>
>     Link: https://lkml.kernel.org/n/tip-4pnkg2vmdvq5u6eivc887wen@git.kernel.org
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 99425d0be6ff..8ec6bc4e5e46 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -180,9 +180,9 @@ $(BPF_IN_SHARED): force elfdep bpfdep bpf_helper_defs.h
>  $(BPF_IN_STATIC): force elfdep bpfdep bpf_helper_defs.h
>         $(Q)$(MAKE) $(build)=libbpf OUTPUT=$(STATIC_OBJDIR)
>
> -bpf_helper_defs.h: $(srctree)/include/uapi/linux/bpf.h
> +bpf_helper_defs.h: $(srctree)/tools/include/uapi/linux/bpf.h
>         $(Q)$(srctree)/scripts/bpf_helpers_doc.py --header              \
> -               --file $(srctree)/include/uapi/linux/bpf.h > bpf_helper_defs.h
> +               --file $(srctree)/tools/include/uapi/linux/bpf.h > bpf_helper_defs.h

fwiw. this bit looks good. Makes sense to do regardless.

>  $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
>
> diff --git a/tools/perf/MANIFEST b/tools/perf/MANIFEST
> index 70f1ff4e2eb4..4934edb5adfd 100644
> --- a/tools/perf/MANIFEST
> +++ b/tools/perf/MANIFEST
> @@ -19,3 +19,4 @@ tools/lib/bitmap.c
>  tools/lib/str_error_r.c
>  tools/lib/vsprintf.c
>  tools/lib/zalloc.c
> +scripts/bpf_helpers_doc.py

This one I don't understand. I couldn't find any piece that uses this file.
Some out of tree usage?
Arnaldo Carvalho de Melo Nov. 26, 2019, 6:30 p.m. UTC | #4
Em Tue, Nov 26, 2019 at 08:53:53AM -0800, Alexei Starovoitov escreveu:
> On Tue, Nov 26, 2019 at 7:10 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Hi guys,
> >
> >    While merging perf/core with mainline I found the problem below for
> > which I'm adding this patch to my perf/core branch, that soon will go
> > Ingo's way, etc. Please let me know if you think this should be handled
> > some other way,
> >
> > Thanks,
> >
> > - Arnaldo
> >
> > commit 94b2e22463f592d2161eb491ddb0b4659e2a91b4
> > Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Date:   Tue Nov 26 11:46:08 2019 -0300
> >
> >     libbpf: Fix up generation of bpf_helper_defs.h
> >
> >     Building perf as a detached tarball, i.e. by using one of:
> >
> >       $ make help | grep perf
> >         perf-tar-src-pkg    - Build perf-5.4.0.tar source tarball
> >         perf-targz-src-pkg  - Build perf-5.4.0.tar.gz source tarball
> >         perf-tarbz2-src-pkg - Build perf-5.4.0.tar.bz2 source tarball
> >         perf-tarxz-src-pkg  - Build perf-5.4.0.tar.xz source tarball
> >       $
> >
> >     And then trying to build the resulting tarball, which is the first thing
> >     that running:
> >
> >       $ make -C tools/perf build-test
> >
> >     does, ends up with these two problems:
> >
> >       make[3]: *** No rule to make target '/tmp/tmp.zq13cHILGB/perf-5.3.0/include/uapi/linux/bpf.h', needed by 'bpf_helper_defs.h'.  Stop.
> >       make[3]: *** Waiting for unfinished jobs....
> >       make[2]: *** [Makefile.perf:757: /tmp/tmp.zq13cHILGB/perf-5.3.0/tools/lib/bpf/libbpf.a] Error 2
> >       make[2]: *** Waiting for unfinished jobs....
> >
> >     Because $(srcdir) points to the /tmp/tmp.zq13cHILGB/perf-5.3.0 directory
> >     and we need '/tools/ after that variable, and after fixing this then we
> >     get to another problem:
> >
> >       /bin/sh: /home/acme/git/perf/tools/scripts/bpf_helpers_doc.py: No such file or directory
> >       make[3]: *** [Makefile:184: bpf_helper_defs.h] Error 127
> >       make[3]: *** Deleting file 'bpf_helper_defs.h'
> >         LD       /tmp/build/perf/libapi-in.o
> >       make[2]: *** [Makefile.perf:778: /tmp/build/perf/libbpf.a] Error 2
> >       make[2]: *** Waiting for unfinished jobs....
> >
> >     Because this requires something outside the tools/ directories that gets
> >     collected into perf's detached tarballs, to fix it just add it to
> >     tools/perf/MANIFEST, which this patch does, now it works for that case
> >     and also for all these other cases after doing a:
> >
> >       $ make -C tools clean
> >
> >     On a kernel sources directory:
> >
> >       $ make -C tools/bpf/bpftool/
> >       make: Entering directory '/home/acme/git/perf/tools/bpf/bpftool'
> >
> >       Auto-detecting system features:
> >       ...                        libbfd: [ on  ]
> >       ...        disassembler-four-args: [ on  ]
> >       ...                          zlib: [ on  ]
> >
> >         CC       map_perf_ring.o
> >       <SNIP>
> >         CC       disasm.o
> >       make[1]: Entering directory '/home/acme/git/perf/tools/lib/bpf'
> >
> >       Auto-detecting system features:
> >       ...                        libelf: [ on  ]
> >       ...                           bpf: [ on  ]
> >
> >         MKDIR    staticobjs/
> >         CC       staticobjs/libbpf.o
> >       <SNIP>
> >         LD       staticobjs/libbpf-in.o
> >         LINK     libbpf.a
> >       make[1]: Leaving directory '/home/acme/git/perf/tools/lib/bpf'
> >         LINK     bpftool
> >       make: Leaving directory '/home/acme/git/perf/tools/bpf/bpftool'
> >       $
> >
> >       $ make -C tools/perf
> >       <SNIP>
> >       Auto-detecting system features:
> >       ...                         dwarf: [ on  ]
> >       ...            dwarf_getlocations: [ on  ]
> >       ...                         glibc: [ on  ]
> >       ...                          gtk2: [ on  ]
> >       ...                      libaudit: [ on  ]
> >       ...                        libbfd: [ on  ]
> >       ...                        libcap: [ on  ]
> >       ...                        libelf: [ on  ]
> >       ...                       libnuma: [ on  ]
> >       ...        numa_num_possible_cpus: [ on  ]
> >       ...                       libperl: [ on  ]
> >       ...                     libpython: [ on  ]
> >       ...                     libcrypto: [ on  ]
> >       ...                     libunwind: [ on  ]
> >       ...            libdw-dwarf-unwind: [ on  ]
> >       ...                          zlib: [ on  ]
> >       ...                          lzma: [ on  ]
> >       ...                     get_cpuid: [ on  ]
> >       ...                           bpf: [ on  ]
> >       ...                        libaio: [ on  ]
> >       ...                       libzstd: [ on  ]
> >       ...        disassembler-four-args: [ on  ]
> >
> >         GEN      common-cmds.h
> >         CC       exec-cmd.o
> >         <SNIP>
> >         CC       util/pmu.o
> >         CC       util/pmu-flex.o
> >         LD       util/perf-in.o
> >         LD       perf-in.o
> >         LINK     perf
> >       make: Leaving directory '/home/acme/git/perf/tools/perf'
> >       $
> >
> >       $ make -C tools/lib/bpf
> >       make: Entering directory '/home/acme/git/perf/tools/lib/bpf'
> >
> >       Auto-detecting system features:
> >       ...                        libelf: [ on  ]
> >       ...                           bpf: [ on  ]
> >
> >         HOSTCC   fixdep.o
> >         HOSTLD   fixdep-in.o
> >         LINK     fixdep
> >       Parsed description of 117 helper function(s)
> >         MKDIR    staticobjs/
> >         CC       staticobjs/libbpf.o
> >         CC       staticobjs/bpf.o
> >         CC       staticobjs/nlattr.o
> >         CC       staticobjs/btf.o
> >         CC       staticobjs/libbpf_errno.o
> >         CC       staticobjs/str_error.o
> >         CC       staticobjs/netlink.o
> >         CC       staticobjs/bpf_prog_linfo.o
> >         CC       staticobjs/libbpf_probes.o
> >         CC       staticobjs/xsk.o
> >         CC       staticobjs/hashmap.o
> >         CC       staticobjs/btf_dump.o
> >         LD       staticobjs/libbpf-in.o
> >         LINK     libbpf.a
> >         MKDIR    sharedobjs/
> >         CC       sharedobjs/libbpf.o
> >         CC       sharedobjs/bpf.o
> >         CC       sharedobjs/nlattr.o
> >         CC       sharedobjs/btf.o
> >         CC       sharedobjs/libbpf_errno.o
> >         CC       sharedobjs/str_error.o
> >         CC       sharedobjs/netlink.o
> >         CC       sharedobjs/bpf_prog_linfo.o
> >         CC       sharedobjs/libbpf_probes.o
> >         CC       sharedobjs/xsk.o
> >         CC       sharedobjs/hashmap.o
> >         CC       sharedobjs/btf_dump.o
> >         LD       sharedobjs/libbpf-in.o
> >         LINK     libbpf.so.0.0.6
> >         GEN      libbpf.pc
> >         LINK     test_libbpf
> >       make: Leaving directory '/home/acme/git/perf/tools/lib/bpf'
> >       $
> >
> >     Fixes: e01a75c15969 ("libbpf: Move bpf_{helpers, helper_defs, endian, tracing}.h into libbpf")
> >     Cc: Adrian Hunter <adrian.hunter@intel.com>
> >     Cc: Alexei Starovoitov <ast@kernel.org>
> >     Cc: Andrii Nakryiko <andriin@fb.com>
> >     Cc: Daniel Borkmann <daniel@iogearbox.net>
> >     Cc: Jiri Olsa <jolsa@kernel.org>
> >     Cc: Martin KaFai Lau <kafai@fb.com>
> >     Cc: Namhyung Kim <namhyung@kernel.org>
> >     Link: https://lkml.kernel.org/n/tip-4pnkg2vmdvq5u6eivc887wen@git.kernel.org
> >     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> > diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> > index 99425d0be6ff..8ec6bc4e5e46 100644
> > --- a/tools/lib/bpf/Makefile
> > +++ b/tools/lib/bpf/Makefile
> > @@ -180,9 +180,9 @@ $(BPF_IN_SHARED): force elfdep bpfdep bpf_helper_defs.h
> >  $(BPF_IN_STATIC): force elfdep bpfdep bpf_helper_defs.h
> >         $(Q)$(MAKE) $(build)=libbpf OUTPUT=$(STATIC_OBJDIR)
> >
> > -bpf_helper_defs.h: $(srctree)/include/uapi/linux/bpf.h
> > +bpf_helper_defs.h: $(srctree)/tools/include/uapi/linux/bpf.h
> >         $(Q)$(srctree)/scripts/bpf_helpers_doc.py --header              \
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > -               --file $(srctree)/include/uapi/linux/bpf.h > bpf_helper_defs.h
> > +               --file $(srctree)/tools/include/uapi/linux/bpf.h > bpf_helper_defs.h
> 
> fwiw. this bit looks good. Makes sense to do regardless.
> 
> >  $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
> >
> > diff --git a/tools/perf/MANIFEST b/tools/perf/MANIFEST
> > index 70f1ff4e2eb4..4934edb5adfd 100644
> > --- a/tools/perf/MANIFEST
> > +++ b/tools/perf/MANIFEST
> > @@ -19,3 +19,4 @@ tools/lib/bitmap.c
> >  tools/lib/str_error_r.c
> >  tools/lib/vsprintf.c
> >  tools/lib/zalloc.c
> > +scripts/bpf_helpers_doc.py
> 
> This one I don't understand. I couldn't find any piece that uses this file.
> Some out of tree usage?

See above on the part that you considered good.

First it couldn't find  $(srctree)/include/uapi/linux/bpf.h when it
tried to handle that bpf_helper_defs.h target, I fixed that by adding
the missing /tools/ bit and then it tried to run
scripts/bpf_helpers_doc.py.

The perf tarball doesn't use anything from the kernel sources (outside
tools/), but since libbpf now uses something that is in the kernel top
level 'scripts' directory, I have to put that script in
tools/perf/MANIFEST so that it can be used when building from the
tarball, detached from the kernel sources.

Or am I still missing something?

- Arnaldo
Arnaldo Carvalho de Melo Nov. 26, 2019, 6:34 p.m. UTC | #5
Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke Høiland-Jørgensen escreveu:
> Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
> 
> > Em Tue, Nov 26, 2019 at 12:10:45PM -0300, Arnaldo Carvalho de Melo escreveu:
> >> Hi guys,
> >> 
> >>    While merging perf/core with mainline I found the problem below for
> >> which I'm adding this patch to my perf/core branch, that soon will go
> >> Ingo's way, etc. Please let me know if you think this should be handled
> >> some other way,
> >
> > This is still not enough, fails building in a container where all we
> > have is the tarball contents, will try to fix later.
> 
> Wouldn't the right thing to do not be to just run the script, and then
> put the generated bpf_helper_defs.h into the tarball?

I would rather continue just running tar and have the build process
in-tree or outside be the same.

- Arnaldo
Toke Høiland-Jørgensen Nov. 26, 2019, 6:50 p.m. UTC | #6
Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:

> Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke Høiland-Jørgensen escreveu:
>> Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
>> 
>> > Em Tue, Nov 26, 2019 at 12:10:45PM -0300, Arnaldo Carvalho de Melo escreveu:
>> >> Hi guys,
>> >> 
>> >>    While merging perf/core with mainline I found the problem below for
>> >> which I'm adding this patch to my perf/core branch, that soon will go
>> >> Ingo's way, etc. Please let me know if you think this should be handled
>> >> some other way,
>> >
>> > This is still not enough, fails building in a container where all we
>> > have is the tarball contents, will try to fix later.
>> 
>> Wouldn't the right thing to do not be to just run the script, and then
>> put the generated bpf_helper_defs.h into the tarball?
>
> I would rather continue just running tar and have the build process
> in-tree or outside be the same.

Hmm, right. Well that Python script basically just parses
include/uapi/linux/bpf.h; and it can be given the path of that file with
the --filename argument. So as long as that file is present, it should
be possible to make it work, I guess?

However, isn't the point of the tarball to make a "stand-alone" source
distribution? I'd argue that it makes more sense to just include the
generated header, then: The point of the Python script is specifically
to extract the latest version of the helper definitions from the kernel
source tree. And if you're "freezing" a version into a tarball, doesn't
it make more sense to also freeze the list of BPF helpers?

-Toke
Arnaldo Carvalho de Melo Nov. 26, 2019, 7:04 p.m. UTC | #7
Em Tue, Nov 26, 2019 at 07:50:44PM +0100, Toke Høiland-Jørgensen escreveu:
> Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
> 
> > Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke Høiland-Jørgensen escreveu:
> >> Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
> >> 
> >> > Em Tue, Nov 26, 2019 at 12:10:45PM -0300, Arnaldo Carvalho de Melo escreveu:
> >> >> Hi guys,
> >> >> 
> >> >>    While merging perf/core with mainline I found the problem below for
> >> >> which I'm adding this patch to my perf/core branch, that soon will go
> >> >> Ingo's way, etc. Please let me know if you think this should be handled
> >> >> some other way,
> >> >
> >> > This is still not enough, fails building in a container where all we
> >> > have is the tarball contents, will try to fix later.
> >> 
> >> Wouldn't the right thing to do not be to just run the script, and then
> >> put the generated bpf_helper_defs.h into the tarball?

> > I would rather continue just running tar and have the build process
> > in-tree or outside be the same.
> 
> Hmm, right. Well that Python script basically just parses
> include/uapi/linux/bpf.h; and it can be given the path of that file with
> the --filename argument. So as long as that file is present, it should
> be possible to make it work, I guess?
 
> However, isn't the point of the tarball to make a "stand-alone" source
> distribution?

Yes, it is, and as far as possible without any prep, just include the
in-source tree files needed to build it.

> I'd argue that it makes more sense to just include the
> generated header, then: The point of the Python script is specifically
> to extract the latest version of the helper definitions from the kernel
> source tree. And if you're "freezing" a version into a tarball, doesn't
> it make more sense to also freeze the list of BPF helpers?

Your suggestion may well even be the only solution, as older systems
don't have python3, and that script requires it :-\

Some containers were showing this:

/bin/sh: 1: /git/linux/scripts/bpf_helpers_doc.py: not found
Makefile:184: recipe for target 'bpf_helper_defs.h' failed
make[3]: *** [bpf_helper_defs.h] Error 127
make[3]: *** Deleting file 'bpf_helper_defs.h'
Makefile.perf:778: recipe for target '/tmp/build/perf/libbpf.a' failed

That "not found" doesn't mean what it looks from staring at the above,
its just that:

nobody@1fb841e33ba3:/tmp/perf-5.4.0$ head -1 /tmp/perf-5.4.0/scripts/bpf_helpers_doc.py
#!/usr/bin/python3
nobody@1fb841e33ba3:/tmp/perf-5.4.0$ ls -la /usr/bin/python3
ls: cannot access /usr/bin/python3: No such file or directory
nobody@1fb841e33ba3:/tmp/perf-5.4.0$

So, for now, I'll keep my fix and start modifying the containers where
this fails and disable testing libbpf/perf integration with BPF on those
containers :-\

I.e. doing:

nobody@1fb841e33ba3:/tmp/perf-5.4.0$ make NO_LIBBPF=1 -C /tmp/perf-5.4.0/tools/perf/ O=/tmp/build/perf

which ends up with a functional perf, just one without libbpf linked in:

nobody@1fb841e33ba3:/tmp/perf-5.4.0$ /tmp/build/perf/perf -vv
perf version 5.4.gf69779ce8f86
                 dwarf: [ on  ]  # HAVE_DWARF_SUPPORT
    dwarf_getlocations: [ OFF ]  # HAVE_DWARF_GETLOCATIONS_SUPPORT
                 glibc: [ on  ]  # HAVE_GLIBC_SUPPORT
                  gtk2: [ on  ]  # HAVE_GTK2_SUPPORT
         syscall_table: [ on  ]  # HAVE_SYSCALL_TABLE_SUPPORT
                libbfd: [ on  ]  # HAVE_LIBBFD_SUPPORT
                libelf: [ on  ]  # HAVE_LIBELF_SUPPORT
               libnuma: [ OFF ]  # HAVE_LIBNUMA_SUPPORT
numa_num_possible_cpus: [ OFF ]  # HAVE_LIBNUMA_SUPPORT
               libperl: [ on  ]  # HAVE_LIBPERL_SUPPORT
             libpython: [ on  ]  # HAVE_LIBPYTHON_SUPPORT
              libslang: [ on  ]  # HAVE_SLANG_SUPPORT
             libcrypto: [ on  ]  # HAVE_LIBCRYPTO_SUPPORT
             libunwind: [ on  ]  # HAVE_LIBUNWIND_SUPPORT
    libdw-dwarf-unwind: [ on  ]  # HAVE_DWARF_SUPPORT
                  zlib: [ on  ]  # HAVE_ZLIB_SUPPORT
                  lzma: [ on  ]  # HAVE_LZMA_SUPPORT
             get_cpuid: [ on  ]  # HAVE_AUXTRACE_SUPPORT
                   bpf: [ OFF ]  # HAVE_LIBBPF_SUPPORT
                   aio: [ on  ]  # HAVE_AIO_SUPPORT
                  zstd: [ OFF ]  # HAVE_ZSTD_SUPPORT
nobody@1fb841e33ba3:/tmp/perf-5.4.0$

The the build tests for libbpf and the bpf support in perf will
continue, but for a reduced set of containers, those with python3.

People wanting to build libbpf on such older systems will hopefully find
this discussion in google, run the script, get the output and have it
working.

- Arnaldo
Andrii Nakryiko Nov. 26, 2019, 10:05 p.m. UTC | #8
On Tue, Nov 26, 2019 at 11:12 AM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Tue, Nov 26, 2019 at 07:50:44PM +0100, Toke Høiland-Jørgensen escreveu:
> > Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
> >
> > > Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke Høiland-Jørgensen escreveu:
> > >> Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
> > >>
> > >> > Em Tue, Nov 26, 2019 at 12:10:45PM -0300, Arnaldo Carvalho de Melo escreveu:
> > >> >> Hi guys,
> > >> >>
> > >> >>    While merging perf/core with mainline I found the problem below for
> > >> >> which I'm adding this patch to my perf/core branch, that soon will go
> > >> >> Ingo's way, etc. Please let me know if you think this should be handled
> > >> >> some other way,
> > >> >
> > >> > This is still not enough, fails building in a container where all we
> > >> > have is the tarball contents, will try to fix later.
> > >>
> > >> Wouldn't the right thing to do not be to just run the script, and then
> > >> put the generated bpf_helper_defs.h into the tarball?
>
> > > I would rather continue just running tar and have the build process
> > > in-tree or outside be the same.
> >
> > Hmm, right. Well that Python script basically just parses
> > include/uapi/linux/bpf.h; and it can be given the path of that file with
> > the --filename argument. So as long as that file is present, it should
> > be possible to make it work, I guess?
>
> > However, isn't the point of the tarball to make a "stand-alone" source
> > distribution?
>
> Yes, it is, and as far as possible without any prep, just include the
> in-source tree files needed to build it.
>
> > I'd argue that it makes more sense to just include the
> > generated header, then: The point of the Python script is specifically
> > to extract the latest version of the helper definitions from the kernel
> > source tree. And if you're "freezing" a version into a tarball, doesn't
> > it make more sense to also freeze the list of BPF helpers?
>
> Your suggestion may well even be the only solution, as older systems
> don't have python3, and that script requires it :-\
>
> Some containers were showing this:
>
> /bin/sh: 1: /git/linux/scripts/bpf_helpers_doc.py: not found
> Makefile:184: recipe for target 'bpf_helper_defs.h' failed
> make[3]: *** [bpf_helper_defs.h] Error 127
> make[3]: *** Deleting file 'bpf_helper_defs.h'
> Makefile.perf:778: recipe for target '/tmp/build/perf/libbpf.a' failed
>
> That "not found" doesn't mean what it looks from staring at the above,
> its just that:
>
> nobody@1fb841e33ba3:/tmp/perf-5.4.0$ head -1 /tmp/perf-5.4.0/scripts/bpf_helpers_doc.py
> #!/usr/bin/python3
> nobody@1fb841e33ba3:/tmp/perf-5.4.0$ ls -la /usr/bin/python3
> ls: cannot access /usr/bin/python3: No such file or directory
> nobody@1fb841e33ba3:/tmp/perf-5.4.0$
>
> So, for now, I'll keep my fix and start modifying the containers where
> this fails and disable testing libbpf/perf integration with BPF on those
> containers :-\

I don't think there is anything Python3-specific in that script. I
changed first line to

#!/usr/bin/env python

and it worked just fine. Do you mind adding this fix and make those
older containers happy(-ier?).

>
> I.e. doing:
>
> nobody@1fb841e33ba3:/tmp/perf-5.4.0$ make NO_LIBBPF=1 -C /tmp/perf-5.4.0/tools/perf/ O=/tmp/build/perf
>
> which ends up with a functional perf, just one without libbpf linked in:
>
> nobody@1fb841e33ba3:/tmp/perf-5.4.0$ /tmp/build/perf/perf -vv
> perf version 5.4.gf69779ce8f86
>                  dwarf: [ on  ]  # HAVE_DWARF_SUPPORT
>     dwarf_getlocations: [ OFF ]  # HAVE_DWARF_GETLOCATIONS_SUPPORT
>                  glibc: [ on  ]  # HAVE_GLIBC_SUPPORT
>                   gtk2: [ on  ]  # HAVE_GTK2_SUPPORT
>          syscall_table: [ on  ]  # HAVE_SYSCALL_TABLE_SUPPORT
>                 libbfd: [ on  ]  # HAVE_LIBBFD_SUPPORT
>                 libelf: [ on  ]  # HAVE_LIBELF_SUPPORT
>                libnuma: [ OFF ]  # HAVE_LIBNUMA_SUPPORT
> numa_num_possible_cpus: [ OFF ]  # HAVE_LIBNUMA_SUPPORT
>                libperl: [ on  ]  # HAVE_LIBPERL_SUPPORT
>              libpython: [ on  ]  # HAVE_LIBPYTHON_SUPPORT
>               libslang: [ on  ]  # HAVE_SLANG_SUPPORT
>              libcrypto: [ on  ]  # HAVE_LIBCRYPTO_SUPPORT
>              libunwind: [ on  ]  # HAVE_LIBUNWIND_SUPPORT
>     libdw-dwarf-unwind: [ on  ]  # HAVE_DWARF_SUPPORT
>                   zlib: [ on  ]  # HAVE_ZLIB_SUPPORT
>                   lzma: [ on  ]  # HAVE_LZMA_SUPPORT
>              get_cpuid: [ on  ]  # HAVE_AUXTRACE_SUPPORT
>                    bpf: [ OFF ]  # HAVE_LIBBPF_SUPPORT
>                    aio: [ on  ]  # HAVE_AIO_SUPPORT
>                   zstd: [ OFF ]  # HAVE_ZSTD_SUPPORT
> nobody@1fb841e33ba3:/tmp/perf-5.4.0$
>
> The the build tests for libbpf and the bpf support in perf will
> continue, but for a reduced set of containers, those with python3.
>
> People wanting to build libbpf on such older systems will hopefully find
> this discussion in google, run the script, get the output and have it
> working.
>
> - Arnaldo
Arnaldo Carvalho de Melo Nov. 26, 2019, 10:10 p.m. UTC | #9
Em Tue, Nov 26, 2019 at 02:05:41PM -0800, Andrii Nakryiko escreveu:
> On Tue, Nov 26, 2019 at 11:12 AM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Em Tue, Nov 26, 2019 at 07:50:44PM +0100, Toke Høiland-Jørgensen escreveu:
> > > Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
> > >
> > > > Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke Høiland-Jørgensen escreveu:
> > > >> Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
> > > >>
> > > >> > Em Tue, Nov 26, 2019 at 12:10:45PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > >> >> Hi guys,
> > > >> >>
> > > >> >>    While merging perf/core with mainline I found the problem below for
> > > >> >> which I'm adding this patch to my perf/core branch, that soon will go
> > > >> >> Ingo's way, etc. Please let me know if you think this should be handled
> > > >> >> some other way,
> > > >> >
> > > >> > This is still not enough, fails building in a container where all we
> > > >> > have is the tarball contents, will try to fix later.
> > > >>
> > > >> Wouldn't the right thing to do not be to just run the script, and then
> > > >> put the generated bpf_helper_defs.h into the tarball?
> >
> > > > I would rather continue just running tar and have the build process
> > > > in-tree or outside be the same.
> > >
> > > Hmm, right. Well that Python script basically just parses
> > > include/uapi/linux/bpf.h; and it can be given the path of that file with
> > > the --filename argument. So as long as that file is present, it should
> > > be possible to make it work, I guess?
> >
> > > However, isn't the point of the tarball to make a "stand-alone" source
> > > distribution?
> >
> > Yes, it is, and as far as possible without any prep, just include the
> > in-source tree files needed to build it.
> >
> > > I'd argue that it makes more sense to just include the
> > > generated header, then: The point of the Python script is specifically
> > > to extract the latest version of the helper definitions from the kernel
> > > source tree. And if you're "freezing" a version into a tarball, doesn't
> > > it make more sense to also freeze the list of BPF helpers?
> >
> > Your suggestion may well even be the only solution, as older systems
> > don't have python3, and that script requires it :-\
> >
> > Some containers were showing this:
> >
> > /bin/sh: 1: /git/linux/scripts/bpf_helpers_doc.py: not found
> > Makefile:184: recipe for target 'bpf_helper_defs.h' failed
> > make[3]: *** [bpf_helper_defs.h] Error 127
> > make[3]: *** Deleting file 'bpf_helper_defs.h'
> > Makefile.perf:778: recipe for target '/tmp/build/perf/libbpf.a' failed
> >
> > That "not found" doesn't mean what it looks from staring at the above,
> > its just that:
> >
> > nobody@1fb841e33ba3:/tmp/perf-5.4.0$ head -1 /tmp/perf-5.4.0/scripts/bpf_helpers_doc.py
> > #!/usr/bin/python3
> > nobody@1fb841e33ba3:/tmp/perf-5.4.0$ ls -la /usr/bin/python3
> > ls: cannot access /usr/bin/python3: No such file or directory
> > nobody@1fb841e33ba3:/tmp/perf-5.4.0$
> >
> > So, for now, I'll keep my fix and start modifying the containers where
> > this fails and disable testing libbpf/perf integration with BPF on those
> > containers :-\
> 
> I don't think there is anything Python3-specific in that script. I
> changed first line to
> 
> #!/usr/bin/env python
> 
> and it worked just fine. Do you mind adding this fix and make those
> older containers happy(-ier?).

I'll try it, was trying the other way around, i.e. adding python3 to
those containers and they got happier, but fatter, so I'll remove that
and try your way, thanks!

I didn't try it that way due to what comes right after the interpreter
line:

#!/usr/bin/python3
# SPDX-License-Identifier: GPL-2.0-only
#
# Copyright (C) 2018-2019 Netronome Systems, Inc.

# In case user attempts to run with Python 2.
from __future__ import print_function

- Arnaldo
 
> >
> > I.e. doing:
> >
> > nobody@1fb841e33ba3:/tmp/perf-5.4.0$ make NO_LIBBPF=1 -C /tmp/perf-5.4.0/tools/perf/ O=/tmp/build/perf
> >
> > which ends up with a functional perf, just one without libbpf linked in:
> >
> > nobody@1fb841e33ba3:/tmp/perf-5.4.0$ /tmp/build/perf/perf -vv
> > perf version 5.4.gf69779ce8f86
> >                  dwarf: [ on  ]  # HAVE_DWARF_SUPPORT
> >     dwarf_getlocations: [ OFF ]  # HAVE_DWARF_GETLOCATIONS_SUPPORT
> >                  glibc: [ on  ]  # HAVE_GLIBC_SUPPORT
> >                   gtk2: [ on  ]  # HAVE_GTK2_SUPPORT
> >          syscall_table: [ on  ]  # HAVE_SYSCALL_TABLE_SUPPORT
> >                 libbfd: [ on  ]  # HAVE_LIBBFD_SUPPORT
> >                 libelf: [ on  ]  # HAVE_LIBELF_SUPPORT
> >                libnuma: [ OFF ]  # HAVE_LIBNUMA_SUPPORT
> > numa_num_possible_cpus: [ OFF ]  # HAVE_LIBNUMA_SUPPORT
> >                libperl: [ on  ]  # HAVE_LIBPERL_SUPPORT
> >              libpython: [ on  ]  # HAVE_LIBPYTHON_SUPPORT
> >               libslang: [ on  ]  # HAVE_SLANG_SUPPORT
> >              libcrypto: [ on  ]  # HAVE_LIBCRYPTO_SUPPORT
> >              libunwind: [ on  ]  # HAVE_LIBUNWIND_SUPPORT
> >     libdw-dwarf-unwind: [ on  ]  # HAVE_DWARF_SUPPORT
> >                   zlib: [ on  ]  # HAVE_ZLIB_SUPPORT
> >                   lzma: [ on  ]  # HAVE_LZMA_SUPPORT
> >              get_cpuid: [ on  ]  # HAVE_AUXTRACE_SUPPORT
> >                    bpf: [ OFF ]  # HAVE_LIBBPF_SUPPORT
> >                    aio: [ on  ]  # HAVE_AIO_SUPPORT
> >                   zstd: [ OFF ]  # HAVE_ZSTD_SUPPORT
> > nobody@1fb841e33ba3:/tmp/perf-5.4.0$
> >
> > The the build tests for libbpf and the bpf support in perf will
> > continue, but for a reduced set of containers, those with python3.
> >
> > People wanting to build libbpf on such older systems will hopefully find
> > this discussion in google, run the script, get the output and have it
> > working.
> >
> > - Arnaldo
Arnaldo Carvalho de Melo Nov. 26, 2019, 10:17 p.m. UTC | #10
Em Tue, Nov 26, 2019 at 07:10:18PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Nov 26, 2019 at 02:05:41PM -0800, Andrii Nakryiko escreveu:
> > On Tue, Nov 26, 2019 at 11:12 AM Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > >
> > > Em Tue, Nov 26, 2019 at 07:50:44PM +0100, Toke Høiland-Jørgensen escreveu:
> > > > Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
> > > >
> > > > > Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke Høiland-Jørgensen escreveu:
> > > > >> Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
> > > > >>
> > > > >> > Em Tue, Nov 26, 2019 at 12:10:45PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > >> >> Hi guys,
> > > > >> >>
> > > > >> >>    While merging perf/core with mainline I found the problem below for
> > > > >> >> which I'm adding this patch to my perf/core branch, that soon will go
> > > > >> >> Ingo's way, etc. Please let me know if you think this should be handled
> > > > >> >> some other way,
> > > > >> >
> > > > >> > This is still not enough, fails building in a container where all we
> > > > >> > have is the tarball contents, will try to fix later.
> > > > >>
> > > > >> Wouldn't the right thing to do not be to just run the script, and then
> > > > >> put the generated bpf_helper_defs.h into the tarball?
> > >
> > > > > I would rather continue just running tar and have the build process
> > > > > in-tree or outside be the same.
> > > >
> > > > Hmm, right. Well that Python script basically just parses
> > > > include/uapi/linux/bpf.h; and it can be given the path of that file with
> > > > the --filename argument. So as long as that file is present, it should
> > > > be possible to make it work, I guess?
> > >
> > > > However, isn't the point of the tarball to make a "stand-alone" source
> > > > distribution?
> > >
> > > Yes, it is, and as far as possible without any prep, just include the
> > > in-source tree files needed to build it.
> > >
> > > > I'd argue that it makes more sense to just include the
> > > > generated header, then: The point of the Python script is specifically
> > > > to extract the latest version of the helper definitions from the kernel
> > > > source tree. And if you're "freezing" a version into a tarball, doesn't
> > > > it make more sense to also freeze the list of BPF helpers?
> > >
> > > Your suggestion may well even be the only solution, as older systems
> > > don't have python3, and that script requires it :-\
> > >
> > > Some containers were showing this:
> > >
> > > /bin/sh: 1: /git/linux/scripts/bpf_helpers_doc.py: not found
> > > Makefile:184: recipe for target 'bpf_helper_defs.h' failed
> > > make[3]: *** [bpf_helper_defs.h] Error 127
> > > make[3]: *** Deleting file 'bpf_helper_defs.h'
> > > Makefile.perf:778: recipe for target '/tmp/build/perf/libbpf.a' failed
> > >
> > > That "not found" doesn't mean what it looks from staring at the above,
> > > its just that:
> > >
> > > nobody@1fb841e33ba3:/tmp/perf-5.4.0$ head -1 /tmp/perf-5.4.0/scripts/bpf_helpers_doc.py
> > > #!/usr/bin/python3
> > > nobody@1fb841e33ba3:/tmp/perf-5.4.0$ ls -la /usr/bin/python3
> > > ls: cannot access /usr/bin/python3: No such file or directory
> > > nobody@1fb841e33ba3:/tmp/perf-5.4.0$
> > >
> > > So, for now, I'll keep my fix and start modifying the containers where
> > > this fails and disable testing libbpf/perf integration with BPF on those
> > > containers :-\
> > 
> > I don't think there is anything Python3-specific in that script. I
> > changed first line to
> > 
> > #!/usr/bin/env python
> > 
> > and it worked just fine. Do you mind adding this fix and make those
> > older containers happy(-ier?).
> 
> I'll try it, was trying the other way around, i.e. adding python3 to
> those containers and they got happier, but fatter, so I'll remove that
> and try your way, thanks!
> 
> I didn't try it that way due to what comes right after the interpreter
> line:
> 
> #!/usr/bin/python3
> # SPDX-License-Identifier: GPL-2.0-only
> #
> # Copyright (C) 2018-2019 Netronome Systems, Inc.
> 
> # In case user attempts to run with Python 2.
> from __future__ import print_function

And that is why I think you got it working, that script uses things
like:

        print('Parsed description of %d helper function(s)' % len(self.helpers),
              file=sys.stderr)

That python2 thinks its science fiction, what tuple is that? Can't
understand, print isn't a function back then.

https://sebastianraschka.com/Articles/2014_python_2_3_key_diff.html#the-print-function

I've been adding python3  to where it is available and not yet in the
container images, most are working after that, some don't need because
they need other packages for BPF to work and those are not available, so
nevermind, lets have just the fix I provided, I'll add python3 and life
goes on.

- Arnaldo
Andrii Nakryiko Nov. 26, 2019, 10:38 p.m. UTC | #11
On Tue, Nov 26, 2019 at 2:17 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Tue, Nov 26, 2019 at 07:10:18PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Tue, Nov 26, 2019 at 02:05:41PM -0800, Andrii Nakryiko escreveu:
> > > On Tue, Nov 26, 2019 at 11:12 AM Arnaldo Carvalho de Melo
> > > <arnaldo.melo@gmail.com> wrote:
> > > >
> > > > Em Tue, Nov 26, 2019 at 07:50:44PM +0100, Toke Høiland-Jørgensen escreveu:
> > > > > Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
> > > > >
> > > > > > Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke Høiland-Jørgensen escreveu:
> > > > > >> Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
> > > > > >>
> > > > > >> > Em Tue, Nov 26, 2019 at 12:10:45PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > > >> >> Hi guys,
> > > > > >> >>
> > > > > >> >>    While merging perf/core with mainline I found the problem below for
> > > > > >> >> which I'm adding this patch to my perf/core branch, that soon will go
> > > > > >> >> Ingo's way, etc. Please let me know if you think this should be handled
> > > > > >> >> some other way,
> > > > > >> >
> > > > > >> > This is still not enough, fails building in a container where all we
> > > > > >> > have is the tarball contents, will try to fix later.
> > > > > >>
> > > > > >> Wouldn't the right thing to do not be to just run the script, and then
> > > > > >> put the generated bpf_helper_defs.h into the tarball?
> > > >
> > > > > > I would rather continue just running tar and have the build process
> > > > > > in-tree or outside be the same.
> > > > >
> > > > > Hmm, right. Well that Python script basically just parses
> > > > > include/uapi/linux/bpf.h; and it can be given the path of that file with
> > > > > the --filename argument. So as long as that file is present, it should
> > > > > be possible to make it work, I guess?
> > > >
> > > > > However, isn't the point of the tarball to make a "stand-alone" source
> > > > > distribution?
> > > >
> > > > Yes, it is, and as far as possible without any prep, just include the
> > > > in-source tree files needed to build it.
> > > >
> > > > > I'd argue that it makes more sense to just include the
> > > > > generated header, then: The point of the Python script is specifically
> > > > > to extract the latest version of the helper definitions from the kernel
> > > > > source tree. And if you're "freezing" a version into a tarball, doesn't
> > > > > it make more sense to also freeze the list of BPF helpers?
> > > >
> > > > Your suggestion may well even be the only solution, as older systems
> > > > don't have python3, and that script requires it :-\
> > > >
> > > > Some containers were showing this:
> > > >
> > > > /bin/sh: 1: /git/linux/scripts/bpf_helpers_doc.py: not found
> > > > Makefile:184: recipe for target 'bpf_helper_defs.h' failed
> > > > make[3]: *** [bpf_helper_defs.h] Error 127
> > > > make[3]: *** Deleting file 'bpf_helper_defs.h'
> > > > Makefile.perf:778: recipe for target '/tmp/build/perf/libbpf.a' failed
> > > >
> > > > That "not found" doesn't mean what it looks from staring at the above,
> > > > its just that:
> > > >
> > > > nobody@1fb841e33ba3:/tmp/perf-5.4.0$ head -1 /tmp/perf-5.4.0/scripts/bpf_helpers_doc.py
> > > > #!/usr/bin/python3
> > > > nobody@1fb841e33ba3:/tmp/perf-5.4.0$ ls -la /usr/bin/python3
> > > > ls: cannot access /usr/bin/python3: No such file or directory
> > > > nobody@1fb841e33ba3:/tmp/perf-5.4.0$
> > > >
> > > > So, for now, I'll keep my fix and start modifying the containers where
> > > > this fails and disable testing libbpf/perf integration with BPF on those
> > > > containers :-\
> > >
> > > I don't think there is anything Python3-specific in that script. I
> > > changed first line to
> > >
> > > #!/usr/bin/env python
> > >
> > > and it worked just fine. Do you mind adding this fix and make those
> > > older containers happy(-ier?).
> >
> > I'll try it, was trying the other way around, i.e. adding python3 to
> > those containers and they got happier, but fatter, so I'll remove that
> > and try your way, thanks!
> >
> > I didn't try it that way due to what comes right after the interpreter
> > line:
> >
> > #!/usr/bin/python3
> > # SPDX-License-Identifier: GPL-2.0-only
> > #
> > # Copyright (C) 2018-2019 Netronome Systems, Inc.
> >
> > # In case user attempts to run with Python 2.
> > from __future__ import print_function
>
> And that is why I think you got it working, that script uses things
> like:
>
>         print('Parsed description of %d helper function(s)' % len(self.helpers),
>               file=sys.stderr)
>
> That python2 thinks its science fiction, what tuple is that? Can't
> understand, print isn't a function back then.

Not a Python expert (or even regular user), but quick googling showed
that this import is the way to go to use Python3 semantics of print
within Python2, so seems like that's fine. But maybe Quentin has
anything to say about this.


>
> https://sebastianraschka.com/Articles/2014_python_2_3_key_diff.html#the-print-function
>
> I've been adding python3  to where it is available and not yet in the
> container images, most are working after that, some don't need because
> they need other packages for BPF to work and those are not available, so
> nevermind, lets have just the fix I provided, I'll add python3 and life
> goes on.
>
> - Arnaldo
Stanislav Fomichev Nov. 26, 2019, 11:10 p.m. UTC | #12
On 11/26, Andrii Nakryiko wrote:
> On Tue, Nov 26, 2019 at 2:17 PM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Em Tue, Nov 26, 2019 at 07:10:18PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Tue, Nov 26, 2019 at 02:05:41PM -0800, Andrii Nakryiko escreveu:
> > > > On Tue, Nov 26, 2019 at 11:12 AM Arnaldo Carvalho de Melo
> > > > <arnaldo.melo@gmail.com> wrote:
> > > > >
> > > > > Em Tue, Nov 26, 2019 at 07:50:44PM +0100, Toke Høiland-Jørgensen escreveu:
> > > > > > Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
> > > > > >
> > > > > > > Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke Høiland-Jørgensen escreveu:
> > > > > > >> Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> writes:
> > > > > > >>
> > > > > > >> > Em Tue, Nov 26, 2019 at 12:10:45PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > > > >> >> Hi guys,
> > > > > > >> >>
> > > > > > >> >>    While merging perf/core with mainline I found the problem below for
> > > > > > >> >> which I'm adding this patch to my perf/core branch, that soon will go
> > > > > > >> >> Ingo's way, etc. Please let me know if you think this should be handled
> > > > > > >> >> some other way,
> > > > > > >> >
> > > > > > >> > This is still not enough, fails building in a container where all we
> > > > > > >> > have is the tarball contents, will try to fix later.
> > > > > > >>
> > > > > > >> Wouldn't the right thing to do not be to just run the script, and then
> > > > > > >> put the generated bpf_helper_defs.h into the tarball?
> > > > >
> > > > > > > I would rather continue just running tar and have the build process
> > > > > > > in-tree or outside be the same.
> > > > > >
> > > > > > Hmm, right. Well that Python script basically just parses
> > > > > > include/uapi/linux/bpf.h; and it can be given the path of that file with
> > > > > > the --filename argument. So as long as that file is present, it should
> > > > > > be possible to make it work, I guess?
> > > > >
> > > > > > However, isn't the point of the tarball to make a "stand-alone" source
> > > > > > distribution?
> > > > >
> > > > > Yes, it is, and as far as possible without any prep, just include the
> > > > > in-source tree files needed to build it.
> > > > >
> > > > > > I'd argue that it makes more sense to just include the
> > > > > > generated header, then: The point of the Python script is specifically
> > > > > > to extract the latest version of the helper definitions from the kernel
> > > > > > source tree. And if you're "freezing" a version into a tarball, doesn't
> > > > > > it make more sense to also freeze the list of BPF helpers?
> > > > >
> > > > > Your suggestion may well even be the only solution, as older systems
> > > > > don't have python3, and that script requires it :-\
> > > > >
> > > > > Some containers were showing this:
> > > > >
> > > > > /bin/sh: 1: /git/linux/scripts/bpf_helpers_doc.py: not found
> > > > > Makefile:184: recipe for target 'bpf_helper_defs.h' failed
> > > > > make[3]: *** [bpf_helper_defs.h] Error 127
> > > > > make[3]: *** Deleting file 'bpf_helper_defs.h'
> > > > > Makefile.perf:778: recipe for target '/tmp/build/perf/libbpf.a' failed
> > > > >
> > > > > That "not found" doesn't mean what it looks from staring at the above,
> > > > > its just that:
> > > > >
> > > > > nobody@1fb841e33ba3:/tmp/perf-5.4.0$ head -1 /tmp/perf-5.4.0/scripts/bpf_helpers_doc.py
> > > > > #!/usr/bin/python3
> > > > > nobody@1fb841e33ba3:/tmp/perf-5.4.0$ ls -la /usr/bin/python3
> > > > > ls: cannot access /usr/bin/python3: No such file or directory
> > > > > nobody@1fb841e33ba3:/tmp/perf-5.4.0$
> > > > >
> > > > > So, for now, I'll keep my fix and start modifying the containers where
> > > > > this fails and disable testing libbpf/perf integration with BPF on those
> > > > > containers :-\
> > > >
> > > > I don't think there is anything Python3-specific in that script. I
> > > > changed first line to
> > > >
> > > > #!/usr/bin/env python
> > > >
> > > > and it worked just fine. Do you mind adding this fix and make those
> > > > older containers happy(-ier?).
> > >
> > > I'll try it, was trying the other way around, i.e. adding python3 to
> > > those containers and they got happier, but fatter, so I'll remove that
> > > and try your way, thanks!
> > >
> > > I didn't try it that way due to what comes right after the interpreter
> > > line:
> > >
> > > #!/usr/bin/python3
> > > # SPDX-License-Identifier: GPL-2.0-only
> > > #
> > > # Copyright (C) 2018-2019 Netronome Systems, Inc.
> > >
> > > # In case user attempts to run with Python 2.
> > > from __future__ import print_function
> >
> > And that is why I think you got it working, that script uses things
> > like:
> >
> >         print('Parsed description of %d helper function(s)' % len(self.helpers),
> >               file=sys.stderr)
> >
> > That python2 thinks its science fiction, what tuple is that? Can't
> > understand, print isn't a function back then.
> 
> Not a Python expert (or even regular user), but quick googling showed
> that this import is the way to go to use Python3 semantics of print
> within Python2, so seems like that's fine. But maybe Quentin has
> anything to say about this.
We are using this script with python2.7, works just fine :-)
So maybe doing s/python3/python/ is the way to go, whatever
default python is installed, it should work with that.

> > https://sebastianraschka.com/Articles/2014_python_2_3_key_diff.html#the-print-function
> >
> > I've been adding python3  to where it is available and not yet in the
> > container images, most are working after that, some don't need because
> > they need other packages for BPF to work and those are not available, so
> > nevermind, lets have just the fix I provided, I'll add python3 and life
> > goes on.
> >
> > - Arnaldo
Jakub Kicinski Nov. 26, 2019, 11:52 p.m. UTC | #13
On Tue, 26 Nov 2019 15:10:30 -0800, Stanislav Fomichev wrote:
> We are using this script with python2.7, works just fine :-)
> So maybe doing s/python3/python/ is the way to go, whatever
> default python is installed, it should work with that.

That increases the risk someone will make a python2-only change 
and break Python 3.

Python 2 is dead, I'm honestly surprised this needs to be said :)
Arnaldo Carvalho de Melo Nov. 27, 2019, 1:39 a.m. UTC | #14
Em Tue, Nov 26, 2019 at 03:52:28PM -0800, Jakub Kicinski escreveu:
> On Tue, 26 Nov 2019 15:10:30 -0800, Stanislav Fomichev wrote:
> > We are using this script with python2.7, works just fine :-)
> > So maybe doing s/python3/python/ is the way to go, whatever
> > default python is installed, it should work with that.

> That increases the risk someone will make a python2-only change 
> and break Python 3.
 
> Python 2 is dead, I'm honestly surprised this needs to be said :)

It shouldn't have to be said, and probably it is old school to try and
keep things portable when there is no need to use new stuff for simple
tasks like this.

Anyway, it seems its just a matter of adding the python3 package to the
old container images and then most of them will work with what is in
that script, what doesn't work is really old and then NO_LIBBPF=1 is the
way to go.

In the end, kinda nothing to see here, go back to adding cool new stuff,
lets not hold eBPF from progressing ;-P

- Arnaldo
Alexei Starovoitov Nov. 28, 2019, 12:31 a.m. UTC | #15
On Tue, Nov 26, 2019 at 5:39 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Tue, Nov 26, 2019 at 03:52:28PM -0800, Jakub Kicinski escreveu:
> > On Tue, 26 Nov 2019 15:10:30 -0800, Stanislav Fomichev wrote:
> > > We are using this script with python2.7, works just fine :-)
> > > So maybe doing s/python3/python/ is the way to go, whatever
> > > default python is installed, it should work with that.
>
> > That increases the risk someone will make a python2-only change
> > and break Python 3.
>
> > Python 2 is dead, I'm honestly surprised this needs to be said :)
>
> It shouldn't have to be said, and probably it is old school to try and
> keep things portable when there is no need to use new stuff for simple
> tasks like this.
>
> Anyway, it seems its just a matter of adding the python3 package to the
> old container images and then most of them will work with what is in
> that script, what doesn't work is really old and then NO_LIBBPF=1 is the
> way to go.
>
> In the end, kinda nothing to see here, go back to adding cool new stuff,
> lets not hold eBPF from progressing ;-P

Absolutely. I think if some distro is still using 32-bit userland it's likely
so much behind anything modern that its kernel is equally old too
and appeal of new features (bpf or anything else) is probably low.
So if I were you I would keep 32-bit builds of perf supported, but with
minimal effort.

Re: patch itself.
I can take it as-is into bpf tree and it will be in Linus's tree in few days.
Or I can take only tools/lib/bpf/Makefile hunk and you can take
tools/perf/MANIFEST via perf tree?
Whichever way is fine.
Arnaldo Carvalho de Melo Nov. 28, 2019, 12:51 a.m. UTC | #16
On November 27, 2019 9:31:41 PM GMT-03:00, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>On Tue, Nov 26, 2019 at 5:39 PM Arnaldo Carvalho de Melo
><acme@kernel.org> wrote:
>>
>> Em Tue, Nov 26, 2019 at 03:52:28PM -0800, Jakub Kicinski escreveu:
>> > On Tue, 26 Nov 2019 15:10:30 -0800, Stanislav Fomichev wrote:
>> > > We are using this script with python2.7, works just fine :-)
>> > > So maybe doing s/python3/python/ is the way to go, whatever
>> > > default python is installed, it should work with that.
>>
>> > That increases the risk someone will make a python2-only change
>> > and break Python 3.
>>
>> > Python 2 is dead, I'm honestly surprised this needs to be said :)
>>
>> It shouldn't have to be said, and probably it is old school to try
>and
>> keep things portable when there is no need to use new stuff for
>simple
>> tasks like this.
>>
>> Anyway, it seems its just a matter of adding the python3 package to
>the
>> old container images and then most of them will work with what is in
>> that script, what doesn't work is really old and then NO_LIBBPF=1 is
>the
>> way to go.
>>
>> In the end, kinda nothing to see here, go back to adding cool new
>stuff,
>> lets not hold eBPF from progressing ;-P
>
>Absolutely. I think if some distro is still using 32-bit userland it's
>likely
>so much behind anything modern that its kernel is equally old too
>and appeal of new features (bpf or anything else) is probably low.
>So if I were you I would keep 32-bit builds of perf supported, but with
>minimal effort.

I try not to assume too much, just try to keep what's being tested to continue to at least build.

>Re: patch itself.
>I can take it as-is into bpf tree and it will be in Linus's tree in few
>days.
>Or I can take only tools/lib/bpf/Makefile hunk and you can take
>tools/perf/MANIFEST via perf tree?
>Whichever way is fine.

Take it as one, I think it's what should have been in the cset it is fixing, that way no breakage would have happened.

- Arnaldo
Alexei Starovoitov Nov. 28, 2019, 12:59 a.m. UTC | #17
On Wed, Nov 27, 2019 at 4:50 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Take it as one, I think it's what should have been in the cset it is fixing, that way no breakage would have happened.

Ok. I trimmed commit log and applied here:
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=1fd450f99272791df8ea8e1b0f5657678e118e90

What about your other fix and my suggestion there?
(__u64) cast instead of PRI ?
We do this already in two places:
libbpf.c:                shdr_idx, (__u64)sym->st_value);
libbpf.c:             (__u64)sym.st_value, GELF_ST_TYPE(sym.st_info),
Arnaldo Carvalho de Melo Nov. 28, 2019, 1:17 a.m. UTC | #18
On November 27, 2019 9:59:15 PM GMT-03:00, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>On Wed, Nov 27, 2019 at 4:50 PM Arnaldo Carvalho de Melo
><arnaldo.melo@gmail.com> wrote:
>>
>> Take it as one, I think it's what should have been in the cset it is
>fixing, that way no breakage would have happened.
>
>Ok. I trimmed commit log and applied here:
>https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=1fd450f99272791df8ea8e1b0f5657678e118e90
>
>What about your other fix and my suggestion there?
>(__u64) cast instead of PRI ?
>We do this already in two places:
>libbpf.c:                shdr_idx, (__u64)sym->st_value);
>libbpf.c:             (__u64)sym.st_value, GELF_ST_TYPE(sym.st_info),


I'm using the smartphone now, but I thought you first suggested using a cast to long, if you mean using %llu + cast to __u64, then should be was ugly as using PRI, IOW, should work on both 64 bit and 32 bit. :-)

- Arnaldo
Alexei Starovoitov Nov. 28, 2019, 1:20 a.m. UTC | #19
On Wed, Nov 27, 2019 at 5:17 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> On November 27, 2019 9:59:15 PM GMT-03:00, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >On Wed, Nov 27, 2019 at 4:50 PM Arnaldo Carvalho de Melo
> ><arnaldo.melo@gmail.com> wrote:
> >>
> >> Take it as one, I think it's what should have been in the cset it is
> >fixing, that way no breakage would have happened.
> >
> >Ok. I trimmed commit log and applied here:
> >https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=1fd450f99272791df8ea8e1b0f5657678e118e90
> >
> >What about your other fix and my suggestion there?
> >(__u64) cast instead of PRI ?
> >We do this already in two places:
> >libbpf.c:                shdr_idx, (__u64)sym->st_value);
> >libbpf.c:             (__u64)sym.st_value, GELF_ST_TYPE(sym.st_info),
>
>
> I'm using the smartphone now, but I thought you first suggested using a cast to long, if you mean using %llu + cast to __u64, then should be was ugly as using PRI, IOW, should work on both 64 bit and 32 bit. :-)

Yes. I suggested (long) first, but then found two cases in libbpf that
solve this with (__u64),
so better to stick to that for consistency.
Arnaldo Carvalho de Melo Nov. 28, 2019, 1:27 a.m. UTC | #20
On November 27, 2019 10:20:17 PM GMT-03:00, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>On Wed, Nov 27, 2019 at 5:17 PM Arnaldo Carvalho de Melo
><arnaldo.melo@gmail.com> wrote:
>>
>> On November 27, 2019 9:59:15 PM GMT-03:00, Alexei Starovoitov
><alexei.starovoitov@gmail.com> wrote:
>> >On Wed, Nov 27, 2019 at 4:50 PM Arnaldo Carvalho de Melo
>> ><arnaldo.melo@gmail.com> wrote:
>> >>
>> >> Take it as one, I think it's what should have been in the cset it
>is
>> >fixing, that way no breakage would have happened.
>> >
>> >Ok. I trimmed commit log and applied here:
>>
>>https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=1fd450f99272791df8ea8e1b0f5657678e118e90
>> >
>> >What about your other fix and my suggestion there?
>> >(__u64) cast instead of PRI ?
>> >We do this already in two places:
>> >libbpf.c:                shdr_idx, (__u64)sym->st_value);
>> >libbpf.c:             (__u64)sym.st_value,
>GELF_ST_TYPE(sym.st_info),
>>
>>
>> I'm using the smartphone now, but I thought you first suggested using
>a cast to long, if you mean using %llu + cast to __u64, then should be
>was ugly as using PRI, IOW, should work on both 64 bit and 32 bit. :-)
>
>Yes. I suggested (long) first, but then found two cases in libbpf that
>solve this with (__u64),
>so better to stick to that for consistency.

If it's already being used elsewhere in lubbpf, it was tested already with the build containers and since nobody complained, go with it :-)

- Arnaldo
Alexei Starovoitov Nov. 28, 2019, 1:52 a.m. UTC | #21
On Wed, Nov 27, 2019 at 5:26 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> On November 27, 2019 10:20:17 PM GMT-03:00, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >On Wed, Nov 27, 2019 at 5:17 PM Arnaldo Carvalho de Melo
> ><arnaldo.melo@gmail.com> wrote:
> >>
> >> On November 27, 2019 9:59:15 PM GMT-03:00, Alexei Starovoitov
> ><alexei.starovoitov@gmail.com> wrote:
> >> >On Wed, Nov 27, 2019 at 4:50 PM Arnaldo Carvalho de Melo
> >> ><arnaldo.melo@gmail.com> wrote:
> >> >>
> >> >> Take it as one, I think it's what should have been in the cset it
> >is
> >> >fixing, that way no breakage would have happened.
> >> >
> >> >Ok. I trimmed commit log and applied here:
> >>
> >>https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=1fd450f99272791df8ea8e1b0f5657678e118e90
> >> >
> >> >What about your other fix and my suggestion there?
> >> >(__u64) cast instead of PRI ?
> >> >We do this already in two places:
> >> >libbpf.c:                shdr_idx, (__u64)sym->st_value);
> >> >libbpf.c:             (__u64)sym.st_value,
> >GELF_ST_TYPE(sym.st_info),
> >>
> >>
> >> I'm using the smartphone now, but I thought you first suggested using
> >a cast to long, if you mean using %llu + cast to __u64, then should be
> >was ugly as using PRI, IOW, should work on both 64 bit and 32 bit. :-)
> >
> >Yes. I suggested (long) first, but then found two cases in libbpf that
> >solve this with (__u64),
> >so better to stick to that for consistency.
>
> If it's already being used elsewhere in lubbpf, it was tested already with the build containers and since nobody complained, go with it :-)

Ok.
Pushed this fix:
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=7c3977d1e80401b1a25efded698b05d60ee26e31
Likely will send PR for bpf tree to Dave tomorrow.
diff mbox series

Patch

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 99425d0be6ff..8ec6bc4e5e46 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -180,9 +180,9 @@  $(BPF_IN_SHARED): force elfdep bpfdep bpf_helper_defs.h
 $(BPF_IN_STATIC): force elfdep bpfdep bpf_helper_defs.h
 	$(Q)$(MAKE) $(build)=libbpf OUTPUT=$(STATIC_OBJDIR)
 
-bpf_helper_defs.h: $(srctree)/include/uapi/linux/bpf.h
+bpf_helper_defs.h: $(srctree)/tools/include/uapi/linux/bpf.h
 	$(Q)$(srctree)/scripts/bpf_helpers_doc.py --header 		\
-		--file $(srctree)/include/uapi/linux/bpf.h > bpf_helper_defs.h
+		--file $(srctree)/tools/include/uapi/linux/bpf.h > bpf_helper_defs.h
 
 $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
 
diff --git a/tools/perf/MANIFEST b/tools/perf/MANIFEST
index 70f1ff4e2eb4..4934edb5adfd 100644
--- a/tools/perf/MANIFEST
+++ b/tools/perf/MANIFEST
@@ -19,3 +19,4 @@  tools/lib/bitmap.c
 tools/lib/str_error_r.c
 tools/lib/vsprintf.c
 tools/lib/zalloc.c
+scripts/bpf_helpers_doc.py