From 427cbb46a3a95e434499b2077a3e1b77360dc903 Mon Sep 17 00:00:00 2001 From: MengjinWu Date: Fri, 1 Jul 2022 12:37:33 +0800 Subject: [PATCH] lib/nvmf: optimize the performance for h2c handle It will not find the h2c related reqs in the tailq now. We can get it from tqpair->reqs directly. Signed-off-by: MengjinWu Change-Id: I25f0900e875b054d7617450477e9719e7a59aa18 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/12861 Tested-by: SPDK CI Jenkins Community-CI: Broadcom CI Reviewed-by: Shuhei Matsumoto Reviewed-by: Aleksey Marchuk --- lib/nvmf/tcp.c | 67 ++++++++++--------------------- test/unit/lib/nvmf/tcp.c/tcp_ut.c | 16 ++------ 2 files changed, 26 insertions(+), 57 deletions(-) diff --git a/lib/nvmf/tcp.c b/lib/nvmf/tcp.c index a1e9e9dc8..c705ec783 100644 --- a/lib/nvmf/tcp.c +++ b/lib/nvmf/tcp.c @@ -1587,37 +1587,6 @@ err: nvmf_tcp_send_c2h_term_req(tqpair, pdu, fes, error_offset); } -static int -nvmf_tcp_find_req_in_state(struct spdk_nvmf_tcp_qpair *tqpair, - enum spdk_nvmf_tcp_req_state state, - uint16_t cid, uint16_t tag, - struct spdk_nvmf_tcp_req **req) -{ - struct spdk_nvmf_tcp_req *tcp_req = NULL; - - TAILQ_FOREACH(tcp_req, &tqpair->tcp_req_working_queue, state_link) { - if (tcp_req->state != state) { - continue; - } - - if (tcp_req->req.cmd->nvme_cmd.cid != cid) { - continue; - } - - if (tcp_req->ttag == tag) { - *req = tcp_req; - return 0; - } - - *req = NULL; - return -1; - } - - /* Didn't find it, but not an error */ - *req = NULL; - return 0; -} - static void nvmf_tcp_h2c_data_hdr_handle(struct spdk_nvmf_tcp_transport *ttransport, struct spdk_nvmf_tcp_qpair *tqpair, @@ -1627,28 +1596,36 @@ 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; - int rc; h2c_data = &pdu->hdr.h2c_data; SPDK_DEBUGLOG(nvmf_tcp, "tqpair=%p, r2t_info: datao=%u, datal=%u, cccid=%u, ttag=%u\n", tqpair, h2c_data->datao, h2c_data->datal, h2c_data->cccid, h2c_data->ttag); - rc = nvmf_tcp_find_req_in_state(tqpair, TCP_REQUEST_STATE_TRANSFERRING_HOST_TO_CONTROLLER, - h2c_data->cccid, h2c_data->ttag, &tcp_req); - if (rc == 0 && tcp_req == NULL) { - rc = nvmf_tcp_find_req_in_state(tqpair, TCP_REQUEST_STATE_AWAITING_R2T_ACK, h2c_data->cccid, - h2c_data->ttag, &tcp_req); + if (h2c_data->ttag > tqpair->resource_count) { + SPDK_DEBUGLOG(nvmf_tcp, "ttag %u is larger than allowed %u.\n", h2c_data->ttag, + tqpair->resource_count); + fes = SPDK_NVME_TCP_TERM_REQ_FES_PDU_SEQUENCE_ERROR; + error_offset = offsetof(struct spdk_nvme_tcp_h2c_data_hdr, ttag); + goto err; } - if (!tcp_req) { - SPDK_DEBUGLOG(nvmf_tcp, "tcp_req is not found for tqpair=%p\n", tqpair); - fes = SPDK_NVME_TCP_TERM_REQ_FES_INVALID_DATA_UNSUPPORTED_PARAMETER; - if (rc == 0) { - error_offset = offsetof(struct spdk_nvme_tcp_h2c_data_hdr, cccid); - } else { - error_offset = offsetof(struct spdk_nvme_tcp_h2c_data_hdr, ttag); - } + tcp_req = &tqpair->reqs[h2c_data->ttag - 1]; + + if (spdk_unlikely(tcp_req->state != TCP_REQUEST_STATE_TRANSFERRING_HOST_TO_CONTROLLER && + tcp_req->state != TCP_REQUEST_STATE_AWAITING_R2T_ACK)) { + SPDK_DEBUGLOG(nvmf_tcp, "tcp_req(%p), tqpair=%p, has error state in %d\n", tcp_req, tqpair, + tcp_req->state); + fes = SPDK_NVME_TCP_TERM_REQ_FES_INVALID_HEADER_FIELD; + error_offset = offsetof(struct spdk_nvme_tcp_h2c_data_hdr, ttag); + goto err; + } + + if (spdk_unlikely(tcp_req->req.cmd->nvme_cmd.cid != h2c_data->cccid)) { + SPDK_DEBUGLOG(nvmf_tcp, "tcp_req(%p), tqpair=%p, expected %u but %u for cccid.\n", tcp_req, tqpair, + tcp_req->req.cmd->nvme_cmd.cid, h2c_data->cccid); + fes = SPDK_NVME_TCP_TERM_REQ_FES_PDU_SEQUENCE_ERROR; + error_offset = offsetof(struct spdk_nvme_tcp_h2c_data_hdr, cccid); goto err; } diff --git a/test/unit/lib/nvmf/tcp.c/tcp_ut.c b/test/unit/lib/nvmf/tcp.c/tcp_ut.c index 2db444b57..d49f69644 100644 --- a/test/unit/lib/nvmf/tcp.c/tcp_ut.c +++ b/test/unit/lib/nvmf/tcp.c/tcp_ut.c @@ -598,11 +598,11 @@ test_nvmf_tcp_h2c_data_hdr_handle(void) struct spdk_nvmf_tcp_req tcp_req = {}; struct spdk_nvme_tcp_h2c_data_hdr *h2c_data; - TAILQ_INIT(&tqpair.tcp_req_working_queue); - /* Set qpair state to make unrelated operations NOP */ tqpair.state = NVME_TCP_QPAIR_STATE_RUNNING; tqpair.recv_state = NVME_TCP_PDU_RECV_STATE_ERROR; + tqpair.resource_count = 1; + tqpair.reqs = &tcp_req; tcp_req.req.iov[0].iov_base = (void *)0xDEADBEEF; tcp_req.req.iov[0].iov_len = 101; @@ -614,14 +614,11 @@ test_nvmf_tcp_h2c_data_hdr_handle(void) tcp_req.req.cmd = (union nvmf_h2c_msg *)&tcp_req.cmd; tcp_req.req.cmd->nvme_cmd.cid = 1; - tcp_req.ttag = 2; - - TAILQ_INSERT_TAIL(&tqpair.tcp_req_working_queue, - &tcp_req, state_link); + tcp_req.ttag = 1; h2c_data = &pdu.hdr.h2c_data; h2c_data->cccid = 1; - h2c_data->ttag = 2; + h2c_data->ttag = 1; h2c_data->datao = 0; h2c_data->datal = 200; @@ -632,11 +629,6 @@ test_nvmf_tcp_h2c_data_hdr_handle(void) CU_ASSERT(pdu.data_iov[0].iov_len == 101); CU_ASSERT((uint64_t)pdu.data_iov[1].iov_base == 0xFEEDBEEF); CU_ASSERT(pdu.data_iov[1].iov_len == 99); - - CU_ASSERT(TAILQ_FIRST(&tqpair.tcp_req_working_queue) == - &tcp_req); - TAILQ_REMOVE(&tqpair.tcp_req_working_queue, - &tcp_req, state_link); }