bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
@ 2019-11-26 15:10 Arnaldo Carvalho de Melo
  2019-11-26 15:48 ` Arnaldo Carvalho de Melo
  2019-11-26 16:53 ` Alexei Starovoitov
  0 siblings, 2 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-26 15:10 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Adrian Hunter, Alexei Starovoitov, Daniel Borkmann, Jiri Olsa,
	Martin KaFai Lau, Namhyung Kim, bpf, netdev, linux-perf-users,
	Linux Kernel Mailing List

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
 
 $(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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 15:10 [PATCH] libbpf: Fix up generation of bpf_helper_defs.h Arnaldo Carvalho de Melo
@ 2019-11-26 15:48 ` Arnaldo Carvalho de Melo
  2019-11-26 16:38   ` Toke Høiland-Jørgensen
  2019-11-26 16:53 ` Alexei Starovoitov
  1 sibling, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-26 15:48 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Adrian Hunter, Alexei Starovoitov, Daniel Borkmann, Jiri Olsa,
	Martin KaFai Lau, Namhyung Kim, bpf, netdev, linux-perf-users,
	Linux Kernel Mailing List

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

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 15:48 ` Arnaldo Carvalho de Melo
@ 2019-11-26 16:38   ` Toke Høiland-Jørgensen
  2019-11-26 18:34     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 30+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-11-26 16:38 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Andrii Nakryiko
  Cc: Adrian Hunter, Alexei Starovoitov, Daniel Borkmann, Jiri Olsa,
	Martin KaFai Lau, Namhyung Kim, bpf, netdev, linux-perf-users,
	Linux Kernel Mailing List

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 15:10 [PATCH] libbpf: Fix up generation of bpf_helper_defs.h Arnaldo Carvalho de Melo
  2019-11-26 15:48 ` Arnaldo Carvalho de Melo
@ 2019-11-26 16:53 ` Alexei Starovoitov
  2019-11-26 18:30   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 30+ messages in thread
From: Alexei Starovoitov @ 2019-11-26 16:53 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Andrii Nakryiko, Adrian Hunter, Alexei Starovoitov,
	Daniel Borkmann, Jiri Olsa, Martin KaFai Lau, Namhyung Kim, bpf,
	Network Development, linux-perf-users, Linux Kernel Mailing List

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?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 16:53 ` Alexei Starovoitov
@ 2019-11-26 18:30   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-26 18:30 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, Adrian Hunter, Alexei Starovoitov,
	Daniel Borkmann, Jiri Olsa, Martin KaFai Lau, Namhyung Kim, bpf,
	Network Development, linux-perf-users, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 16:38   ` Toke Høiland-Jørgensen
@ 2019-11-26 18:34     ` Arnaldo Carvalho de Melo
  2019-11-26 18:50       ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-26 18:34 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Arnaldo Carvalho de Melo, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, netdev, linux-perf-users,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 18:34     ` Arnaldo Carvalho de Melo
@ 2019-11-26 18:50       ` Toke Høiland-Jørgensen
  2019-11-26 19:04         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 30+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-11-26 18:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Arnaldo Carvalho de Melo, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, netdev, linux-perf-users,
	Linux Kernel Mailing List

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 18:50       ` Toke Høiland-Jørgensen
@ 2019-11-26 19:04         ` Arnaldo Carvalho de Melo
  2019-11-26 22:05           ` Andrii Nakryiko
  0 siblings, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-26 19:04 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Arnaldo Carvalho de Melo, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, netdev, linux-perf-users,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 19:04         ` Arnaldo Carvalho de Melo
@ 2019-11-26 22:05           ` Andrii Nakryiko
  2019-11-26 22:10             ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 30+ messages in thread
From: Andrii Nakryiko @ 2019-11-26 22:05 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 22:05           ` Andrii Nakryiko
@ 2019-11-26 22:10             ` Arnaldo Carvalho de Melo
  2019-11-26 22:17               ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-26 22:10 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Arnaldo Carvalho de Melo, Toke Høiland-Jørgensen,
	Andrii Nakryiko, Adrian Hunter, Alexei Starovoitov,
	Daniel Borkmann, Jiri Olsa, Martin KaFai Lau, Namhyung Kim, bpf,
	Networking, linux-perf-users, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 22:10             ` Arnaldo Carvalho de Melo
@ 2019-11-26 22:17               ` Arnaldo Carvalho de Melo
  2019-11-26 22:38                 ` Andrii Nakryiko
  0 siblings, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-26 22:17 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Arnaldo Carvalho de Melo, Toke Høiland-Jørgensen,
	Andrii Nakryiko, Adrian Hunter, Alexei Starovoitov,
	Daniel Borkmann, Jiri Olsa, Martin KaFai Lau, Namhyung Kim, bpf,
	Networking, linux-perf-users, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 22:17               ` Arnaldo Carvalho de Melo
@ 2019-11-26 22:38                 ` Andrii Nakryiko
  2019-11-26 23:10                   ` Stanislav Fomichev
  0 siblings, 1 reply; 30+ messages in thread
From: Andrii Nakryiko @ 2019-11-26 22:38 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 22:38                 ` Andrii Nakryiko
@ 2019-11-26 23:10                   ` Stanislav Fomichev
  2019-11-26 23:52                     ` Jakub Kicinski
  0 siblings, 1 reply; 30+ messages in thread
From: Stanislav Fomichev @ 2019-11-26 23:10 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Arnaldo Carvalho de Melo, Toke Høiland-Jørgensen,
	Andrii Nakryiko, Adrian Hunter, Alexei Starovoitov,
	Daniel Borkmann, Jiri Olsa, Martin KaFai Lau, Namhyung Kim, bpf,
	Networking, linux-perf-users, Linux Kernel Mailing List,
	Quentin Monnet

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 23:10                   ` Stanislav Fomichev
@ 2019-11-26 23:52                     ` Jakub Kicinski
  2019-11-27  1:39                       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 30+ messages in thread
From: Jakub Kicinski @ 2019-11-26 23:52 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

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 :)

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-26 23:52                     ` Jakub Kicinski
@ 2019-11-27  1:39                       ` Arnaldo Carvalho de Melo
  2019-11-27 13:45                         ` [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches Arnaldo Carvalho de Melo
  2019-11-28  0:31                         ` [PATCH] libbpf: Fix up generation of bpf_helper_defs.h Alexei Starovoitov
  0 siblings, 2 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-27  1:39 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Stanislav Fomichev, Andrii Nakryiko, Arnaldo Carvalho de Melo,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
  2019-11-27  1:39                       ` Arnaldo Carvalho de Melo
@ 2019-11-27 13:45                         ` Arnaldo Carvalho de Melo
  2019-11-27 16:39                           ` Alexei Starovoitov
                                             ` (2 more replies)
  2019-11-28  0:31                         ` [PATCH] libbpf: Fix up generation of bpf_helper_defs.h Alexei Starovoitov
  1 sibling, 3 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-27 13:45 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Jakub Kicinski, Stanislav Fomichev,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

Another fix I'm carrying in my perf/core branch,

Regards,

- Arnaldo

commit 98bb09f90a0ae33125fabc8f41529345382f1498
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Wed Nov 27 09:26:54 2019 -0300

    libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
    
    The st_value field is a 64-bit value, so use PRIu64 to fix this error on
    32-bit arches:
    
      In file included from libbpf.c:52:
      libbpf.c: In function 'bpf_program__record_reloc':
      libbpf_internal.h:59:22: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'Elf64_Addr' {aka 'const long long unsigned int'} [-Werror=format=]
        libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
                            ^~~~~~~~~~
      libbpf_internal.h:62:27: note: in expansion of macro '__pr'
       #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__)
                                 ^~~~
      libbpf.c:1822:4: note: in expansion of macro 'pr_warn'
          pr_warn("bad call relo offset: %lu\n", sym->st_value);
          ^~~~~~~
      libbpf.c:1822:37: note: format string is defined here
          pr_warn("bad call relo offset: %lu\n", sym->st_value);
                                         ~~^
                                         %llu
    
    Fixes: 1f8e2bcb2cd5 ("libbpf: Refactor relocation handling")
    Cc: Alexei Starovoitov <ast@kernel.org>
    Cc: Andrii Nakryiko <andriin@fb.com>
    Link: https://lkml.kernel.org/n/tip-iabs1wq19c357bkk84p7blif@git.kernel.org
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b20f82e58989..6b0eae5c8a94 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1819,7 +1819,7 @@ static int bpf_program__record_reloc(struct bpf_program *prog,
 			return -LIBBPF_ERRNO__RELOC;
 		}
 		if (sym->st_value % 8) {
-			pr_warn("bad call relo offset: %lu\n", sym->st_value);
+			pr_warn("bad call relo offset: %" PRIu64 "\n", sym->st_value);
 			return -LIBBPF_ERRNO__RELOC;
 		}
 		reloc_desc->type = RELO_CALL;

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
  2019-11-27 13:45                         ` [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches Arnaldo Carvalho de Melo
@ 2019-11-27 16:39                           ` Alexei Starovoitov
  2019-11-27 18:45                             ` Arnaldo Carvalho de Melo
  2019-11-27 19:33                           ` Alexei Starovoitov
  2019-12-03 13:50                           ` Naresh Kamboju
  2 siblings, 1 reply; 30+ messages in thread
From: Alexei Starovoitov @ 2019-11-27 16:39 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Andrii Nakryiko, Jakub Kicinski, Stanislav Fomichev,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Another fix I'm carrying in my perf/core branch,

Why in perf/core?
I very much prefer all libbpf patches to go via normal route via bpf/net trees.
We had enough conflicts in this merge window. Let's avoid them.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
  2019-11-27 16:39                           ` Alexei Starovoitov
@ 2019-11-27 18:45                             ` Arnaldo Carvalho de Melo
  2019-11-27 18:55                               ` Alexei Starovoitov
  0 siblings, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-27 18:45 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, Jakub Kicinski, Stanislav Fomichev,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

Em Wed, Nov 27, 2019 at 08:39:28AM -0800, Alexei Starovoitov escreveu:
> On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Another fix I'm carrying in my perf/core branch,
 
> Why in perf/core?
> I very much prefer all libbpf patches to go via normal route via bpf/net trees.
> We had enough conflicts in this merge window. Let's avoid them.

Humm, if we both carry the same patch the merge process can do its magic
and nobody gets hurt? Besides these are really minor things, no?

- Arnaldo

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
  2019-11-27 18:45                             ` Arnaldo Carvalho de Melo
@ 2019-11-27 18:55                               ` Alexei Starovoitov
  2019-11-27 19:39                                 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 30+ messages in thread
From: Alexei Starovoitov @ 2019-11-27 18:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Andrii Nakryiko, Jakub Kicinski, Stanislav Fomichev,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

On Wed, Nov 27, 2019 at 10:45 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Wed, Nov 27, 2019 at 08:39:28AM -0800, Alexei Starovoitov escreveu:
> > On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> > >
> > > Another fix I'm carrying in my perf/core branch,
>
> > Why in perf/core?
> > I very much prefer all libbpf patches to go via normal route via bpf/net trees.
> > We had enough conflicts in this merge window. Let's avoid them.
>
> Humm, if we both carry the same patch the merge process can do its magic
> and nobody gets hurt? Besides these are really minor things, no?

I thought so too, but learned the hard lesson recently.
We should try to avoid that as much as possible.
Andrii's is fixing stuff in the same lines:
https://patchwork.ozlabs.org/patch/1201344/
these two patches will likely conflict. I'd rather have them both in bpf tree.
What is the value for this patch in perf tree?
To fix the build on 32-bit arches, right?
But how urgent is it? Can you wait few days until this one and other
libbpf fixes
land via bpf/net trees?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
  2019-11-27 13:45                         ` [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches Arnaldo Carvalho de Melo
  2019-11-27 16:39                           ` Alexei Starovoitov
@ 2019-11-27 19:33                           ` Alexei Starovoitov
  2019-12-03 13:50                           ` Naresh Kamboju
  2 siblings, 0 replies; 30+ messages in thread
From: Alexei Starovoitov @ 2019-11-27 19:33 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Andrii Nakryiko, Jakub Kicinski, Stanislav Fomichev,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Another fix I'm carrying in my perf/core branch,
>
> Regards,
>
> - Arnaldo
>
> commit 98bb09f90a0ae33125fabc8f41529345382f1498
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Wed Nov 27 09:26:54 2019 -0300
>
>     libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
>
>     The st_value field is a 64-bit value, so use PRIu64 to fix this error on
>     32-bit arches:
>
>       In file included from libbpf.c:52:
>       libbpf.c: In function 'bpf_program__record_reloc':
>       libbpf_internal.h:59:22: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'Elf64_Addr' {aka 'const long long unsigned int'} [-Werror=format=]
>         libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
>                             ^~~~~~~~~~
>       libbpf_internal.h:62:27: note: in expansion of macro '__pr'
>        #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__)
>                                  ^~~~
>       libbpf.c:1822:4: note: in expansion of macro 'pr_warn'
>           pr_warn("bad call relo offset: %lu\n", sym->st_value);
>           ^~~~~~~
>       libbpf.c:1822:37: note: format string is defined here
>           pr_warn("bad call relo offset: %lu\n", sym->st_value);
>                                          ~~^
>                                          %llu
>
>     Fixes: 1f8e2bcb2cd5 ("libbpf: Refactor relocation handling")
>     Cc: Alexei Starovoitov <ast@kernel.org>
>     Cc: Andrii Nakryiko <andriin@fb.com>
>     Link: https://lkml.kernel.org/n/tip-iabs1wq19c357bkk84p7blif@git.kernel.org
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b20f82e58989..6b0eae5c8a94 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1819,7 +1819,7 @@ static int bpf_program__record_reloc(struct bpf_program *prog,
>                         return -LIBBPF_ERRNO__RELOC;
>                 }
>                 if (sym->st_value % 8) {
> -                       pr_warn("bad call relo offset: %lu\n", sym->st_value);
> +                       pr_warn("bad call relo offset: %" PRIu64 "\n", sym->st_value);

Looking at this more... I never liked this PRI stuff. It makes for
such unreadable code.
How about just typecasting st_value to (long) ?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
  2019-11-27 18:55                               ` Alexei Starovoitov
@ 2019-11-27 19:39                                 ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-27 19:39 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, Jakub Kicinski, Stanislav Fomichev,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

Em Wed, Nov 27, 2019 at 10:55:31AM -0800, Alexei Starovoitov escreveu:
> On Wed, Nov 27, 2019 at 10:45 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Wed, Nov 27, 2019 at 08:39:28AM -0800, Alexei Starovoitov escreveu:
> > > On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo
> > > <acme@kernel.org> wrote:
> > > >
> > > > Another fix I'm carrying in my perf/core branch,
> >
> > > Why in perf/core?
> > > I very much prefer all libbpf patches to go via normal route via bpf/net trees.
> > > We had enough conflicts in this merge window. Let's avoid them.
> >
> > Humm, if we both carry the same patch the merge process can do its magic
> > and nobody gets hurt? Besides these are really minor things, no?
> 
> I thought so too, but learned the hard lesson recently.
> We should try to avoid that as much as possible.
> Andrii's is fixing stuff in the same lines:
> https://patchwork.ozlabs.org/patch/1201344/
> these two patches will likely conflict. I'd rather have them both in bpf tree.
> What is the value for this patch in perf tree?
> To fix the build on 32-bit arches, right?
> But how urgent is it? Can you wait few days until this one and other
> libbpf fixes
> land via bpf/net trees?

Ok, I'll add a note to the pull request report about where the perf
build is clean in all containers because I added these two patches, but
that they'll go via the bpf tree, as soon as that gets merged, the
problem will go away.

And I wasn't strictly defending that I should carry this in perf/core,
just said I was, to fix something minor that I found while doing my
usual testing, patch was posted, you got notified and got the patch,
I'll remove it from perf/core now since you stated that it'll eventually
land upstream.

Thanks,

- Arnaldo

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-27  1:39                       ` Arnaldo Carvalho de Melo
  2019-11-27 13:45                         ` [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches Arnaldo Carvalho de Melo
@ 2019-11-28  0:31                         ` Alexei Starovoitov
  2019-11-28  0:51                           ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 30+ messages in thread
From: Alexei Starovoitov @ 2019-11-28  0:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jakub Kicinski, Stanislav Fomichev, Andrii Nakryiko,
	Arnaldo Carvalho de Melo, Toke Høiland-Jørgensen,
	Andrii Nakryiko, Adrian Hunter, Alexei Starovoitov,
	Daniel Borkmann, Jiri Olsa, Martin KaFai Lau, Namhyung Kim, bpf,
	Networking, linux-perf-users, Linux Kernel Mailing List,
	Quentin Monnet

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.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-28  0:31                         ` [PATCH] libbpf: Fix up generation of bpf_helper_defs.h Alexei Starovoitov
@ 2019-11-28  0:51                           ` Arnaldo Carvalho de Melo
  2019-11-28  0:59                             ` Alexei Starovoitov
  0 siblings, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-28  0:51 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jakub Kicinski, Stanislav Fomichev, Andrii Nakryiko,
	Arnaldo Carvalho de Melo, Toke Høiland-Jørgensen,
	Andrii Nakryiko, Adrian Hunter, Alexei Starovoitov,
	Daniel Borkmann, Jiri Olsa, Martin KaFai Lau, Namhyung Kim, bpf,
	Networking, linux-perf-users, Linux Kernel Mailing List,
	Quentin Monnet

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-28  0:51                           ` Arnaldo Carvalho de Melo
@ 2019-11-28  0:59                             ` Alexei Starovoitov
  2019-11-28  1:17                               ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 30+ messages in thread
From: Alexei Starovoitov @ 2019-11-28  0:59 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jakub Kicinski, Stanislav Fomichev, Andrii Nakryiko,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

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),

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-28  0:59                             ` Alexei Starovoitov
@ 2019-11-28  1:17                               ` Arnaldo Carvalho de Melo
  2019-11-28  1:20                                 ` Alexei Starovoitov
  0 siblings, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-28  1:17 UTC (permalink / raw)
  To: Alexei Starovoitov, Arnaldo Carvalho de Melo
  Cc: Jakub Kicinski, Stanislav Fomichev, Andrii Nakryiko,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-28  1:17                               ` Arnaldo Carvalho de Melo
@ 2019-11-28  1:20                                 ` Alexei Starovoitov
  2019-11-28  1:27                                   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 30+ messages in thread
From: Alexei Starovoitov @ 2019-11-28  1:20 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jakub Kicinski, Stanislav Fomichev, Andrii Nakryiko,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

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.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-28  1:20                                 ` Alexei Starovoitov
@ 2019-11-28  1:27                                   ` Arnaldo Carvalho de Melo
  2019-11-28  1:52                                     ` Alexei Starovoitov
  0 siblings, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-28  1:27 UTC (permalink / raw)
  To: Alexei Starovoitov, Arnaldo Carvalho de Melo
  Cc: Jakub Kicinski, Stanislav Fomichev, Andrii Nakryiko,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h
  2019-11-28  1:27                                   ` Arnaldo Carvalho de Melo
@ 2019-11-28  1:52                                     ` Alexei Starovoitov
  0 siblings, 0 replies; 30+ messages in thread
From: Alexei Starovoitov @ 2019-11-28  1:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jakub Kicinski, Stanislav Fomichev, Andrii Nakryiko,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet

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.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
  2019-11-27 13:45                         ` [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches Arnaldo Carvalho de Melo
  2019-11-27 16:39                           ` Alexei Starovoitov
  2019-11-27 19:33                           ` Alexei Starovoitov
@ 2019-12-03 13:50                           ` Naresh Kamboju
  2019-12-03 14:41                             ` Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 30+ messages in thread
From: Naresh Kamboju @ 2019-12-03 13:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Andrii Nakryiko, Jakub Kicinski, Stanislav Fomichev,
	Toke Høiland-Jørgensen, Andrii Nakryiko, Adrian Hunter,
	Alexei Starovoitov, Daniel Borkmann, Jiri Olsa, Martin KaFai Lau,
	Namhyung Kim, bpf, Networking, linux-perf-users,
	Linux Kernel Mailing List, Quentin Monnet, Leo Yan

Hi Arnaldo,

FYI,

On Wed, 27 Nov 2019 at 19:15, Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Another fix I'm carrying in my perf/core branch,
>
> Regards,
>
> - Arnaldo
>
> commit 98bb09f90a0ae33125fabc8f41529345382f1498
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Wed Nov 27 09:26:54 2019 -0300
>
>     libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
>
>     The st_value field is a 64-bit value, so use PRIu64 to fix this error on
>     32-bit arches:
>
>       In file included from libbpf.c:52:
>       libbpf.c: In function 'bpf_program__record_reloc':
>       libbpf_internal.h:59:22: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'Elf64_Addr' {aka 'const long long unsigned int'} [-Werror=format=]
>         libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
>                             ^~~~~~~~~~
>       libbpf_internal.h:62:27: note: in expansion of macro '__pr'
>        #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__)
>                                  ^~~~
>       libbpf.c:1822:4: note: in expansion of macro 'pr_warn'
>           pr_warn("bad call relo offset: %lu\n", sym->st_value);
>           ^~~~~~~
>       libbpf.c:1822:37: note: format string is defined here
>           pr_warn("bad call relo offset: %lu\n", sym->st_value);
>                                          ~~^
>                                          %llu

This build error is been noticed on Linux mainline kernel for 32-bit
architectures from Nov 26.

Full build log,
https://ci.linaro.org/job/openembedded-lkft-linux-mainline/DISTRO=lkft,MACHINE=intel-core2-32,label=docker-lkft/2297/consoleText
https://ci.linaro.org/job/openembedded-lkft-linux-mainline/

- Naresh

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
  2019-12-03 13:50                           ` Naresh Kamboju
@ 2019-12-03 14:41                             ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-12-03 14:41 UTC (permalink / raw)
  To: Naresh Kamboju
  Cc: Arnaldo Carvalho de Melo, Andrii Nakryiko, Jakub Kicinski,
	Stanislav Fomichev, Toke Høiland-Jørgensen,
	Andrii Nakryiko, Adrian Hunter, Alexei Starovoitov,
	Daniel Borkmann, Jiri Olsa, Martin KaFai Lau, Namhyung Kim, bpf,
	Networking, linux-perf-users, Linux Kernel Mailing List,
	Quentin Monnet, Leo Yan

Em Tue, Dec 03, 2019 at 07:20:08PM +0530, Naresh Kamboju escreveu:
> Hi Arnaldo,
> 
> FYI,
> 
> On Wed, 27 Nov 2019 at 19:15, Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Another fix I'm carrying in my perf/core branch,
> >
> > Regards,
> >
> > - Arnaldo
> >
> > commit 98bb09f90a0ae33125fabc8f41529345382f1498
> > Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Date:   Wed Nov 27 09:26:54 2019 -0300
> >
> >     libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
> >
> >     The st_value field is a 64-bit value, so use PRIu64 to fix this error on
> >     32-bit arches:
> >
> >       In file included from libbpf.c:52:
> >       libbpf.c: In function 'bpf_program__record_reloc':
> >       libbpf_internal.h:59:22: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'Elf64_Addr' {aka 'const long long unsigned int'} [-Werror=format=]
> >         libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
> >                             ^~~~~~~~~~
> >       libbpf_internal.h:62:27: note: in expansion of macro '__pr'
> >        #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__)
> >                                  ^~~~
> >       libbpf.c:1822:4: note: in expansion of macro 'pr_warn'
> >           pr_warn("bad call relo offset: %lu\n", sym->st_value);
> >           ^~~~~~~
> >       libbpf.c:1822:37: note: format string is defined here
> >           pr_warn("bad call relo offset: %lu\n", sym->st_value);
> >                                          ~~^
> >                                          %llu
> 
> This build error is been noticed on Linux mainline kernel for 32-bit
> architectures from Nov 26.

Right, the fix is in the bpf tree:

https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=7c3977d1e804

Should go upstream soon.

- Arnaldo
 
> Full build log,
> https://ci.linaro.org/job/openembedded-lkft-linux-mainline/DISTRO=lkft,MACHINE=intel-core2-32,label=docker-lkft/2297/consoleText
> https://ci.linaro.org/job/openembedded-lkft-linux-mainline/
> 
> - Naresh

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2019-12-03 14:41 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26 15:10 [PATCH] libbpf: Fix up generation of bpf_helper_defs.h Arnaldo Carvalho de Melo
2019-11-26 15:48 ` Arnaldo Carvalho de Melo
2019-11-26 16:38   ` Toke Høiland-Jørgensen
2019-11-26 18:34     ` Arnaldo Carvalho de Melo
2019-11-26 18:50       ` Toke Høiland-Jørgensen
2019-11-26 19:04         ` Arnaldo Carvalho de Melo
2019-11-26 22:05           ` Andrii Nakryiko
2019-11-26 22:10             ` Arnaldo Carvalho de Melo
2019-11-26 22:17               ` Arnaldo Carvalho de Melo
2019-11-26 22:38                 ` Andrii Nakryiko
2019-11-26 23:10                   ` Stanislav Fomichev
2019-11-26 23:52                     ` Jakub Kicinski
2019-11-27  1:39                       ` Arnaldo Carvalho de Melo
2019-11-27 13:45                         ` [PATCH] libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches Arnaldo Carvalho de Melo
2019-11-27 16:39                           ` Alexei Starovoitov
2019-11-27 18:45                             ` Arnaldo Carvalho de Melo
2019-11-27 18:55                               ` Alexei Starovoitov
2019-11-27 19:39                                 ` Arnaldo Carvalho de Melo
2019-11-27 19:33                           ` Alexei Starovoitov
2019-12-03 13:50                           ` Naresh Kamboju
2019-12-03 14:41                             ` Arnaldo Carvalho de Melo
2019-11-28  0:31                         ` [PATCH] libbpf: Fix up generation of bpf_helper_defs.h Alexei Starovoitov
2019-11-28  0:51                           ` Arnaldo Carvalho de Melo
2019-11-28  0:59                             ` Alexei Starovoitov
2019-11-28  1:17                               ` Arnaldo Carvalho de Melo
2019-11-28  1:20                                 ` Alexei Starovoitov
2019-11-28  1:27                                   ` Arnaldo Carvalho de Melo
2019-11-28  1:52                                     ` Alexei Starovoitov
2019-11-26 16:53 ` Alexei Starovoitov
2019-11-26 18:30   ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).