bpf: xdp asm volatile fix in relation to reg spill #11152
Merged
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
borkmann
added
pending-review
release-note/misc
borkmann
force-pushed
the
pr/xdp-follow-ups-4
branch
from
6c9e3d4
to
726ee32
Compare
jrfastab
approved these changes
Apr 25, 2020
joestringer
approved these changes
Apr 25, 2020
|
test-focus K8sService.* |
|
restart-ginkgo |
|
restart-gke |
pchaigno
approved these changes
Apr 25, 2020
qmonnet
approved these changes
Apr 27, 2020
|
test-focus K8sService.* |
|
test-gke |
aanm
requested changes
Apr 27, 2020
XDP accelerated NodePort and routing to remote backends via vxlan or geneve is not supported yet at this point, therefore bail out. Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
There was one missing struct dsr_opt_v6 we didn't mark. Did not cause an issue, but it's a candidate similar to the other dsr_opt_v6 we've marked. Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
I've recently seen an odd verifier error similar to this:
[...]
8276: (61) r1 = *(u32 *)(r6 +0)
8277: (61) r2 = *(u32 *)(r6 +4)
8278: (25) if r5 > 0xff goto pc+6
R0=inv(id=0) R1=pkt(id=0,off=0,r=0,imm=0) R2=pkt_end(id=0,off=0,imm=0) R4=inv(id=0,umax_value=65535,var_off=(0x0; 0xffff)) R5=inv(id=0,umax_value=255,var_off=(0x0; 0xff)) R6=ctx(id=0,off=0,imm=0) R7=inv16 R8=map_value(id=0,off=0,ks=38,vs=56,imm=0) R9=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R10=fp0 fp-168=??mmmmmm fp-176=mmmmmmmm fp-184=mmmmmmmm fp-192=mmmmmmmm fp-200=mmmmmmmm fp-208=00000000 fp-216=00000000 fp-224=00000000 fp-232=00000000 fp-240=00000000 fp-248=00000000 fp-256=00000000 fp-264=00000000 fp-272=mmmmmmmm fp-280=mmmmmmmm fp-288=00000000
8279: (0f) r1 += r5
from 8278 to 8285: R0=inv(id=0) R1=pkt(id=0,off=0,r=0,imm=0) R2=pkt_end(id=0,off=0,imm=0) R4=inv(id=0,umax_value=65535,var_off=(0x0; 0xffff)) R5=inv(id=0,smin_value=-2147483648,smax_value=2147483647,umin_value=256) R6=ctx(id=0,off=0,imm=0) R7=inv16 R8=map_value(id=0,off=0,ks=38,vs=56,imm=0) R9=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R10=fp0 fp-168=??mmmmmm fp-176=mmmmmmmm fp-184=mmmmmmmm fp-192=mmmmmmmm fp-200=mmmmmmmm fp-208=00000000 fp-216=00000000 fp-224=00000000 fp-232=00000000 fp-240=00000000 fp-248=00000000 fp-256=00000000 fp-264=00000000 fp-272=mmmmmmmm fp-280=mmmmmmmm fp-288=00000000
8285: (b7) r5 = -22
8286: (7b) *(u64 *)(r10 -296) = r3
R3 !read_ok
[...]
This was related to asm volatile interactions in XDP code. Looking at
the LLVM objdump from a prog with ~8900 BPF insns, I've noticed two
occasions where there is something weird in the LLVM codegen:
000000000000fef0 LBB5_796:
8158: 7b 2a e0 fe 00 00 00 00 *(u64 *)(r10 - 288) = r2
8159: 79 a5 f0 fe 00 00 00 00 r5 = *(u64 *)(r10 - 272)
8160: bf 71 00 00 00 00 00 00 r1 = r7
8161: 0f 15 00 00 00 00 00 00 r5 += r1
8162: 67 05 00 00 20 00 00 00 r5 <<= 32
8163: c7 05 00 00 20 00 00 00 r5 s>>= 32
8164: 61 61 00 00 00 00 00 00 r1 = *(u32 *)(r6 + 0)
8165: 61 62 04 00 00 00 00 00 r2 = *(u32 *)(r6 + 4)
8166: 25 05 06 00 ff 00 00 00 if r5 > 255 goto +6 <LBB5_796+0x78> <---
8167: 0f 51 00 00 00 00 00 00 r1 += r5
8168: bf 13 00 00 00 00 00 00 r3 = r1 <---
8169: 07 01 00 00 02 00 00 00 r1 += 2
8170: 2d 21 02 00 00 00 00 00 if r1 > r2 goto +2 <LBB5_796+0x78>
8171: b7 05 00 00 00 00 00 00 r5 = 0
8172: 05 00 01 00 00 00 00 00 goto +1 <LBB5_796+0x80>
8173: b7 05 00 00 ea ff ff ff r5 = -22
8174: 7b 3a d8 fe 00 00 00 00 *(u64 *)(r10 - 296) = r3 <---
8175: 67 05 00 00 20 00 00 00 r5 <<= 32
8176: bf 51 00 00 00 00 00 00 r1 = r5
8177: 77 01 00 00 20 00 00 00 r1 >>= 32
8178: 55 01 ce 00 00 00 00 00 if r1 != 0 goto +206 <LBB5_811>
8179: 79 a1 e8 fe 00 00 00 00 r1 = *(u64 *)(r10 - 280)
8180: 79 a2 d8 fe 00 00 00 00 r2 = *(u64 *)(r10 - 296) <---
8181: 79 a3 e0 fe 00 00 00 00 r3 = *(u64 *)(r10 - 288)
8182: 15 01 02 00 00 00 00 00 if r1 == 0 goto +2 <LBB5_799>
8183: 69 21 00 00 00 00 00 00 r1 = *(u16 *)(r2 + 0) <---
8184: 15 01 cd 00 00 00 00 00 if r1 == 0 goto +205 <LBB5_812>
Basically, LLVM spills r3 with ctx->data* to stack, however, from the
path `if r5 > 255` it does not yet contain the ctx->data from r1. The
`r3 = r1` happens in the fall-through path.
Verifier is more strict on us here since it requires a register to be
initialized, and r3 from prior paths was uninitialized e.g. due to
helper call. While logically the code is correct, for BPF we need to
initialize the reg first in such case.
Simply setting r3 to 0 did not help, presumably as it tries to reuse
the spilled register elsewhere again (though w/o throwing an error or
such).
A different workaround we can do here is to just mask the offset with
the max offset we ever allow (e.g. we don't go beyond 0xff anyway).
This keeps the spilled reg valid as well and resolves the issue.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
borkmann
force-pushed
the
pr/xdp-follow-ups-4
branch
from
726ee32
to
9603277
Compare
|
test-me-please |
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.
See commit msg.