Commit 384aa16d authored by Dmitry Safonov's avatar Dmitry Safonov Committed by Jakub Kicinski

selftests/net: Rectify key counters checks

As the names of (struct test_key) members didn't reflect whether the key
was used for TX or RX, the verification for the counters was done
incorrectly for asymmetrical selftests.

Rename these with _tx appendix and fix checks in verify_counters().
While at it, as the checks are now correct, introduce skip_counters_checks,
which is intended for tests where it's expected that a key that was set
with setsockopt(sk, IPPROTO_TCP, TCP_AO_INFO, ...) might had no chance
of getting used on the wire.

Fixes the following failures, exposed by the previous commit:
> not ok 51 server: Check current != rnext keys set before connect(): Counter pkt_good was expected to increase 0 => 0 for key 132:5
> not ok 52 server: Check current != rnext keys set before connect(): Counter pkt_good was not expected to increase 0 => 21 for key 137:10
>
> not ok 63 server: Check current flapping back on peer's RnextKey request: Counter pkt_good was expected to increase 0 => 0 for key 132:5
> not ok 64 server: Check current flapping back on peer's RnextKey request: Counter pkt_good was not expected to increase 0 => 40 for key 137:10

Cc: Mohammad Nassiri <mnassiri@ciena.com>
Fixes: 3c3ead55 ("selftests/net: Add TCP-AO key-management test")
Signed-off-by: default avatarDmitry Safonov <dima@arista.com>
Link: https://lore.kernel.org/r/20240130-tcp-ao-test-key-mgmt-v2-2-d190430a6c60@arista.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent d8f5df1f
...@@ -417,9 +417,9 @@ struct test_key { ...@@ -417,9 +417,9 @@ struct test_key {
matches_vrf : 1, matches_vrf : 1,
is_current : 1, is_current : 1,
is_rnext : 1, is_rnext : 1,
used_on_handshake : 1, used_on_server_tx : 1,
used_after_accept : 1, used_on_client_tx : 1,
used_on_client : 1; skip_counters_checks : 1;
}; };
struct key_collection { struct key_collection {
...@@ -609,16 +609,14 @@ static int key_collection_socket(bool server, unsigned int port) ...@@ -609,16 +609,14 @@ static int key_collection_socket(bool server, unsigned int port)
addr = &this_ip_dest; addr = &this_ip_dest;
sndid = key->client_keyid; sndid = key->client_keyid;
rcvid = key->server_keyid; rcvid = key->server_keyid;
set_current = key->is_current; key->used_on_client_tx = set_current = key->is_current;
set_rnext = key->is_rnext; key->used_on_server_tx = set_rnext = key->is_rnext;
} }
if (test_add_key_cr(sk, key->password, key->len, if (test_add_key_cr(sk, key->password, key->len,
*addr, vrf, sndid, rcvid, key->maclen, *addr, vrf, sndid, rcvid, key->maclen,
key->alg, set_current, set_rnext)) key->alg, set_current, set_rnext))
test_key_error("setsockopt(TCP_AO_ADD_KEY)", key); test_key_error("setsockopt(TCP_AO_ADD_KEY)", key);
if (set_current || set_rnext)
key->used_on_handshake = 1;
#ifdef DEBUG #ifdef DEBUG
test_print("%s [%u/%u] key: { %s, %u:%u, %u, %u:%u:%u:%u (%u)}", test_print("%s [%u/%u] key: { %s, %u:%u, %u, %u:%u:%u:%u (%u)}",
server ? "server" : "client", i, collection.nr_keys, server ? "server" : "client", i, collection.nr_keys,
...@@ -640,22 +638,22 @@ static void verify_counters(const char *tst_name, bool is_listen_sk, bool server ...@@ -640,22 +638,22 @@ static void verify_counters(const char *tst_name, bool is_listen_sk, bool server
for (i = 0; i < collection.nr_keys; i++) { for (i = 0; i < collection.nr_keys; i++) {
struct test_key *key = &collection.keys[i]; struct test_key *key = &collection.keys[i];
uint8_t sndid, rcvid; uint8_t sndid, rcvid;
bool was_used; bool rx_cnt_expected;
if (key->skip_counters_checks)
continue;
if (server) { if (server) {
sndid = key->server_keyid; sndid = key->server_keyid;
rcvid = key->client_keyid; rcvid = key->client_keyid;
if (is_listen_sk) rx_cnt_expected = key->used_on_client_tx;
was_used = key->used_on_handshake;
else
was_used = key->used_after_accept;
} else { } else {
sndid = key->client_keyid; sndid = key->client_keyid;
rcvid = key->server_keyid; rcvid = key->server_keyid;
was_used = key->used_on_client; rx_cnt_expected = key->used_on_server_tx;
} }
test_tcp_ao_key_counters_cmp(tst_name, a, b, was_used, test_tcp_ao_key_counters_cmp(tst_name, a, b,
rx_cnt_expected ? TEST_CNT_KEY_GOOD : 0,
sndid, rcvid); sndid, rcvid);
} }
test_tcp_ao_counters_free(a); test_tcp_ao_counters_free(a);
...@@ -916,9 +914,8 @@ static int run_client(const char *tst_name, unsigned int port, ...@@ -916,9 +914,8 @@ static int run_client(const char *tst_name, unsigned int port,
current_index = nr_keys - 1; current_index = nr_keys - 1;
if (rnext_index < 0) if (rnext_index < 0)
rnext_index = nr_keys - 1; rnext_index = nr_keys - 1;
collection.keys[current_index].used_on_handshake = 1; collection.keys[current_index].used_on_client_tx = 1;
collection.keys[rnext_index].used_after_accept = 1; collection.keys[rnext_index].used_on_server_tx = 1;
collection.keys[rnext_index].used_on_client = 1;
synchronize_threads(); /* 3: accepted => send data */ synchronize_threads(); /* 3: accepted => send data */
if (test_client_verify(sk, msg_sz, msg_nr, TEST_TIMEOUT_SEC)) { if (test_client_verify(sk, msg_sz, msg_nr, TEST_TIMEOUT_SEC)) {
...@@ -1059,7 +1056,16 @@ static void check_current_back(const char *tst_name, unsigned int port, ...@@ -1059,7 +1056,16 @@ static void check_current_back(const char *tst_name, unsigned int port,
test_error("Can't change the current key"); test_error("Can't change the current key");
if (test_client_verify(sk, msg_len, nr_packets, TEST_TIMEOUT_SEC)) if (test_client_verify(sk, msg_len, nr_packets, TEST_TIMEOUT_SEC))
test_fail("verify failed"); test_fail("verify failed");
collection.keys[rotate_to_index].used_after_accept = 1; /* There is a race here: between setting the current_key with
* setsockopt(TCP_AO_INFO) and starting to send some data - there
* might have been a segment received with the desired
* RNext_key set. In turn that would mean that the first outgoing
* segment will have the desired current_key (flipped back).
* Which is what the user/test wants. As it's racy, skip checking
* the counters, yet check what are the resulting current/rnext
* keys on both sides.
*/
collection.keys[rotate_to_index].skip_counters_checks = 1;
end_client(tst_name, sk, nr_keys, current_index, rnext_index, &tmp); end_client(tst_name, sk, nr_keys, current_index, rnext_index, &tmp);
} }
...@@ -1089,7 +1095,7 @@ static void roll_over_keys(const char *tst_name, unsigned int port, ...@@ -1089,7 +1095,7 @@ static void roll_over_keys(const char *tst_name, unsigned int port,
} }
verify_current_rnext(tst_name, sk, -1, verify_current_rnext(tst_name, sk, -1,
collection.keys[i].server_keyid); collection.keys[i].server_keyid);
collection.keys[i].used_on_client = 1; collection.keys[i].used_on_server_tx = 1;
synchronize_threads(); /* verify current/rnext */ synchronize_threads(); /* verify current/rnext */
} }
end_client(tst_name, sk, nr_keys, current_index, rnext_index, &tmp); end_client(tst_name, sk, nr_keys, current_index, rnext_index, &tmp);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment