From 010e9a7338e238d026e781dc7443e18dbae94f7d Mon Sep 17 00:00:00 2001 From: Ziye Yang Date: Mon, 11 Feb 2019 14:43:39 +0800 Subject: [PATCH] nvme/tcp: fix the lvol creation failure issue The patch is used to fix issue: https://github.com/spdk/spdk/issues/638 Reason: For supporting sgl, the implementation of function nvme_tcp_pdu_set_data_buf is not correct. The translation is not correct for incapsule data when using SGL. In order not to do the translation via calling sgl function again, we use a variable to store the buf. Change-Id: I580d266d85a1a805b5f168271acac25e5fd60190 Signed-off-by: Ziye Yang Reviewed-on: https://review.gerrithub.io/c/444066 (master) Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/447584 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Ben Walker --- lib/nvme/nvme_tcp.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/lib/nvme/nvme_tcp.c b/lib/nvme/nvme_tcp.c index 743f100a2..cd1ba0931 100644 --- a/lib/nvme/nvme_tcp.c +++ b/lib/nvme/nvme_tcp.c @@ -117,6 +117,7 @@ struct nvme_tcp_req { uint32_t r2tl_remain; bool in_capsule_data; struct nvme_tcp_pdu send_pdu; + void *buf; TAILQ_ENTRY(nvme_tcp_req) link; TAILQ_ENTRY(nvme_tcp_req) active_r2t_link; }; @@ -154,6 +155,7 @@ nvme_tcp_req_get(struct nvme_tcp_qpair *tqpair) tcp_req->req = NULL; tcp_req->in_capsule_data = false; tcp_req->r2tl_remain = 0; + tcp_req->buf = NULL; memset(&tcp_req->send_pdu, 0, sizeof(tcp_req->send_pdu)); TAILQ_INSERT_TAIL(&tqpair->outstanding_reqs, tcp_req, link); @@ -509,14 +511,14 @@ nvme_tcp_qpair_write_pdu(struct nvme_tcp_qpair *tqpair, * Build SGL describing contiguous payload buffer. */ static int -nvme_tcp_build_contig_request(struct nvme_tcp_qpair *tqpair, struct nvme_request *req) +nvme_tcp_build_contig_request(struct nvme_tcp_qpair *tqpair, struct nvme_tcp_req *tcp_req) { - void *payload = req->payload.contig_or_cb_arg + req->payload_offset; + struct nvme_request *req = tcp_req->req; + tcp_req->buf = req->payload.contig_or_cb_arg + req->payload_offset; SPDK_DEBUGLOG(SPDK_LOG_NVME, "enter\n"); assert(nvme_payload_type(&req->payload) == NVME_PAYLOAD_TYPE_CONTIG); - req->cmd.dptr.sgl1.address = (uint64_t)payload; return 0; } @@ -525,11 +527,11 @@ nvme_tcp_build_contig_request(struct nvme_tcp_qpair *tqpair, struct nvme_request * Build SGL describing scattered payload buffer. */ static int -nvme_tcp_build_sgl_request(struct nvme_tcp_qpair *tqpair, struct nvme_request *req) +nvme_tcp_build_sgl_request(struct nvme_tcp_qpair *tqpair, struct nvme_tcp_req *tcp_req) { int rc; - void *virt_addr; uint32_t length; + struct nvme_request *req = tcp_req->req; SPDK_DEBUGLOG(SPDK_LOG_NVME, "enter\n"); @@ -540,7 +542,8 @@ nvme_tcp_build_sgl_request(struct nvme_tcp_qpair *tqpair, struct nvme_request *r req->payload.reset_sgl_fn(req->payload.contig_or_cb_arg, req->payload_offset); /* TODO: for now, we only support a single SGL entry */ - rc = req->payload.next_sge_fn(req->payload.contig_or_cb_arg, &virt_addr, &length); + rc = req->payload.next_sge_fn(req->payload.contig_or_cb_arg, &tcp_req->buf, &length); + if (rc) { return -1; } @@ -550,8 +553,6 @@ nvme_tcp_build_sgl_request(struct nvme_tcp_qpair *tqpair, struct nvme_request *r return -1; } - req->cmd.dptr.sgl1.address = (uint64_t)virt_addr; - return 0; } @@ -578,9 +579,9 @@ nvme_tcp_req_init(struct nvme_tcp_qpair *tqpair, struct nvme_request *req, req->cmd.dptr.sgl1.unkeyed.length = req->payload_size; if (nvme_payload_type(&req->payload) == NVME_PAYLOAD_TYPE_CONTIG) { - rc = nvme_tcp_build_contig_request(tqpair, req); + rc = nvme_tcp_build_contig_request(tqpair, tcp_req); } else if (nvme_payload_type(&req->payload) == NVME_PAYLOAD_TYPE_SGL) { - rc = nvme_tcp_build_sgl_request(tqpair, req); + rc = nvme_tcp_build_sgl_request(tqpair, tcp_req); } else { rc = -1; } @@ -622,14 +623,7 @@ static void nvme_tcp_pdu_set_data_buf(struct nvme_tcp_pdu *pdu, struct nvme_tcp_req *tcp_req) { - /* Here is the tricky, we should consider different NVME data command type: SGL with continue or - scatter data, now we only consider continous data, which is not exactly correct, shoud be fixed */ - if (spdk_unlikely(!tcp_req->req->cmd.dptr.sgl1.address)) { - pdu->data = (void *)tcp_req->req->payload.contig_or_cb_arg + tcp_req->datao; - } else { - pdu->data = (void *)tcp_req->req->cmd.dptr.sgl1.address + tcp_req->datao; - } - + pdu->data = (void *)((uint64_t)tcp_req->buf + tcp_req->datao); } static int