From 4c2e52b93588f3f614ba4f6288699e13ca1275ee Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Thu, 20 Jun 2019 16:25:25 +0900 Subject: [PATCH] dif: Process partial data block properly in _dif_update_crc32c_split For NVMe/TCP target, data segments which correspond to H2C or C2H PDU will have any alignment, and _dif_update_crc32c_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 On the other hand, _dif_update_crc32c_split had assumed that passed block is always a complete block. This patch exposes offset_in_block, data_len, and guard as parameters of _dif_update_crc32c_split() and make _dif_verify_split() process the above two types of data block properly. The next patch will utilize the updated _dif_update_crc32c_split to add spdk_dif_update_crc32c_stream(). Signed-off-by: Shuhei Matsumoto Change-Id: Iee29377ad49d4f209673fffb4de4a23a54f31766 Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/458918 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Changpeng Liu --- lib/util/dif.c | 17 +++++----- test/unit/lib/util/dif.c/dif_ut.c | 52 ++++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/lib/util/dif.c b/lib/util/dif.c index 7ac007be0..cfc13524c 100644 --- a/lib/util/dif.c +++ b/lib/util/dif.c @@ -712,27 +712,28 @@ dif_update_crc32c(struct _dif_sgl *sgl, uint32_t num_blocks, } static uint32_t -_dif_update_crc32c_split(struct _dif_sgl *sgl, uint32_t crc32c, - const struct spdk_dif_ctx *ctx) +_dif_update_crc32c_split(struct _dif_sgl *sgl, uint32_t offset_in_block, uint32_t data_len, + uint32_t crc32c, const struct spdk_dif_ctx *ctx) { - uint32_t data_block_size, offset_in_block, buf_len; + uint32_t data_block_size, buf_len; void *buf; data_block_size = ctx->block_size - ctx->md_size; - offset_in_block = 0; - while (offset_in_block < ctx->block_size) { + assert(offset_in_block + data_len <= ctx->block_size); + + while (data_len != 0) { _dif_sgl_get_buf(sgl, &buf, &buf_len); + buf_len = spdk_min(buf_len, data_len); if (offset_in_block < data_block_size) { buf_len = spdk_min(buf_len, data_block_size - offset_in_block); crc32c = spdk_crc32c_update(buf, buf_len, crc32c); - } else { - buf_len = spdk_min(buf_len, ctx->block_size - offset_in_block); } _dif_sgl_advance(sgl, buf_len); offset_in_block += buf_len; + data_len -= buf_len; } return crc32c; @@ -745,7 +746,7 @@ dif_update_crc32c_split(struct _dif_sgl *sgl, uint32_t num_blocks, uint32_t offset_blocks; for (offset_blocks = 0; offset_blocks < num_blocks; offset_blocks++) { - crc32c = _dif_update_crc32c_split(sgl, crc32c, ctx); + crc32c = _dif_update_crc32c_split(sgl, 0, ctx->block_size, crc32c, ctx); } return crc32c; diff --git a/test/unit/lib/util/dif.c/dif_ut.c b/test/unit/lib/util/dif.c/dif_ut.c index 188a4e79c..9f066417b 100644 --- a/test/unit/lib/util/dif.c/dif_ut.c +++ b/test/unit/lib/util/dif.c/dif_ut.c @@ -2233,6 +2233,54 @@ update_crc32c_test(void) } } +static void +_dif_update_crc32c_split_test(void) +{ + struct spdk_dif_ctx ctx = {}; + struct iovec iov; + uint8_t *buf; + struct _dif_sgl sgl; + uint32_t dif_flags, crc32c, prev_crc32c; + 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); + + buf = calloc(1, 4096 + 128); + SPDK_CU_ASSERT_FATAL(buf != NULL); + _iov_set_buf(&iov, buf, 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); + + _dif_sgl_init(&sgl, &iov, 1); + + crc32c = _dif_update_crc32c_split(&sgl, 0, 1000, UT_CRC32C_XOR, &ctx); + CU_ASSERT(crc32c == spdk_crc32c_update(buf, 1000, UT_CRC32C_XOR)); + + prev_crc32c = crc32c; + + crc32c = _dif_update_crc32c_split(&sgl, 1000, 3000, prev_crc32c, &ctx); + CU_ASSERT(crc32c == spdk_crc32c_update(buf + 1000, 3000, prev_crc32c)); + + prev_crc32c = crc32c; + + crc32c = _dif_update_crc32c_split(&sgl, 4000, 96 + 128, prev_crc32c, &ctx); + CU_ASSERT(crc32c == spdk_crc32c_update(buf + 4000, 96, prev_crc32c)); + + CU_ASSERT(crc32c == spdk_crc32c_update(buf, 4096, UT_CRC32C_XOR)); + + free(buf); +} + int main(int argc, char **argv) { @@ -2329,7 +2377,9 @@ main(int argc, char **argv) CU_add_test(suite, "_dif_verify_split_test", _dif_verify_split_test) == NULL || CU_add_test(suite, "dif_verify_stream_multi_segments_test", dif_verify_stream_multi_segments_test) == NULL || - CU_add_test(suite, "update_crc32c_test", update_crc32c_test) == NULL + CU_add_test(suite, "update_crc32c_test", update_crc32c_test) == NULL || + CU_add_test(suite, "_dif_update_crc32c_split_test", + _dif_update_crc32c_split_test) == NULL ) { CU_cleanup_registry(); return CU_get_error();