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 <[email protected]>
> 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 <[email protected]>
> Cc: Alexei Starovoitov <[email protected]>
> Cc: Andrii Nakryiko <[email protected]>
> Cc: Daniel Borkmann <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Martin KaFai Lau <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Link: https://lkml.kernel.org/n/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> 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
Arnaldo Carvalho de Melo <[email protected]> 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
Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke H?iland-J?rgensen escreveu:
> Arnaldo Carvalho de Melo <[email protected]> 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
Arnaldo Carvalho de Melo <[email protected]> writes:
> Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke Høiland-Jørgensen escreveu:
>> Arnaldo Carvalho de Melo <[email protected]> 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
Em Tue, Nov 26, 2019 at 07:50:44PM +0100, Toke H?iland-J?rgensen escreveu:
> Arnaldo Carvalho de Melo <[email protected]> writes:
>
> > Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke H?iland-J?rgensen escreveu:
> >> Arnaldo Carvalho de Melo <[email protected]> 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
On Tue, Nov 26, 2019 at 11:12 AM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Em Tue, Nov 26, 2019 at 07:50:44PM +0100, Toke Høiland-Jørgensen escreveu:
> > Arnaldo Carvalho de Melo <[email protected]> writes:
> >
> > > Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke Høiland-Jørgensen escreveu:
> > >> Arnaldo Carvalho de Melo <[email protected]> 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
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
> <[email protected]> wrote:
> >
> > Em Tue, Nov 26, 2019 at 07:50:44PM +0100, Toke H?iland-J?rgensen escreveu:
> > > Arnaldo Carvalho de Melo <[email protected]> writes:
> > >
> > > > Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke H?iland-J?rgensen escreveu:
> > > >> Arnaldo Carvalho de Melo <[email protected]> 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
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
> > <[email protected]> wrote:
> > >
> > > Em Tue, Nov 26, 2019 at 07:50:44PM +0100, Toke H?iland-J?rgensen escreveu:
> > > > Arnaldo Carvalho de Melo <[email protected]> writes:
> > > >
> > > > > Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke H?iland-J?rgensen escreveu:
> > > > >> Arnaldo Carvalho de Melo <[email protected]> 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
On Tue, Nov 26, 2019 at 2:17 PM Arnaldo Carvalho de Melo
<[email protected]> 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
> > > <[email protected]> wrote:
> > > >
> > > > Em Tue, Nov 26, 2019 at 07:50:44PM +0100, Toke Høiland-Jørgensen escreveu:
> > > > > Arnaldo Carvalho de Melo <[email protected]> writes:
> > > > >
> > > > > > Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke Høiland-Jørgensen escreveu:
> > > > > >> Arnaldo Carvalho de Melo <[email protected]> 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
On 11/26, Andrii Nakryiko wrote:
> On Tue, Nov 26, 2019 at 2:17 PM Arnaldo Carvalho de Melo
> <[email protected]> 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
> > > > <[email protected]> wrote:
> > > > >
> > > > > Em Tue, Nov 26, 2019 at 07:50:44PM +0100, Toke H?iland-J?rgensen escreveu:
> > > > > > Arnaldo Carvalho de Melo <[email protected]> writes:
> > > > > >
> > > > > > > Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke H?iland-J?rgensen escreveu:
> > > > > > >> Arnaldo Carvalho de Melo <[email protected]> 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
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 :)
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
Another fix I'm carrying in my perf/core branch,
Regards,
- Arnaldo
commit 98bb09f90a0ae33125fabc8f41529345382f1498
Author: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Andrii Nakryiko <[email protected]>
Link: https://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
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;
On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo
<[email protected]> 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.
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
> <[email protected]> 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
On Wed, Nov 27, 2019 at 10:45 AM Arnaldo Carvalho de Melo
<[email protected]> 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
> > <[email protected]> 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?
On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Another fix I'm carrying in my perf/core branch,
>
> Regards,
>
> - Arnaldo
>
> commit 98bb09f90a0ae33125fabc8f41529345382f1498
> Author: Arnaldo Carvalho de Melo <[email protected]>
> 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 <[email protected]>
> Cc: Andrii Nakryiko <[email protected]>
> Link: https://lkml.kernel.org/n/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> 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) ?
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
> <[email protected]> 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
> > > <[email protected]> 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
On Tue, Nov 26, 2019 at 5:39 PM Arnaldo Carvalho de Melo
<[email protected]> 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.
On November 27, 2019 9:31:41 PM GMT-03:00, Alexei Starovoitov <[email protected]> wrote:
>On Tue, Nov 26, 2019 at 5:39 PM Arnaldo Carvalho de Melo
><[email protected]> 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
On Wed, Nov 27, 2019 at 4:50 PM Arnaldo Carvalho de Melo
<[email protected]> 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),
On November 27, 2019 9:59:15 PM GMT-03:00, Alexei Starovoitov <[email protected]> wrote:
>On Wed, Nov 27, 2019 at 4:50 PM Arnaldo Carvalho de Melo
><[email protected]> 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
On Wed, Nov 27, 2019 at 5:17 PM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> On November 27, 2019 9:59:15 PM GMT-03:00, Alexei Starovoitov <[email protected]> wrote:
> >On Wed, Nov 27, 2019 at 4:50 PM Arnaldo Carvalho de Melo
> ><[email protected]> 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.
On November 27, 2019 10:20:17 PM GMT-03:00, Alexei Starovoitov <[email protected]> wrote:
>On Wed, Nov 27, 2019 at 5:17 PM Arnaldo Carvalho de Melo
><[email protected]> wrote:
>>
>> On November 27, 2019 9:59:15 PM GMT-03:00, Alexei Starovoitov
><[email protected]> wrote:
>> >On Wed, Nov 27, 2019 at 4:50 PM Arnaldo Carvalho de Melo
>> ><[email protected]> 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
On Wed, Nov 27, 2019 at 5:26 PM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> On November 27, 2019 10:20:17 PM GMT-03:00, Alexei Starovoitov <[email protected]> wrote:
> >On Wed, Nov 27, 2019 at 5:17 PM Arnaldo Carvalho de Melo
> ><[email protected]> wrote:
> >>
> >> On November 27, 2019 9:59:15 PM GMT-03:00, Alexei Starovoitov
> ><[email protected]> wrote:
> >> >On Wed, Nov 27, 2019 at 4:50 PM Arnaldo Carvalho de Melo
> >> ><[email protected]> 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.
Hi Arnaldo,
FYI,
On Wed, 27 Nov 2019 at 19:15, Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Another fix I'm carrying in my perf/core branch,
>
> Regards,
>
> - Arnaldo
>
> commit 98bb09f90a0ae33125fabc8f41529345382f1498
> Author: Arnaldo Carvalho de Melo <[email protected]>
> 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
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
> <[email protected]> wrote:
> >
> > Another fix I'm carrying in my perf/core branch,
> >
> > Regards,
> >
> > - Arnaldo
> >
> > commit 98bb09f90a0ae33125fabc8f41529345382f1498
> > Author: Arnaldo Carvalho de Melo <[email protected]>
> > 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