bpf: avoid encrypt_key map lookup if IPsec is disabled #17840
Conversation
tklauser
added
sig/datapath
|
/test |
So far I haven't tried loading them, just incidentally noticed these errors while compile testing some other changes locally where I apparently have LLVM 14 installed. What would be the best way to load-test the programs? (Sorry, I'm not that experienced with datapath development yet). |
Building the BPF datapath with LLVM 14 leads to the following errors:
bpf_lxc.c:101:16: error: variable 'daddr' set but not used [-Werror,-Wunused-but-set-variable]
union v6addr *daddr, orig_dip;
^
bpf_lxc.c:103:7: error: variable 'encrypt_key' set but not used [-Werror,-Wunused-but-set-variable]
__u8 encrypt_key = 0;
^
bpf_lxc.c:102:8: error: variable 'tunnel_endpoint' set but not used [-Werror,-Wunused-but-set-variable]
__u32 tunnel_endpoint = 0;
^
bpf_lxc.c:526:7: error: variable 'encrypt_key' set but not used [-Werror,-Wunused-but-set-variable]
__u8 encrypt_key = 0;
^
bpf_lxc.c:525:8: error: variable 'tunnel_endpoint' set but not used [-Werror,-Wunused-but-set-variable]
__u32 tunnel_endpoint = 0;
^
These are normally warnings, but errors in this case due to the use of
-Werror when compiling Cilium's bpf programs. Fix these by marking the
affected variables as __maybe_unused.
Signed-off-by: Tobias Klauser <tobias@cilium.io>
In the bpf_lxc program's functions ipv6_l3_from_lxc and handle_ipv4_from_lxc, currently encrypt_key is always looked up in the encrypt map, regardless of whether IPsec is enabled or not. However, its value is only actually used when IPsec is enabled. Thus, the call can be avoid when IPsec is disabled. This also slightly reduces program size if !defined(ENABLE_IPSEC). Signed-off-by: Tobias Klauser <tobias@cilium.io>
tklauser
force-pushed
the
bpf-fix-llvm-14-warnings
branch
from
6ce7a2d
to
fb2510f
Compare
You don't need to. I was just curious because I'd expect loading with LLVM 14 to fail given it fails with LLVM 12+. |
|
/test Job 'Cilium-PR-K8s-1.21-kernel-4.19' failed and has not been observed before, so may be related to your PR: Click to show.Test NameFailure OutputIf it is a flake, comment |
pchaigno
added
the
area/encryption
|
test-1.21-4.19 previous run hit #12690: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.21-kernel-4.19/136/ |
|
Paul approved and CI is green, marking as ready to merge. |
tklauser
added
the
ready-to-merge
|
Let me request backporting this PR to 1.10 branch. The background is this issue (#18591). According to the user
We can say two things from here
I could reproduce the issue in my local (see the original issue), and when I backport this PR over That's why I think it's worth backporting this PR to |
Avoid an unnecessary map lookup for encrypt_key in the bpf_lxc program in case IPsec is disabled. Also fix warnings/errors when building with LLVM 14. See individual commit messages for details.