cilium, bpf: select v3 BPF CPU in llc for 5.7+ kernels #10804
Closed
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
Given this kernel has full 32bit subregister tracking, enable llc to emit instructions for those which also helps us to reduce complexity on the verifier. See also complexity comparison in recent commit 4baaa2c ("docker, runtime: upgrade to recent clang/llvm image in runtime"). From CI side, we are good as well here since test-me-please runs for: - 4.9 kernel compiles with v1 CPU - 4.19 kernel compiles with v2 CPU - bpf-next kernel compiles with v3 CPU Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
borkmann
added
pending-review
sig/datapath
|
test-me-please |
brb
approved these changes
Apr 1, 2020
|
test-me-please |
|
Unrelated but rather odd one from go build: |
pchaigno
approved these changes
Apr 1, 2020
Could this be related to the operator split (#9920) that @errordeveloper has been working on? Specifically #10689? |
|
test-me-please |
1 similar comment
|
test-me-please |
|
Build finished. |
qmonnet
approved these changes
Apr 6, 2020
Does that issue still persist? |
Didn't see it happen in the last few days. Looks like it was fixed in the meantime. |
|
restart-ginkgo |
|
test-with-kernel |
|
test-gke |
|
test-me-please jenkins history was gone and ginkgo-tests have failed. |
joestringer
reviewed
Apr 20, 2020
There was a problem hiding this comment.
LGTM, one question about whether we'll misdetect the presence of the subregister backtracking changes.
We'll also need to keep this in mind as we think about slimming down the images: #9411
| // but the jmp/alu32 handling is not suited for pre 5.7 due to | ||
| // lack of 32 bit subreg tracking. | ||
| if h := probes.NewProbeManager().GetHelpers("cgroup_sock_addr"); h != nil { | ||
| if _, ok := h["bpf_get_netns_cookie"]; ok { |
There was a problem hiding this comment.
Is there any chance that the subregister backtracking won't be backported but the netns_cookie changes will be? I think the netns_cookie helper is quite powerful with a relatively minimal set of changes.
|
(pr currently on hold given v3 complexity) |
|
this PR has been marked as a draft PR since it had a WIP label. Please click in "Ready for review" [below vvv ] once the PR is ready to be reviewed. CI will still run for draft PRs. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
stale
bot
added
the
stale
|
Closing for now. We'll revisit mcpu=v3 at a later point in time. |
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.
Given this kernel has full 32bit subregister tracking, enable llc to
emit instructions for those which also helps us to reduce complexity
on the verifier.
See also complexity comparison in recent commit 4baaa2c ("docker,
runtime: upgrade to recent clang/llvm image in runtime").
From CI side, we are good as well here since test-me-please runs for:
Signed-off-by: Daniel Borkmann daniel@iogearbox.net