[PATCH 51/98] nvmet-tcp: look again after a short read with TLS

[Jens Axboe <axboe@kernel.dk>]

From: Josef Bacik <josef@toxicpanda.com>

The receive helpers return -EAGAIN both for a short read and for a
dropped non-data TLS record, and nvmet_tcp_try_recv() takes that as
"socket empty" and waits for the next data_ready. On plain TCP that's
right. tls_sw_recvmsg() stops where the record type changes though, so
what follows an alert or a handshake record may already be queued and
nobody looks at it until the host sends something else.

Return 1 in those cases so nvmet_tcp_try_recv() counts it against the
budget and tries again until the socket itself says -EAGAIN. While at
it, drain the rest of a dropped record using MSG_EOR instead of pulling
it in PDU-sized pieces, and ratelimit the messages since the host
controls how many there are.

Fixes: a1c5dd8355b1 ("nvmet-tcp: control messages for recvmsg()")
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 | 54 +++++++++++++++++++++++++++++++--------
 1 file changed, 43 insertions(+), 11 deletions(-)

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 261672ab5c1f..a2a5f39d2bb7 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -1173,6 +1173,24 @@ static inline bool nvmet_tcp_pdu_valid(u8 type)
 	return false;
 }
 
+/*
+ * The buffer took as much of the record as the PDU still wants, which may be
+ * one byte.  The rest comes with the type of the record, never with that
+ * which follows: throw it away at once.
+ */
+static void nvmet_tcp_tls_drop_rest(struct nvmet_tcp_queue *queue)
+{
+	char cbuf[CMSG_LEN(sizeof(char))] = {};
+	struct msghdr msg = {
+		.msg_control = cbuf,
+		.msg_controllen = sizeof(cbuf),
+		.msg_flags = MSG_DONTWAIT,
+	};
+
+	iov_iter_discard(&msg.msg_iter, ITER_DEST, TLS_MAX_PAYLOAD_SIZE);
+	sock_recvmsg(queue->sock, &msg, msg.msg_flags);
+}
+
 static int nvmet_tcp_tls_record_ok(struct nvmet_tcp_queue *queue,
 		struct msghdr *msg, char *cbuf, int len)
 {
@@ -1190,26 +1208,38 @@ static int nvmet_tcp_tls_record_ok(struct nvmet_tcp_queue *queue,
 		iov_iter_revert(&msg->msg_iter, len);
 		tls_alert_recv(queue->sock->sk, msg, &level, &description);
 		if (level == TLS_ALERT_LEVEL_FATAL) {
-			pr_err("queue %d: TLS Alert desc %u\n",
-			       queue->idx, description);
+			pr_err_ratelimited("queue %d: TLS Alert desc %u\n",
+					   queue->idx, description);
 			ret = -ENOTCONN;
 		} else {
-			pr_warn("queue %d: TLS Alert desc %u\n",
-			       queue->idx, description);
+			pr_warn_ratelimited("queue %d: TLS Alert desc %u\n",
+					    queue->idx, description);
 			ret = -EAGAIN;
 		}
 		break;
 	default:
 		/* discard this record type */
 		iov_iter_revert(&msg->msg_iter, len);
-		pr_err("queue %d: TLS record %d unhandled\n",
-		       queue->idx, ctype);
+		pr_err_ratelimited("queue %d: TLS record %d unhandled\n",
+				   queue->idx, ctype);
 		ret = -EAGAIN;
 		break;
 	}
+	if (ret == -EAGAIN && !(msg->msg_flags & MSG_EOR))
+		nvmet_tcp_tls_drop_rest(queue);
 	return ret;
 }
 
+/*
+ * A receive from a TLS socket ends with the record, or where the type of the
+ * records changes.  There may be more behind a record that has been dropped,
+ * and nobody will tell: have nvmet_tcp_try_recv() look again.
+ */
+static int nvmet_tcp_tls_again(int ret)
+{
+	return ret == -EAGAIN ? 1 : ret;
+}
+
 static int nvmet_tcp_try_recv_pdu(struct nvmet_tcp_queue *queue)
 {
 	struct nvme_tcp_hdr *hdr = &queue->pdu.cmd.hdr;
@@ -1224,6 +1254,7 @@ static int nvmet_tcp_try_recv_pdu(struct nvmet_tcp_queue *queue)
 	if (queue->tls_pskid) {
 		msg.msg_control = cbuf;
 		msg.msg_controllen = sizeof(cbuf);
+		msg.msg_flags &= ~MSG_EOR;
 	}
 	len = kernel_recvmsg(queue->sock, &msg, &iov, 1,
 			iov.iov_len, msg.msg_flags);
@@ -1232,13 +1263,13 @@ static int nvmet_tcp_try_recv_pdu(struct nvmet_tcp_queue *queue)
 	if (queue->tls_pskid) {
 		ret = nvmet_tcp_tls_record_ok(queue, &msg, cbuf, len);
 		if (ret < 0)
-			return ret;
+			return nvmet_tcp_tls_again(ret);
 	}
 
 	queue->offset += len;
 	queue->left -= len;
 	if (queue->left)
-		return -EAGAIN;
+		return queue->tls_pskid && len ? 1 : -EAGAIN;
 
 	if (queue->offset == sizeof(struct nvme_tcp_hdr)) {
 		u8 hdgst = nvmet_tcp_hdgst_len(queue);
@@ -1290,6 +1321,7 @@ static int nvmet_tcp_try_recv_data(struct nvmet_tcp_queue *queue)
 			/* The last receive has used it up */
 			cmd->recv_msg.msg_control = cmd->recv_cbuf;
 			cmd->recv_msg.msg_controllen = sizeof(cmd->recv_cbuf);
+			cmd->recv_msg.msg_flags &= ~MSG_EOR;
 		}
 		len = sock_recvmsg(cmd->queue->sock, &cmd->recv_msg,
 			cmd->recv_msg.msg_flags);
@@ -1299,7 +1331,7 @@ static int nvmet_tcp_try_recv_data(struct nvmet_tcp_queue *queue)
 			ret = nvmet_tcp_tls_record_ok(cmd->queue,
 					&cmd->recv_msg, cmd->recv_cbuf, len);
 			if (ret < 0)
-				return ret;
+				return nvmet_tcp_tls_again(ret);
 		}
 
 		cmd->pdu_recv += len;
@@ -1340,13 +1372,13 @@ static int nvmet_tcp_try_recv_ddgst(struct nvmet_tcp_queue *queue)
 	if (queue->tls_pskid) {
 		ret = nvmet_tcp_tls_record_ok(queue, &msg, cbuf, len);
 		if (ret < 0)
-			return ret;
+			return nvmet_tcp_tls_again(ret);
 	}
 
 	queue->offset += len;
 	queue->left -= len;
 	if (queue->left)
-		return -EAGAIN;
+		return queue->tls_pskid && len ? 1 : -EAGAIN;
 
 	if (queue->data_digest && cmd->exp_ddgst != cmd->recv_ddgst) {
 		pr_err("queue %d: cmd %d pdu (%d) data digest error: recv %#x expected %#x\n",
-- 
2.56.0