Commit f84c916c authored by Ben Walker's avatar Ben Walker Committed by Tomasz Zawadzki
Browse files

nvmf/tcp: Correctly kick the recv state machine when a request is freed 



When a command arrives and no requests are available, the socket
recv state machine sits in the RECV_STATE_AWAIT_REQ state until another
network event occurs. If this I/O was the last one sent, this leaves the
target hung. To fix this, when a request is completed, kick the state
machine to make forward progress.

In practice, this can only occur once the pdu send acknowledgements are
asynchronous relative to arriving commands. That only begins happening
with the use of MSG_ZEROCOPY. When MSG_ZEROCOPY is turned on, it's
possible receive the next PDU in a chain for a command prior to seeing
the acknowledgement that the response that triggered that PDU actually
sent.

Change-Id: I556f31ad56970d36aa3538cfde375d35f3d4e551
Signed-off-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/480002


Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent 48a547fd

lib/nvmf/tcp.c

+24 −3
Original line number Diff line number Diff line
@@ -267,6 +267,7 @@ struct spdk_nvmf_tcp_poll_group { 
	struct spdk_sock_group			*sock_group;



	TAILQ_HEAD(, spdk_nvmf_tcp_qpair)	qpairs;

	TAILQ_HEAD(, spdk_nvmf_tcp_qpair)	await_req;

};



struct spdk_nvmf_tcp_port {
@@ -1025,6 +1026,7 @@ spdk_nvmf_tcp_poll_group_create(struct spdk_nvmf_transport *transport) 
	}



	TAILQ_INIT(&tgroup->qpairs);

	TAILQ_INIT(&tgroup->await_req);



	return &tgroup->group;
@@ -1094,15 +1096,24 @@ spdk_nvmf_tcp_qpair_set_recv_state(struct spdk_nvmf_tcp_qpair *tqpair, 
		return;

	}



	if (tqpair->recv_state == NVME_TCP_PDU_RECV_STATE_AWAIT_REQ) {

		/* When leaving the await req state, move the qpair to the main list */

		TAILQ_REMOVE(&tqpair->group->await_req, tqpair, link);

		TAILQ_INSERT_TAIL(&tqpair->group->qpairs, tqpair, link);

	}



	SPDK_DEBUGLOG(SPDK_LOG_NVMF_TCP, "tqpair(%p) recv state=%d\n", tqpair, state);

	tqpair->recv_state = state;



	switch (state) {

	case NVME_TCP_PDU_RECV_STATE_AWAIT_PDU_CH:

	case NVME_TCP_PDU_RECV_STATE_AWAIT_PDU_PSH:

	case NVME_TCP_PDU_RECV_STATE_AWAIT_REQ:

	case NVME_TCP_PDU_RECV_STATE_AWAIT_PDU_PAYLOAD:

		break;

	case NVME_TCP_PDU_RECV_STATE_AWAIT_REQ:

		TAILQ_REMOVE(&tqpair->group->qpairs, tqpair, link);

		TAILQ_INSERT_TAIL(&tqpair->group->await_req, tqpair, link);

		break;

	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));
@@ -2462,7 +2473,12 @@ spdk_nvmf_tcp_poll_group_remove(struct spdk_nvmf_transport_poll_group *group, 
	assert(tqpair->group == tgroup);



	SPDK_DEBUGLOG(SPDK_LOG_NVMF_TCP, "remove tqpair=%p from the tgroup=%p\n", tqpair, tgroup);

	if (tqpair->recv_state == NVME_TCP_PDU_RECV_STATE_AWAIT_REQ) {

		TAILQ_REMOVE(&tgroup->await_req, tqpair, link);

	} else {

		TAILQ_REMOVE(&tgroup->qpairs, tqpair, link);

	}



	rc = spdk_sock_group_remove_sock(tgroup->sock_group, tqpair->sock);

	if (rc != 0) {

		SPDK_ERRLOG("Could not remove sock from sock_group: %s (%d)\n",
@@ -2506,12 +2522,13 @@ spdk_nvmf_tcp_poll_group_poll(struct spdk_nvmf_transport_poll_group *group) 
	int rc;

	struct spdk_nvmf_request *req, *req_tmp;

	struct spdk_nvmf_tcp_req *tcp_req;

	struct spdk_nvmf_tcp_qpair *tqpair, *tqpair_tmp;

	struct spdk_nvmf_tcp_transport *ttransport = SPDK_CONTAINEROF(group->transport,

			struct spdk_nvmf_tcp_transport, transport);



	tgroup = SPDK_CONTAINEROF(group, struct spdk_nvmf_tcp_poll_group, group);



	if (spdk_unlikely(TAILQ_EMPTY(&tgroup->qpairs))) {

	if (spdk_unlikely(TAILQ_EMPTY(&tgroup->qpairs) && TAILQ_EMPTY(&tgroup->await_req))) {

		return 0;

	}
@@ -2527,6 +2544,10 @@ spdk_nvmf_tcp_poll_group_poll(struct spdk_nvmf_transport_poll_group *group) 
		SPDK_ERRLOG("Failed to poll sock_group=%p\n", tgroup->sock_group);

	}



	TAILQ_FOREACH_SAFE(tqpair, &tgroup->await_req, link, tqpair_tmp) {

		spdk_nvmf_tcp_sock_process(tqpair);

	}



	return rc;

}