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

[Aurelien Aptel <aaptel@nvidia.com>]

Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com> writes:
> 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?

Any target should do, but the host needs to use a NIC that supports DDP
offload. This means the loop/localhost target cannot be used.

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

It requires hardware that supports the ULP DDP infrastructure
(specifically nvme-tcp). This includes ConnectX 7 NIC and above or
BlueField 3 DPU and above.

>> +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?

The DDP offload is part of the nvme-tcp offload series [1], and the only
missing part is the netdev maintainer (Jakub) request of including the
tests for the feature. We agreed to have those tests as part of blktests,
which we will then run in a CI.

>> +     _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.

These are not declared as local because they are global, see the top of
the file.

I will declare the local ones on line when it makes it more readable.

Thanks.

1: https://lore.kernel.org/netdev/20240529160053.111531-1-aaptel@nvidia.com/