Closed anakryiko closed 1 year ago
@qmonnet it seems like something about building bpftool from submodule is broken, it expects LLVM-14 which is not present? Any idea what's going on? Is the fix needed on bpftool side or retsnoop side? See https://github.com/anakryiko/retsnoop/actions/runs/6368016638/job/17287065875
Hi Andrii, I don't know where the issue comes from.
It looks like this is coming from bpftool side, given that the feature probe for LLVM passes but the build fails to find libLLVM-14.so
:
Makefile.feature:158: Probing: feature-llvm
Makefile.feature:159: printf '%b\n' '#include <llvm-c/Core.h>\n' '#include <llvm-c/TargetMachine.h>\n' 'int main(void) {' ' char *triple = LLVMNormalizeTargetTriple("");' ' LLVMDisposeMessage(triple);' ' return 0;' '}' | cc -I/usr/lib/llvm-14/include -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -L/usr/lib/llvm-14/lib -Wall -Werror -x c - -lLLVM-14 -o /dev/null >/dev/null && (echo 1 && >&2 echo result: 1) || (echo 0 && >&2 echo result: 0)
result: 1
... libbfd: [ OFF ]
... clang-bpf-co-re: [ on ]
... llvm: [ on ]
... libcap: [ OFF ]
[...]
gcc [...] -lLLVM-14 [...] -o /home/runner/work/retsnoop/retsnoop/retsnoop/src/.output/bpftool/bootstrap/bpftool
/usr/bin/ld: cannot find -lLLVM-14: No such file or directory
I can't reproduce locally, I've got consistent results between the probe and the build on my setup (the probe does fail if libLLVM-14.so
is missing). I'll try to reproduce on a GH runner to investigate.
Do you have any idea when the workflow broke? I note that you updated bpftool yesterday, but I can't find changes for bpftool's build system on this diff. There were some changes that you pulled in June, do you know if bpftool built successfully after that?
I can reproduce, this happens when I pass CFLAGS=--static
to bpftool. This is because bpftool expects the --static
flag in EXTRA_CFLAGS
, and not in CFLAGS
(which gets overwritten when compiling in the kernel repo). In particular, this commit makes it true for feature probes in the mirror repo, to align the behaviour with the kernel repo.
Bpftool's README.md tells to use EXTRA_CFLAGS
, but I had not considered the case when we inherit the flag from the CFLAGS
of a parent Makefile
. Maybe we could look into making bpftool's Makefile
more robust in that regard :thinking:.
In the meantime, the simple way to fix this is to change the Makefile
on retsnoop
side: we can pass retsnoop's CFLAGS
to bpftool via the EXTRA_CFLAGS
, something like this, which fixes the probe:
diff --git a/src/Makefile b/src/Makefile
index 8090f4236249..c6d2627afbea 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -96,7 +96,7 @@ $(LIBBPF_OBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(OUTPU
$(BPFTOOL): $(LIBBPF_OBJ) | $(BPFTOOL_OUTPUT)
$(call msg,BPFTOOL,$@)
$(Q) [ -d ../bpftool/src ] || git submodule update --init
- $(Q)$(MAKE) ARCH= CROSS_COMPILE= \
+ $(Q)CFLAGS= EXTRA_CFLAGS="$(CFLAGS)" $(MAKE) ARCH= CROSS_COMPILE= \
BPF_DIR=$(LIBBPF_SRC) OUTPUT=$(BPFTOOL_OUTPUT)/ \
-C $(BPFTOOL_SRC) bootstrap
I can open a PR with the above if you're happy with it?
hey @qmonnet, thanks for taking a look! I don't know when it broke, as this was the first time I used new action to create a release. I updated bpftool after I ran into problems in hope that latest version of bpftool might have a fix.
This fix for retsnoop's Makefile looks good, you can probably put those CFLAGS and EXTRA_CFLAGS after $(MAKE), though? Please do open a PR!
you can probably put those CFLAGS and EXTRA_CFLAGS after $(MAKE), though?
Nope, secret Makefile feature - it's not the same. Passing before passes through the environment and does not override the Makefile contents; passing after passes via the command line and overwrites the contents, making the assignments inside of the Makefile useless (more details), and the build fail.
Please do open a PR!
OK will do :)
Yeah, I knew about the difference for before or after make "envvars", but I just didn't think it matters so much here. Good to know, put that bit into the PR as well, for posterity. Thanks!
Sure, I had documented it in the commit log already. The PR is at #52
According to Github Actions, this should allow to trigger the workflow manually. Let's see.
Cc: Quentin Monnet quentin@isovalent.com