Commit 9dd9adda authored by Ziye Yang's avatar Ziye Yang Committed by Changpeng Liu
Browse files

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: default avatarZiye Yang <optimistyzy@gmail.com>
Reviewed-on: https://review.gerrithub.io/c/441830


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
parent ac5b26e4

lib/nvmf/tcp.c

+21 −14
Original line number Diff line number Diff line
@@ -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);