From f6a91b3b407abf01f6e6b598aaacf0ea41ded58c Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Mon, 17 Jun 2019 15:02:15 +0900 Subject: [PATCH] dif: Process partial data block properly in _dif_generate_split() For NVMe/TCP target, data segments which correspond to H2C or C2H PDU will have any alignment, and _dif_generate_split will have to process partial data block, particularly the following types: - start and end are both within a data block. - start is within a data block, and end is at the end of a block According to the refactoring done in the last patch, this patch exposes offset_in_block, data_len, and guard as parameters of _dif_generate_split() and make _dif_generate_split() process the above two types of data block properly. The next patch will utilize the updated _dif_generate_split in spdk_dif_generate_stream(). Signed-off-by: Shuhei Matsumoto Change-Id: I4211e65ead7fc256a40748412c670e46f83b1731 Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/457544 Tested-by: SPDK CI Jenkins Reviewed-by: Darek Stojaczyk Reviewed-by: Changpeng Liu --- lib/util/dif.c | 44 ++++++++++++----- test/unit/lib/util/dif.c/dif_ut.c | 80 +++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 12 deletions(-) diff --git a/lib/util/dif.c b/lib/util/dif.c index 671262ae5..484113b59 100644 --- a/lib/util/dif.c +++ b/lib/util/dif.c @@ -314,23 +314,22 @@ dif_generate(struct _dif_sgl *sgl, uint32_t num_blocks, const struct spdk_dif_ct } } -static void -_dif_generate_split(struct _dif_sgl *sgl, uint32_t offset_blocks, - const struct spdk_dif_ctx *ctx) +static uint16_t +_dif_generate_split(struct _dif_sgl *sgl, uint32_t offset_in_block, uint32_t data_len, + uint16_t guard, uint32_t offset_blocks, const struct spdk_dif_ctx *ctx) { - uint32_t offset_in_block, offset_in_dif, buf_len; + uint32_t offset_in_dif, buf_len; void *buf; - uint16_t guard = 0; struct spdk_dif dif = {}; - if (ctx->dif_flags & SPDK_DIF_FLAGS_GUARD_CHECK) { - guard = ctx->guard_seed; - } - offset_in_block = 0; + assert(offset_in_block < ctx->guard_interval); + assert(offset_in_block + data_len < ctx->guard_interval || + offset_in_block + data_len == ctx->block_size); /* Compute CRC over split logical block data. */ - while (offset_in_block < ctx->guard_interval) { + while (data_len != 0 && offset_in_block < ctx->guard_interval) { _dif_sgl_get_buf(sgl, &buf, &buf_len); + buf_len = spdk_min(buf_len, data_len); buf_len = spdk_min(buf_len, ctx->guard_interval - offset_in_block); if (ctx->dif_flags & SPDK_DIF_FLAGS_GUARD_CHECK) { @@ -339,6 +338,11 @@ _dif_generate_split(struct _dif_sgl *sgl, uint32_t offset_blocks, _dif_sgl_advance(sgl, buf_len); offset_in_block += buf_len; + data_len -= buf_len; + } + + if (offset_in_block < ctx->guard_interval) { + return guard; } /* If a whole logical block data is parsed, generate DIF @@ -364,6 +368,12 @@ _dif_generate_split(struct _dif_sgl *sgl, uint32_t offset_blocks, _dif_sgl_advance(sgl, buf_len); offset_in_block += buf_len; } + + if (ctx->dif_flags & SPDK_DIF_FLAGS_GUARD_CHECK) { + guard = ctx->guard_seed; + } + + return guard; } static void @@ -371,9 +381,14 @@ dif_generate_split(struct _dif_sgl *sgl, uint32_t num_blocks, const struct spdk_dif_ctx *ctx) { uint32_t offset_blocks; + uint16_t guard = 0; + + if (ctx->dif_flags & SPDK_DIF_FLAGS_GUARD_CHECK) { + guard = ctx->guard_seed; + } for (offset_blocks = 0; offset_blocks < num_blocks; offset_blocks++) { - _dif_generate_split(sgl, offset_blocks, ctx); + _dif_generate_split(sgl, 0, ctx->block_size, guard, offset_blocks, ctx); } } @@ -1453,6 +1468,7 @@ spdk_dif_generate_stream(struct iovec *iovs, int iovcnt, const struct spdk_dif_ctx *ctx) { uint32_t data_block_size, offset_blocks, num_blocks, i; + uint16_t guard = 0; struct _dif_sgl sgl; if (iovs == NULL || iovcnt == 0) { @@ -1461,6 +1477,10 @@ spdk_dif_generate_stream(struct iovec *iovs, int iovcnt, data_block_size = ctx->block_size - ctx->md_size; + if (ctx->dif_flags & SPDK_DIF_FLAGS_GUARD_CHECK) { + guard = ctx->guard_seed; + } + offset_blocks = data_offset / data_block_size; data_len += data_offset % data_block_size; @@ -1476,7 +1496,7 @@ spdk_dif_generate_stream(struct iovec *iovs, int iovcnt, _dif_sgl_advance(&sgl, data_offset); for (i = 0; i < num_blocks; i++) { - _dif_generate_split(&sgl, offset_blocks + i, ctx); + _dif_generate_split(&sgl, 0, ctx->block_size, guard, offset_blocks + i, ctx); } return 0; diff --git a/test/unit/lib/util/dif.c/dif_ut.c b/test/unit/lib/util/dif.c/dif_ut.c index e3a76f409..da0ed1c3d 100644 --- a/test/unit/lib/util/dif.c/dif_ut.c +++ b/test/unit/lib/util/dif.c/dif_ut.c @@ -1778,6 +1778,85 @@ set_md_interleave_iovs_alignment_test(void) CU_ASSERT(_iov_check(&dif_iovs[1], (void *)0xC0FFEE, 24) == true); } +static void +_dif_generate_split_test(void) +{ + struct spdk_dif_ctx ctx = {}; + struct iovec iov; + uint8_t *buf1, *buf2; + struct _dif_sgl sgl; + uint16_t guard = 0, prev_guard; + uint32_t dif_flags; + int rc; + + dif_flags = SPDK_DIF_FLAGS_GUARD_CHECK | SPDK_DIF_FLAGS_APPTAG_CHECK | + SPDK_DIF_FLAGS_REFTAG_CHECK; + + rc = spdk_dif_ctx_init(&ctx, 4096 + 128, 128, true, false, SPDK_DIF_TYPE1, + dif_flags, 0, 0, 0, 0, GUARD_SEED); + CU_ASSERT(rc == 0); + + buf1 = calloc(1, 4096 + 128); + SPDK_CU_ASSERT_FATAL(buf1 != NULL); + _iov_set_buf(&iov, buf1, 4096 + 128); + + rc = ut_data_pattern_generate(&iov, 1, 4096 + 128, 128, 1); + CU_ASSERT(rc == 0); + + _dif_sgl_init(&sgl, &iov, 1); + + guard = GUARD_SEED; + prev_guard = GUARD_SEED; + + guard = _dif_generate_split(&sgl, 0, 1000, guard, 0, &ctx); + CU_ASSERT(sgl.iov_offset == 1000); + CU_ASSERT(guard == spdk_crc16_t10dif(prev_guard, buf1, 1000)); + + prev_guard = guard; + + guard = _dif_generate_split(&sgl, 1000, 3000, guard, 0, &ctx); + CU_ASSERT(sgl.iov_offset == 4000); + CU_ASSERT(guard == spdk_crc16_t10dif(prev_guard, buf1 + 1000, 3000)); + + guard = _dif_generate_split(&sgl, 4000, 96 + 128, guard, 0, &ctx); + CU_ASSERT(guard == GUARD_SEED); + CU_ASSERT(sgl.iov_offset == 0); + CU_ASSERT(sgl.iovcnt == 0); + + rc = ut_data_pattern_verify(&iov, 1, 4096 + 128, 128, 1); + CU_ASSERT(rc == 0); + + _dif_sgl_init(&sgl, &iov, 1); + + rc = dif_verify(&sgl, 1, &ctx, NULL); + CU_ASSERT(rc == 0); + + buf2 = calloc(1, 4096 + 128); + SPDK_CU_ASSERT_FATAL(buf2 != NULL); + _iov_set_buf(&iov, buf2, 4096 + 128); + + rc = ut_data_pattern_generate(&iov, 1, 4096 + 128, 128, 1); + CU_ASSERT(rc == 0); + + _dif_sgl_init(&sgl, &iov, 1); + + dif_generate(&sgl, 1, &ctx); + + rc = ut_data_pattern_verify(&iov, 1, 4096 + 128, 128, 1); + CU_ASSERT(rc == 0); + + _dif_sgl_init(&sgl, &iov, 1); + + rc = dif_verify(&sgl, 1, &ctx, NULL); + CU_ASSERT(rc == 0); + + rc = memcmp(buf1, buf2, 4096 + 128); + CU_ASSERT(rc == 0); + + free(buf1); + free(buf2); +} + #define UT_CRC32C_XOR 0xffffffffUL static void @@ -1961,6 +2040,7 @@ main(int argc, char **argv) CU_add_test(suite, "dif_generate_stream_test", dif_generate_stream_test) == NULL || CU_add_test(suite, "set_md_interleave_iovs_alignment_test", set_md_interleave_iovs_alignment_test) == NULL || + CU_add_test(suite, "_dif_generate_split_test", _dif_generate_split_test) == NULL || CU_add_test(suite, "update_crc32c_test", update_crc32c_test) == NULL ) { CU_cleanup_registry();