Commit d3032139 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Konrad Sztyber
Browse files

dif: Remove alignment requirement from spdk_dif_generate_copy()



spdk_dif_generate_copy() required each iovec of bounce buffer to be
multiple of block size length. This was too strong.

Software implementation should not have strong limitation on
configuration.

Remove the limitation in this patch.

Modify the functional test of the DIF feature of the accel framework
slightly. The total length of the iovec still should be larger than
payload length.

spdk_dif_verify_copy() have the same strong limitation. However, in unit
tests, spdk_dif_generate_copy() and spdk_dif_verify_copy() are a pair
and share the same bounce buffer.

If we add bugs to both of spdk_dif_generate_copy() and
spdk_dif_verify_copy(), it is likely that the bugs are not found in
unit tests. Hence, this patch changes only spdk_dif_generate_copy().
The next patch will change spdk_dif_verify_copy().

Signed-off-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Change-Id: Ib50299a8b337a4825708891f9739e72a862fbf49
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/23846


Community-CI: Mellanox Build Bot
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
parent aee021cd
Loading
Loading
Loading
Loading
+35 −33
Original line number Diff line number Diff line
@@ -345,21 +345,24 @@ _dif_generate_guard_copy(uint64_t guard_seed, void *dst, void *src, size_t buf_l
}

static uint64_t
_dif_generate_guard_copy_split_src(uint64_t guard, uint8_t *dst,
_dif_generate_guard_copy_split(uint64_t guard, struct _dif_sgl *dst_sgl,
			       struct _dif_sgl *src_sgl, uint32_t data_len,
			       enum spdk_dif_pi_format dif_pi_format)
{
	uint32_t offset = 0, src_len;
	uint8_t *src;
	uint32_t offset = 0, src_len, dst_len, buf_len;
	uint8_t *src, *dst;

	while (offset < data_len) {
		_dif_sgl_get_buf(src_sgl, &src, &src_len);
		src_len = spdk_min(src_len, data_len - offset);
		_dif_sgl_get_buf(dst_sgl, &dst, &dst_len);
		buf_len = spdk_min(src_len, dst_len);
		buf_len = spdk_min(buf_len, data_len - offset);

		guard = _dif_generate_guard_copy(guard, dst + offset, src, src_len, dif_pi_format);
		guard = _dif_generate_guard_copy(guard, dst, src, buf_len, dif_pi_format);

		_dif_sgl_advance(src_sgl, src_len);
		offset += src_len;
		_dif_sgl_advance(src_sgl, buf_len);
		_dif_sgl_advance(dst_sgl, buf_len);
		offset += buf_len;
	}

	return guard;
@@ -387,19 +390,22 @@ _dif_generate_guard_copy_split_dst(uint64_t guard, uint8_t *src,
}

static void
_data_copy_split_src(uint8_t *dst, struct _dif_sgl *src_sgl, uint32_t data_len)
_data_copy_split(struct _dif_sgl *dst_sgl, struct _dif_sgl *src_sgl, uint32_t data_len)
{
	uint32_t offset = 0, src_len;
	uint8_t *src;
	uint32_t offset = 0, src_len, dst_len, buf_len;
	uint8_t *src, *dst;

	while (offset < data_len) {
		_dif_sgl_get_buf(src_sgl, &src, &src_len);
		src_len = spdk_min(src_len, data_len - offset);
		_dif_sgl_get_buf(dst_sgl, &dst, &dst_len);
		buf_len = spdk_min(src_len, dst_len);
		buf_len = spdk_min(buf_len, data_len - offset);

		memcpy(dst + offset, src, src_len);
		memcpy(dst, src, buf_len);

		_dif_sgl_advance(src_sgl, src_len);
		offset += src_len;
		_dif_sgl_advance(src_sgl, buf_len);
		_dif_sgl_advance(dst_sgl, buf_len);
		offset += buf_len;
	}
}

@@ -1247,25 +1253,24 @@ _dif_generate_copy_split(struct _dif_sgl *src_sgl, struct _dif_sgl *dst_sgl,
			 uint32_t offset_blocks, const struct spdk_dif_ctx *ctx)
{
	uint32_t data_block_size;
	uint8_t *dst;
	uint64_t guard = 0;

	_dif_sgl_get_buf(dst_sgl, &dst, NULL);
	struct spdk_dif dif = {};

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

	if (ctx->dif_flags & SPDK_DIF_FLAGS_GUARD_CHECK) {
		guard = _dif_generate_guard_copy_split_src(ctx->guard_seed, dst, src_sgl,
		guard = _dif_generate_guard_copy_split(ctx->guard_seed, dst_sgl, src_sgl,
						       data_block_size, ctx->dif_pi_format);
		guard = _dif_generate_guard(guard, dst + data_block_size,
					    ctx->guard_interval - data_block_size, ctx->dif_pi_format);
		guard = dif_generate_guard_split(guard, dst_sgl, data_block_size,
						 ctx->guard_interval - data_block_size, ctx);
	} else {
		_data_copy_split_src(dst, src_sgl, data_block_size);
		_data_copy_split(dst_sgl, src_sgl, data_block_size);
		_dif_sgl_advance(dst_sgl, ctx->guard_interval - data_block_size);
	}

	_dif_sgl_advance(dst_sgl, ctx->block_size);
	_dif_generate(&dif, guard, offset_blocks, ctx);

	_dif_generate(dst + ctx->guard_interval, guard, offset_blocks, ctx);
	dif_store_split(dst_sgl, &dif, ctx);
}

static void
@@ -1292,21 +1297,18 @@ spdk_dif_generate_copy(struct iovec *iovs, int iovcnt, struct iovec *bounce_iovs

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

	if (!_dif_sgl_is_valid(&src_sgl, data_block_size * num_blocks)) {
	if (!_dif_sgl_is_valid(&src_sgl, data_block_size * num_blocks) ||
	    !_dif_sgl_is_valid(&dst_sgl, ctx->block_size * num_blocks)) {
		SPDK_ERRLOG("Size of iovec arrays are not valid.\n");
		return -EINVAL;
	}

	if (!_dif_sgl_is_valid_block_aligned(&dst_sgl, num_blocks, ctx->block_size)) {
		SPDK_ERRLOG("Size of bounce_iovs arrays are not valid or misaligned with block_size.\n");
		return -EINVAL;
	}

	if (_dif_is_disabled(ctx->dif_type)) {
		return 0;
	}

	if (_dif_sgl_is_bytes_multiple(&src_sgl, data_block_size)) {
	if (_dif_sgl_is_bytes_multiple(&src_sgl, data_block_size) &&
	    _dif_sgl_is_bytes_multiple(&dst_sgl, ctx->block_size)) {
		dif_generate_copy(&src_sgl, &dst_sgl, num_blocks, ctx);
	} else {
		dif_generate_copy_split(&src_sgl, &dst_sgl, num_blocks, ctx);
+1 −1
Original line number Diff line number Diff line
@@ -1264,7 +1264,7 @@ accel_dif_generate_copy_op_iovecs_len_validate(void)
	req.channel = g_channel;
	req.dst_iovs = task->dst_iovs;
	/* Make iov_len param incorrect */
	req.dst_iovs->iov_len += 16;
	req.dst_iovs->iov_len -= 16;
	req.dst_iovcnt = task->dst_iovcnt;
	req.src_iovs = task->src_iovs;
	req.src_iovcnt = task->src_iovcnt;