From fa7dd957ee5437ec7b1f985e3c2b078fd8b2a3a3 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Wed, 27 Feb 2019 11:32:33 +0900 Subject: [PATCH] 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: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446176 Reviewed-by: Ziye Yang Reviewed-by: Jim Harris Reviewed-by: Ben Walker Tested-by: SPDK CI Jenkins --- lib/iscsi/conn.c | 6 +-- lib/iscsi/iscsi.c | 19 ++++++++- lib/iscsi/iscsi.h | 5 +-- test/unit/lib/iscsi/conn.c/conn_ut.c | 2 +- test/unit/lib/iscsi/iscsi.c/iscsi_ut.c | 53 ++++++++++++++++++++++---- 5 files changed, 69 insertions(+), 16 deletions(-) diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index 1693cbb85..4f7e2b412 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -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); } diff --git a/lib/iscsi/iscsi.c b/lib/iscsi/iscsi.c index 1525c1082..aa01f0152 100644 --- a/lib/iscsi/iscsi.c +++ b/lib/iscsi/iscsi.c @@ -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; } diff --git a/lib/iscsi/iscsi.h b/lib/iscsi/iscsi.h index 91b436955..401d7ea5d 100644 --- a/lib/iscsi/iscsi.h +++ b/lib/iscsi/iscsi.h @@ -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); diff --git a/test/unit/lib/iscsi/conn.c/conn_ut.c b/test/unit/lib/iscsi/conn.c/conn_ut.c index aa7362333..e6ebeae77 100644 --- a/test/unit/lib/iscsi/conn.c/conn_ut.c +++ b/test/unit/lib/iscsi/conn.c/conn_ut.c @@ -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); diff --git a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c index 631ca917d..5e53e47cd 100644 --- a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c +++ b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c @@ -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); }