From cb448c1bd7ff328a9b6604e4f4d9513bfe661b3c Mon Sep 17 00:00:00 2001 From: Ben Walker Date: Fri, 27 Sep 2019 14:19:19 -0700 Subject: [PATCH] Revert "nvmf/tcp: Remove the potential pdu hdr memory copy." This reverts commit 5e7b8d18f3287c88f24d27686e8e0b76deccbdd3. This code is moving from the nvmf target down into the posix sock layer in this series. Change-Id: Iea9a7cef5bedd6a34edf7b4c87825897279829c3 Signed-off-by: Ben Walker Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/444 Tested-by: SPDK CI Jenkins Reviewed-by: Shuhei Matsumoto Reviewed-by: Jim Harris --- lib/nvmf/tcp.c | 56 ++++++++++++++------------------------------------ 1 file changed, 15 insertions(+), 41 deletions(-) diff --git a/lib/nvmf/tcp.c b/lib/nvmf/tcp.c index 8cc239951..d1f18fa89 100644 --- a/lib/nvmf/tcp.c +++ b/lib/nvmf/tcp.c @@ -838,7 +838,7 @@ spdk_nvmf_tcp_qpair_init_mem_resource(struct spdk_nvmf_tcp_qpair *tqpair) tqpair->pdu_recv_buf.size); return -1; } - tqpair->pdu_in_progress.hdr = (union nvme_tcp_pdu_hdr *)tqpair->pdu_recv_buf.buf; + tqpair->pdu_in_progress.hdr = &tqpair->pdu_in_progress.hdr_mem; return 0; } @@ -862,6 +862,8 @@ spdk_nvmf_tcp_qpair_init(struct spdk_nvmf_qpair *qpair) tqpair->host_hdgst_enable = true; tqpair->host_ddgst_enable = true; + tqpair->pdu_in_progress.hdr = &tqpair->pdu_in_progress.hdr_mem; + return 0; } @@ -1022,30 +1024,6 @@ spdk_nvmf_tcp_poll_group_destroy(struct spdk_nvmf_transport_poll_group *group) free(tgroup); } -static inline void -spdk_nvmf_tcp_reset_pdu_in_process(struct spdk_nvmf_tcp_qpair *tqpair) -{ - struct nvme_tcp_pdu_recv_buf *pdu_recv_buf = &tqpair->pdu_recv_buf; - char *dst, *src; - - if (spdk_unlikely((pdu_recv_buf->off + sizeof(union nvme_tcp_pdu_hdr)) > - pdu_recv_buf->size)) { - if (pdu_recv_buf->remain_size) { - dst = pdu_recv_buf->buf; - src = (char *)((void *)pdu_recv_buf->buf + pdu_recv_buf->off); - - /* purpose: to avoid overlap copy, so do not use memcpy if there is overlap case */ - memmove(dst, src, pdu_recv_buf->remain_size); - } - tqpair->pdu_recv_buf.off = 0; - } else if (!pdu_recv_buf->remain_size) { - tqpair->pdu_recv_buf.off = 0; - } - - tqpair->pdu_in_progress.hdr = (union nvme_tcp_pdu_hdr *)((void *)pdu_recv_buf->buf + - pdu_recv_buf->off); -} - static void spdk_nvmf_tcp_qpair_set_recv_state(struct spdk_nvmf_tcp_qpair *tqpair, enum nvme_tcp_pdu_recv_state state) @@ -1077,7 +1055,7 @@ spdk_nvmf_tcp_qpair_set_recv_state(struct spdk_nvmf_tcp_qpair *tqpair, case NVME_TCP_PDU_RECV_STATE_ERROR: case NVME_TCP_PDU_RECV_STATE_AWAIT_PDU_READY: memset(&tqpair->pdu_in_progress, 0, sizeof(tqpair->pdu_in_progress)); - spdk_nvmf_tcp_reset_pdu_in_process(tqpair); + tqpair->pdu_in_progress.hdr = &tqpair->pdu_in_progress.hdr_mem; break; default: SPDK_ERRLOG("The state(%d) is invalid\n", state); @@ -1737,8 +1715,10 @@ nvme_tcp_recv_buf_read(struct spdk_sock *sock, struct nvme_tcp_pdu_recv_buf *pdu { int rc; - rc = nvme_tcp_read_data(sock, pdu_recv_buf->size - pdu_recv_buf->off, - (void *)pdu_recv_buf->buf + pdu_recv_buf->off); + assert(pdu_recv_buf->off == 0); + assert(pdu_recv_buf->remain_size == 0); + rc = nvme_tcp_read_data(sock, pdu_recv_buf->size, + pdu_recv_buf->buf); if (rc < 0) { SPDK_DEBUGLOG(SPDK_LOG_NVMF_TCP, "will disconnect sock=%p\n", sock); } else if (rc > 0) { @@ -1758,12 +1738,12 @@ nvme_tcp_read_data_from_pdu_recv_buf(struct nvme_tcp_pdu_recv_buf *pdu_recv_buf, assert(pdu_recv_buf->remain_size > 0); size = spdk_min(expected_size, pdu_recv_buf->remain_size); - if (dst) { - memcpy(dst, (void *)pdu_recv_buf->buf + pdu_recv_buf->off, size); - } + memcpy(dst, (void *)pdu_recv_buf->buf + pdu_recv_buf->off, size); pdu_recv_buf->off += size; pdu_recv_buf->remain_size -= size; - + if (spdk_unlikely(!pdu_recv_buf->remain_size)) { + pdu_recv_buf->off = 0; + } return size; } @@ -1775,7 +1755,6 @@ nvme_tcp_read_payload_data_from_pdu_recv_buf(struct nvme_tcp_pdu_recv_buf *pdu_r struct iovec iov[NVME_TCP_MAX_SGL_DESCRIPTORS + 1]; int iovcnt, i; uint32_t size = 0; - void *dst; assert(pdu_recv_buf->remain_size > 0); iovcnt = nvme_tcp_build_payload_iovs(iov, NVME_TCP_MAX_SGL_DESCRIPTORS + 1, pdu, @@ -1785,12 +1764,7 @@ nvme_tcp_read_payload_data_from_pdu_recv_buf(struct nvme_tcp_pdu_recv_buf *pdu_r if (!pdu_recv_buf->remain_size) { break; } - - dst = NULL; - if (pdu->hdr->common.pdu_type != SPDK_NVME_TCP_PDU_TYPE_H2C_TERM_REQ) { - dst = iov[i].iov_base; - } - size += nvme_tcp_read_data_from_pdu_recv_buf(pdu_recv_buf, iov[i].iov_len, dst); + size += nvme_tcp_read_data_from_pdu_recv_buf(pdu_recv_buf, iov[i].iov_len, iov[i].iov_base); } return size; @@ -1828,7 +1802,7 @@ spdk_nvmf_tcp_sock_process(struct spdk_nvmf_tcp_qpair *tqpair) } rc = nvme_tcp_read_data_from_pdu_recv_buf(&tqpair->pdu_recv_buf, sizeof(struct spdk_nvme_tcp_common_pdu_hdr) - pdu->ch_valid_bytes, - NULL); + (void *)&pdu->hdr->common + pdu->ch_valid_bytes); pdu->ch_valid_bytes += rc; if (spdk_likely(tqpair->recv_state == NVME_TCP_PDU_RECV_STATE_AWAIT_PDU_READY)) { spdk_nvmf_tcp_qpair_set_recv_state(tqpair, NVME_TCP_PDU_RECV_STATE_AWAIT_PDU_CH); @@ -1852,7 +1826,7 @@ spdk_nvmf_tcp_sock_process(struct spdk_nvmf_tcp_qpair *tqpair) rc = nvme_tcp_read_data_from_pdu_recv_buf(&tqpair->pdu_recv_buf, pdu->psh_len - pdu->psh_valid_bytes, - NULL); + (void *)&pdu->hdr->raw + sizeof(struct spdk_nvme_tcp_common_pdu_hdr) + pdu->psh_valid_bytes); pdu->psh_valid_bytes += rc; if (pdu->psh_valid_bytes < pdu->psh_len) { return NVME_TCP_PDU_IN_PROGRESS;