Skip to content

Commit 97de5a1

Browse files
q2venPaolo Abeni
authored andcommitted
selftest: Don't reuse port for SO_INCOMING_CPU test.
Jakub reported that ASSERT_EQ(cpu, i) in so_incoming_cpu.c seems to fire somewhat randomly. # # RUN so_incoming_cpu.before_reuseport.test3 ... # # so_incoming_cpu.c:191:test3:Expected cpu (32) == i (0) # # test3: Test terminated by assertion # # FAIL so_incoming_cpu.before_reuseport.test3 # not ok 3 so_incoming_cpu.before_reuseport.test3 When the test failed, not-yet-accepted CLOSE_WAIT sockets received SYN with a "challenging" SEQ number, which was sent from an unexpected CPU that did not create the receiver. The test basically does: 1. for each cpu: 1-1. create a server 1-2. set SO_INCOMING_CPU 2. for each cpu: 2-1. set cpu affinity 2-2. create some clients 2-3. let clients connect() to the server on the same cpu 2-4. close() clients 3. for each server: 3-1. accept() all child sockets 3-2. check if all children have the same SO_INCOMING_CPU with the server The root cause was the close() in 2-4. and net.ipv4.tcp_tw_reuse. In a loop of 2., close() changed the client state to FIN_WAIT_2, and the peer transitioned to CLOSE_WAIT. In another loop of 2., connect() happened to select the same port of the FIN_WAIT_2 socket, and it was reused as the default value of net.ipv4.tcp_tw_reuse is 2. As a result, the new client sent SYN to the CLOSE_WAIT socket from a different CPU, and the receiver's sk_incoming_cpu was overwritten with unexpected CPU ID. Also, the SYN had a different SEQ number, so the CLOSE_WAIT socket responded with Challenge ACK. The new client properly returned RST and effectively killed the CLOSE_WAIT socket. This way, all clients were created successfully, but the error was detected later by 3-2., ASSERT_EQ(cpu, i). To avoid the failure, let's make sure that (i) the number of clients is less than the number of available ports and (ii) such reuse never happens. Fixes: 6df9614 ("selftest: Add test for SO_INCOMING_CPU.") Reported-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> Tested-by: Jakub Kicinski <kuba@kernel.org> Link: https://lore.kernel.org/r/20240120031642.67014-1-kuniyu@amazon.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
1 parent 7267e8d commit 97de5a1

File tree

1 file changed

+50
-18
lines changed

1 file changed

+50
-18
lines changed

tools/testing/selftests/net/so_incoming_cpu.c

Lines changed: 50 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,16 @@
33
#define _GNU_SOURCE
44
#include <sched.h>
55

6+
#include <fcntl.h>
7+
68
#include <netinet/in.h>
79
#include <sys/socket.h>
810
#include <sys/sysinfo.h>
911

1012
#include "../kselftest_harness.h"
1113

12-
#define CLIENT_PER_SERVER 32 /* More sockets, more reliable */
13-
#define NR_SERVER self->nproc
14-
#define NR_CLIENT (CLIENT_PER_SERVER * NR_SERVER)
15-
1614
FIXTURE(so_incoming_cpu)
1715
{
18-
int nproc;
1916
int *servers;
2017
union {
2118
struct sockaddr addr;
@@ -56,12 +53,47 @@ FIXTURE_VARIANT_ADD(so_incoming_cpu, after_all_listen)
5653
.when_to_set = AFTER_ALL_LISTEN,
5754
};
5855

56+
static void write_sysctl(struct __test_metadata *_metadata,
57+
char *filename, char *string)
58+
{
59+
int fd, len, ret;
60+
61+
fd = open(filename, O_WRONLY);
62+
ASSERT_NE(fd, -1);
63+
64+
len = strlen(string);
65+
ret = write(fd, string, len);
66+
ASSERT_EQ(ret, len);
67+
}
68+
69+
static void setup_netns(struct __test_metadata *_metadata)
70+
{
71+
ASSERT_EQ(unshare(CLONE_NEWNET), 0);
72+
ASSERT_EQ(system("ip link set lo up"), 0);
73+
74+
write_sysctl(_metadata, "/proc/sys/net/ipv4/ip_local_port_range", "10000 60001");
75+
write_sysctl(_metadata, "/proc/sys/net/ipv4/tcp_tw_reuse", "0");
76+
}
77+
78+
#define NR_PORT (60001 - 10000 - 1)
79+
#define NR_CLIENT_PER_SERVER_DEFAULT 32
80+
static int nr_client_per_server, nr_server, nr_client;
81+
5982
FIXTURE_SETUP(so_incoming_cpu)
6083
{
61-
self->nproc = get_nprocs();
62-
ASSERT_LE(2, self->nproc);
84+
setup_netns(_metadata);
85+
86+
nr_server = get_nprocs();
87+
ASSERT_LE(2, nr_server);
88+
89+
if (NR_CLIENT_PER_SERVER_DEFAULT * nr_server < NR_PORT)
90+
nr_client_per_server = NR_CLIENT_PER_SERVER_DEFAULT;
91+
else
92+
nr_client_per_server = NR_PORT / nr_server;
93+
94+
nr_client = nr_client_per_server * nr_server;
6395

64-
self->servers = malloc(sizeof(int) * NR_SERVER);
96+
self->servers = malloc(sizeof(int) * nr_server);
6597
ASSERT_NE(self->servers, NULL);
6698

6799
self->in_addr.sin_family = AF_INET;
@@ -74,7 +106,7 @@ FIXTURE_TEARDOWN(so_incoming_cpu)
74106
{
75107
int i;
76108

77-
for (i = 0; i < NR_SERVER; i++)
109+
for (i = 0; i < nr_server; i++)
78110
close(self->servers[i]);
79111

80112
free(self->servers);
@@ -110,10 +142,10 @@ int create_server(struct __test_metadata *_metadata,
110142
if (variant->when_to_set == BEFORE_LISTEN)
111143
set_so_incoming_cpu(_metadata, fd, cpu);
112144

113-
/* We don't use CLIENT_PER_SERVER here not to block
145+
/* We don't use nr_client_per_server here not to block
114146
* this test at connect() if SO_INCOMING_CPU is broken.
115147
*/
116-
ret = listen(fd, NR_CLIENT);
148+
ret = listen(fd, nr_client);
117149
ASSERT_EQ(ret, 0);
118150

119151
if (variant->when_to_set == AFTER_LISTEN)
@@ -128,7 +160,7 @@ void create_servers(struct __test_metadata *_metadata,
128160
{
129161
int i, ret;
130162

131-
for (i = 0; i < NR_SERVER; i++) {
163+
for (i = 0; i < nr_server; i++) {
132164
self->servers[i] = create_server(_metadata, self, variant, i);
133165

134166
if (i == 0) {
@@ -138,7 +170,7 @@ void create_servers(struct __test_metadata *_metadata,
138170
}
139171

140172
if (variant->when_to_set == AFTER_ALL_LISTEN) {
141-
for (i = 0; i < NR_SERVER; i++)
173+
for (i = 0; i < nr_server; i++)
142174
set_so_incoming_cpu(_metadata, self->servers[i], i);
143175
}
144176
}
@@ -149,7 +181,7 @@ void create_clients(struct __test_metadata *_metadata,
149181
cpu_set_t cpu_set;
150182
int i, j, fd, ret;
151183

152-
for (i = 0; i < NR_SERVER; i++) {
184+
for (i = 0; i < nr_server; i++) {
153185
CPU_ZERO(&cpu_set);
154186

155187
CPU_SET(i, &cpu_set);
@@ -162,7 +194,7 @@ void create_clients(struct __test_metadata *_metadata,
162194
ret = sched_setaffinity(0, sizeof(cpu_set), &cpu_set);
163195
ASSERT_EQ(ret, 0);
164196

165-
for (j = 0; j < CLIENT_PER_SERVER; j++) {
197+
for (j = 0; j < nr_client_per_server; j++) {
166198
fd = socket(AF_INET, SOCK_STREAM, 0);
167199
ASSERT_NE(fd, -1);
168200

@@ -180,8 +212,8 @@ void verify_incoming_cpu(struct __test_metadata *_metadata,
180212
int i, j, fd, cpu, ret, total = 0;
181213
socklen_t len = sizeof(int);
182214

183-
for (i = 0; i < NR_SERVER; i++) {
184-
for (j = 0; j < CLIENT_PER_SERVER; j++) {
215+
for (i = 0; i < nr_server; i++) {
216+
for (j = 0; j < nr_client_per_server; j++) {
185217
/* If we see -EAGAIN here, SO_INCOMING_CPU is broken */
186218
fd = accept(self->servers[i], &self->addr, &self->addrlen);
187219
ASSERT_NE(fd, -1);
@@ -195,7 +227,7 @@ void verify_incoming_cpu(struct __test_metadata *_metadata,
195227
}
196228
}
197229

198-
ASSERT_EQ(total, NR_CLIENT);
230+
ASSERT_EQ(total, nr_client);
199231
TH_LOG("SO_INCOMING_CPU is very likely to be "
200232
"working correctly with %d sockets.", total);
201233
}

0 commit comments

Comments
 (0)