From 9dd9adda38a3a5ad43abc7cfa01021c2bebe3b02 Mon Sep 17 00:00:00 2001 From: Ziye Yang Date: Thu, 24 Jan 2019 22:29:18 +0800 Subject: [PATCH] nvmf: To correctly handle the socket read error. If there is socket read error, we should directly disconnect the socket instead of set the tqpair into RECV_ERROR state. When it is in ERROR_RECV state, it does not mean that we should close the socket immediately. Change-Id: I975906653c13eb3fa5195799c517015435176785 Signed-off-by: Ziye Yang Reviewed-on: https://review.gerrithub.io/c/441830 Tested-by: SPDK CI Jenkins Chandler-Test-Pool: SPDK Automated Test System Reviewed-by: Ben Walker Reviewed-by: Shuhei Matsumoto Reviewed-by: Changpeng Liu --- lib/nvmf/tcp.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/lib/nvmf/tcp.c b/lib/nvmf/tcp.c index 7088e8a4b..1c50ef97a 100644 --- a/lib/nvmf/tcp.c +++ b/lib/nvmf/tcp.c @@ -1987,8 +1987,7 @@ spdk_nvmf_tcp_sock_process(struct nvme_tcp_qpair *tqpair) (void *)&pdu->hdr.common + pdu->ch_valid_bytes); if (rc < 0) { SPDK_DEBUGLOG(SPDK_LOG_NVMF_TCP, "will disconnect tqpair=%p\n", tqpair); - spdk_nvmf_tcp_qpair_set_recv_state(tqpair, NVME_TCP_PDU_RECV_STATE_ERROR); - break; + return NVME_TCP_PDU_FATAL; } pdu->ch_valid_bytes += rc; if (pdu->ch_valid_bytes < sizeof(struct spdk_nvme_tcp_common_pdu_hdr)) { @@ -2028,8 +2027,7 @@ spdk_nvmf_tcp_sock_process(struct nvme_tcp_qpair *tqpair) psh_len - pdu->psh_valid_bytes, (void *)&pdu->hdr.raw + sizeof(struct spdk_nvme_tcp_common_pdu_hdr) + pdu->psh_valid_bytes); if (rc < 0) { - spdk_nvmf_tcp_qpair_set_recv_state(tqpair, NVME_TCP_PDU_RECV_STATE_ERROR); - break; + return NVME_TCP_PDU_FATAL; } pdu->psh_valid_bytes += rc; @@ -2055,8 +2053,7 @@ spdk_nvmf_tcp_sock_process(struct nvme_tcp_qpair *tqpair) rc = nvme_tcp_read_data(tqpair->sock, data_len - pdu->data_valid_bytes, (void *)pdu->data + pdu->data_valid_bytes); if (rc < 0) { - spdk_nvmf_tcp_qpair_set_recv_state(tqpair, NVME_TCP_PDU_RECV_STATE_ERROR); - break; + return NVME_TCP_PDU_FATAL; } pdu->data_valid_bytes += rc; @@ -2072,8 +2069,7 @@ spdk_nvmf_tcp_sock_process(struct nvme_tcp_qpair *tqpair) SPDK_NVME_TCP_DIGEST_LEN - pdu->ddigest_valid_bytes, pdu->data_digest + pdu->ddigest_valid_bytes); if (rc < 0) { - spdk_nvmf_tcp_qpair_set_recv_state(tqpair, NVME_TCP_PDU_RECV_STATE_ERROR); - break; + return NVME_TCP_PDU_FATAL; } pdu->ddigest_valid_bytes += rc; @@ -2086,7 +2082,12 @@ spdk_nvmf_tcp_sock_process(struct nvme_tcp_qpair *tqpair) spdk_nvmf_tcp_pdu_payload_handle(tqpair); break; case NVME_TCP_PDU_RECV_STATE_ERROR: - rc = NVME_TCP_PDU_FATAL; + pdu = &tqpair->pdu_in_progress; + /* Check whether the connection is closed. Each time, we only read 1 byte every time */ + rc = nvme_tcp_read_data(tqpair->sock, 1, (void *)&pdu->hdr.common); + if (rc < 0) { + return NVME_TCP_PDU_FATAL; + } break; default: assert(0); @@ -2646,6 +2647,11 @@ spdk_nvmf_tcp_qpair_process_pending(struct spdk_nvmf_tcp_transport *ttransport, { struct nvme_tcp_req *tcp_req, *req_tmp; + /* Tqpair is not in a good state, so return it */ + if (spdk_unlikely(tqpair->recv_state == NVME_TCP_PDU_RECV_STATE_ERROR)) { + return; + } + spdk_nvmf_tcp_handle_queued_r2t_req(tqpair); TAILQ_FOREACH_SAFE(tcp_req, &tqpair->ch->pending_data_buf_queue, link, req_tmp) { @@ -2664,14 +2670,15 @@ spdk_nvmf_tcp_sock_cb(void *arg, struct spdk_sock_group *group, struct spdk_sock assert(tqpair != NULL); - if (tqpair->recv_state == NVME_TCP_PDU_RECV_STATE_ERROR) { - return; - } - ttransport = SPDK_CONTAINEROF(tqpair->qpair.transport, struct spdk_nvmf_tcp_transport, transport); spdk_nvmf_tcp_qpair_process_pending(ttransport, tqpair); rc = spdk_nvmf_tcp_sock_process(tqpair); - if (rc < 0) { + + /* check the following two factors: + * rc: The socket is closed + * State of tqpair: The tqpair is in EXITING state due to internal error + */ + if ((rc < 0) || (tqpair->state == NVME_TCP_QPAIR_STATE_EXITING)) { tqpair->state = NVME_TCP_QPAIR_STATE_EXITED; spdk_nvmf_tcp_qpair_flush_pdus(tqpair); SPDK_DEBUGLOG(SPDK_LOG_NVMF_TCP, "will disconect the tqpair=%p\n", tqpair);