Commit 73b02ffd authored by Ben Walker's avatar Ben Walker Committed by Jim Harris
Browse files

nvme: In nvme_tcp_qpair_process_completions, do not call 


nvme_tcp_read_pdu in a loop

nvme_tcp_read_pdu itself has a loop in it that runs until no more data
is available, so the extra loop does nothing.

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


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 8b29c211

lib/nvme/nvme_tcp.c

+20 −27
Original line number Diff line number Diff line
@@ -1600,8 +1600,15 @@ nvme_tcp_read_pdu(struct nvme_tcp_qpair *tqpair, uint32_t *reaped, uint32_t max_ 
	uint32_t data_len;

	enum nvme_tcp_pdu_recv_state prev_state;



	*reaped = tqpair->async_complete;

	tqpair->async_complete = 0;



	/* The loop here is to allow for several back-to-back state changes. */

	do {

		if (*reaped >= max_completions) {

			break;

		}



		prev_state = tqpair->recv_state;

		pdu = tqpair->recv_pdu;

		switch (tqpair->recv_state) {
@@ -1622,8 +1629,7 @@ nvme_tcp_read_pdu(struct nvme_tcp_qpair *tqpair, uint32_t *reaped, uint32_t max_ 
			}

			pdu->ch_valid_bytes += rc;

			if (pdu->ch_valid_bytes < sizeof(struct spdk_nvme_tcp_common_pdu_hdr)) {

				rc =  NVME_TCP_PDU_IN_PROGRESS;

				goto out;

				return NVME_TCP_PDU_IN_PROGRESS;

			}



			/* The command header of this PDU has now been read from the socket. */
@@ -1642,8 +1648,7 @@ nvme_tcp_read_pdu(struct nvme_tcp_qpair *tqpair, uint32_t *reaped, uint32_t max_ 


			pdu->psh_valid_bytes += rc;

			if (pdu->psh_valid_bytes < pdu->psh_len) {

				rc =  NVME_TCP_PDU_IN_PROGRESS;

				goto out;

				return NVME_TCP_PDU_IN_PROGRESS;

			}



			/* All header(ch, psh, head digist) of this PDU has now been read from the socket. */
@@ -1671,8 +1676,7 @@ nvme_tcp_read_pdu(struct nvme_tcp_qpair *tqpair, uint32_t *reaped, uint32_t max_ 


			pdu->rw_offset += rc;

			if (pdu->rw_offset < data_len) {

				rc =  NVME_TCP_PDU_IN_PROGRESS;

				goto out;

				return NVME_TCP_PDU_IN_PROGRESS;

			}



			assert(pdu->rw_offset == data_len);
@@ -1681,19 +1685,14 @@ nvme_tcp_read_pdu(struct nvme_tcp_qpair *tqpair, uint32_t *reaped, uint32_t max_ 
			break;

		case NVME_TCP_PDU_RECV_STATE_ERROR:

			memset(pdu, 0, sizeof(struct nvme_tcp_pdu));

			rc = NVME_TCP_PDU_FATAL;

			break;

			return NVME_TCP_PDU_FATAL;

		default:

			assert(0);

			break;

		}

	} while (prev_state != tqpair->recv_state && *reaped + tqpair->async_complete < max_completions);



out:

	*reaped += tqpair->async_complete;

	tqpair->async_complete = 0;

	} while (prev_state != tqpair->recv_state);



	return rc;

	return rc > 0 ? 0 : rc;

}



static void
@@ -1758,25 +1757,19 @@ nvme_tcp_qpair_process_completions(struct spdk_nvme_qpair *qpair, uint32_t max_c 
	}



	if (max_completions == 0) {

		max_completions = tqpair->num_entries;

		max_completions = spdk_max(tqpair->num_entries, 1);

	} else {

		max_completions = spdk_min(max_completions, tqpair->num_entries);

	}



	reaped = 0;

	do {

	rc = nvme_tcp_read_pdu(tqpair, &reaped, max_completions);

	if (rc < 0) {

		SPDK_DEBUGLOG(nvme, "Error polling CQ! (%d): %s\n",

			      errno, spdk_strerror(errno));

		goto fail;

		} else if (rc == 0) {

			/* Partial PDU is read */

			break;

	}



	} while (reaped < max_completions);



	if (spdk_unlikely(tqpair->qpair.ctrlr->timeout_enabled)) {

		nvme_tcp_qpair_check_timeout(qpair);

	}

test/unit/lib/nvme/nvme_tcp.c/nvme_tcp_ut.c

+1 −0
Original line number Diff line number Diff line
@@ -1376,6 +1376,7 @@ test_nvme_tcp_ctrlr_connect_qpair(void) 
	tqpair->send_pdu = &pdu;

	tqpair->qpair.ctrlr = &ctrlr;

	tqpair->qpair.state = NVME_QPAIR_CONNECTING;

	tqpair->num_entries = 128;

	ic_req = &pdu.hdr.ic_req;



	tqpair->recv_pdu->hdr.common.pdu_type = SPDK_NVME_TCP_PDU_TYPE_IC_RESP;