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: 3c3ead555648 ("selftests/net: Add TCP-AO key-management test")
Signed-off-by: Dmitry Safonov <dima@arista.com>
Link: https://lore.kernel.org/r/20240130-tcp-ao-test-key-mgmt-v2-2-d190430a6c60@arista.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/tools/testing/selftests/net/tcp_ao/key-management.c b/tools/testing/selftests/net/tcp_ao/key-management.c
index f6a9395e3cd7dc3e00cb15ae00135c032d160db1..24e62120b7924d3a1555a7e42097f9e55338b4db 100644 (file)
--- a/tools/testing/selftests/net/tcp_ao/key-management.c
+++ b/tools/testing/selftests/net/tcp_ao/key-management.c
@@ -417,9 +417,9 @@ struct test_key {
                matches_vrf             : 1,
                is_current              : 1,
                is_rnext                : 1,
-               used_on_handshake       : 1,
-               used_after_accept       : 1,
-               used_on_client          : 1;
+               used_on_server_tx       : 1,
+               used_on_client_tx       : 1,
+               skip_counters_checks    : 1;
 };
 
 struct key_collection {
@@ -609,16 +609,14 @@ static int key_collection_socket(bool server, unsigned int port)
                                addr = &this_ip_dest;
                        sndid = key->client_keyid;
                        rcvid = key->server_keyid;
-                       set_current = key->is_current;
-                       set_rnext = key->is_rnext;
+                       key->used_on_client_tx = set_current = key->is_current;
+                       key->used_on_server_tx = set_rnext = key->is_rnext;
                }
 
                if (test_add_key_cr(sk, key->password, key->len,
                                    *addr, vrf, sndid, rcvid, key->maclen,
                                    key->alg, set_current, set_rnext))
                        test_key_error("setsockopt(TCP_AO_ADD_KEY)", key);
-               if (set_current || set_rnext)
-                       key->used_on_handshake = 1;
 #ifdef DEBUG
                test_print("%s [%u/%u] key: { %s, %u:%u, %u, %u:%u:%u:%u (%u)}",
                           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
        for (i = 0; i < collection.nr_keys; i++) {
                struct test_key *key = &collection.keys[i];
                uint8_t sndid, rcvid;
-               bool was_used;
+               bool rx_cnt_expected;
 
+               if (key->skip_counters_checks)
+                       continue;
                if (server) {
                        sndid = key->server_keyid;
                        rcvid = key->client_keyid;
-                       if (is_listen_sk)
-                               was_used = key->used_on_handshake;
-                       else
-                               was_used = key->used_after_accept;
+                       rx_cnt_expected = key->used_on_client_tx;
                } else {
                        sndid = key->client_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);
        }
        test_tcp_ao_counters_free(a);
@@ -916,9 +914,8 @@ static int run_client(const char *tst_name, unsigned int port,
                current_index = nr_keys - 1;
        if (rnext_index < 0)
                rnext_index = nr_keys - 1;
-       collection.keys[current_index].used_on_handshake = 1;
-       collection.keys[rnext_index].used_after_accept = 1;
-       collection.keys[rnext_index].used_on_client = 1;
+       collection.keys[current_index].used_on_client_tx = 1;
+       collection.keys[rnext_index].used_on_server_tx = 1;
 
        synchronize_threads(); /* 3: accepted => send data */
        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,
                test_error("Can't change the current key");
        if (test_client_verify(sk, msg_len, nr_packets, TEST_TIMEOUT_SEC))
                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);
 }
@@ -1089,7 +1095,7 @@ static void roll_over_keys(const char *tst_name, unsigned int port,
                }
                verify_current_rnext(tst_name, sk, -1,
                                     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 */
        }
        end_client(tst_name, sk, nr_keys, current_index, rnext_index, &tmp);