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

iscsi: Reduce required free iovecs from 5 to 1 to flush PDUs 



Upcoming patches will support DIF insert and strip feature in
iSCSI target and the feature will be implemented by utilizing
iovecs.

Even when we support the DIF feature, we want to keep current
batched PDU flush, and current requirement that there must be
enough free iovecs to map all segments of a PDU is too strict.

This patch alleviates the requirement by passing remaining
number of iovecs to spdk_iscsi_build_iovs().

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


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 ca0022f9

lib/iscsi/conn.c

+3 −3
Original line number Diff line number Diff line
@@ -1172,9 +1172,9 @@ spdk_iscsi_conn_flush_pdus_internal(struct spdk_iscsi_conn *conn) 
	 *  spdk_iscsi_build_iovs() and so applied to remaining PDUs too.

	 *  But extra overhead is negligible.

	 */

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

		iovcnt += spdk_iscsi_build_iovs(conn, &iovs[iovcnt], pdu,

						&mapped_length);

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

		iovcnt += spdk_iscsi_build_iovs(conn, &iovs[iovcnt], num_iovs - iovcnt,

						pdu, &mapped_length);

		total_length += mapped_length;

		pdu = TAILQ_NEXT(pdu, tailq);

	}

lib/iscsi/iscsi.c

+18 −1
Original line number Diff line number Diff line
@@ -564,7 +564,7 @@ spdk_iscsi_read_pdu(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu **_pdu) 
}



int

spdk_iscsi_build_iovs(struct spdk_iscsi_conn *conn, struct iovec *iovs,

spdk_iscsi_build_iovs(struct spdk_iscsi_conn *conn, struct iovec *iovs, int num_iovs,

		      struct spdk_iscsi_pdu *pdu, uint32_t *_mapped_length)

{

	int iovcnt = 0;
@@ -574,6 +574,10 @@ spdk_iscsi_build_iovs(struct spdk_iscsi_conn *conn, struct iovec *iovs, 
	uint32_t iov_offset;

	uint32_t mapped_length = 0;



	if (num_iovs == 0) {

		return 0;

	}



	total_ahs_len = pdu->bhs.total_ahs_len;

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

	data_len = ISCSI_ALIGN(data_len);
@@ -595,6 +599,9 @@ spdk_iscsi_build_iovs(struct spdk_iscsi_conn *conn, struct iovec *iovs, 
		mapped_length += ISCSI_BHS_LEN - iov_offset;

		iov_offset = 0;

		iovcnt++;

		if (iovcnt == num_iovs) {

			goto end;

		}

	}

	/* AHS */

	if (total_ahs_len > 0) {
@@ -606,6 +613,9 @@ spdk_iscsi_build_iovs(struct spdk_iscsi_conn *conn, struct iovec *iovs, 
			mapped_length += 4 * total_ahs_len - iov_offset;

			iov_offset = 0;

			iovcnt++;

			if (iovcnt == num_iovs) {

				goto end;

			}

		}

	}
@@ -619,6 +629,9 @@ spdk_iscsi_build_iovs(struct spdk_iscsi_conn *conn, struct iovec *iovs, 
			mapped_length += ISCSI_DIGEST_LEN - iov_offset;

			iov_offset = 0;

			iovcnt++;

			if (iovcnt == num_iovs) {

				goto end;

			}

		}

	}
@@ -632,6 +645,9 @@ spdk_iscsi_build_iovs(struct spdk_iscsi_conn *conn, struct iovec *iovs, 
			mapped_length += data_len - iov_offset;

			iov_offset = 0;

			iovcnt++;

			if (iovcnt == num_iovs) {

				goto end;

			}

		}

	}
@@ -645,6 +661,7 @@ spdk_iscsi_build_iovs(struct spdk_iscsi_conn *conn, struct iovec *iovs, 
		}

	}



end:

	if (_mapped_length != NULL) {

		*_mapped_length = mapped_length;

	}

lib/iscsi/iscsi.h

+2 −3
Original line number Diff line number Diff line
@@ -404,9 +404,8 @@ void spdk_iscsi_send_nopin(struct spdk_iscsi_conn *conn); 
void spdk_iscsi_task_response(struct spdk_iscsi_conn *conn,

			      struct spdk_iscsi_task *task);

int spdk_iscsi_execute(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu);

int spdk_iscsi_build_iovs(struct spdk_iscsi_conn *conn,

			  struct iovec *iovs, struct spdk_iscsi_pdu *pdu,

			  uint32_t *mapped_length);

int spdk_iscsi_build_iovs(struct spdk_iscsi_conn *conn, struct iovec *iovs, int num_iovs,

			  struct spdk_iscsi_pdu *pdu, uint32_t *mapped_length);

int spdk_iscsi_read_pdu(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu **_pdu);

void spdk_iscsi_task_mgmt_response(struct spdk_iscsi_conn *conn,

				   struct spdk_iscsi_task *task);

test/unit/lib/iscsi/conn.c/conn_ut.c

+1 −1
Original line number Diff line number Diff line
@@ -138,7 +138,7 @@ DEFINE_STUB_V(spdk_clear_all_transfer_task, 
	       struct spdk_iscsi_pdu *pdu));



DEFINE_STUB(spdk_iscsi_build_iovs, int,

	    (struct spdk_iscsi_conn *conn, struct iovec *iov,

	    (struct spdk_iscsi_conn *conn, struct iovec *iov, int num_iovs,

	     struct spdk_iscsi_pdu *pdu, uint32_t *mapped_length),

	    0);

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

+45 −8
Original line number Diff line number Diff line
@@ -1224,7 +1224,7 @@ build_iovs_test(void) 
	pdu.bhs.opcode = ISCSI_OP_SCSI;



	pdu.writev_offset = 0;

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

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

	CU_ASSERT(rc == 4);

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

	CU_ASSERT(iovs[0].iov_len == ISCSI_BHS_LEN);
@@ -1237,7 +1237,7 @@ build_iovs_test(void) 
	CU_ASSERT(mapped_length == ISCSI_BHS_LEN + ISCSI_DIGEST_LEN + 512 + ISCSI_DIGEST_LEN);



	pdu.writev_offset = ISCSI_BHS_LEN / 2;

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

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

	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);
@@ -1250,7 +1250,7 @@ build_iovs_test(void) 
	CU_ASSERT(mapped_length == ISCSI_BHS_LEN / 2 + ISCSI_DIGEST_LEN + 512 + ISCSI_DIGEST_LEN);



	pdu.writev_offset = ISCSI_BHS_LEN;

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

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

	CU_ASSERT(rc == 3);

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

	CU_ASSERT(iovs[0].iov_len == ISCSI_DIGEST_LEN);
@@ -1261,7 +1261,7 @@ build_iovs_test(void) 
	CU_ASSERT(mapped_length == ISCSI_DIGEST_LEN + 512 + ISCSI_DIGEST_LEN);



	pdu.writev_offset = ISCSI_BHS_LEN + ISCSI_DIGEST_LEN / 2;

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

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

	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);
@@ -1272,7 +1272,7 @@ build_iovs_test(void) 
	CU_ASSERT(mapped_length == ISCSI_DIGEST_LEN / 2 + 512 + ISCSI_DIGEST_LEN);



	pdu.writev_offset = ISCSI_BHS_LEN + ISCSI_DIGEST_LEN;

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

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

	CU_ASSERT(rc == 2);

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

	CU_ASSERT(iovs[0].iov_len == 512);
@@ -1281,24 +1281,61 @@ build_iovs_test(void) 
	CU_ASSERT(mapped_length == 512 + ISCSI_DIGEST_LEN);



	pdu.writev_offset = ISCSI_BHS_LEN + ISCSI_DIGEST_LEN + 512;

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

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

	CU_ASSERT(rc == 1);

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

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

	CU_ASSERT(mapped_length == 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, &mapped_length);

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

	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);

	CU_ASSERT(mapped_length == 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, &mapped_length);

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

	CU_ASSERT(rc == 0);

	CU_ASSERT(mapped_length == 0);



	pdu.writev_offset = 0;

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

	CU_ASSERT(rc == 1);

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

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

	CU_ASSERT(mapped_length == ISCSI_BHS_LEN);



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

	CU_ASSERT(rc == 2);

	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(mapped_length == ISCSI_BHS_LEN + ISCSI_DIGEST_LEN);



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

	CU_ASSERT(rc == 3);

	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(mapped_length == ISCSI_BHS_LEN + ISCSI_DIGEST_LEN + 512);



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

	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);

	CU_ASSERT(mapped_length == ISCSI_BHS_LEN + ISCSI_DIGEST_LEN + 512 + ISCSI_DIGEST_LEN);



	free(data);

}