From 3184884f9da54ddfbd0a4dd0b927b273273cbd16 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Wed, 29 May 2019 17:57:50 +0900 Subject: [PATCH] nvmf/tcp: Properly handle multiple iovecs in processing H2C and C2H NVMe/TCP target had assumed the size of each iovec was io_unit_size. Using nvme_tcp_pdu_set_data_buf() instead removes the assumption and supports any alignment transparently. Hence this patch moves nvme_tcp_pdu_set_data_buf() to include/spdk_internal/nvme_tcp.h and replaces the current code to use it. Besides, this patch simplifies spdk_nvmf_tcp_calc_c2h_data_pdu_num() because sum of iov_len of iovecs is equal to the variable length now. We cannot separate code movement (lib/nvme/nvme_tcp.c to include/ spdk_internal/nvme_tcp.h) and code replacement (lib/nvmf/tcp.c) because moved functions are static and compiler give warning if they are not referenced in lib/nvmf/tcp.c. The next patch will add UT code. Change-Id: Iaece5639c6d9a41bd35ee4eb2b75220682dcecd1 Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/455625 Tested-by: SPDK CI Jenkins Reviewed-by: Ziye Yang Reviewed-by: Darek Stojaczyk Reviewed-by: Changpeng Liu --- include/spdk_internal/nvme_tcp.h | 66 ++++++++++++++++++++++++++++++++ lib/nvme/nvme_tcp.c | 66 -------------------------------- lib/nvmf/tcp.c | 30 +++++---------- 3 files changed, 75 insertions(+), 87 deletions(-) diff --git a/include/spdk_internal/nvme_tcp.h b/include/spdk_internal/nvme_tcp.h index a4ed3a97dd..bbc6926abb 100644 --- a/include/spdk_internal/nvme_tcp.h +++ b/include/spdk_internal/nvme_tcp.h @@ -200,6 +200,32 @@ _nvme_tcp_sgl_init(struct _nvme_tcp_sgl *s, struct iovec *iov, int iovcnt, s->total_size = 0; } +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 inline bool _nvme_tcp_sgl_append(struct _nvme_tcp_sgl *s, uint8_t *data, uint32_t data_len) { @@ -419,4 +445,44 @@ nvme_tcp_pdu_set_data(struct nvme_tcp_pdu *pdu, void *data, uint32_t data_len) pdu->data_iovcnt = 1; } +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 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 { + pdu_sgl = &pdu->sgl; + + _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) { + _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, buf, len)) { + break; + } + } + + assert(remain_len == 0); + assert(pdu_sgl->total_size == data_len); + + pdu->data_iovcnt = NVME_TCP_MAX_SGL_DESCRIPTORS - pdu_sgl->iovcnt; + pdu->data_len = data_len; + } +} + #endif /* SPDK_INTERNAL_NVME_TCP_H */ diff --git a/lib/nvme/nvme_tcp.c b/lib/nvme/nvme_tcp.c index 497f507f33..b38a57c329 100644 --- a/lib/nvme/nvme_tcp.c +++ b/lib/nvme/nvme_tcp.c @@ -619,72 +619,6 @@ 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 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 { - pdu_sgl = &pdu->sgl; - - _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) { - _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, buf, len)) { - break; - } - } - - assert(remain_len == 0); - assert(pdu_sgl->total_size == data_len); - - pdu->data_iovcnt = NVME_TCP_MAX_SGL_DESCRIPTORS - pdu_sgl->iovcnt; - pdu->data_len = data_len; - } -} - static int nvme_tcp_qpair_capsule_cmd_send(struct nvme_tcp_qpair *tqpair, struct nvme_tcp_req *tcp_req) diff --git a/lib/nvmf/tcp.c b/lib/nvmf/tcp.c index 16c725f413..3633aa9d5e 100644 --- a/lib/nvmf/tcp.c +++ b/lib/nvmf/tcp.c @@ -1348,7 +1348,6 @@ spdk_nvmf_tcp_h2c_data_hdr_handle(struct spdk_nvmf_tcp_transport *ttransport, uint32_t error_offset = 0; enum spdk_nvme_tcp_term_req_fes fes = 0; struct spdk_nvme_tcp_h2c_data_hdr *h2c_data; - uint32_t iov_index; bool ttag_offset_error = false; h2c_data = &pdu->hdr.h2c_data; @@ -1403,9 +1402,8 @@ spdk_nvmf_tcp_h2c_data_hdr_handle(struct spdk_nvmf_tcp_transport *ttransport, } pdu->ctx = tcp_req; - iov_index = pdu->hdr.h2c_data.datao / ttransport->transport.opts.io_unit_size; - nvme_tcp_pdu_set_data(pdu, tcp_req->req.iov[iov_index].iov_base + (pdu->hdr.h2c_data.datao % - ttransport->transport.opts.io_unit_size), h2c_data->datal); + nvme_tcp_pdu_set_data_buf(pdu, tcp_req->req.iov, tcp_req->req.iovcnt, + h2c_data->datao, h2c_data->datal); spdk_nvmf_tcp_qpair_set_recv_state(tqpair, NVME_TCP_PDU_RECV_STATE_AWAIT_PDU_PAYLOAD); return; @@ -2185,14 +2183,10 @@ spdk_nvmf_tcp_send_c2h_data(struct spdk_nvmf_tcp_qpair *tqpair, { struct nvme_tcp_pdu *rsp_pdu; struct spdk_nvme_tcp_c2h_data_hdr *c2h_data; - uint32_t plen, pdo, alignment, offset, iov_index; + uint32_t plen, pdo, alignment; SPDK_DEBUGLOG(SPDK_LOG_NVMF_TCP, "enter\n"); - /* always use the first iov_len, which is correct */ - iov_index = tcp_req->c2h_data_offset / tcp_req->req.iov[0].iov_len; - offset = tcp_req->c2h_data_offset % tcp_req->req.iov[0].iov_len; - rsp_pdu = spdk_nvmf_tcp_pdu_get(tqpair); assert(rsp_pdu != NULL); @@ -2208,7 +2202,7 @@ spdk_nvmf_tcp_send_c2h_data(struct spdk_nvmf_tcp_qpair *tqpair, /* set the psh */ c2h_data->cccid = tcp_req->req.cmd->nvme_cmd.cid; c2h_data->datal = spdk_min(NVMF_TCP_PDU_MAX_C2H_DATA_SIZE, - (tcp_req->req.iov[iov_index].iov_len - offset)); + tcp_req->req.length - tcp_req->c2h_data_offset); c2h_data->datao = tcp_req->c2h_data_offset; /* set the padding */ @@ -2231,10 +2225,11 @@ spdk_nvmf_tcp_send_c2h_data(struct spdk_nvmf_tcp_qpair *tqpair, c2h_data->common.plen = plen; - nvme_tcp_pdu_set_data(rsp_pdu, tcp_req->req.iov[iov_index].iov_base + offset, c2h_data->datal); + nvme_tcp_pdu_set_data_buf(rsp_pdu, tcp_req->req.iov, tcp_req->req.iovcnt, + c2h_data->datao, c2h_data->datal); tcp_req->c2h_data_offset += c2h_data->datal; - if (iov_index == (tcp_req->req.iovcnt - 1) && (tcp_req->c2h_data_offset == tcp_req->req.length)) { + if (tcp_req->c2h_data_offset == tcp_req->req.length) { SPDK_DEBUGLOG(SPDK_LOG_NVMF_TCP, "Last pdu for tcp_req=%p on tqpair=%p\n", tcp_req, tqpair); c2h_data->common.flags |= SPDK_NVME_TCP_C2H_DATA_FLAGS_LAST_PDU; if (tqpair->qpair.transport->opts.c2h_success) { @@ -2250,15 +2245,8 @@ spdk_nvmf_tcp_send_c2h_data(struct spdk_nvmf_tcp_qpair *tqpair, static int spdk_nvmf_tcp_calc_c2h_data_pdu_num(struct spdk_nvmf_tcp_req *tcp_req) { - uint32_t i, iov_cnt, pdu_num = 0; - - iov_cnt = tcp_req->req.iovcnt; - for (i = 0; i < iov_cnt; i++) { - pdu_num += (tcp_req->req.iov[i].iov_len + NVMF_TCP_PDU_MAX_C2H_DATA_SIZE - 1) / - NVMF_TCP_PDU_MAX_C2H_DATA_SIZE; - } - - return pdu_num; + return (tcp_req->req.length + NVMF_TCP_PDU_MAX_C2H_DATA_SIZE - 1) / + NVMF_TCP_PDU_MAX_C2H_DATA_SIZE; } static void