Two typo fixes, noticed by Mohammad's review.
And a fix for an issue that got uncovered.
Signed-off-by: Dmitry Safonov <[email protected]>
---
Dmitry Safonov (2):
selftests/net: Rectify key counters checks
selftests/net: Clean-up double assignment
Mohammad Nassiri (1):
selftests/net: Argument value mismatch when calling verify_counters()
.../testing/selftests/net/tcp_ao/key-management.c | 46 ++++++++++++----------
tools/testing/selftests/net/tcp_ao/lib/sock.c | 1 -
2 files changed, 26 insertions(+), 21 deletions(-)
---
base-commit: 296455ade1fdcf5f8f8c033201633b60946c589a
change-id: 20240118-tcp-ao-test-key-mgmt-bb51a5fe15a2
Best regards,
--
Dmitry Safonov <[email protected]>
From: Mohammad Nassiri <[email protected]>
The end_server() function only operates in the server thread
and always takes an accept socket instead of a listen socket as
its input argument. To align with this, invert the boolean values
used when calling verify_counters() within the end_server() function.
Fixes: 3c3ead555648 ("selftests/net: Add TCP-AO key-management test")
Signed-off-by: Mohammad Nassiri <[email protected]>
Link: https://lore.kernel.org/all/[email protected]/
[amended 'Fixes' tag and carried-over to lkml]
Signed-off-by: Dmitry Safonov <[email protected]>
---
tools/testing/selftests/net/tcp_ao/key-management.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/tcp_ao/key-management.c b/tools/testing/selftests/net/tcp_ao/key-management.c
index c48b4970ca17..f6a9395e3cd7 100644
--- a/tools/testing/selftests/net/tcp_ao/key-management.c
+++ b/tools/testing/selftests/net/tcp_ao/key-management.c
@@ -843,7 +843,7 @@ static void end_server(const char *tst_name, int sk,
synchronize_threads(); /* 4: verified => closed */
close(sk);
- verify_counters(tst_name, true, false, begin, &end);
+ verify_counters(tst_name, false, true, begin, &end);
synchronize_threads(); /* 5: counters */
}
--
2.43.0
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: 3c3ead555648 ("selftests/net: Add TCP-AO key-management test")
Signed-off-by: Dmitry Safonov <[email protected]>
---
.../testing/selftests/net/tcp_ao/key-management.c | 44 ++++++++++++----------
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/net/tcp_ao/key-management.c b/tools/testing/selftests/net/tcp_ao/key-management.c
index f6a9395e3cd7..24e62120b792 100644
--- 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);
--
2.43.0
Yeah, copy'n'paste typo.
Fixes: 3c3ead555648 ("selftests/net: Add TCP-AO key-management test")
Reported-by: Nassiri, Mohammad <[email protected]>
Closes: https://lore.kernel.org/all/DM6PR04MB4202BC58A9FD5BDD24A16E8EC56F2@DM6PR04MB4202.namprd04.prod.outlook.com/
Signed-off-by: Dmitry Safonov <[email protected]>
---
tools/testing/selftests/net/tcp_ao/lib/sock.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/net/tcp_ao/lib/sock.c b/tools/testing/selftests/net/tcp_ao/lib/sock.c
index c75d82885a2e..923a9bb4f1ca 100644
--- a/tools/testing/selftests/net/tcp_ao/lib/sock.c
+++ b/tools/testing/selftests/net/tcp_ao/lib/sock.c
@@ -377,7 +377,6 @@ int test_get_tcp_ao_counters(int sk, struct tcp_ao_counters *out)
key_dump[0].nkeys = nr_keys;
key_dump[0].get_all = 1;
- key_dump[0].get_all = 1;
err = getsockopt(sk, IPPROTO_TCP, TCP_AO_GET_KEYS,
key_dump, &key_dump_sz);
if (err) {
--
2.43.0
On Thu, 18 Jan 2024 02:51:33 +0000 Dmitry Safonov wrote:
> Two typo fixes, noticed by Mohammad's review.
> And a fix for an issue that got uncovered.
Somewhat unrelated to these fixes but related to the tcp_ao selftests
in general - could you please also add a config file so that it's
easy to build a minimal kernel for running the tests?
Something like:
make defconfig
make kvm_guest.config
make tools/testing/selftests/net/tcp_ao/config
should give us a suitable config. Differently put it'd be great to have
a config we can pass to vmtest or virtme-ng and run the tests.
On 1/18/24 16:51, Jakub Kicinski wrote:
> On Thu, 18 Jan 2024 02:51:33 +0000 Dmitry Safonov wrote:
>> Two typo fixes, noticed by Mohammad's review.
>> And a fix for an issue that got uncovered.
>
> Somewhat unrelated to these fixes but related to the tcp_ao selftests
> in general - could you please also add a config file so that it's
> easy to build a minimal kernel for running the tests?
>
> Something like:
>
> make defconfig
> make kvm_guest.config
> make tools/testing/selftests/net/tcp_ao/config
Yep, sounds good to me.
I'll take as a base tools/testing/selftests/net/config and add any
needed config options on the top.
> should give us a suitable config. Differently put it'd be great to have
> a config we can pass to vmtest or virtme-ng and run the tests.
Will check that it works with them.
Thanks,
Dmitry
On Thu, 18 Jan 2024 17:04:25 +0000 Dmitry Safonov wrote:
> > Somewhat unrelated to these fixes but related to the tcp_ao selftests
> > in general - could you please also add a config file so that it's
> > easy to build a minimal kernel for running the tests?
> >
> > Something like:
> >
> > make defconfig
> > make kvm_guest.config
> > make tools/testing/selftests/net/tcp_ao/config
>
> Yep, sounds good to me.
> I'll take as a base tools/testing/selftests/net/config and add any
> needed config options on the top.
You probably want something smaller to be honest.
tools/testing/selftests/net/config has a lot of stuff in it
and it's actually missing a lot more. I'm working thru adding
the missing options to tools/testing/selftests/net/config
right now so far I got:
# tun / tap
+CONFIG_TUN=y
+CONFIG_MACVLAN=y
+CONFIG_MACVTAP=y
+CONFIG_NET_SCH_FQ_CODEL=m
+# l2tp
+CONFIG_L2TP=m
+CONFIG_L2TP_V3=y
+CONFIG_L2TP_IP=m
+CONFIG_L2TP_ETH=m
+# sctp-vrf (need SCTP_DIAG to appear)
+CONFIG_INET_DIAG=y
+# txtimestamp
+CONFIG_NET_CLS_U32=m
+# test-vxlan-mdb-sh etc.
+CONFIG_BRIDGE_VLAN_FILTERING=y
+# gre_gso.sh etc.
+CONFIG_NET_IPGRE_DEMUX=m
+CONFIG_IP_GRE=m
+CONFIG_IPV6_GRE=m
+# ./srv6_end_dt*_l3vpn_test.sh
+CONFIG_IPV6_SEG6_LWTUNNEL=y
+# local port something..
+CONFIG_MPTCP=y
+# fib_test.sh
+CONFIG_NET_CLS_BASIC=m
On Thu, Jan 18, 2024 at 02:51:36AM +0000, Dmitry Safonov wrote:
> Yeah, copy'n'paste typo.
>
> Fixes: 3c3ead555648 ("selftests/net: Add TCP-AO key-management test")
> Reported-by: Nassiri, Mohammad <[email protected]>
> Closes: https://lore.kernel.org/all/DM6PR04MB4202BC58A9FD5BDD24A16E8EC56F2@DM6PR04MB4202.namprd04.prod.outlook.com/
> Signed-off-by: Dmitry Safonov <[email protected]>
> ---
Hi Dmitry,
This seems more like a clean-up than a fix.
> tools/testing/selftests/net/tcp_ao/lib/sock.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/tools/testing/selftests/net/tcp_ao/lib/sock.c b/tools/testing/selftests/net/tcp_ao/lib/sock.c
> index c75d82885a2e..923a9bb4f1ca 100644
> --- a/tools/testing/selftests/net/tcp_ao/lib/sock.c
> +++ b/tools/testing/selftests/net/tcp_ao/lib/sock.c
> @@ -377,7 +377,6 @@ int test_get_tcp_ao_counters(int sk, struct tcp_ao_counters *out)
>
> key_dump[0].nkeys = nr_keys;
> key_dump[0].get_all = 1;
> - key_dump[0].get_all = 1;
> err = getsockopt(sk, IPPROTO_TCP, TCP_AO_GET_KEYS,
> key_dump, &key_dump_sz);
> if (err) {
>
> --
> 2.43.0
>
On Thu, Jan 18, 2024 at 02:51:34AM +0000, Dmitry Safonov wrote:
> From: Mohammad Nassiri <[email protected]>
>
> The end_server() function only operates in the server thread
> and always takes an accept socket instead of a listen socket as
> its input argument. To align with this, invert the boolean values
> used when calling verify_counters() within the end_server() function.
Hi Dmitry,
I see that this is correct, but I am wondering
if it fixes a user-visible bug. If so, could
some text about that go here?
>
> Fixes: 3c3ead555648 ("selftests/net: Add TCP-AO key-management test")
> Signed-off-by: Mohammad Nassiri <[email protected]>
> Link: https://lore.kernel.org/all/[email protected]/
> [amended 'Fixes' tag and carried-over to lkml]
> Signed-off-by: Dmitry Safonov <[email protected]>
..
Hi Simon,
On 1/19/24 16:25, Simon Horman wrote:
> On Thu, Jan 18, 2024 at 02:51:36AM +0000, Dmitry Safonov wrote:
>> Yeah, copy'n'paste typo.
>>
>> Fixes: 3c3ead555648 ("selftests/net: Add TCP-AO key-management test")
>> Reported-by: Nassiri, Mohammad <[email protected]>
>> Closes: https://lore.kernel.org/all/DM6PR04MB4202BC58A9FD5BDD24A16E8EC56F2@DM6PR04MB4202.namprd04.prod.outlook.com/
>> Signed-off-by: Dmitry Safonov <[email protected]>
>> ---
>
> Hi Dmitry,
>
> This seems more like a clean-up than a fix.
Do you mean to remove the tag or that you'd prefer it rather go
to net-next?
>> tools/testing/selftests/net/tcp_ao/lib/sock.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/net/tcp_ao/lib/sock.c b/tools/testing/selftests/net/tcp_ao/lib/sock.c
>> index c75d82885a2e..923a9bb4f1ca 100644
>> --- a/tools/testing/selftests/net/tcp_ao/lib/sock.c
>> +++ b/tools/testing/selftests/net/tcp_ao/lib/sock.c
>> @@ -377,7 +377,6 @@ int test_get_tcp_ao_counters(int sk, struct tcp_ao_counters *out)
>>
>> key_dump[0].nkeys = nr_keys;
>> key_dump[0].get_all = 1;
>> - key_dump[0].get_all = 1;
>> err = getsockopt(sk, IPPROTO_TCP, TCP_AO_GET_KEYS,
>> key_dump, &key_dump_sz);
>> if (err) {
>>
>> --
>> 2.43.0
>>
Thanks,
Dmitry
Hi Simon,
On 1/19/24 16:26, Simon Horman wrote:
> On Thu, Jan 18, 2024 at 02:51:34AM +0000, Dmitry Safonov wrote:
>> From: Mohammad Nassiri <[email protected]>
>>
>> The end_server() function only operates in the server thread
>> and always takes an accept socket instead of a listen socket as
>> its input argument. To align with this, invert the boolean values
>> used when calling verify_counters() within the end_server() function.
>
> Hi Dmitry,
>
> I see that this is correct, but I am wondering
> if it fixes a user-visible bug. If so, could
> some text about that go here?
Yep. The test as the result doesn't correctly check tcp counters for
non-symmetrical scenario, where peer-A uses a key <a:a2> to send data,
but peer-B uses <b:b2> key to send its data. So, in simple terms,
different keys for TX and RX on peers.
I'll send v2 with an improved patch message.
>> Fixes: 3c3ead555648 ("selftests/net: Add TCP-AO key-management test")
>> Signed-off-by: Mohammad Nassiri <[email protected]>
>> Link: https://lore.kernel.org/all/[email protected]/
>> [amended 'Fixes' tag and carried-over to lkml]
>> Signed-off-by: Dmitry Safonov <[email protected]>
>
> ...
Thanks,
Dmitry
On 1/18/24 17:13, Jakub Kicinski wrote:
> On Thu, 18 Jan 2024 17:04:25 +0000 Dmitry Safonov wrote:
>>> Somewhat unrelated to these fixes but related to the tcp_ao selftests
>>> in general - could you please also add a config file so that it's
>>> easy to build a minimal kernel for running the tests?
>>>
>>> Something like:
>>>
>>> make defconfig
>>> make kvm_guest.config
>>> make tools/testing/selftests/net/tcp_ao/config
>>
>> Yep, sounds good to me.
>> I'll take as a base tools/testing/selftests/net/config and add any
>> needed config options on the top.
>
> You probably want something smaller to be honest.
> tools/testing/selftests/net/config has a lot of stuff in it
> and it's actually missing a lot more. I'm working thru adding
> the missing options to tools/testing/selftests/net/config
> right now so far I got:
Thanks!
I'll send a patch for it in version 2 (as I anyway need to address
Simon's feedback).
>
> # tun / tap
> +CONFIG_TUN=y
> +CONFIG_MACVLAN=y
> +CONFIG_MACVTAP=y
> +CONFIG_NET_SCH_FQ_CODEL=m
> +# l2tp
> +CONFIG_L2TP=m
> +CONFIG_L2TP_V3=y
> +CONFIG_L2TP_IP=m
> +CONFIG_L2TP_ETH=m
> +# sctp-vrf (need SCTP_DIAG to appear)
> +CONFIG_INET_DIAG=y
> +# txtimestamp
> +CONFIG_NET_CLS_U32=m
> +# test-vxlan-mdb-sh etc.
> +CONFIG_BRIDGE_VLAN_FILTERING=y
> +# gre_gso.sh etc.
> +CONFIG_NET_IPGRE_DEMUX=m
> +CONFIG_IP_GRE=m
> +CONFIG_IPV6_GRE=m
> +# ./srv6_end_dt*_l3vpn_test.sh
> +CONFIG_IPV6_SEG6_LWTUNNEL=y
> +# local port something..
> +CONFIG_MPTCP=y
> +# fib_test.sh
> +CONFIG_NET_CLS_BASIC=m
Thanks,
Dmitry
On Fri, Jan 19, 2024 at 06:37:46PM +0000, Dmitry Safonov wrote:
> Hi Simon,
>
> On 1/19/24 16:25, Simon Horman wrote:
> > On Thu, Jan 18, 2024 at 02:51:36AM +0000, Dmitry Safonov wrote:
> >> Yeah, copy'n'paste typo.
> >>
> >> Fixes: 3c3ead555648 ("selftests/net: Add TCP-AO key-management test")
> >> Reported-by: Nassiri, Mohammad <[email protected]>
> >> Closes: https://lore.kernel.org/all/DM6PR04MB4202BC58A9FD5BDD24A16E8EC56F2@DM6PR04MB4202.namprd04.prod.outlook.com/
> >> Signed-off-by: Dmitry Safonov <[email protected]>
> >> ---
> >
> > Hi Dmitry,
> >
> > This seems more like a clean-up than a fix.
>
> Do you mean to remove the tag or that you'd prefer it rather go
> to net-next?
Both :)
..
On Fri, 19 Jan 2024 18:39:14 +0000 Dmitry Safonov wrote:
> > You probably want something smaller to be honest.
> > tools/testing/selftests/net/config has a lot of stuff in it
> > and it's actually missing a lot more. I'm working thru adding
> > the missing options to tools/testing/selftests/net/config
> > right now so far I got:
>
> Thanks!
>
> I'll send a patch for it in version 2 (as I anyway need to address
> Simon's feedback).
Hi Dmitry!
I put TCP_AO and VETH in the config and the tests seem to fail with
selftests: net/tcp_ao: rst_ipv4
not ok 1 # error 834[lib/kconfig.c:143] Failed to initialize kconfig 2: No such file or directory
# Planned tests != run tests (0 != 1)
# Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:1
The script does:
target=net/tcp_ao
make mrproper
vng -v -b -f tools/testing/selftests/$target
# build the scripts
make headers
make -C tools/testing/selftests/$target
vng -v -r arch/x86/boot/bzImage --user root
# inside the VM
make -C tools/testing/selftests TARGETS=$target run_tests
On Wed, 24 Jan 2024 17:46:10 +0000 Dmitry Safonov wrote:
> >> Thanks!
> >>
> >> I'll send a patch for it in version 2 (as I anyway need to address
> >> Simon's feedback).
> >
> > Hi Dmitry!
> >
> > I put TCP_AO and VETH in the config and the tests seem to fail with
>
> Thanks for wiring it up and for https://netdev.bots.linux.dev/status.html!
>
> > selftests: net/tcp_ao: rst_ipv4
> > not ok 1 # error 834[lib/kconfig.c:143] Failed to initialize kconfig 2: No such file or directory
> > # Planned tests != run tests (0 != 1)
> > # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:1
>
> Hehe, yeah I wanted to detect kernels with !CONFIG_TCP_AO, to SKIP the
> test, rather than FAIL it, which this lib/kconfig.c does.
> But from a glance, I think it's failing in your run because there are
> checks with and without TCP_AO, but I didn't think of checking for
> the hashing algorithms support.
>
> I think what happens is has_tcp_ao():
> : strcpy(tmp.alg_name, "hmac(sha1)");
> ...
> : if (setsockopt(sk, IPPROTO_TCP, TCP_AO_ADD_KEY, &tmp, sizeof(tmp)) < 0)
>
> Could you check that what I suppose is failing, is actually failing?
> [dima@Mindolluin linux-master]$ grep -e '\<CONFIG_CRYPTO_SHA1\>' -e
> '\<CONFIG_CRYPTO_HMAC\>' .config
> CONFIG_CRYPTO_HMAC=y
> CONFIG_CRYPTO_SHA1=y
FWIW the config used is uploaded with the results. If you click on
the remote it should take you to a location like this:
https://netdev-2.bots.linux.dev/vmksft-tcp-ao/results/435369/
and there should be a config file in there.
> If that's the case, I'll add the detection for hashing algorithms to
> lib/kconfig.c (together with a patch for
> tools/testing/selftests/net/config).
> And also heads up for key-management.c - that tries a bunch of hashing
> algorithms to check that the work and that the key rotation between
> different algorithms works:
>
> : const char *test_algos[] = {
> : "cmac(aes128)",
> : "hmac(sha1)", "hmac(sha512)", "hmac(sha384)", "hmac(sha256)",
> : "hmac(sha224)", "hmac(sha3-512)",
> : /* only if !CONFIG_FIPS */
> : #define TEST_NON_FIPS_ALGOS 2
> : "hmac(rmd160)", "hmac(md5)"
> : };
I was stuck in a meeting and I started playing around with the options
for TCP-AO :) I added these options now:
CONFIG_CRYPTO_HMAC=y
CONFIG_CRYPTO_SHA1=y
CONFIG_CRYPTO_RMD160=y
CONFIG_IPV6=y
CONFIG_TCP_AO=y
CONFIG_TCP_MD5SIG=y
CONFIG_VETH=m
And it looks much better! There are still some failures:
https://netdev.bots.linux.dev/contest.html?branch=net-next-2024-01-24--18-00&executor=vmksft-tcp-ao
I added VRF so that should hopefully take care of the MD5 skips
on the next run. But the failures of the rst-ip* tests don't look
like an obvious config problem.
On 1/24/24 19:04, Jakub Kicinski wrote:
> On Wed, 24 Jan 2024 17:46:10 +0000 Dmitry Safonov wrote:
>>>> Thanks!
>>>>
>>>> I'll send a patch for it in version 2 (as I anyway need to address
>>>> Simon's feedback).
>>>
>>> Hi Dmitry!
>>>
>>> I put TCP_AO and VETH in the config and the tests seem to fail with
>>
>> Thanks for wiring it up and for https://netdev.bots.linux.dev/status.html!
>>
>>> selftests: net/tcp_ao: rst_ipv4
>>> not ok 1 # error 834[lib/kconfig.c:143] Failed to initialize kconfig 2: No such file or directory
>>> # Planned tests != run tests (0 != 1)
>>> # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:1
>>
>> Hehe, yeah I wanted to detect kernels with !CONFIG_TCP_AO, to SKIP the
>> test, rather than FAIL it, which this lib/kconfig.c does.
>> But from a glance, I think it's failing in your run because there are
>> checks with and without TCP_AO, but I didn't think of checking for
>> the hashing algorithms support.
>>
>> I think what happens is has_tcp_ao():
>> : strcpy(tmp.alg_name, "hmac(sha1)");
>> ...
>> : if (setsockopt(sk, IPPROTO_TCP, TCP_AO_ADD_KEY, &tmp, sizeof(tmp)) < 0)
>>
>> Could you check that what I suppose is failing, is actually failing?
>> [dima@Mindolluin linux-master]$ grep -e '\<CONFIG_CRYPTO_SHA1\>' -e
>> '\<CONFIG_CRYPTO_HMAC\>' .config
>> CONFIG_CRYPTO_HMAC=y
>> CONFIG_CRYPTO_SHA1=y
>
> FWIW the config used is uploaded with the results. If you click on
> the remote it should take you to a location like this:
>
> https://netdev-2.bots.linux.dev/vmksft-tcp-ao/results/435369/
>
> and there should be a config file in there.
>
>> If that's the case, I'll add the detection for hashing algorithms to
>> lib/kconfig.c (together with a patch for
>> tools/testing/selftests/net/config).
>> And also heads up for key-management.c - that tries a bunch of hashing
>> algorithms to check that the work and that the key rotation between
>> different algorithms works:
>>
>> : const char *test_algos[] = {
>> : "cmac(aes128)",
>> : "hmac(sha1)", "hmac(sha512)", "hmac(sha384)", "hmac(sha256)",
>> : "hmac(sha224)", "hmac(sha3-512)",
>> : /* only if !CONFIG_FIPS */
>> : #define TEST_NON_FIPS_ALGOS 2
>> : "hmac(rmd160)", "hmac(md5)"
>> : };
>
> I was stuck in a meeting and I started playing around with the options
> for TCP-AO :)
Haha, the same: 3 hour-long meetings for today :-/
> I added these options now:
>
> CONFIG_CRYPTO_HMAC=y
> CONFIG_CRYPTO_SHA1=y
> CONFIG_CRYPTO_RMD160=y
> CONFIG_IPV6=y
> CONFIG_TCP_AO=y
> CONFIG_TCP_MD5SIG=y
> CONFIG_VETH=m
>
> And it looks much better! There are still some failures:
>
> https://netdev.bots.linux.dev/contest.html?branch=net-next-2024-01-24--18-00&executor=vmksft-tcp-ao
Wow! Nice, thank you!
>
> I added VRF so that should hopefully take care of the MD5 skips
> on the next run. But the failures of the rst-ip* tests don't look
> like an obvious config problem.
Yep, I'll look into this this week.
Thanks,
Dmitry