diff mbox series

[net,2/2] wireguard: selftests: add metadata_dst xmit selftest

Message ID 20220414104458.3097244-3-razor@blackwall.org (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series wireguard: device: fix metadata_dst xmit null pointer dereference | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net, async
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 13 maintainers not CCed: songliubraving@fb.com andrii@kernel.org shuah@kernel.org nathan@kernel.org kafai@fb.com ndesaulniers@google.com linux-kselftest@vger.kernel.org llvm@lists.linux.dev yhs@fb.com bpf@vger.kernel.org john.fastabend@gmail.com kpsingh@kernel.org ast@kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: line length of 128 exceeds 80 columns WARNING: line length of 81 exceeds 80 columns WARNING: line length of 90 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Nikolay Aleksandrov April 14, 2022, 10:44 a.m. UTC
Add a selftest for transmitting skb with md_dst attached. It is done via
a bpf program which uses bpf_skb_set_tunnel_key on wireguard's egress
path. It requires clang and tc to be installed. If the test finishes
without a crash it is considered successful.

CC: wireguard@lists.zx2c4.com
CC: Jason A. Donenfeld <Jason@zx2c4.com>
CC: Daniel Borkmann <daniel@iogearbox.net>
CC: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
Executed the prep compilation commands with n1 to make them visible.

 tools/testing/selftests/wireguard/netns.sh | 63 ++++++++++++++++++++++
 1 file changed, 63 insertions(+)

Comments

Jason A. Donenfeld April 14, 2022, 12:06 p.m. UTC | #1
Hi Nikolay,

These tests need to run in the minimal fast-to-compile test harness
inside of tools/testing/selftests/wireguard/qemu, which you can try
out with:

$ make -C tools/testing/selftests/wireguard/qemu -j$(nproc)

Currently iproute2 is built, but only ip(8) is used in the image, so
you'll need to add tc(8) to there. Clang, however, seems a bit
heavyweight. I suspect it'd make more sense to just base64 up that
object file and include it as a string in the file? Or, alternatively,
we could just not move ahead with this rather niche test, and revisit
the issue if we wind up wanting to test for lots of bpf things.
Thoughts on that?

Jason
Nikolay Aleksandrov April 14, 2022, 12:12 p.m. UTC | #2
On 14/04/2022 15:06, Jason A. Donenfeld wrote:
> Hi Nikolay,
> 
> These tests need to run in the minimal fast-to-compile test harness
> inside of tools/testing/selftests/wireguard/qemu, which you can try
> out with:
> 
> $ make -C tools/testing/selftests/wireguard/qemu -j$(nproc)
> 
> Currently iproute2 is built, but only ip(8) is used in the image, so
> you'll need to add tc(8) to there. Clang, however, seems a bit
> heavyweight. I suspect it'd make more sense to just base64 up that
> object file and include it as a string in the file? Or, alternatively,
> we could just not move ahead with this rather niche test, and revisit
> the issue if we wind up wanting to test for lots of bpf things.
> Thoughts on that?
> 
> Jason

Hi Jason,
My bad, I completely missed the qemu part. I'll look into including the
ready object file. If it works out, looks compact and well
I'll post v2.

Thanks,
 Nik
Jason A. Donenfeld April 14, 2022, 12:24 p.m. UTC | #3
On Thu, Apr 14, 2022 at 2:12 PM Nikolay Aleksandrov <razor@blackwall.org> wrote:
> My bad, I completely missed the qemu part.

If you're curious, by the way, there's this running all those tests
for every commit https://www.wireguard.com/build-status/

Jason
diff mbox series

Patch

diff --git a/tools/testing/selftests/wireguard/netns.sh b/tools/testing/selftests/wireguard/netns.sh
index 8a9461aa0878..b492dbb94245 100755
--- a/tools/testing/selftests/wireguard/netns.sh
+++ b/tools/testing/selftests/wireguard/netns.sh
@@ -156,6 +156,67 @@  tests() {
 	done
 }
 
+md_dst_test_cleanup() {
+	rm -rf /tmp/test_wg_tun.c /tmp/test_wg_tun.ll /tmp/test_wg_tun.o
+	n1 tc qdisc del dev wg0 clsact
+}
+
+# test for md dst on wireguard's egress path
+md_dst_test() {
+	# clang is required for the test
+	if [[ ! -x "$(command -v "clang")" ]]; then
+		return
+	fi
+
+	# attach md dst to the skb on egress using bpf_skb_set_tunnel_key
+	n1 cat > /tmp/test_wg_tun.c <<EOF
+#include <linux/bpf.h>
+
+#ifndef TC_ACT_OK
+# define TC_ACT_OK 0
+#endif
+
+static long (*bpf_skb_set_tunnel_key)(struct __sk_buff *skb, struct bpf_tunnel_key *key, __u32 size, __u64 flags) = (void *) 21;
+
+__attribute__((section("egress"), used))
+int tc_egress(struct __sk_buff *skb)
+{
+	struct bpf_tunnel_key key = {};
+
+        bpf_skb_set_tunnel_key(skb, &key, sizeof(key), 0);
+
+	return TC_ACT_OK;
+}
+
+char __license[] __attribute__((section("license"), used)) = "GPL";
+EOF
+
+	n1 clang -O2 -emit-llvm -c /tmp/test_wg_tun.c -o /tmp/test_wg_tun.ll
+	if [[ ! -f "/tmp/test_wg_tun.ll" ]]; then
+		md_dst_test_cleanup
+		return
+	fi
+	n1 llc -march=bpf -filetype=obj -o /tmp/test_wg_tun.o /tmp/test_wg_tun.ll
+	if [[ ! -f "/tmp/test_wg_tun.o" ]]; then
+		md_dst_test_cleanup
+		return
+	fi
+
+	n1 tc qdisc add dev wg0 clsact
+	if [[ $? -ne 0 ]]; then
+		md_dst_test_cleanup
+		return
+	fi
+	n1 tc filter add dev wg0 egress basic action bpf obj /tmp/test_wg_tun.o sec egress
+	if [[ $? -ne 0 ]]; then
+		md_dst_test_cleanup
+		return
+	fi
+	n1 ping -c 2 -f -W 1 192.168.241.2
+	# if we reach here without a crash the test passed
+	md_dst_test_cleanup
+}
+
 [[ $(ip1 link show dev wg0) =~ mtu\ ([0-9]+) ]] && orig_mtu="${BASH_REMATCH[1]}"
 big_mtu=$(( 34816 - 1500 + $orig_mtu ))
 
@@ -175,6 +236,8 @@  read _ rx_bytes tx_bytes < <(n1 wg show wg0 transfer)
 read _ timestamp < <(n1 wg show wg0 latest-handshakes)
 (( timestamp != 0 ))
 
+md_dst_test
+
 tests
 ip1 link set wg0 mtu $big_mtu
 ip2 link set wg0 mtu $big_mtu