From f62d5ccbe62326cbb75c1a9cfceca991a5937e1b Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Fri, 24 May 2019 10:59:54 +0900 Subject: [PATCH] nvme/tcp: Properly handle multiple iovecs in nvme_tcp_pdu_set_data_buf nvme_tcp_pdu_set_data_buf() has been used to process C2H and H2C for NVMe/TCP initiator. In this case, NVMe/TCP cuts out the part of the input data buffer and transfers the part, and repeats these cut and transfers until the whole data buffer is transferred. NVMe/TCP uses two SGLs, and use one to parse from the offset datao to datao + datal and another to append from the offset 0 to datal. However, the current nvme_tcp_pdu_set_data_buf() had used data_length as not data length of this transfer but total length of the whole transfers by mistake. Recently DIF library updated to properly handle very similar cases, and so this patch takes DIF library as a reference and corrects the implementation. The next patch will add UT code to verify the bug will be fixed. The code size is pretty large and so UT code is separated. Change-Id: Ibeed4de182b8b8740566e874e2757280dc21f9e8 Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/455623 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Changpeng Liu Reviewed-by: Ziye Yang --- lib/nvme/nvme_tcp.c | 53 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/lib/nvme/nvme_tcp.c b/lib/nvme/nvme_tcp.c index 7013bec0c..497f507f3 100644 --- a/lib/nvme/nvme_tcp.c +++ b/lib/nvme/nvme_tcp.c @@ -619,35 +619,68 @@ nvme_tcp_qpair_cmd_send_complete(void *cb_arg) { } +static inline void +_nvme_tcp_sgl_advance(struct _nvme_tcp_sgl *s, uint32_t step) +{ + s->iov_offset += step; + while (s->iovcnt > 0) { + if (s->iov_offset < s->iov->iov_len) { + break; + } + + s->iov_offset -= s->iov->iov_len; + s->iov++; + s->iovcnt--; + } +} + +static inline void +_nvme_tcp_sgl_get_buf(struct _nvme_tcp_sgl *s, void **_buf, uint32_t *_buf_len) +{ + if (_buf != NULL) { + *_buf = s->iov->iov_base + s->iov_offset; + } + if (_buf_len != NULL) { + *_buf_len = s->iov->iov_len - s->iov_offset; + } +} + static void nvme_tcp_pdu_set_data_buf(struct nvme_tcp_pdu *pdu, struct iovec *iov, int iovcnt, uint32_t data_offset, uint32_t data_len) { - uint32_t i, remain_len, len; - struct _nvme_tcp_sgl *pdu_sgl; + uint32_t remain_len, len; + uint8_t *buf; + struct _nvme_tcp_sgl *pdu_sgl, buf_sgl; if (iovcnt == 1) { nvme_tcp_pdu_set_data(pdu, (void *)((uint64_t)iov[0].iov_base + data_offset), data_len); } else { - i = 0; pdu_sgl = &pdu->sgl; - assert(iovcnt <= NVME_TCP_MAX_SGL_DESCRIPTORS); - _nvme_tcp_sgl_init(pdu_sgl, pdu->data_iov, iovcnt, data_offset); + + _nvme_tcp_sgl_init(pdu_sgl, pdu->data_iov, NVME_TCP_MAX_SGL_DESCRIPTORS, 0); + _nvme_tcp_sgl_init(&buf_sgl, iov, iovcnt, 0); + + _nvme_tcp_sgl_advance(&buf_sgl, data_offset); remain_len = data_len; while (remain_len > 0) { - assert(i < NVME_TCP_MAX_SGL_DESCRIPTORS); - len = spdk_min(remain_len, iov[i].iov_len); + _nvme_tcp_sgl_get_buf(&buf_sgl, (void *)&buf, &len); + len = spdk_min(len, remain_len); + + _nvme_tcp_sgl_advance(&buf_sgl, len); remain_len -= len; - if (!_nvme_tcp_sgl_append(pdu_sgl, iov[i].iov_base, len)) { + + if (!_nvme_tcp_sgl_append(pdu_sgl, buf, len)) { break; } - i++; } assert(remain_len == 0); - pdu->data_iovcnt = iovcnt - pdu_sgl->iovcnt; + assert(pdu_sgl->total_size == data_len); + + pdu->data_iovcnt = NVME_TCP_MAX_SGL_DESCRIPTORS - pdu_sgl->iovcnt; pdu->data_len = data_len; } }