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@xxxxxxxxxx>
> Signed-off-by: Shai Malin smalin@xxxxxxxxxx
> Reviewed-by: Daniel Wagner <dwagner@xxxxxxx>
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@xxxxxxxxxx>
> +#
> +# 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.
