[PATCH blktests v4 5/5] nvme/055: add test for nvme-tcp zero-copy offload

[Shinichiro Kawasaki]

On Nov 26, 2024 / 22:38, Aurelien Aptel wrote:
> This commit adds a new test for the kernel ULP DDP (Direct Data
> Placement) feature with NVMe-TCP.
> 
> Configuration of DDP is per NIC and is done through a script in the
> kernel source. For this reason we add 2 new config vars:
> - KERNELSRC: path to the running kernel sources
> - NVME_IFACE: name of the network interface to configure the offload on
> 
> Signed-off-by: Aurelien Aptel <aaptel at nvidia.com>
> Signed-off-by: Shai Malin smalin at nvidia.com
> Reviewed-by: Daniel Wagner <dwagner at suse.de>

This test is interesting!

[...]

> diff --git a/tests/nvme/055 b/tests/nvme/055
> new file mode 100755
> index 0000000..7e76126
> --- /dev/null
> +++ b/tests/nvme/055
> @@ -0,0 +1,285 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2024 Aurelien Aptel <aaptel at nvidia.com>
> +#
> +# zero-copy offload

My understanding is that this test case requires the target set up by
NVME_TARGET_CONTROL. Is it beneficial to explain what kind of target set
up is required here?

Does this test case require specific hardware for nvme-tcp and zero-copy?
If so, it can be described here also, probably.

> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="enable zero copy offload and run rw traffic"
> +TIMED=1
> +
> +iface_idx=""
> +
> +# these vars get updated after each call to connect_run_disconnect()
> +nb_packets=0
> +nb_bytes=0
> +nb_offload_packets=0
> +nb_offload_bytes=0
> +offload_bytes_ratio=0
> +offload_packets_ratio=0
> +
> +requires() {
> +	_nvme_requires
> +	_require_remote_nvme_target
> +	_require_nvme_trtype tcp
> +	_have_kernel_option ULP_DDP
> +	# require nvme-tcp as a module to be able to change the ddp_offload param
> +	_have_module nvme_tcp && _have_module_param nvme_tcp ddp_offload

I checked the latest kernel source code but could not find the ddp_offload
parameter. Do I miss anything? or Do you plan to post kernel patches for it?

> +	_have_fio
> +	_have_program ip
> +	_have_program ethtool
> +	_have_kernel_source && have_netlink_cli && _have_program python3
> +	have_iface
> +}
> +
[...]
> +
> +connect_run_disconnect() {
> +	local io_size
> +	local nvme_dev
> +	local nb_drop
> +	local drop_ratio
> +	local nb_resync
> +	local resync_ratio

Nit: some local variables misses declarations here: nb_packets,
nb_offload_packets, etc. It might be good to declare multiple variables
in one line, like "local nb_drop nb_resync nb_packets ..." to reduce number
of lines.