bpf: Test DEBUG configurations, fix a newly exposed verifier error #32170
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
|
/test |
gentoo-root
force-pushed
the
pr/max/complexity-with-debug
branch
from
3cdb91c
to
9097f8a
Compare
|
/test |
gentoo-root
force-pushed
the
pr/max/complexity-with-debug
branch
2 times, most recently
from
3638663
to
f5e8005
Compare
|
/test |
When DEBUG is enabled, Clang 17 optimizes the final part of assign_socket_tcp and assign_socket_udp by keeping only one instance of the code and jumping to it from two places. It means that the same struct bpf_sock *sk pointer can point to the return value of either skc_lookup_tcp or sk_lookup_udp. Unfortunately, for the verifier these pointer types are different (PTR_TO_SOCK_COMMON and PTR_TO_SOCKET), and it doesn't allow such pattern with the following error: ; sk = sk_lookup_udp(ctx, tuple, len, BPF_F_CURRENT_NETNS, 0); 829: (bf) r1 = r6 830: (b4) w3 = 36 831: (b7) r4 = -1 832: (b7) r5 = 0 833: (85) call bpf_sk_lookup_udp#85 last_idx 833 first_idx 827 regs=8 stack=0 before 832: (b7) r5 = 0 regs=8 stack=0 before 831: (b7) r4 = -1 regs=8 stack=0 before 830: (b4) w3 = 36 834: (b4) w7 = -178 ; if (!sk) 835: (15) if r0 == 0x0 goto pc+38 R0_w=sock(id=0,ref_obj_id=29,off=0,imm=0) R6=ctx(id=0,off=0,imm=0) R7_w=inv4294967118 R8=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R9=inv(id=22,umax_value=65535,var_off=(0x0; 0xffff)) R10=fp0 fp-16=???mmmmm fp-24=mmmmmmmm fp-32=mmmmmmmm fp-40=mmmmmmmm fp-48=mmmmmmmm fp-72=????mmmm fp-80=mmmmmmmm fp-88=mmmmmmmm refs=29 ; 836: (61) r1 = *(u32 *)(r6 +16) 837: (63) *(u32 *)(r10 -104) = r1 838: (61) r7 = *(u32 *)(r0 +4) same insn cannot be used with different pointers processed 1261 insns (limit 1000000) max_states_per_insn 4 total_states 68 peak_states 67 mark_read 19 Work around it by adding READ_ONCE to the problematic dereference. Although it doesn't guarantee that the same problem won't happen, it helps in this case. Ref: #32169 Fixes: 673d481 ("images: Update LLVM to 17.0.6") Reported-by: Marco Hofstetter <marco.hofstetter@isovalent.com> Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com> Co-developed-by: Marco Hofstetter <marco.hofstetter@isovalent.com> Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
Run the verifier tests on the debug configurations, because it may expose verifier errors. Ref: #32169 Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
gentoo-root
force-pushed
the
pr/max/complexity-with-debug
branch
from
f5e8005
to
b4bd642
Compare
|
/test |
gentoo-root
changed the title
mhofstetter
approved these changes
May 6, 2024
There was a problem hiding this comment.
LGTM ✔️ Thanks a lot @gentoo-root!
I verified my (previously) failing setup locally - it works with this PR! 🎉
markpash
approved these changes
May 7, 2024
maintainer-s-little-helper
bot
added
the
ready-to-merge
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.
Run the verifier tests on the debug configurations, because it may expose verifier errors.
Fix one error exposed with the new test that happens after the upgrade to LLVM 17.
Ref: #32169