Note that there's another post-merge fix for TCP-AO selftests, but that doesn't conflict with these, so I don't resend that:
https://lore.kernel.org/all/20231219-b4-tcp-ao-selftests-out-of-tree-v1-1-0f...
Signed-off-by: Dmitry Safonov dima@arista.com --- Dmitry Safonov (2): selftest/tcp-ao: Set routes in a proper VRF table id selftest/tcp-ao: Work on namespace-ified sysctl_optmem_max
tools/testing/selftests/net/tcp_ao/bench-lookups.c | 4 ++- tools/testing/selftests/net/tcp_ao/lib/netlink.c | 4 +-- tools/testing/selftests/net/tcp_ao/lib/setup.c | 35 +++++++++++++++++----- tools/testing/selftests/net/tcp_ao/unsigned-md5.c | 11 ++++--- 4 files changed, 36 insertions(+), 18 deletions(-) --- base-commit: 857647efa9be89a13cf8963c7e167fab062b28bb change-id: 20231222-selftests-tcp-ao-fixups-ce70a60e6f57
Best regards,
In unsigned-md5 selftests ip_route_add() is not needed in client_add_ip(): the route was pre-setup in __test_init() => link_init() for subnet, rather than a specific ip-address.
Currently, __ip_route_add() mistakenly always sets VRF table to RT_TABLE_MAIN - this seems to have sneaked in during unsigned-md5 tests debugging. That also explains, why ip_route_add_vrf() ignored EEXIST, returned by fib6.
Yet, keep EEXIST ignoring in bench-lookups selftests as it's expected that those selftests may add the same (duplicate) routes.
Reported-by: Hangbin Liu liuhangbin@gmail.com Signed-off-by: Dmitry Safonov dima@arista.com --- tools/testing/selftests/net/tcp_ao/bench-lookups.c | 4 +++- tools/testing/selftests/net/tcp_ao/lib/netlink.c | 4 +--- tools/testing/selftests/net/tcp_ao/unsigned-md5.c | 11 +++++------ 3 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/net/tcp_ao/bench-lookups.c b/tools/testing/selftests/net/tcp_ao/bench-lookups.c index 7be8a7d9308c..a1e6e007c291 100644 --- a/tools/testing/selftests/net/tcp_ao/bench-lookups.c +++ b/tools/testing/selftests/net/tcp_ao/bench-lookups.c @@ -46,8 +46,10 @@ static void test_add_routes(union tcp_addr *ips, size_t ips_nr)
for (i = 0; i < ips_nr; i++) { union tcp_addr *p = (union tcp_addr *)&ips[i]; + int err;
- if (ip_route_add(veth_name, TEST_FAMILY, this_ip_addr, *p)) + err = ip_route_add(veth_name, TEST_FAMILY, this_ip_addr, *p); + if (err && err != -EEXIST) test_error("Failed to add route"); } } diff --git a/tools/testing/selftests/net/tcp_ao/lib/netlink.c b/tools/testing/selftests/net/tcp_ao/lib/netlink.c index b731f2c84083..7f108493a29a 100644 --- a/tools/testing/selftests/net/tcp_ao/lib/netlink.c +++ b/tools/testing/selftests/net/tcp_ao/lib/netlink.c @@ -261,7 +261,7 @@ static int __ip_route_add(int sock, uint32_t seq, const char *intf, int family, req.nh.nlmsg_seq = seq; req.rt.rtm_family = family; req.rt.rtm_dst_len = (family == AF_INET) ? 32 : 128; - req.rt.rtm_table = RT_TABLE_MAIN; + req.rt.rtm_table = vrf; req.rt.rtm_protocol = RTPROT_BOOT; req.rt.rtm_scope = RT_SCOPE_UNIVERSE; req.rt.rtm_type = RTN_UNICAST; @@ -294,8 +294,6 @@ int ip_route_add_vrf(const char *intf, int family,
ret = __ip_route_add(route_sock, route_seq++, intf, family, src, dst, vrf); - if (ret == -EEXIST) /* ignoring */ - ret = 0;
close(route_sock); return ret; diff --git a/tools/testing/selftests/net/tcp_ao/unsigned-md5.c b/tools/testing/selftests/net/tcp_ao/unsigned-md5.c index 7cffde02d2be..f5b6d488d501 100644 --- a/tools/testing/selftests/net/tcp_ao/unsigned-md5.c +++ b/tools/testing/selftests/net/tcp_ao/unsigned-md5.c @@ -30,7 +30,7 @@ static void setup_vrfs(void) err = ip_route_add_vrf(veth_name, TEST_FAMILY, this_ip_addr, this_ip_dest, test_vrf_tabid); if (err) - test_error("Failed to add a route to VRF"); + test_error("Failed to add a route to VRF: %d", err); }
static void try_accept(const char *tst_name, unsigned int port, @@ -494,15 +494,14 @@ static void try_to_add(const char *tst_name, unsigned int port,
static void client_add_ip(union tcp_addr *client, const char *ip) { - int family = TEST_FAMILY; + int err, family = TEST_FAMILY;
if (inet_pton(family, ip, client) != 1) test_error("Can't convert ip address %s", ip);
- if (ip_addr_add(veth_name, family, *client, TEST_PREFIX)) - test_error("Failed to add ip address"); - if (ip_route_add(veth_name, family, *client, this_ip_dest)) - test_error("Failed to add route"); + err = ip_addr_add(veth_name, family, *client, TEST_PREFIX); + if (err) + test_error("Failed to add ip address: %d", err); }
static void client_add_ips(void)
Since commit f5769faeec36 ("net: Namespace-ify sysctl_optmem_max") optmem_max is per-netns, so need of switching to root namespace. It seems trivial to keep the old logic working, so going to keep it for a while (at least, until kernel with netns-optmem_max will be release).
Currently, there is a test that checks that optmem_max limit applies to TCP-AO keys and a little benchmark that measures linked-list TCP-AO keys scaling, those are fixed by this.
Cc: Eric Dumazet edumazet@google.com Signed-off-by: Dmitry Safonov dima@arista.com --- tools/testing/selftests/net/tcp_ao/lib/setup.c | 35 ++++++++++++++++++++------ 1 file changed, 27 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/net/tcp_ao/lib/setup.c b/tools/testing/selftests/net/tcp_ao/lib/setup.c index 374b27c26ebd..92276f916f2f 100644 --- a/tools/testing/selftests/net/tcp_ao/lib/setup.c +++ b/tools/testing/selftests/net/tcp_ao/lib/setup.c @@ -277,22 +277,38 @@ void __test_init(unsigned int ntests, int family, unsigned int prefix,
/* /proc/sys/net/core/optmem_max artifically limits the amount of memory * that can be allocated with sock_kmalloc() on each socket in the system. - * It is not virtualized, so it has to written outside test namespaces. - * To be nice a test will revert optmem back to the old value. + * It is not virtualized in v6.7, so it has to written outside test + * namespaces. To be nice a test will revert optmem back to the old value. * Keeping it simple without any file lock, which means the tests that * need to set/increase optmem value shouldn't run in parallel. * Also, not re-entrant. + * Since commit f5769faeec36 ("net: Namespace-ify sysctl_optmem_max") + * it is per-namespace, keeping logic for non-virtualized optmem_max + * for v6.7, which supports TCP-AO. */ static const char *optmem_file = "/proc/sys/net/core/optmem_max"; static size_t saved_optmem; +static int optmem_ns = -1; + +static bool is_optmem_namespaced(void) +{ + if (optmem_ns == -1) { + int old_ns = switch_save_ns(nsfd_child); + + optmem_ns = !access(optmem_file, F_OK); + switch_ns(old_ns); + } + return !!optmem_ns; +}
size_t test_get_optmem(void) { + int old_ns = 0; FILE *foptmem; - int old_ns; size_t ret;
- old_ns = switch_save_ns(nsfd_outside); + if (!is_optmem_namespaced()) + old_ns = switch_save_ns(nsfd_outside); foptmem = fopen(optmem_file, "r"); if (!foptmem) test_error("failed to open %s", optmem_file); @@ -300,19 +316,21 @@ size_t test_get_optmem(void) if (fscanf(foptmem, "%zu", &ret) != 1) test_error("can't read from %s", optmem_file); fclose(foptmem); - switch_ns(old_ns); + if (!is_optmem_namespaced()) + switch_ns(old_ns); return ret; }
static void __test_set_optmem(size_t new, size_t *old) { + int old_ns = 0; FILE *foptmem; - int old_ns;
if (old != NULL) *old = test_get_optmem();
- old_ns = switch_save_ns(nsfd_outside); + if (!is_optmem_namespaced()) + old_ns = switch_save_ns(nsfd_outside); foptmem = fopen(optmem_file, "w"); if (!foptmem) test_error("failed to open %s", optmem_file); @@ -320,7 +338,8 @@ static void __test_set_optmem(size_t new, size_t *old) if (fprintf(foptmem, "%zu", new) <= 0) test_error("can't write %zu to %s", new, optmem_file); fclose(foptmem); - switch_ns(old_ns); + if (!is_optmem_namespaced()) + switch_ns(old_ns); }
static void test_revert_optmem(void)
On Fri, Dec 22, 2023 at 01:59:05AM +0000, Dmitry Safonov wrote:
Note that there's another post-merge fix for TCP-AO selftests, but that doesn't conflict with these, so I don't resend that:
https://lore.kernel.org/all/20231219-b4-tcp-ao-selftests-out-of-tree-v1-1-0f...
Signed-off-by: Dmitry Safonov dima@arista.com
Tested-by: Hangbin Liu liuhangbin@gmail.com Reviewed-by: Hangbin Liu liuhangbin@gmail.com
Hello:
This series was applied to netdev/net-next.git (main) by David S. Miller davem@davemloft.net:
On Fri, 22 Dec 2023 01:59:05 +0000 you wrote:
Note that there's another post-merge fix for TCP-AO selftests, but that doesn't conflict with these, so I don't resend that:
https://lore.kernel.org/all/20231219-b4-tcp-ao-selftests-out-of-tree-v1-1-0f...
Signed-off-by: Dmitry Safonov dima@arista.com
[...]
Here is the summary with links: - [net-next,1/2] selftest/tcp-ao: Set routes in a proper VRF table id https://git.kernel.org/netdev/net-next/c/72cd9f8d5a99 - [net-next,2/2] selftest/tcp-ao: Work on namespace-ified sysctl_optmem_max https://git.kernel.org/netdev/net-next/c/80057b2080a8
You are awesome, thank you!
linux-kselftest-mirror@lists.linaro.org
-
Dmitry Safonov -
Hangbin Liu -
patchwork-bot+netdevbpf＠kernel.org