Commit 879386d7 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Jim Harris
Browse files

iscsi: Unify build and fast-forward iovecs to flush PDUs 



Building iovecs had been done in spdk_iscsi_build_iovs() and
fast-forwarding iovecs for the partially written first PDU had
been done in-line separately.

This patch unifies these two operations into spdk_iscsi_build_iovs().

Fast-forwarding iovecs is necessary only for the first PDU, but the
operation is applied to all PDUs after this patch.

Extra overhead will be negligible because usually at most two
iovecs are consumed, one is for the base header segment and another
is for the data segment.

Change-Id: I8feafac6d280439eb7cf73136107adbac820ae09
Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446174


Reviewed-by: default avatarZiye Yang <ziye.yang@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent 42596fb6

lib/iscsi/conn.c

+5 −17
Original line number Diff line number Diff line
@@ -1166,7 +1166,11 @@ spdk_iscsi_conn_flush_pdus_internal(struct spdk_iscsi_conn *conn) 


	/*

	 * Build up a list of iovecs for the first few PDUs in the

	 *  connection's write_pdu_list.

	 *  connection's write_pdu_list. For the first PDU, check if it was

	 *  partially written out the last time this function was called, and

	 *  if so adjust the iovec array accordingly. This check is done in

	 *  spdk_iscsi_build_iovs() and so applied to remaining PDUs too.

	 *  But extra overhead is negligible.

	 */

	while (pdu != NULL && ((num_iovs - iovcnt) >= 5)) {

		pdu_length = spdk_iscsi_get_pdu_length(pdu,
@@ -1177,24 +1181,8 @@ spdk_iscsi_conn_flush_pdus_internal(struct spdk_iscsi_conn *conn) 
		pdu = TAILQ_NEXT(pdu, tailq);

	}



	/*

	 * Check if the first PDU was partially written out the last time

	 *  this function was called, and if so adjust the iovec array

	 *  accordingly.

	 */

	writev_offset = TAILQ_FIRST(&conn->write_pdu_list)->writev_offset;

	total_length -= writev_offset;

	while (writev_offset > 0) {

		if (writev_offset >= iov->iov_len) {

			writev_offset -= iov->iov_len;

			iov++;

			iovcnt--;

		} else {

			iov->iov_len -= writev_offset;

			iov->iov_base = (char *)iov->iov_base + writev_offset;

			writev_offset = 0;

		}

	}



	spdk_trace_record(TRACE_ISCSI_FLUSH_WRITEBUF_START, conn->id, total_length, 0, iovcnt);

lib/iscsi/iscsi.c

+43 −18
Original line number Diff line number Diff line
@@ -569,11 +569,13 @@ spdk_iscsi_build_iovs(struct spdk_iscsi_conn *conn, struct iovec *iovs, 
{

	int iovcnt = 0;

	int enable_digest;

	int total_ahs_len;

	int data_len;

	uint32_t total_ahs_len;

	uint32_t data_len;

	uint32_t iov_offset;



	total_ahs_len = pdu->bhs.total_ahs_len;

	data_len = DGET24(pdu->bhs.data_segment_len);

	data_len = ISCSI_ALIGN(data_len);



	enable_digest = 1;

	if (pdu->bhs.opcode == ISCSI_OP_LOGIN_RSP) {
@@ -581,38 +583,61 @@ spdk_iscsi_build_iovs(struct spdk_iscsi_conn *conn, struct iovec *iovs, 
		enable_digest = 0;

	}



	iov_offset = pdu->writev_offset;



	/* BHS */

	iovs[iovcnt].iov_base = &pdu->bhs;

	iovs[iovcnt].iov_len = ISCSI_BHS_LEN;

	if (iov_offset >= ISCSI_BHS_LEN) {

		iov_offset -= ISCSI_BHS_LEN;

	} else {

		iovs[iovcnt].iov_base = (uint8_t *)&pdu->bhs + iov_offset;

		iovs[iovcnt].iov_len = ISCSI_BHS_LEN - iov_offset;

		iov_offset = 0;

		iovcnt++;



	}

	/* AHS */

	if (total_ahs_len > 0) {

		iovs[iovcnt].iov_base = pdu->ahs;

		iovs[iovcnt].iov_len = 4 * total_ahs_len;

		if (iov_offset >= 4 * total_ahs_len) {

			iov_offset -= 4 * total_ahs_len;

		} else {

			iovs[iovcnt].iov_base = pdu->ahs + iov_offset;

			iovs[iovcnt].iov_len = 4 * total_ahs_len - iov_offset;

			iov_offset = 0;

			iovcnt++;

		}

	}



	/* Header Digest */

	if (enable_digest && conn->header_digest) {

		iovs[iovcnt].iov_base = pdu->header_digest;

		iovs[iovcnt].iov_len = ISCSI_DIGEST_LEN;

		if (iov_offset >= ISCSI_DIGEST_LEN) {

			iov_offset -= ISCSI_DIGEST_LEN;

		} else {

			iovs[iovcnt].iov_base = pdu->header_digest + iov_offset;

			iovs[iovcnt].iov_len = ISCSI_DIGEST_LEN - iov_offset;

			iov_offset = 0;

			iovcnt++;

		}

	}



	/* Data Segment */

	if (data_len > 0) {

		iovs[iovcnt].iov_base = pdu->data;

		iovs[iovcnt].iov_len = ISCSI_ALIGN(data_len);

		if (iov_offset >= data_len) {

			iov_offset -= data_len;

		} else {

			iovs[iovcnt].iov_base = pdu->data + iov_offset;

			iovs[iovcnt].iov_len = data_len - iov_offset;

			iov_offset = 0;

			iovcnt++;

		}

	}



	/* Data Digest */

	if (enable_digest && conn->data_digest && data_len != 0) {

		iovs[iovcnt].iov_base = pdu->data_digest;

		iovs[iovcnt].iov_len = ISCSI_DIGEST_LEN;

		if (iov_offset < ISCSI_DIGEST_LEN) {

			iovs[iovcnt].iov_base = pdu->data_digest + iov_offset;

			iovs[iovcnt].iov_len = ISCSI_DIGEST_LEN - iov_offset;

			iovcnt++;

		}

	}



	return iovcnt;

}

test/unit/lib/iscsi/iscsi.c/iscsi_ut.c

+92 −0
Original line number Diff line number Diff line
@@ -1202,6 +1202,97 @@ abort_queued_datain_tasks_test(void) 
	spdk_put_pdu(pdu1);

}



static void

build_iovs_test(void)

{

	struct spdk_iscsi_conn conn = {};

	struct spdk_iscsi_pdu pdu = {};

	struct iovec iovs[5] = {};

	uint8_t *data;

	int rc;



	conn.header_digest = true;

	conn.data_digest = true;



	DSET24(&pdu.bhs.data_segment_len, 512);

	data = calloc(1, 512);

	SPDK_CU_ASSERT_FATAL(data != NULL);

	pdu.data = data;



	pdu.bhs.total_ahs_len = 0;

	pdu.bhs.opcode = ISCSI_OP_SCSI;



	pdu.writev_offset = 0;

	rc = spdk_iscsi_build_iovs(&conn, iovs, &pdu);

	CU_ASSERT(rc == 4);

	CU_ASSERT(iovs[0].iov_base == (void *)&pdu.bhs);

	CU_ASSERT(iovs[0].iov_len == ISCSI_BHS_LEN);

	CU_ASSERT(iovs[1].iov_base == (void *)pdu.header_digest);

	CU_ASSERT(iovs[1].iov_len == ISCSI_DIGEST_LEN);

	CU_ASSERT(iovs[2].iov_base == (void *)pdu.data);

	CU_ASSERT(iovs[2].iov_len == 512);

	CU_ASSERT(iovs[3].iov_base == (void *)pdu.data_digest);

	CU_ASSERT(iovs[3].iov_len == ISCSI_DIGEST_LEN);



	pdu.writev_offset = ISCSI_BHS_LEN / 2;

	rc = spdk_iscsi_build_iovs(&conn, iovs, &pdu);

	CU_ASSERT(rc == 4);

	CU_ASSERT(iovs[0].iov_base == (void *)((uint8_t *)&pdu.bhs + ISCSI_BHS_LEN / 2));

	CU_ASSERT(iovs[0].iov_len == ISCSI_BHS_LEN / 2);

	CU_ASSERT(iovs[1].iov_base == (void *)pdu.header_digest);

	CU_ASSERT(iovs[1].iov_len == ISCSI_DIGEST_LEN);

	CU_ASSERT(iovs[2].iov_base == (void *)pdu.data);

	CU_ASSERT(iovs[2].iov_len == 512);

	CU_ASSERT(iovs[3].iov_base == (void *)pdu.data_digest);

	CU_ASSERT(iovs[3].iov_len == ISCSI_DIGEST_LEN);



	pdu.writev_offset = ISCSI_BHS_LEN;

	rc = spdk_iscsi_build_iovs(&conn, iovs, &pdu);

	CU_ASSERT(rc == 3);

	CU_ASSERT(iovs[0].iov_base == (void *)pdu.header_digest);

	CU_ASSERT(iovs[0].iov_len == ISCSI_DIGEST_LEN);

	CU_ASSERT(iovs[1].iov_base == (void *)pdu.data);

	CU_ASSERT(iovs[1].iov_len == 512);

	CU_ASSERT(iovs[2].iov_base == (void *)pdu.data_digest);

	CU_ASSERT(iovs[2].iov_len == ISCSI_DIGEST_LEN);



	pdu.writev_offset = ISCSI_BHS_LEN + ISCSI_DIGEST_LEN / 2;

	rc = spdk_iscsi_build_iovs(&conn, iovs, &pdu);

	CU_ASSERT(rc == 3);

	CU_ASSERT(iovs[0].iov_base == (void *)((uint8_t *)pdu.header_digest + ISCSI_DIGEST_LEN / 2));

	CU_ASSERT(iovs[0].iov_len == ISCSI_DIGEST_LEN / 2);

	CU_ASSERT(iovs[1].iov_base == (void *)pdu.data);

	CU_ASSERT(iovs[1].iov_len == 512);

	CU_ASSERT(iovs[2].iov_base == (void *)pdu.data_digest);

	CU_ASSERT(iovs[2].iov_len == ISCSI_DIGEST_LEN);



	pdu.writev_offset = ISCSI_BHS_LEN + ISCSI_DIGEST_LEN;

	rc = spdk_iscsi_build_iovs(&conn, iovs, &pdu);

	CU_ASSERT(rc == 2);

	CU_ASSERT(iovs[0].iov_base == (void *)pdu.data);

	CU_ASSERT(iovs[0].iov_len == 512);

	CU_ASSERT(iovs[1].iov_base == (void *)pdu.data_digest);

	CU_ASSERT(iovs[1].iov_len == ISCSI_DIGEST_LEN);



	pdu.writev_offset = ISCSI_BHS_LEN + ISCSI_DIGEST_LEN + 512;

	rc = spdk_iscsi_build_iovs(&conn, iovs, &pdu);

	CU_ASSERT(rc == 1);

	CU_ASSERT(iovs[0].iov_base == (void *)pdu.data_digest);

	CU_ASSERT(iovs[0].iov_len == ISCSI_DIGEST_LEN);



	pdu.writev_offset = ISCSI_BHS_LEN + ISCSI_DIGEST_LEN + 512 + ISCSI_DIGEST_LEN / 2;

	rc = spdk_iscsi_build_iovs(&conn, iovs, &pdu);

	CU_ASSERT(rc == 1);

	CU_ASSERT(iovs[0].iov_base == (void *)((uint8_t *)pdu.data_digest + ISCSI_DIGEST_LEN / 2));

	CU_ASSERT(iovs[0].iov_len == ISCSI_DIGEST_LEN / 2);



	pdu.writev_offset = ISCSI_BHS_LEN + ISCSI_DIGEST_LEN + 512 + ISCSI_DIGEST_LEN;

	rc = spdk_iscsi_build_iovs(&conn, iovs, &pdu);

	CU_ASSERT(rc == 0);



	free(data);

}



int

main(int argc, char **argv)

{
@@ -1238,6 +1329,7 @@ main(int argc, char **argv) 
			       abort_queued_datain_task_test) == NULL

		|| CU_add_test(suite, "abort_queued_datain_tasks_test",

			       abort_queued_datain_tasks_test) == NULL

		|| CU_add_test(suite, "build_iovs_test", build_iovs_test) == NULL

	) {

		CU_cleanup_registry();

		return CU_get_error();