[PATCH 70/98] nvmet-tcp: don't wait for the host in the accept work

[Jens Axboe <axboe@kernel.dk>]

From: Josef Bacik <josef@toxicpanda.com>

On a port that offers TLS but doesn't require it,
nvmet_tcp_alloc_queue() peeks at the first bytes of a new connection to
tell an ICReq from a TLS handshake. That receive blocks until the host
sends something or closes, and it runs in the accept work, of which
there is one per port. A connection that stays silent blocks every
other connect on that port for as long as it's held open, no secret
needed. And nvmet_tcp_remove_port() waits for the accept work with
nvmet_config_sem held for writing, so an admin taking the port down
hangs there and no port of the target answers a Connect anymore.

Install the socket callbacks first and peek without blocking from the
io work, which data_ready() reruns. If it's a handshake, restore the
default callbacks for the upcall as before, and make the upcall from
one more run of the io work so no run is left that could steal data
from the handshake. The state change happens under state_lock and only
if the queue isn't already going away. The io work doesn't hold the
port like the accept work did, so note the keyring serial, which is all
the upcall needs from the port, at allocation.

A partially arrived header used to be taken for "no TLS", now the rest
is waited for. A silent connection still holds its queue, as on a port
without TLS.

Fixes: 70525e5d82f6 ("nvmet-tcp: peek icreq before starting TLS")
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 drivers/nvme/target/tcp.c | 67 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 59 insertions(+), 8 deletions(-)

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index c8f0c52a1b95..3d24efe3b7c3 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -140,6 +140,7 @@ struct nvmet_tcp_cmd {
 
 enum nvmet_tcp_queue_state {
 	NVMET_TCP_Q_CONNECTING,
+	NVMET_TCP_Q_TLS_PEEK,
 	NVMET_TCP_Q_TLS_HANDSHAKE,
 	NVMET_TCP_Q_LIVE,
 	NVMET_TCP_Q_DISCONNECTING,
@@ -176,6 +177,7 @@ struct nvmet_tcp_queue {
 
 	/* TLS state */
 	key_serial_t		tls_pskid;
+	key_serial_t		tls_keyring;
 	struct delayed_work	tls_handshake_tmo_work;
 
 	unsigned long           poll_end;
@@ -1496,6 +1498,8 @@ static bool nvmet_tcp_check_queue_deadline(struct nvmet_tcp_queue *queue,
 	return !time_after(jiffies, queue->poll_end);
 }
 
+static bool nvmet_tcp_tls_peek(struct nvmet_tcp_queue *queue);
+
 static void nvmet_tcp_io_work(struct work_struct *w)
 {
 	struct nvmet_tcp_queue *queue =
@@ -1503,6 +1507,11 @@ static void nvmet_tcp_io_work(struct work_struct *w)
 	bool pending;
 	int ret, ops = 0;
 
+	if ((queue->state == NVMET_TCP_Q_TLS_PEEK ||
+	     queue->state == NVMET_TCP_Q_TLS_HANDSHAKE) &&
+	    !nvmet_tcp_tls_peek(queue))
+		return;
+
 	do {
 		pending = false;
 
@@ -1828,12 +1837,9 @@ static int nvmet_tcp_try_peek_pdu(struct nvmet_tcp_queue *queue)
 	struct msghdr msg = {
 		.msg_control = cbuf,
 		.msg_controllen = sizeof(cbuf),
-		.msg_flags = MSG_PEEK,
+		.msg_flags = MSG_PEEK | MSG_DONTWAIT,
 	};
 
-	if (nvmet_port_secure_channel_required(queue->port->nport))
-		return 0;
-
 	len = kernel_recvmsg(queue->sock, &msg, &iov, 1,
 			iov.iov_len, msg.msg_flags);
 	if (unlikely(len < 0)) {
@@ -1955,7 +1961,7 @@ static int nvmet_tcp_tls_handshake(struct nvmet_tcp_queue *queue)
 	args.ta_sock = queue->sock;
 	args.ta_done = nvmet_tcp_tls_handshake_done;
 	args.ta_data = queue;
-	args.ta_keyring = key_serial(queue->port->nport->keyring);
+	args.ta_keyring = queue->tls_keyring;
 	args.ta_timeout_ms = tls_handshake_timeout * 1000;
 
 	ret = tls_server_hello_psk(&args, GFP_KERNEL);
@@ -1968,8 +1974,51 @@ static int nvmet_tcp_tls_handshake(struct nvmet_tcp_queue *queue)
 	}
 	return ret;
 }
+
+/*
+ * TLS is offered, but not required: the first bytes tell what the host wants.
+ * Returns true if the queue goes on without TLS.
+ */
+static bool nvmet_tcp_tls_peek(struct nvmet_tcp_queue *queue)
+{
+	int ret;
+
+	if (queue->state == NVMET_TCP_Q_TLS_HANDSHAKE) {
+		if (nvmet_tcp_tls_handshake(queue))
+			nvmet_tcp_schedule_release_queue(queue);
+		return false;
+	}
+
+	/* ->data_ready() brings us back */
+	ret = nvmet_tcp_try_peek_pdu(queue);
+	if (ret == -EAGAIN)
+		return false;
+
+	/* The upcall wants the default callbacks */
+	if (!ret)
+		nvmet_tcp_restore_socket_callbacks(queue);
+
+	spin_lock_bh(&queue->state_lock);
+	if (queue->state != NVMET_TCP_Q_TLS_PEEK) {
+		spin_unlock_bh(&queue->state_lock);
+		return false;
+	}
+	queue->state = ret ? NVMET_TCP_Q_CONNECTING : NVMET_TCP_Q_TLS_HANDSHAKE;
+	spin_unlock_bh(&queue->state_lock);
+	if (ret)
+		return true;
+
+	/*
+	 * ->data_ready() may have queued the io work meanwhile, nothing does
+	 * from now on. The handshake must not start before that run is over,
+	 * so leave it to that run.
+	 */
+	queue_work_on(queue_cpu(queue), nvmet_tcp_wq, &queue->io_work);
+	return false;
+}
 #else
 static void nvmet_tcp_tls_handshake_timeout(struct work_struct *w) {}
+static bool nvmet_tcp_tls_peek(struct nvmet_tcp_queue *queue) { return true; }
 #endif
 
 static void nvmet_tcp_alloc_queue(struct nvmet_tcp_port *port,
@@ -2039,14 +2088,16 @@ static void nvmet_tcp_alloc_queue(struct nvmet_tcp_port *port,
 		sk->sk_user_data = NULL;
 		sk->sk_data_ready = port->data_ready;
 		write_unlock_bh(&sk->sk_callback_lock);
-		if (!nvmet_tcp_try_peek_pdu(queue)) {
+		/* The port may be gone when the upcall is made */
+		queue->tls_keyring = key_serial(port->nport->keyring);
+		if (nvmet_port_secure_channel_required(port->nport)) {
 			if (!nvmet_tcp_tls_handshake(queue))
 				return;
 			/* TLS handshake failed, terminate the connection */
 			goto out_destroy_sq;
 		}
-		/* Not a TLS connection, continue with normal processing */
-		queue->state = NVMET_TCP_Q_CONNECTING;
+		/* Don't wait for the host here: others want to connect, too */
+		queue->state = NVMET_TCP_Q_TLS_PEEK;
 	}
 #endif
 
-- 
2.56.0