From beefcd1ae580943ea51413b5a903b26ce829d206 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Wed, 19 Jun 2019 10:08:17 +0900 Subject: [PATCH] iscsi: Fix the bug to add wrong size in _iscsi_sgl_append_with_md This bug was found by code inspection. When PDU read restarts after data segment, iov_offset must be reduced by data length. However iov_offset had been reduced by buffer length by mistake. Lack of UT code was the main reason of this bug. Hence add UT code to test the fix of the bug together in this patch. Signed-off-by: Shuhei Matsumoto Change-Id: I74f2f6ae8dca2e78a64bfdef8080c8031dfabb87 Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/458530 Tested-by: SPDK CI Jenkins Reviewed-by: Darek Stojaczyk Reviewed-by: Changpeng Liu --- lib/iscsi/iscsi.c | 2 +- test/unit/lib/iscsi/iscsi.c/iscsi_ut.c | 76 ++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/lib/iscsi/iscsi.c b/lib/iscsi/iscsi.c index 792c8575f..2f8861459 100644 --- a/lib/iscsi/iscsi.c +++ b/lib/iscsi/iscsi.c @@ -636,7 +636,7 @@ _iscsi_sgl_append_with_md(struct _iscsi_sgl *s, struct iovec buf_iov; if (s->iov_offset >= data_len) { - s->iov_offset -= buf_len; + s->iov_offset -= data_len; } else { buf_iov.iov_base = buf; buf_iov.iov_len = buf_len; diff --git a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c index 372a04019..11ddd9568 100644 --- a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c +++ b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c @@ -1440,6 +1440,81 @@ build_iovs_test(void) free(data); } +static void +build_iovs_with_md_test(void) +{ + struct spdk_iscsi_conn conn = {}; + struct spdk_iscsi_pdu pdu = {}; + struct iovec iovs[6] = {}; + uint8_t *data; + uint32_t mapped_length = 0; + int rc; + + conn.header_digest = true; + conn.data_digest = true; + + DSET24(&pdu.bhs.data_segment_len, 4096 * 2); + data = calloc(1, (4096 + 128) * 2); + SPDK_CU_ASSERT_FATAL(data != NULL); + pdu.data = data; + pdu.data_buf_len = (4096 + 128) * 2; + + pdu.bhs.total_ahs_len = 0; + pdu.bhs.opcode = ISCSI_OP_SCSI; + + rc = spdk_dif_ctx_init(&pdu.dif_ctx, 4096 + 128, 128, true, false, SPDK_DIF_TYPE1, + 0, 0, 0, 0, 0, 0); + CU_ASSERT(rc == 0); + + pdu.dif_insert_or_strip = true; + + pdu.writev_offset = 0; + rc = spdk_iscsi_build_iovs(&conn, iovs, 6, &pdu, &mapped_length); + CU_ASSERT(rc == 5); + 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 == 4096); + CU_ASSERT(iovs[3].iov_base == (void *)(pdu.data + 4096 + 128)); + CU_ASSERT(iovs[3].iov_len == 4096); + CU_ASSERT(iovs[4].iov_base == (void *)pdu.data_digest); + CU_ASSERT(iovs[4].iov_len == ISCSI_DIGEST_LEN); + CU_ASSERT(mapped_length == ISCSI_BHS_LEN + ISCSI_DIGEST_LEN + 4096 * 2 + ISCSI_DIGEST_LEN); + + pdu.writev_offset = ISCSI_BHS_LEN + ISCSI_DIGEST_LEN + 2048; + rc = spdk_iscsi_build_iovs(&conn, iovs, 6, &pdu, &mapped_length); + CU_ASSERT(rc == 3); + CU_ASSERT(iovs[0].iov_base == (void *)(pdu.data + 2048)); + CU_ASSERT(iovs[0].iov_len == 2048); + CU_ASSERT(iovs[1].iov_base == (void *)(pdu.data + 4096 + 128)); + CU_ASSERT(iovs[1].iov_len == 4096); + CU_ASSERT(iovs[2].iov_base == (void *)pdu.data_digest); + CU_ASSERT(iovs[2].iov_len == ISCSI_DIGEST_LEN); + CU_ASSERT(mapped_length == 2048 + 4096 + ISCSI_DIGEST_LEN); + + pdu.writev_offset = ISCSI_BHS_LEN + ISCSI_DIGEST_LEN + 4096 * 2; + rc = spdk_iscsi_build_iovs(&conn, iovs, 6, &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 = 0; + 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 == 4096); + CU_ASSERT(mapped_length == ISCSI_BHS_LEN + ISCSI_DIGEST_LEN + 4096); + + free(data); +} + int main(int argc, char **argv) { @@ -1478,6 +1553,7 @@ main(int argc, char **argv) || 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_add_test(suite, "build_iovs_with_md_test", build_iovs_with_md_test) == NULL ) { CU_cleanup_registry(); return CU_get_error();