nvme/tcp: optimize nvme_tcp_build_iovecs function.

Borrow the ideas from iSCSI and optimize
the nvme_tcp_build_iovecs function.

Change-Id: I19b165b5f6dc34b4bf655157170dec5c2ce3e19a
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446836
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>

include/spdk_internal/nvme_tcp.h

@@ -137,6 +137,14 @@ enum nvme_tcp_qpair_state {
 	NVME_TCP_QPAIR_STATE_EXITED = 3,
 };
 
+struct _iov_ctx {
+	struct iovec    *iov;
+	int             num_iovs;
+	uint32_t        iov_offset;
+	int             iovcnt;
+	uint32_t        mapped_len;
+};
+
 static uint32_t
 nvme_tcp_pdu_calc_header_digest(struct nvme_tcp_pdu *pdu)
 {
@@ -173,17 +181,52 @@ nvme_tcp_pdu_calc_data_digest(struct nvme_tcp_pdu *pdu)
 	return crc32c;
 }
 
-static int
+static inline void
-nvme_tcp_build_iovecs(struct iovec *iovec, struct nvme_tcp_pdu *pdu,
+_iov_ctx_init(struct _iov_ctx *ctx, struct iovec *iovs, int num_iovs,
-		      bool hdgst_enable, bool ddgst_enable)
+	      uint32_t iov_offset)
 {
+	ctx->iov = iovs;
+	ctx->num_iovs = num_iovs;
+	ctx->iov_offset = iov_offset;
+	ctx->iovcnt = 0;
+	ctx->mapped_len = 0;
+}
 
-	int iovec_cnt = 0;
+static inline bool
+_iov_ctx_set_iov(struct _iov_ctx *ctx, uint8_t *data, uint32_t data_len)
+{
+	if (ctx->iov_offset >= data_len) {
+		ctx->iov_offset -= data_len;
+	} else {
+		ctx->iov->iov_base = data + ctx->iov_offset;
+		ctx->iov->iov_len = data_len - ctx->iov_offset;
+		ctx->mapped_len += data_len - ctx->iov_offset;
+		ctx->iov_offset = 0;
+		ctx->iov++;
+		ctx->iovcnt++;
+		if (ctx->iovcnt == ctx->num_iovs) {
+			return false;
+		}
+	}
+
+	return true;
+}
+
+static int
+nvme_tcp_build_iovecs(struct iovec *iovec, int num_iovs, struct nvme_tcp_pdu *pdu,
+		      bool hdgst_enable, bool ddgst_enable, uint32_t *_mapped_length)
+{
+	struct _iov_ctx ctx;
 	int enable_digest;
-	int hlen;
+	uint32_t hlen, plen;
-	uint32_t plen;
 
+	if (num_iovs == 0) {
+		return 0;
+	}
+
+	_iov_ctx_init(&ctx, iovec, num_iovs, pdu->writev_offset);
 	hlen = pdu->hdr.common.hlen;
+	plen = hlen;
 	enable_digest = 1;
 	if (pdu->hdr.common.pdu_type == SPDK_NVME_TCP_PDU_TYPE_IC_REQ ||
 	    pdu->hdr.common.pdu_type == SPDK_NVME_TCP_PDU_TYPE_IC_RESP ||
@@ -198,39 +241,45 @@ nvme_tcp_build_iovecs(struct iovec *iovec, struct nvme_tcp_pdu *pdu,
 		hlen += SPDK_NVME_TCP_DIGEST_LEN;
 	}
 
-	/* PDU header + possible header digest */
-	iovec[iovec_cnt].iov_base = &pdu->hdr.raw;
-	iovec[iovec_cnt].iov_len = hlen;
-	plen = iovec[iovec_cnt].iov_len;
-	iovec_cnt++;
-
 	if (!pdu->data_len || !pdu->data) {
+		/* PDU header + possible header digest */
+		_iov_ctx_set_iov(&ctx, (uint8_t *)&pdu->hdr.raw, hlen);
 		goto end;
 	}
 
-	/* Padding  */
+	/* Padding */
 	if (pdu->padding_len > 0) {
-		iovec[iovec_cnt - 1].iov_len += pdu->padding_len;
+		hlen += pdu->padding_len;
-		plen = iovec[iovec_cnt - 1].iov_len;
+		plen = hlen;
+	}
+
+	if (!_iov_ctx_set_iov(&ctx, (uint8_t *)&pdu->hdr.raw, hlen)) {
+		goto end;
 	}
 
 	/* Data Segment */
-	iovec[iovec_cnt].iov_base = pdu->data;
+	plen += pdu->data_len;
-	iovec[iovec_cnt].iov_len = pdu->data_len;
+	if (!_iov_ctx_set_iov(&ctx, pdu->data, pdu->data_len)) {
-	plen += iovec[iovec_cnt].iov_len;
+		goto end;
-	iovec_cnt++;
+	}
 
 	/* Data Digest */
 	if (enable_digest && ddgst_enable) {
-		iovec[iovec_cnt].iov_base = pdu->data_digest;
+		plen += SPDK_NVME_TCP_DIGEST_LEN;
-		iovec[iovec_cnt].iov_len = SPDK_NVME_TCP_DIGEST_LEN;
+		_iov_ctx_set_iov(&ctx, pdu->data_digest, SPDK_NVME_TCP_DIGEST_LEN);
-		plen += iovec[iovec_cnt].iov_len;
-		iovec_cnt++;
 	}
 
 end:
-	assert(plen == pdu->hdr.common.plen);
+	if (_mapped_length != NULL) {
-	return iovec_cnt;
+		*_mapped_length = ctx.mapped_len;
+	}
+
+	/* check the plen for the first time constructing iov */
+	if (!pdu->writev_offset) {
+		assert(plen == pdu->hdr.common.plen);
+	}
+
+	return ctx.iovcnt;
 }
 
 static int

lib/nvme/nvme_tcp.c

@@ -380,7 +380,7 @@ nvme_tcp_qpair_process_send_queue(struct nvme_tcp_qpair *tqpair)
 	struct iovec	*iov = iovec_array;
 	int iovec_cnt = 0;
 	int bytes = 0;
-	uint32_t writev_offset;
+	uint32_t mapped_length;
 	struct nvme_tcp_pdu *pdu;
 	int pdu_length;
 	TAILQ_HEAD(, nvme_tcp_pdu) completed_pdus_list;
@@ -396,30 +396,12 @@ nvme_tcp_qpair_process_send_queue(struct nvme_tcp_qpair *tqpair)
 	 *  tqpair 's send_queue.
 	 */
 	while (pdu != NULL && ((array_size - iovec_cnt) >= 3)) {
-		iovec_cnt += nvme_tcp_build_iovecs(&iovec_array[iovec_cnt],
+		iovec_cnt += nvme_tcp_build_iovecs(&iovec_array[iovec_cnt], array_size - iovec_cnt,
 						   pdu, tqpair->host_hdgst_enable,
-						   tqpair->host_ddgst_enable);
+						   tqpair->host_ddgst_enable, &mapped_length);
 		pdu = TAILQ_NEXT(pdu, tailq);
 	}
 
-	/*
-	 * Check if the first PDU was partially written out the last time
-	 *  this function was called, and if so adjust the iovec array
-	 *  accordingly.
-	 */
-	writev_offset = TAILQ_FIRST(&tqpair->send_queue)->writev_offset;
-	while ((writev_offset > 0) && (iovec_cnt > 0)) {
-		if (writev_offset >= iov->iov_len) {
-			writev_offset -= iov->iov_len;
-			iov++;
-			iovec_cnt--;
-		} else {
-			iov->iov_len -= writev_offset;
-			iov->iov_base = (char *)iov->iov_base + writev_offset;
-			writev_offset = 0;
-		}
-	}
-
 	bytes = spdk_sock_writev(tqpair->sock, iov, iovec_cnt);
 	SPDK_DEBUGLOG(SPDK_LOG_NVME, "bytes=%d are out\n", bytes);
 	if (bytes == -1) {

lib/nvmf/tcp.c

@@ -773,7 +773,7 @@ spdk_nvmf_tcp_qpair_flush_pdus_internal(struct spdk_nvmf_tcp_qpair *tqpair)
 	int iovec_cnt = 0;
 	int bytes = 0;
 	int total_length = 0;
-	uint32_t writev_offset;
+	uint32_t mapped_length;
 	struct nvme_tcp_pdu *pdu;
 	int pdu_length;
 	TAILQ_HEAD(, nvme_tcp_pdu) completed_pdus_list;
@@ -791,32 +791,15 @@ spdk_nvmf_tcp_qpair_flush_pdus_internal(struct spdk_nvmf_tcp_qpair *tqpair)
 	 */
 	while (pdu != NULL && ((array_size - iovec_cnt) >= 3)) {
 		iovec_cnt += nvme_tcp_build_iovecs(&iovec_array[iovec_cnt],
+						   array_size - iovec_cnt,
 						   pdu,
 						   tqpair->host_hdgst_enable,
-						   tqpair->host_ddgst_enable);
+						   tqpair->host_ddgst_enable,
-		total_length += pdu->hdr.common.plen;
+						   &mapped_length);
+		total_length += mapped_length;
 		pdu = TAILQ_NEXT(pdu, tailq);
 	}
 
-	/*
-	 * Check if the first PDU was partially written out the last time
-	 *  this function was called, and if so adjust the iovec array
-	 *  accordingly.
-	 */
-	writev_offset = TAILQ_FIRST(&tqpair->send_queue)->writev_offset;
-	total_length -= writev_offset;
-	while ((writev_offset > 0) && (iovec_cnt > 0)) {
-		if (writev_offset >= iov->iov_len) {
-			writev_offset -= iov->iov_len;
-			iov++;
-			iovec_cnt--;
-		} else {
-			iov->iov_len -= writev_offset;
-			iov->iov_base = (char *)iov->iov_base + writev_offset;
-			writev_offset = 0;
-		}
-	}
-
 	spdk_trace_record(TRACE_TCP_FLUSH_WRITEBUF_START, 0, total_length, 0, iovec_cnt);
 
 	bytes = spdk_sock_writev(tqpair->sock, iov, iovec_cnt);