build: use upstream clang instead of forked version for BPF programs #146
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
willfindlay
force-pushed
the
pr/willfindlay/use-upstream-clang
branch
12 times, most recently
from
1ddc072
to
605901c
Compare
willfindlay
force-pushed
the
pr/willfindlay/use-upstream-clang
branch
from
cb3a78c
to
a32fd65
Compare
willfindlay
force-pushed
the
pr/willfindlay/use-upstream-clang
branch
from
d630ecb
to
3954fcb
Compare
willfindlay
force-pushed
the
pr/willfindlay/use-upstream-clang
branch
from
3954fcb
to
7ea8dd5
Compare
willfindlay
force-pushed
the
pr/willfindlay/use-upstream-clang
branch
from
7ea8dd5
to
52b8f81
Compare
We were defining a0, a1, a2, a3, and a4 in every tail call program, even though we really only needed one per program. Remove the extras to fix the unused variable error when compiling on latest clang. Signed-off-by: William Findlay <will@isovalent.com>
We have a nop binary now but it wasn't being ignored in VCS. Let's fix that so we don't check it in by accident. Signed-off-by: William Findlay <will@isovalent.com>
Use apt-get instead of apt to silence warnings. Remove CLANG_INSTALL_DIR from the tools-install target because it no longer exists. Signed-off-by: William Findlay <will@isovalent.com>
Introduce a new Docker image that provides a suitable clang version for compiling our BPF programs. Signed-off-by: William Findlay <will@isovalent.com>
willfindlay
force-pushed
the
pr/willfindlay/use-upstream-clang
branch
from
52b8f81
to
14a5870
Compare
|
Note, I cancelled the clang build job since I already ran this on a previous revision of this PR and the Dockerfile has not changed. So we can ignore the "failure" here. |
willfindlay
changed the title
|
I wouldn't block this for this, but why not clang14? |
jrfastab
approved these changes
Jun 14, 2022
|
@jrfastab honestly no reason, I'll bump it and see if anything breaks. |
|
Hitting over 1M instructions on execve programs under clang-14. I'll create an issue for now and we can ship this with clang-13. Edit: issue is here #164 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use an upstreamed version of clang (clang-13) instead of our forked version. We provide a Docker image for convenience, but any distribution's clang should work provided it is >= clang-13. See individual commits.
Fixes #142