Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0536fcc0 authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller
Browse files

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: default avatarEric Dumazet <edumazet@google.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 2985aaac
Loading
Loading
Loading
Loading
+4 −18
Original line number Original line Diff line number Diff line
@@ -382,25 +382,11 @@ static inline bool tcp_passive_fastopen(const struct sock *sk)
		tcp_sk(sk)->fastopen_rsk != NULL);
		tcp_sk(sk)->fastopen_rsk != NULL);
}
}


extern void tcp_sock_destruct(struct sock *sk);
static inline void fastopen_queue_tune(struct sock *sk, int backlog)

static inline int fastopen_init_queue(struct sock *sk, int backlog)
{
{
	struct request_sock_queue *queue =
	struct request_sock_queue *queue = &inet_csk(sk)->icsk_accept_queue;
	    &inet_csk(sk)->icsk_accept_queue;


	queue->fastopenq.max_qlen = backlog;
	if (queue->fastopenq == NULL) {
		queue->fastopenq = kzalloc(
		    sizeof(struct fastopen_queue),
		    sk->sk_allocation);
		if (queue->fastopenq == NULL)
			return -ENOMEM;

		sk->sk_destruct = tcp_sock_destruct;
		spin_lock_init(&queue->fastopenq->lock);
	}
	queue->fastopenq->max_qlen = backlog;
	return 0;
}
}


static inline void tcp_saved_syn_free(struct tcp_sock *tp)
static inline void tcp_saved_syn_free(struct tcp_sock *tp)
+2 −5
Original line number Original line Diff line number Diff line
@@ -180,11 +180,8 @@ struct request_sock_queue {
	struct request_sock	*rskq_accept_tail;
	struct request_sock	*rskq_accept_tail;
	u8			rskq_defer_accept;
	u8			rskq_defer_accept;
	struct listen_sock	*listen_opt;
	struct listen_sock	*listen_opt;
	struct fastopen_queue	*fastopenq; /* This is non-NULL iff TFO has been
	struct fastopen_queue	fastopenq;  /* Check max_qlen != 0 to determine
					     * enabled on this listener. Check
					     * if TFO is enabled.
					     * max_qlen != 0 in fastopen_queue
					     * to determine if TFO is enabled
					     * right at this moment.
					     */
					     */


	/* temporary alignment, our goal is to get rid of this lock */
	/* temporary alignment, our goal is to get rid of this lock */
+8 −1
Original line number Original line Diff line number Diff line
@@ -59,6 +59,13 @@ int reqsk_queue_alloc(struct request_sock_queue *queue,


	get_random_bytes(&lopt->hash_rnd, sizeof(lopt->hash_rnd));
	get_random_bytes(&lopt->hash_rnd, sizeof(lopt->hash_rnd));
	spin_lock_init(&queue->syn_wait_lock);
	spin_lock_init(&queue->syn_wait_lock);

	spin_lock_init(&queue->fastopenq.lock);
	queue->fastopenq.rskq_rst_head = NULL;
	queue->fastopenq.rskq_rst_tail = NULL;
	queue->fastopenq.qlen = 0;
	queue->fastopenq.max_qlen = 0;

	queue->rskq_accept_head = NULL;
	queue->rskq_accept_head = NULL;
	lopt->nr_table_entries = nr_table_entries;
	lopt->nr_table_entries = nr_table_entries;
	lopt->max_qlen_log = ilog2(nr_table_entries);
	lopt->max_qlen_log = ilog2(nr_table_entries);
@@ -174,7 +181,7 @@ void reqsk_fastopen_remove(struct sock *sk, struct request_sock *req,
	struct sock *lsk = req->rsk_listener;
	struct sock *lsk = req->rsk_listener;
	struct fastopen_queue *fastopenq;
	struct fastopen_queue *fastopenq;


	fastopenq = inet_csk(lsk)->icsk_accept_queue.fastopenq;
	fastopenq = &inet_csk(lsk)->icsk_accept_queue.fastopenq;


	tcp_sk(sk)->fastopen_rsk = NULL;
	tcp_sk(sk)->fastopen_rsk = NULL;
	spin_lock_bh(&fastopenq->lock);
	spin_lock_bh(&fastopenq->lock);
+3 −7
Original line number Original line Diff line number Diff line
@@ -219,17 +219,13 @@ int inet_listen(struct socket *sock, int backlog)
		 * shutdown() (rather than close()).
		 * shutdown() (rather than close()).
		 */
		 */
		if ((sysctl_tcp_fastopen & TFO_SERVER_ENABLE) != 0 &&
		if ((sysctl_tcp_fastopen & TFO_SERVER_ENABLE) != 0 &&
		    !inet_csk(sk)->icsk_accept_queue.fastopenq) {
		    !inet_csk(sk)->icsk_accept_queue.fastopenq.max_qlen) {
			if ((sysctl_tcp_fastopen & TFO_SERVER_WO_SOCKOPT1) != 0)
			if ((sysctl_tcp_fastopen & TFO_SERVER_WO_SOCKOPT1) != 0)
				err = fastopen_init_queue(sk, backlog);
				fastopen_queue_tune(sk, backlog);
			else if ((sysctl_tcp_fastopen &
			else if ((sysctl_tcp_fastopen &
				  TFO_SERVER_WO_SOCKOPT2) != 0)
				  TFO_SERVER_WO_SOCKOPT2) != 0)
				err = fastopen_init_queue(sk,
				fastopen_queue_tune(sk,
				    ((uint)sysctl_tcp_fastopen) >> 16);
				    ((uint)sysctl_tcp_fastopen) >> 16);
			else
				err = 0;
			if (err)
				goto out;


			tcp_fastopen_init_key_once(true);
			tcp_fastopen_init_key_once(true);
		}
		}
+8 −9
Original line number Original line Diff line number Diff line
@@ -335,9 +335,8 @@ struct sock *inet_csk_accept(struct sock *sk, int flags, int *err)


	sk_acceptq_removed(sk);
	sk_acceptq_removed(sk);
	if (sk->sk_protocol == IPPROTO_TCP &&
	if (sk->sk_protocol == IPPROTO_TCP &&
	    tcp_rsk(req)->tfo_listener &&
	    tcp_rsk(req)->tfo_listener) {
	    queue->fastopenq) {
		spin_lock_bh(&queue->fastopenq.lock);
		spin_lock_bh(&queue->fastopenq->lock);
		if (tcp_rsk(req)->tfo_listener) {
		if (tcp_rsk(req)->tfo_listener) {
			/* We are still waiting for the final ACK from 3WHS
			/* We are still waiting for the final ACK from 3WHS
			 * so can't free req now. Instead, we set req->sk to
			 * 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)
			req->sk = NULL;
			req->sk = NULL;
			req = NULL;
			req = NULL;
		}
		}
		spin_unlock_bh(&queue->fastopenq->lock);
		spin_unlock_bh(&queue->fastopenq.lock);
	}
	}
out:
out:
	release_sock(sk);
	release_sock(sk);
@@ -886,12 +885,12 @@ void inet_csk_listen_stop(struct sock *sk)
		sk_acceptq_removed(sk);
		sk_acceptq_removed(sk);
		reqsk_put(req);
		reqsk_put(req);
	}
	}
	if (queue->fastopenq) {
	if (queue->fastopenq.rskq_rst_head) {
		/* Free all the reqs queued in rskq_rst_head. */
		/* Free all the reqs queued in rskq_rst_head. */
		spin_lock_bh(&queue->fastopenq->lock);
		spin_lock_bh(&queue->fastopenq.lock);
		acc_req = queue->fastopenq->rskq_rst_head;
		acc_req = queue->fastopenq.rskq_rst_head;
		queue->fastopenq->rskq_rst_head = NULL;
		queue->fastopenq.rskq_rst_head = NULL;
		spin_unlock_bh(&queue->fastopenq->lock);
		spin_unlock_bh(&queue->fastopenq.lock);
		while ((req = acc_req) != NULL) {
		while ((req = acc_req) != NULL) {
			acc_req = req->dl_next;
			acc_req = req->dl_next;
			reqsk_put(req);
			reqsk_put(req);
Loading