Skip to content

Commit 0536fcc

Browse files
edumazetdavem330
authored andcommitted
tcp: prepare fastopen code for upcoming listener changes
While auditing TCP stack for upcoming 'lockless' listener changes, I found I had to change fastopen_init_queue() to properly init the object before publishing it. Otherwise an other cpu could try to lock the spinlock before it gets properly initialized. Instead of adding appropriate barriers, just remove dynamic memory allocations : - Structure is 28 bytes on 64bit arches. Using additional 8 bytes for holding a pointer seems overkill. - Two listeners can share same cache line and performance would suffer. If we really want to save few bytes, we would instead dynamically allocate whole struct request_sock_queue in the future. Signed-off-by: Eric Dumazet <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 2985aaa commit 0536fcc

File tree

9 files changed

+35
-60
lines changed

9 files changed

+35
-60
lines changed

include/linux/tcp.h

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -382,25 +382,11 @@ static inline bool tcp_passive_fastopen(const struct sock *sk)
382382
tcp_sk(sk)->fastopen_rsk != NULL);
383383
}
384384

385-
extern void tcp_sock_destruct(struct sock *sk);
386-
387-
static inline int fastopen_init_queue(struct sock *sk, int backlog)
385+
static inline void fastopen_queue_tune(struct sock *sk, int backlog)
388386
{
389-
struct request_sock_queue *queue =
390-
&inet_csk(sk)->icsk_accept_queue;
391-
392-
if (queue->fastopenq == NULL) {
393-
queue->fastopenq = kzalloc(
394-
sizeof(struct fastopen_queue),
395-
sk->sk_allocation);
396-
if (queue->fastopenq == NULL)
397-
return -ENOMEM;
398-
399-
sk->sk_destruct = tcp_sock_destruct;
400-
spin_lock_init(&queue->fastopenq->lock);
401-
}
402-
queue->fastopenq->max_qlen = backlog;
403-
return 0;
387+
struct request_sock_queue *queue = &inet_csk(sk)->icsk_accept_queue;
388+
389+
queue->fastopenq.max_qlen = backlog;
404390
}
405391

406392
static inline void tcp_saved_syn_free(struct tcp_sock *tp)

include/net/request_sock.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -180,11 +180,8 @@ struct request_sock_queue {
180180
struct request_sock *rskq_accept_tail;
181181
u8 rskq_defer_accept;
182182
struct listen_sock *listen_opt;
183-
struct fastopen_queue *fastopenq; /* This is non-NULL iff TFO has been
184-
* enabled on this listener. Check
185-
* max_qlen != 0 in fastopen_queue
186-
* to determine if TFO is enabled
187-
* right at this moment.
183+
struct fastopen_queue fastopenq; /* Check max_qlen != 0 to determine
184+
* if TFO is enabled.
188185
*/
189186

190187
/* temporary alignment, our goal is to get rid of this lock */

net/core/request_sock.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,13 @@ int reqsk_queue_alloc(struct request_sock_queue *queue,
5959

6060
get_random_bytes(&lopt->hash_rnd, sizeof(lopt->hash_rnd));
6161
spin_lock_init(&queue->syn_wait_lock);
62+
63+
spin_lock_init(&queue->fastopenq.lock);
64+
queue->fastopenq.rskq_rst_head = NULL;
65+
queue->fastopenq.rskq_rst_tail = NULL;
66+
queue->fastopenq.qlen = 0;
67+
queue->fastopenq.max_qlen = 0;
68+
6269
queue->rskq_accept_head = NULL;
6370
lopt->nr_table_entries = nr_table_entries;
6471
lopt->max_qlen_log = ilog2(nr_table_entries);
@@ -174,7 +181,7 @@ void reqsk_fastopen_remove(struct sock *sk, struct request_sock *req,
174181
struct sock *lsk = req->rsk_listener;
175182
struct fastopen_queue *fastopenq;
176183

177-
fastopenq = inet_csk(lsk)->icsk_accept_queue.fastopenq;
184+
fastopenq = &inet_csk(lsk)->icsk_accept_queue.fastopenq;
178185

179186
tcp_sk(sk)->fastopen_rsk = NULL;
180187
spin_lock_bh(&fastopenq->lock);

net/ipv4/af_inet.c

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -219,17 +219,13 @@ int inet_listen(struct socket *sock, int backlog)
219219
* shutdown() (rather than close()).
220220
*/
221221
if ((sysctl_tcp_fastopen & TFO_SERVER_ENABLE) != 0 &&
222-
!inet_csk(sk)->icsk_accept_queue.fastopenq) {
222+
!inet_csk(sk)->icsk_accept_queue.fastopenq.max_qlen) {
223223
if ((sysctl_tcp_fastopen & TFO_SERVER_WO_SOCKOPT1) != 0)
224-
err = fastopen_init_queue(sk, backlog);
224+
fastopen_queue_tune(sk, backlog);
225225
else if ((sysctl_tcp_fastopen &
226226
TFO_SERVER_WO_SOCKOPT2) != 0)
227-
err = fastopen_init_queue(sk,
227+
fastopen_queue_tune(sk,
228228
((uint)sysctl_tcp_fastopen) >> 16);
229-
else
230-
err = 0;
231-
if (err)
232-
goto out;
233229

234230
tcp_fastopen_init_key_once(true);
235231
}

net/ipv4/inet_connection_sock.c

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -335,9 +335,8 @@ struct sock *inet_csk_accept(struct sock *sk, int flags, int *err)
335335

336336
sk_acceptq_removed(sk);
337337
if (sk->sk_protocol == IPPROTO_TCP &&
338-
tcp_rsk(req)->tfo_listener &&
339-
queue->fastopenq) {
340-
spin_lock_bh(&queue->fastopenq->lock);
338+
tcp_rsk(req)->tfo_listener) {
339+
spin_lock_bh(&queue->fastopenq.lock);
341340
if (tcp_rsk(req)->tfo_listener) {
342341
/* We are still waiting for the final ACK from 3WHS
343342
* so can't free req now. Instead, we set req->sk to
@@ -348,7 +347,7 @@ struct sock *inet_csk_accept(struct sock *sk, int flags, int *err)
348347
req->sk = NULL;
349348
req = NULL;
350349
}
351-
spin_unlock_bh(&queue->fastopenq->lock);
350+
spin_unlock_bh(&queue->fastopenq.lock);
352351
}
353352
out:
354353
release_sock(sk);
@@ -886,12 +885,12 @@ void inet_csk_listen_stop(struct sock *sk)
886885
sk_acceptq_removed(sk);
887886
reqsk_put(req);
888887
}
889-
if (queue->fastopenq) {
888+
if (queue->fastopenq.rskq_rst_head) {
890889
/* Free all the reqs queued in rskq_rst_head. */
891-
spin_lock_bh(&queue->fastopenq->lock);
892-
acc_req = queue->fastopenq->rskq_rst_head;
893-
queue->fastopenq->rskq_rst_head = NULL;
894-
spin_unlock_bh(&queue->fastopenq->lock);
890+
spin_lock_bh(&queue->fastopenq.lock);
891+
acc_req = queue->fastopenq.rskq_rst_head;
892+
queue->fastopenq.rskq_rst_head = NULL;
893+
spin_unlock_bh(&queue->fastopenq.lock);
895894
while ((req = acc_req) != NULL) {
896895
acc_req = req->dl_next;
897896
reqsk_put(req);

net/ipv4/tcp.c

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2253,13 +2253,6 @@ int tcp_disconnect(struct sock *sk, int flags)
22532253
}
22542254
EXPORT_SYMBOL(tcp_disconnect);
22552255

2256-
void tcp_sock_destruct(struct sock *sk)
2257-
{
2258-
inet_sock_destruct(sk);
2259-
2260-
kfree(inet_csk(sk)->icsk_accept_queue.fastopenq);
2261-
}
2262-
22632256
static inline bool tcp_can_repair_sock(const struct sock *sk)
22642257
{
22652258
return ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN) &&
@@ -2581,7 +2574,7 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
25812574
TCPF_LISTEN))) {
25822575
tcp_fastopen_init_key_once(true);
25832576

2584-
err = fastopen_init_queue(sk, val);
2577+
fastopen_queue_tune(sk, val);
25852578
} else {
25862579
err = -EINVAL;
25872580
}
@@ -2849,10 +2842,7 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
28492842
break;
28502843

28512844
case TCP_FASTOPEN:
2852-
if (icsk->icsk_accept_queue.fastopenq)
2853-
val = icsk->icsk_accept_queue.fastopenq->max_qlen;
2854-
else
2855-
val = 0;
2845+
val = icsk->icsk_accept_queue.fastopenq.max_qlen;
28562846
break;
28572847

28582848
case TCP_TIMESTAMP:

net/ipv4/tcp_fastopen.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,9 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
142142
if (!child)
143143
return NULL;
144144

145-
spin_lock(&queue->fastopenq->lock);
146-
queue->fastopenq->qlen++;
147-
spin_unlock(&queue->fastopenq->lock);
145+
spin_lock(&queue->fastopenq.lock);
146+
queue->fastopenq.qlen++;
147+
spin_unlock(&queue->fastopenq.lock);
148148

149149
/* Initialize the child socket. Have to fix some values to take
150150
* into account the child is a Fast Open socket and is created
@@ -237,8 +237,8 @@ static bool tcp_fastopen_queue_check(struct sock *sk)
237237
* between qlen overflow causing Fast Open to be disabled
238238
* temporarily vs a server not supporting Fast Open at all.
239239
*/
240-
fastopenq = inet_csk(sk)->icsk_accept_queue.fastopenq;
241-
if (!fastopenq || fastopenq->max_qlen == 0)
240+
fastopenq = &inet_csk(sk)->icsk_accept_queue.fastopenq;
241+
if (fastopenq->max_qlen == 0)
242242
return false;
243243

244244
if (fastopenq->qlen >= fastopenq->max_qlen) {

net/ipv4/tcp_ipv4.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2186,7 +2186,7 @@ static void get_tcp4_sock(struct sock *sk, struct seq_file *f, int i)
21862186
const struct tcp_sock *tp = tcp_sk(sk);
21872187
const struct inet_connection_sock *icsk = inet_csk(sk);
21882188
const struct inet_sock *inet = inet_sk(sk);
2189-
struct fastopen_queue *fastopenq = icsk->icsk_accept_queue.fastopenq;
2189+
const struct fastopen_queue *fastopenq = &icsk->icsk_accept_queue.fastopenq;
21902190
__be32 dest = inet->inet_daddr;
21912191
__be32 src = inet->inet_rcv_saddr;
21922192
__u16 destp = ntohs(inet->inet_dport);

net/ipv6/tcp_ipv6.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1672,7 +1672,7 @@ static void get_tcp6_sock(struct seq_file *seq, struct sock *sp, int i)
16721672
const struct inet_sock *inet = inet_sk(sp);
16731673
const struct tcp_sock *tp = tcp_sk(sp);
16741674
const struct inet_connection_sock *icsk = inet_csk(sp);
1675-
struct fastopen_queue *fastopenq = icsk->icsk_accept_queue.fastopenq;
1675+
const struct fastopen_queue *fastopenq = &icsk->icsk_accept_queue.fastopenq;
16761676

16771677
dest = &sp->sk_v6_daddr;
16781678
src = &sp->sk_v6_rcv_saddr;
@@ -1716,7 +1716,7 @@ static void get_tcp6_sock(struct seq_file *seq, struct sock *sp, int i)
17161716
(icsk->icsk_ack.quick << 1) | icsk->icsk_ack.pingpong,
17171717
tp->snd_cwnd,
17181718
sp->sk_state == TCP_LISTEN ?
1719-
(fastopenq ? fastopenq->max_qlen : 0) :
1719+
fastopenq->max_qlen :
17201720
(tcp_in_initial_slowstart(tp) ? -1 : tp->snd_ssthresh)
17211721
);
17221722
}

0 commit comments

Comments
 (0)