Skip to content

Commit 9b70c36

Browse files
kuba-mookuba-moo
committed
Merge branch 'net-fix-passive-tfo-socket-having-invalid-napi-id'
David Wei says: ==================== net: fix passive TFO socket having invalid NAPI ID Found a bug where an accepted passive TFO socket returns an invalid NAPI ID (i.e. 0) from SO_INCOMING_NAPI_ID. Add a selftest for this using netdevsim and fix the bug. Patch 1 is a drive-by fix for the lib.sh include in an existing drivers/net/netdevsim/peer.sh selftest. Patch 2 adds a test binary for a simple TFO server/client. Patch 3 adds a selftest for checking that the NAPI ID of a passive TFO socket is valid. This will currently fail. Patch 4 adds a fix for the bug. ==================== Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
2 parents f82727a + dbe0ca8 commit 9b70c36

File tree

6 files changed

+291
-1
lines changed

6 files changed

+291
-1
lines changed

‎net/ipv4/tcp_fastopen.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <linux/tcp.h>
44
#include <linux/rcupdate.h>
55
#include <net/tcp.h>
6+
#include <net/busy_poll.h>
67

78
void tcp_fastopen_init_key_once(struct net *net)
89
{
@@ -279,6 +280,8 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
279280

280281
refcount_set(&req->rsk_refcnt, 2);
281282

283+
sk_mark_napi_id_set(child, skb);
284+
282285
/* Now finish processing the fastopen child socket. */
283286
tcp_init_transfer(child, BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB, skb);
284287

‎tools/testing/selftests/drivers/net/netdevsim/peer.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
#!/bin/bash
22
# SPDX-License-Identifier: GPL-2.0-only
33

4-
source ../../../net/lib.sh
4+
lib_dir=$(dirname $0)/../../../net
5+
source $lib_dir/lib.sh
56

67
NSIM_DEV_1_ID=$((256 + RANDOM % 256))
78
NSIM_DEV_1_SYS=/sys/bus/netdevsim/devices/netdevsim$NSIM_DEV_1_ID

‎tools/testing/selftests/net/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ tap
5050
tcp_fastopen_backup_key
5151
tcp_inq
5252
tcp_mmap
53+
tfo
5354
timestamping
5455
tls
5556
toeplitz

‎tools/testing/selftests/net/Makefile

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,8 @@ TEST_GEN_PROGS += proc_net_pktgen
110110
TEST_PROGS += lwt_dst_cache_ref_loop.sh
111111
TEST_PROGS += skf_net_off.sh
112112
TEST_GEN_FILES += skf_net_off
113+
TEST_GEN_FILES += tfo
114+
TEST_PROGS += tfo_passive.sh
113115

114116
# YNL files, must be before "include ..lib.mk"
115117
YNL_GEN_FILES := busy_poller netlink-dumps

‎tools/testing/selftests/net/tfo.c

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
#include <error.h>
3+
#include <fcntl.h>
4+
#include <limits.h>
5+
#include <stdbool.h>
6+
#include <stdint.h>
7+
#include <stdio.h>
8+
#include <stdlib.h>
9+
#include <string.h>
10+
#include <unistd.h>
11+
#include <arpa/inet.h>
12+
#include <sys/socket.h>
13+
#include <netinet/tcp.h>
14+
#include <errno.h>
15+
16+
static int cfg_server;
17+
static int cfg_client;
18+
static int cfg_port = 8000;
19+
static struct sockaddr_in6 cfg_addr;
20+
static char *cfg_outfile;
21+
22+
static int parse_address(const char *str, int port, struct sockaddr_in6 *sin6)
23+
{
24+
int ret;
25+
26+
sin6->sin6_family = AF_INET6;
27+
sin6->sin6_port = htons(port);
28+
29+
ret = inet_pton(sin6->sin6_family, str, &sin6->sin6_addr);
30+
if (ret != 1) {
31+
/* fallback to plain IPv4 */
32+
ret = inet_pton(AF_INET, str, &sin6->sin6_addr.s6_addr32[3]);
33+
if (ret != 1)
34+
return -1;
35+
36+
/* add ::ffff prefix */
37+
sin6->sin6_addr.s6_addr32[0] = 0;
38+
sin6->sin6_addr.s6_addr32[1] = 0;
39+
sin6->sin6_addr.s6_addr16[4] = 0;
40+
sin6->sin6_addr.s6_addr16[5] = 0xffff;
41+
}
42+
43+
return 0;
44+
}
45+
46+
static void run_server(void)
47+
{
48+
unsigned long qlen = 32;
49+
int fd, opt, connfd;
50+
socklen_t len;
51+
char buf[64];
52+
FILE *outfile;
53+
54+
outfile = fopen(cfg_outfile, "w");
55+
if (!outfile)
56+
error(1, errno, "fopen() outfile");
57+
58+
fd = socket(AF_INET6, SOCK_STREAM, 0);
59+
if (fd == -1)
60+
error(1, errno, "socket()");
61+
62+
opt = 1;
63+
if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)) < 0)
64+
error(1, errno, "setsockopt(SO_REUSEADDR)");
65+
66+
if (setsockopt(fd, SOL_TCP, TCP_FASTOPEN, &qlen, sizeof(qlen)) < 0)
67+
error(1, errno, "setsockopt(TCP_FASTOPEN)");
68+
69+
if (bind(fd, (struct sockaddr *)&cfg_addr, sizeof(cfg_addr)) < 0)
70+
error(1, errno, "bind()");
71+
72+
if (listen(fd, 5) < 0)
73+
error(1, errno, "listen()");
74+
75+
len = sizeof(cfg_addr);
76+
connfd = accept(fd, (struct sockaddr *)&cfg_addr, &len);
77+
if (connfd < 0)
78+
error(1, errno, "accept()");
79+
80+
len = sizeof(opt);
81+
if (getsockopt(connfd, SOL_SOCKET, SO_INCOMING_NAPI_ID, &opt, &len) < 0)
82+
error(1, errno, "getsockopt(SO_INCOMING_NAPI_ID)");
83+
84+
read(connfd, buf, 64);
85+
fprintf(outfile, "%d\n", opt);
86+
87+
fclose(outfile);
88+
close(connfd);
89+
close(fd);
90+
}
91+
92+
static void run_client(void)
93+
{
94+
int fd;
95+
char *msg = "Hello, world!";
96+
97+
fd = socket(AF_INET6, SOCK_STREAM, 0);
98+
if (fd == -1)
99+
error(1, errno, "socket()");
100+
101+
sendto(fd, msg, strlen(msg), MSG_FASTOPEN, (struct sockaddr *)&cfg_addr, sizeof(cfg_addr));
102+
103+
close(fd);
104+
}
105+
106+
static void usage(const char *filepath)
107+
{
108+
error(1, 0, "Usage: %s (-s|-c) -h<server_ip> -p<port> -o<outfile> ", filepath);
109+
}
110+
111+
static void parse_opts(int argc, char **argv)
112+
{
113+
struct sockaddr_in6 *addr6 = (void *) &cfg_addr;
114+
char *addr = NULL;
115+
int ret;
116+
int c;
117+
118+
if (argc <= 1)
119+
usage(argv[0]);
120+
121+
while ((c = getopt(argc, argv, "sch:p:o:")) != -1) {
122+
switch (c) {
123+
case 's':
124+
if (cfg_client)
125+
error(1, 0, "Pass one of -s or -c");
126+
cfg_server = 1;
127+
break;
128+
case 'c':
129+
if (cfg_server)
130+
error(1, 0, "Pass one of -s or -c");
131+
cfg_client = 1;
132+
break;
133+
case 'h':
134+
addr = optarg;
135+
break;
136+
case 'p':
137+
cfg_port = strtoul(optarg, NULL, 0);
138+
break;
139+
case 'o':
140+
cfg_outfile = strdup(optarg);
141+
if (!cfg_outfile)
142+
error(1, 0, "outfile invalid");
143+
break;
144+
}
145+
}
146+
147+
if (cfg_server && addr)
148+
error(1, 0, "Server cannot have -h specified");
149+
150+
memset(addr6, 0, sizeof(*addr6));
151+
addr6->sin6_family = AF_INET6;
152+
addr6->sin6_port = htons(cfg_port);
153+
addr6->sin6_addr = in6addr_any;
154+
if (addr) {
155+
ret = parse_address(addr, cfg_port, addr6);
156+
if (ret)
157+
error(1, 0, "Client address parse error: %s", addr);
158+
}
159+
}
160+
161+
int main(int argc, char **argv)
162+
{
163+
parse_opts(argc, argv);
164+
165+
if (cfg_server)
166+
run_server();
167+
else if (cfg_client)
168+
run_client();
169+
170+
return 0;
171+
}

‎tools/testing/selftests/net/tfo_passive.sh

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
#!/bin/bash
2+
# SPDX-License-Identifier: GPL-2.0
3+
source lib.sh
4+
5+
NSIM_SV_ID=$((256 + RANDOM % 256))
6+
NSIM_SV_SYS=/sys/bus/netdevsim/devices/netdevsim$NSIM_SV_ID
7+
NSIM_CL_ID=$((512 + RANDOM % 256))
8+
NSIM_CL_SYS=/sys/bus/netdevsim/devices/netdevsim$NSIM_CL_ID
9+
10+
NSIM_DEV_SYS_NEW=/sys/bus/netdevsim/new_device
11+
NSIM_DEV_SYS_DEL=/sys/bus/netdevsim/del_device
12+
NSIM_DEV_SYS_LINK=/sys/bus/netdevsim/link_device
13+
NSIM_DEV_SYS_UNLINK=/sys/bus/netdevsim/unlink_device
14+
15+
SERVER_IP=192.168.1.1
16+
CLIENT_IP=192.168.1.2
17+
SERVER_PORT=48675
18+
19+
setup_ns()
20+
{
21+
set -e
22+
ip netns add nssv
23+
ip netns add nscl
24+
25+
NSIM_SV_NAME=$(find $NSIM_SV_SYS/net -maxdepth 1 -type d ! \
26+
-path $NSIM_SV_SYS/net -exec basename {} \;)
27+
NSIM_CL_NAME=$(find $NSIM_CL_SYS/net -maxdepth 1 -type d ! \
28+
-path $NSIM_CL_SYS/net -exec basename {} \;)
29+
30+
ip link set $NSIM_SV_NAME netns nssv
31+
ip link set $NSIM_CL_NAME netns nscl
32+
33+
ip netns exec nssv ip addr add "${SERVER_IP}/24" dev $NSIM_SV_NAME
34+
ip netns exec nscl ip addr add "${CLIENT_IP}/24" dev $NSIM_CL_NAME
35+
36+
ip netns exec nssv ip link set dev $NSIM_SV_NAME up
37+
ip netns exec nscl ip link set dev $NSIM_CL_NAME up
38+
39+
# Enable passive TFO
40+
ip netns exec nssv sysctl -w net.ipv4.tcp_fastopen=519 > /dev/null
41+
42+
set +e
43+
}
44+
45+
cleanup_ns()
46+
{
47+
ip netns del nscl
48+
ip netns del nssv
49+
}
50+
51+
###
52+
### Code start
53+
###
54+
55+
modprobe netdevsim
56+
57+
# linking
58+
59+
echo $NSIM_SV_ID > $NSIM_DEV_SYS_NEW
60+
echo $NSIM_CL_ID > $NSIM_DEV_SYS_NEW
61+
udevadm settle
62+
63+
setup_ns
64+
65+
NSIM_SV_FD=$((256 + RANDOM % 256))
66+
exec {NSIM_SV_FD}</var/run/netns/nssv
67+
NSIM_SV_IFIDX=$(ip netns exec nssv cat /sys/class/net/$NSIM_SV_NAME/ifindex)
68+
69+
NSIM_CL_FD=$((256 + RANDOM % 256))
70+
exec {NSIM_CL_FD}</var/run/netns/nscl
71+
NSIM_CL_IFIDX=$(ip netns exec nscl cat /sys/class/net/$NSIM_CL_NAME/ifindex)
72+
73+
echo "$NSIM_SV_FD:$NSIM_SV_IFIDX $NSIM_CL_FD:$NSIM_CL_IFIDX" > \
74+
$NSIM_DEV_SYS_LINK
75+
76+
if [ $? -ne 0 ]; then
77+
echo "linking netdevsim1 with netdevsim2 should succeed"
78+
cleanup_ns
79+
exit 1
80+
fi
81+
82+
out_file=$(mktemp)
83+
84+
timeout -k 1s 30s ip netns exec nssv ./tfo \
85+
-s \
86+
-p ${SERVER_PORT} \
87+
-o ${out_file}&
88+
89+
wait_local_port_listen nssv ${SERVER_PORT} tcp
90+
91+
ip netns exec nscl ./tfo -c -h ${SERVER_IP} -p ${SERVER_PORT}
92+
93+
wait
94+
95+
res=$(cat $out_file)
96+
rm $out_file
97+
98+
if [ $res -eq 0 ]; then
99+
echo "got invalid NAPI ID from passive TFO socket"
100+
cleanup_ns
101+
exit 1
102+
fi
103+
104+
echo "$NSIM_SV_FD:$NSIM_SV_IFIDX" > $NSIM_DEV_SYS_UNLINK
105+
106+
echo $NSIM_CL_ID > $NSIM_DEV_SYS_DEL
107+
108+
cleanup_ns
109+
110+
modprobe -r netdevsim
111+
112+
exit 0

0 commit comments

Comments
 (0)