Skip to content

Commit 384aa16

Browse files
0x7f454c46kuba-moo
authored andcommitted
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 <[email protected]> Fixes: 3c3ead5 ("selftests/net: Add TCP-AO key-management test") Signed-off-by: Dmitry Safonov <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent d8f5df1 commit 384aa16

File tree

1 file changed

+25
-19
lines changed

1 file changed

+25
-19
lines changed

tools/testing/selftests/net/tcp_ao/key-management.c

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -417,9 +417,9 @@ struct test_key {
417417
matches_vrf : 1,
418418
is_current : 1,
419419
is_rnext : 1,
420-
used_on_handshake : 1,
421-
used_after_accept : 1,
422-
used_on_client : 1;
420+
used_on_server_tx : 1,
421+
used_on_client_tx : 1,
422+
skip_counters_checks : 1;
423423
};
424424

425425
struct key_collection {
@@ -609,16 +609,14 @@ static int key_collection_socket(bool server, unsigned int port)
609609
addr = &this_ip_dest;
610610
sndid = key->client_keyid;
611611
rcvid = key->server_keyid;
612-
set_current = key->is_current;
613-
set_rnext = key->is_rnext;
612+
key->used_on_client_tx = set_current = key->is_current;
613+
key->used_on_server_tx = set_rnext = key->is_rnext;
614614
}
615615

616616
if (test_add_key_cr(sk, key->password, key->len,
617617
*addr, vrf, sndid, rcvid, key->maclen,
618618
key->alg, set_current, set_rnext))
619619
test_key_error("setsockopt(TCP_AO_ADD_KEY)", key);
620-
if (set_current || set_rnext)
621-
key->used_on_handshake = 1;
622620
#ifdef DEBUG
623621
test_print("%s [%u/%u] key: { %s, %u:%u, %u, %u:%u:%u:%u (%u)}",
624622
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
640638
for (i = 0; i < collection.nr_keys; i++) {
641639
struct test_key *key = &collection.keys[i];
642640
uint8_t sndid, rcvid;
643-
bool was_used;
641+
bool rx_cnt_expected;
644642

643+
if (key->skip_counters_checks)
644+
continue;
645645
if (server) {
646646
sndid = key->server_keyid;
647647
rcvid = key->client_keyid;
648-
if (is_listen_sk)
649-
was_used = key->used_on_handshake;
650-
else
651-
was_used = key->used_after_accept;
648+
rx_cnt_expected = key->used_on_client_tx;
652649
} else {
653650
sndid = key->client_keyid;
654651
rcvid = key->server_keyid;
655-
was_used = key->used_on_client;
652+
rx_cnt_expected = key->used_on_server_tx;
656653
}
657654

658-
test_tcp_ao_key_counters_cmp(tst_name, a, b, was_used,
655+
test_tcp_ao_key_counters_cmp(tst_name, a, b,
656+
rx_cnt_expected ? TEST_CNT_KEY_GOOD : 0,
659657
sndid, rcvid);
660658
}
661659
test_tcp_ao_counters_free(a);
@@ -916,9 +914,8 @@ static int run_client(const char *tst_name, unsigned int port,
916914
current_index = nr_keys - 1;
917915
if (rnext_index < 0)
918916
rnext_index = nr_keys - 1;
919-
collection.keys[current_index].used_on_handshake = 1;
920-
collection.keys[rnext_index].used_after_accept = 1;
921-
collection.keys[rnext_index].used_on_client = 1;
917+
collection.keys[current_index].used_on_client_tx = 1;
918+
collection.keys[rnext_index].used_on_server_tx = 1;
922919

923920
synchronize_threads(); /* 3: accepted => send data */
924921
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,
10591056
test_error("Can't change the current key");
10601057
if (test_client_verify(sk, msg_len, nr_packets, TEST_TIMEOUT_SEC))
10611058
test_fail("verify failed");
1062-
collection.keys[rotate_to_index].used_after_accept = 1;
1059+
/* There is a race here: between setting the current_key with
1060+
* setsockopt(TCP_AO_INFO) and starting to send some data - there
1061+
* might have been a segment received with the desired
1062+
* RNext_key set. In turn that would mean that the first outgoing
1063+
* segment will have the desired current_key (flipped back).
1064+
* Which is what the user/test wants. As it's racy, skip checking
1065+
* the counters, yet check what are the resulting current/rnext
1066+
* keys on both sides.
1067+
*/
1068+
collection.keys[rotate_to_index].skip_counters_checks = 1;
10631069

10641070
end_client(tst_name, sk, nr_keys, current_index, rnext_index, &tmp);
10651071
}
@@ -1089,7 +1095,7 @@ static void roll_over_keys(const char *tst_name, unsigned int port,
10891095
}
10901096
verify_current_rnext(tst_name, sk, -1,
10911097
collection.keys[i].server_keyid);
1092-
collection.keys[i].used_on_client = 1;
1098+
collection.keys[i].used_on_server_tx = 1;
10931099
synchronize_threads(); /* verify current/rnext */
10941100
}
10951101
end_client(tst_name, sk, nr_keys, current_index, rnext_index, &tmp);

0 commit comments

Comments
 (0)