Commit 8b24fc4a authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Changpeng Liu
Browse files

dif: Process unaligned end of data segment in spdk_dif_set_md_interleave_iovs() 



For NVMe/TCP target, data segments which correspond to H2C or C2H PDU
will have any alignment. spdk_dif_set_md_interleave_iovs() have allowed
reading data to have any alignment but had required data segment to
be a multiple of block size.

In other words, spdk_dif_set_md_interleave_iovs() had required that both
ctx->data_offset and (data_offset + data_len) must be a multiple of the
data block size.

This patch refines the algorithm to remove the latter requirement.

The update implies that spdk_dif_set_md_interleave_iovs support any
data buffer whose size is less than a single data block.

The update doesn't change parameters of spdk_dif_set_md_interleave_iovs
and existing UT should be passed.

This patch adds additional UT code to test these updates.

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


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarDarek Stojaczyk <dariusz.stojaczyk@intel.com>
parent 2bb4fb4c

include/spdk/dif.h

+7 −2
Original line number Diff line number Diff line
@@ -288,12 +288,17 @@ int spdk_dix_inject_error(struct iovec *iovs, int iovcnt, struct iovec *md_iov, 
 * This function removes the necessity of data copy in the SPDK application

 * during DIF insertion and strip.

 *

 * When the extended LBA payload is splitted into multiple data segments,

 * start of each data segment is passed through the DIF context. data_offset

 * and data_len is within a data segment.

 *

 * \param iovs iovec array set by this function.

 * \param iovcnt Number of elements in the iovec array.

 * \param buf_iovs SGL for the buffer to create extended LBA payload.

 * \param buf_iovcnt Size of the SGL for the buffer to create extended LBA payload.

 * \param data_offset Offset to store the next incoming data.

 * \param data_len Expected length of the newly read data in the extended LBA payload.

 * \param data_offset Offset to store the next incoming data in the current data segment.

 * \param data_len Expected length of the newly read data in the current data segment of

 * the extended LBA payload.

 * \param mapped_len Output parameter that will contain data length mapped by

 * the iovec array.

 * \param ctx DIF context.

lib/util/dif.c

+16 −16
Original line number Diff line number Diff line
@@ -1385,6 +1385,12 @@ spdk_dix_inject_error(struct iovec *iovs, int iovcnt, struct iovec *md_iov, 
	return 0;

}



static uint32_t

_to_next_boundary(uint32_t offset, uint32_t boundary)

{

	return boundary - (offset % boundary);

}



int

spdk_dif_set_md_interleave_iovs(struct iovec *iovs, int iovcnt,

				struct iovec *buf_iovs, int buf_iovcnt,
@@ -1392,8 +1398,8 @@ spdk_dif_set_md_interleave_iovs(struct iovec *iovs, int iovcnt, 
				uint32_t *_mapped_len,

				const struct spdk_dif_ctx *ctx)

{

	uint32_t data_block_size, head_unalign;

	uint32_t num_blocks, offset_blocks;

	uint32_t data_block_size, head_unalign, tail_unalign;

	uint32_t num_blocks, offset_blocks, len;

	struct _dif_sgl dif_sgl;

	struct _dif_sgl buf_sgl;
@@ -1403,17 +1409,13 @@ spdk_dif_set_md_interleave_iovs(struct iovec *iovs, int iovcnt, 


	data_block_size = ctx->block_size - ctx->md_size;



	if (((data_offset + data_len) % data_block_size) != 0) {

		SPDK_ERRLOG("Data offset + length must be a multiple of data block size\n");

		return -EINVAL;

	}



	num_blocks = (data_offset + data_len) / data_block_size;

	tail_unalign = (data_offset + data_len) % data_block_size;



	_dif_sgl_init(&dif_sgl, iovs, iovcnt);

	_dif_sgl_init(&buf_sgl, buf_iovs, buf_iovcnt);



	if (!_dif_sgl_is_valid(&buf_sgl, num_blocks * ctx->block_size)) {

	if (!_dif_sgl_is_valid(&buf_sgl, num_blocks * ctx->block_size + tail_unalign)) {

		SPDK_ERRLOG("Buffer overflow will occur.\n");

		return -ERANGE;

	}
@@ -1421,19 +1423,17 @@ spdk_dif_set_md_interleave_iovs(struct iovec *iovs, int iovcnt, 
	offset_blocks = data_offset / data_block_size;

	head_unalign = data_offset % data_block_size;



	_dif_sgl_advance(&buf_sgl, offset_blocks * ctx->block_size);

	_dif_sgl_advance(&buf_sgl, offset_blocks * ctx->block_size + head_unalign);



	while (offset_blocks < num_blocks) {

		_dif_sgl_advance(&buf_sgl, head_unalign);

	while (data_len != 0) {

		len = spdk_min(data_len, _to_next_boundary(data_offset, data_block_size));



		if (!_dif_sgl_append_split(&dif_sgl, &buf_sgl,

					   data_block_size - head_unalign)) {

		if (!_dif_sgl_append_split(&dif_sgl, &buf_sgl, len)) {

			break;

		}

		_dif_sgl_advance(&buf_sgl, ctx->md_size);

		offset_blocks++;



		head_unalign = 0;

		data_offset += len;

		data_len -= len;

	}



	if (_mapped_len != NULL) {

test/unit/lib/util/dif.c/dif_ut.c

+61 −0
Original line number Diff line number Diff line
@@ -1719,6 +1719,65 @@ dif_generate_stream_test(void) 
	_iov_free_buf(&iov);

}



static void

set_md_interleave_iovs_alignment_test(void)

{

	struct iovec iovs[3], dif_iovs[5];

	uint32_t mapped_len = 0;

	int rc;

	struct spdk_dif_ctx ctx;



	rc = spdk_dif_ctx_init(&ctx, 512 + 8, 8, true, false, SPDK_DIF_TYPE1,

			       0, 0, 0, 0, 0, 0);

	CU_ASSERT(rc == 0);



	/* The case that buffer size is smaller than necessary. */

	_iov_set_buf(&iovs[0], (uint8_t *)0xDEADBEEF, 1024);

	_iov_set_buf(&iovs[1], (uint8_t *)0xFEEDBEEF, 1024);

	_iov_set_buf(&iovs[2], (uint8_t *)0xC0FFEE, 24);



	rc = spdk_dif_set_md_interleave_iovs(dif_iovs, 5, iovs, 3, 0, 2048, &mapped_len, &ctx);

	CU_ASSERT(rc == -ERANGE);



	/* The folllowing are the normal cases. */

	_iov_set_buf(&iovs[2], (uint8_t *)0xC0FFEE, 32);



	/* data length is less than a data block size. */

	rc = spdk_dif_set_md_interleave_iovs(dif_iovs, 5, iovs, 3, 0, 500, &mapped_len, &ctx);

	CU_ASSERT(rc == 1);

	CU_ASSERT(mapped_len == 500);

	CU_ASSERT(_iov_check(&dif_iovs[0], (void *)0xDEADBEEF, 500) == true);



	/* Pass enough number of iovecs */

	rc = spdk_dif_set_md_interleave_iovs(dif_iovs, 5, iovs, 3, 500, 1000, &mapped_len, &ctx);

	CU_ASSERT(rc == 4);

	CU_ASSERT(mapped_len == 1000);

	CU_ASSERT(_iov_check(&dif_iovs[0], (void *)(0xDEADBEEF + 500), 12) == true);

	CU_ASSERT(_iov_check(&dif_iovs[1], (void *)(0xDEADBEEF + 520), 504) == true);

	CU_ASSERT(_iov_check(&dif_iovs[2], (void *)0xFEEDBEEF, 8) == true);

	CU_ASSERT(_iov_check(&dif_iovs[3], (void *)(0xFEEDBEEF + 16), 476) == true);



	/* Pass iovecs smaller than necessary */

	rc = spdk_dif_set_md_interleave_iovs(dif_iovs, 3, iovs, 3, 500, 1000, &mapped_len, &ctx);

	CU_ASSERT(rc == 3);

	CU_ASSERT(mapped_len == 524);

	CU_ASSERT(_iov_check(&dif_iovs[0], (void *)(0xDEADBEEF + 500), 12) == true);

	CU_ASSERT(_iov_check(&dif_iovs[1], (void *)(0xDEADBEEF + 520), 504) == true);

	CU_ASSERT(_iov_check(&dif_iovs[2], (void *)0xFEEDBEEF, 8) == true);



	rc = spdk_dif_set_md_interleave_iovs(dif_iovs, 5, iovs, 3, 1500, 500, &mapped_len, &ctx);

	CU_ASSERT(rc == 2);

	CU_ASSERT(mapped_len == 500);

	CU_ASSERT(_iov_check(&dif_iovs[0], (void *)(0xFEEDBEEF + 492), 36) == true);

	CU_ASSERT(_iov_check(&dif_iovs[1], (void *)(0xFEEDBEEF + 536), 464) == true);



	rc = spdk_dif_set_md_interleave_iovs(dif_iovs, 5, iovs, 3, 2000, 48, &mapped_len, &ctx);

	CU_ASSERT(rc == 2);

	CU_ASSERT(mapped_len == 48);

	CU_ASSERT(_iov_check(&dif_iovs[0], (void *)0xFEEDBEEF + 1000, 24) == true);

	CU_ASSERT(_iov_check(&dif_iovs[1], (void *)0xC0FFEE, 24) ==  true);

}



#define UT_CRC32C_XOR	0xffffffffUL



static void
@@ -1900,6 +1959,8 @@ main(int argc, char **argv) 
		CU_add_test(suite, "set_md_interleave_iovs_split_test",

			    set_md_interleave_iovs_split_test) == NULL ||

		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, "update_crc32c_test", update_crc32c_test) == NULL

	) {

		CU_cleanup_registry();