Commit 7c89562f authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'net-selftests-tcp-ao-selftests-updates'

Dmitry Safonov via says:

====================
net/selftests: TCP-AO selftests updates

First 3 patches are more-or-less cleanups/preparations.

Patches 4/5 are fixes for netns file descriptors leaks/open.

Patch 6 was sent to me/contributed off-list by Mohammad, who wants 32-bit
kernels to run TCP-AO.

Patch 7 is a workaround/fix for slow VMs. Albeit, I can't reproduce
the issue, but I hope it will fix netdev flakes for connect-deny-*
tests.

And the biggest change is adding TCP-AO tracepoints to selftests.
I think it's a good addition by the following reasons:
- The related tracepoints are now tested;
- It allows tcp-ao selftests to raise expectations on the kernel
  behavior - up from the syscalls exit statuses + net counters.
- Provides tracepoints usage samples.

As tracepoints are not a stable ABI, any kernel changes done to them
will be reflected to the selftests, which also will allow users
to see how to change their code. It's quite better than parsing dmesg
(what BGP was doing pre-tracepoints, ugh).

Somewhat arguably, the code parses trace_pipe, rather than uses
libtraceevent (which any sane user should do). The reason behind that is
the same as for rt-netlink macros instead of libmnl: I'm trying
to minimize the library dependencies of the selftests. And the
performance of formatting text in kernel and parsing it again in a test
is not critical.

Current output sample:
> ok 73 Trace events matched expectations: 13 tcp_hash_md5_required[2] tcp_hash_md5_unexpected[4] tcp_hash_ao_required[3] tcp_ao_key_not_found[4]

Previously, tracepoints selftests were part of kernel tcp tracepoints
submission [1], but since then the code was quite changed:
- Now generic tracing setup is in lib/ftrace.c, separate from
  lib/ftrace-tcp.c which utilizes TCP trace points. This separation
  allows future selftests to trace non-TCP events, i.e. to find out
  an skb's drop reason, which was useful in the creation of TCP-CLOSE
  stress-test (not in this patch set, but used in attempt to reproduce
  the issue from [2]).
- Another change is that in the previous submission the trace events
  where used only to detect unexpected TCP-AO/TCP-MD5 events. In this
  version the selftests will fail if an expected trace event didn't
  appear.
  Let's see how reliable this is on the netdev bot - it obviously passes
  on my testing, but potentially may require a temporary XFAIL patch
  if it misbehaves on a slow VM.

[1] https://lore.kernel.org/lkml/20240224-tcp-ao-tracepoints-v1-0-15f31b7f30a7@arista.com/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=33700a0c9b56

v3: https://lore.kernel.org/20240815-tcp-ao-selftests-upd-6-12-v3-0-7bd2e22bb81c@gmail.com
v2: https://lore.kernel.org/20240802-tcp-ao-selftests-upd-6-12-v2-0-370c99358161@gmail.com
v1: https://lore.kernel.org/20240730-tcp-ao-selftests-upd-6-12-v1-0-ffd4bf15d638@gmail.com
====================

Link: https://patch.msgid.link/20240823-tcp-ao-selftests-upd-6-12-v4-0-05623636fe8c@gmail.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 73d33bd0 586d8702
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -31,7 +31,8 @@ CFLAGS += $(KHDR_INCLUDES)
CFLAGS	+= -iquote ./lib/ -I ../../../../include/

# Library
LIBSRC	:= kconfig.c netlink.c proc.c repair.c setup.c sock.c utils.c
LIBSRC	:= ftrace.c ftrace-tcp.c kconfig.c netlink.c
LIBSRC	+= proc.c repair.c setup.c sock.c utils.c
LIBOBJ	:= $(LIBSRC:%.c=$(LIBDIR)/%.o)
EXTRA_CLEAN += $(LIBOBJ) $(LIB)

+1 −1
Original line number Diff line number Diff line
@@ -355,6 +355,6 @@ static void *client_fn(void *arg)

int main(int argc, char *argv[])
{
	test_init(30, server_fn, client_fn);
	test_init(31, server_fn, client_fn);
	return 0;
}
+1 −0
Original line number Diff line number Diff line
@@ -7,4 +7,5 @@ CONFIG_NET_L3_MASTER_DEV=y
CONFIG_NET_VRF=y
CONFIG_TCP_AO=y
CONFIG_TCP_MD5SIG=y
CONFIG_TRACEPOINTS=y
CONFIG_VETH=m
+21 −4
Original line number Diff line number Diff line
@@ -71,10 +71,12 @@ static void try_accept(const char *tst_name, unsigned int port, const char *pwd,
		}
	}

	synchronize_threads(); /* before counter checks */
	if (pwd && test_get_tcp_ao_counters(lsk, &ao_cnt2))
		test_error("test_get_tcp_ao_counters()");

	close(lsk);

	if (pwd)
		test_tcp_ao_counters_cmp(tst_name, &ao_cnt1, &ao_cnt2, cnt_expected);

@@ -84,10 +86,10 @@ static void try_accept(const char *tst_name, unsigned int port, const char *pwd,
	after_cnt = netstat_get_one(cnt_name, NULL);

	if (after_cnt <= before_cnt) {
		test_fail("%s: %s counter did not increase: %zu <= %zu",
		test_fail("%s: %s counter did not increase: %" PRIu64 " <= %" PRIu64,
				tst_name, cnt_name, after_cnt, before_cnt);
	} else {
		test_ok("%s: counter %s increased %zu => %zu",
		test_ok("%s: counter %s increased %" PRIu64  " => %" PRIu64,
			tst_name, cnt_name, before_cnt, after_cnt);
	}

@@ -180,6 +182,7 @@ static void try_connect(const char *tst_name, unsigned int port,
	timeout = fault(TIMEOUT) ? TEST_RETRANSMIT_SEC : TEST_TIMEOUT_SEC;
	ret = _test_connect_socket(sk, this_ip_dest, port, timeout);

	synchronize_threads(); /* before counter checks */
	if (ret < 0) {
		if (fault(KEYREJECT) && ret == -EKEYREJECTED) {
			test_ok("%s: connect() was prevented", tst_name);
@@ -212,30 +215,44 @@ static void try_connect(const char *tst_name, unsigned int port,

static void *client_fn(void *arg)
{
	union tcp_addr wrong_addr, network_addr;
	union tcp_addr wrong_addr, network_addr, addr_any = {};
	unsigned int port = test_server_port;

	if (inet_pton(TEST_FAMILY, TEST_WRONG_IP, &wrong_addr) != 1)
		test_error("Can't convert ip address %s", TEST_WRONG_IP);

	trace_ao_event_expect(TCP_AO_KEY_NOT_FOUND, this_ip_addr, this_ip_dest,
			      -1, port, 0, 0, 1, 0, 0, 0, 100, 100, -1);
	try_connect("Non-AO server + AO client", port++, DEFAULT_TEST_PASSWORD,
			this_ip_dest, -1, 100, 100, 0, FAULT_TIMEOUT);

	trace_hash_event_expect(TCP_HASH_AO_REQUIRED, this_ip_addr, this_ip_dest,
				-1, port, 0, 0, 1, 0, 0, 0);
	try_connect("AO server + Non-AO client", port++, NULL,
			this_ip_dest, -1, 100, 100, 0, FAULT_TIMEOUT);

	trace_ao_event_expect(TCP_AO_MISMATCH, this_ip_addr, this_ip_dest,
			      -1, port, 0, 0, 1, 0, 0, 0, 100, 100, -1);
	try_connect("Wrong password", port++, DEFAULT_TEST_PASSWORD,
			this_ip_dest, -1, 100, 100, 0, FAULT_TIMEOUT);

	trace_ao_event_expect(TCP_AO_KEY_NOT_FOUND, this_ip_addr, this_ip_dest,
			      -1, port, 0, 0, 1, 0, 0, 0, 100, 100, -1);
	try_connect("Wrong rcv id", port++, DEFAULT_TEST_PASSWORD,
			this_ip_dest, -1, 100, 100, 0, FAULT_TIMEOUT);

	trace_ao_event_sk_expect(TCP_AO_SYNACK_NO_KEY, this_ip_dest, addr_any,
				 port, 0, 100, 100);
	try_connect("Wrong snd id", port++, DEFAULT_TEST_PASSWORD,
			this_ip_dest, -1, 100, 100, 0, FAULT_TIMEOUT);

	trace_ao_event_expect(TCP_AO_WRONG_MACLEN, this_ip_addr, this_ip_dest,
			      -1, port, 0, 0, 1, 0, 0, 0, 100, 100, -1);
	try_connect("Different maclen", port++, DEFAULT_TEST_PASSWORD,
			this_ip_dest, -1, 100, 100, 0, FAULT_TIMEOUT);

	trace_ao_event_expect(TCP_AO_KEY_NOT_FOUND, this_ip_addr, this_ip_dest,
			      -1, port, 0, 0, 1, 0, 0, 0, 100, 100, -1);
	try_connect("Server: Wrong addr", port++, DEFAULT_TEST_PASSWORD,
			this_ip_dest, -1, 100, 100, 0, FAULT_TIMEOUT);

@@ -259,6 +276,6 @@ static void *client_fn(void *arg)

int main(int argc, char *argv[])
{
	test_init(21, server_fn, client_fn);
	test_init(22, server_fn, client_fn);
	return 0;
}
+3 −3
Original line number Diff line number Diff line
@@ -67,14 +67,14 @@ static void *client_fn(void *arg)
	netstat_free(ns_after);

	if (nr_packets > (after_aogood - before_aogood)) {
		test_fail("TCPAOGood counter mismatch: %zu > (%zu - %zu)",
		test_fail("TCPAOGood counter mismatch: %zu > (%" PRIu64 " - %" PRIu64 ")",
				nr_packets, after_aogood, before_aogood);
		return NULL;
	}
	if (test_tcp_ao_counters_cmp("connect", &ao1, &ao2, TEST_CNT_GOOD))
		return NULL;

	test_ok("connect TCPAOGood %" PRIu64 "/%" PRIu64 "/%" PRIu64 " => %" PRIu64 "/%" PRIu64 "/%" PRIu64 ", sent %" PRIu64,
	test_ok("connect TCPAOGood %" PRIu64 "/%" PRIu64 "/%" PRIu64 " => %" PRIu64 "/%" PRIu64 "/%" PRIu64 ", sent %zu",
			before_aogood, ao1.ao_info_pkt_good,
			ao1.key_cnts[0].pkt_good,
			after_aogood, ao2.ao_info_pkt_good,
@@ -85,6 +85,6 @@ static void *client_fn(void *arg)

int main(int argc, char *argv[])
{
	test_init(1, server_fn, client_fn);
	test_init(2, server_fn, client_fn);
	return 0;
}
Loading