Thread (30 messages) 30 messages, 7 authors, 2019-12-03

Re: [PATCH] libbpf: Fix up generation of bpf_helper_defs.h

From: Andrii Nakryiko <hidden>
Date: 2019-11-26 22:38:43
Also in: bpf, linux-perf-users, lkml

On Tue, Nov 26, 2019 at 2:17 PM Arnaldo Carvalho de Melo
[off-list ref] wrote:
Em Tue, Nov 26, 2019 at 07:10:18PM -0300, Arnaldo Carvalho de Melo escreveu:
quoted
Em Tue, Nov 26, 2019 at 02:05:41PM -0800, Andrii Nakryiko escreveu:
quoted
On Tue, Nov 26, 2019 at 11:12 AM Arnaldo Carvalho de Melo
[off-list ref] wrote:
quoted
Em Tue, Nov 26, 2019 at 07:50:44PM +0100, Toke Høiland-Jørgensen escreveu:
quoted
Arnaldo Carvalho de Melo [off-list ref] writes:
quoted
Em Tue, Nov 26, 2019 at 05:38:18PM +0100, Toke Høiland-Jørgensen escreveu:
quoted
Arnaldo Carvalho de Melo [off-list ref] writes:
quoted
Em Tue, Nov 26, 2019 at 12:10:45PM -0300, Arnaldo Carvalho de Melo escreveu:
quoted
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?
quoted
quoted
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?
quoted
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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help