[PATCH blktests v2 0/4] blktests: add target test cases

[PATCH blktests v2 0/4] blktests: add target test cases

[Daniel Wagner]

For testing the nvmet-fcloop series I used these two test cases to
exercise the target setup/teardown code paths.

Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
Changes in v2:
- fixes shellcheck warnings
- removed dead code
- reworded comments
- updated requires section
- moved nvmf_wait_for_state to common code
- collected tags
- Link to v1: https://lore.kernel.org/r/20250318-test-target-v1-0-01e01142cf2b@kernel.org

---
Daniel Wagner (4):
      common/nvme: add debug nvmet path variable
      common/nvme: move nvmf_wait_for_state to common code
      nvme/060: add test nvme fabrics target resetting during I/O
      nvme/061: add test teardown and setup fabrics target during I/O

 common/nvme        | 29 ++++++++++++++++++++++++
 tests/nvme/048     | 31 ++-----------------------
 tests/nvme/060     | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/060.out |  2 ++
 tests/nvme/061     | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/061.out | 21 +++++++++++++++++
 6 files changed, 182 insertions(+), 29 deletions(-)
---
base-commit: 236edfd5d892f0abb0747f2668d1b9734349e2f6
change-id: 20250318-test-target-0d63599d94b0

Best regards,
-- 
Daniel Wagner <wagi@kernel.org>

[PATCH blktests v2 1/4] common/nvme: add debug nvmet path variable

[Daniel Wagner]

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
 common/nvme | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/nvme b/common/nvme
index 3761329d39e3763136f60a4751ad15de347f6e9b..04b49c2c1f9edc6de516398b537502fc30a92969 100644
--- a/common/nvme
+++ b/common/nvme
@@ -26,6 +26,8 @@ nvmet_blkdev_type=${nvmet_blkdev_type:-"device"}
 NVMET_BLKDEV_TYPES=${NVMET_BLKDEV_TYPES:-"device file"}
 nvme_target_control="${NVME_TARGET_CONTROL:-}"
 NVMET_CFS="/sys/kernel/config/nvmet/"
+# shellcheck disable=SC2034
+NVMET_DFS="/sys/kernel/debug/nvmet/"
 nvme_trtype=${nvme_trtype:-}
 nvme_adrfam=${nvme_adrfam:-}
 

-- 
2.49.0

[PATCH blktests v2 2/4] common/nvme: move nvmf_wait_for_state to common code

[Daniel Wagner]

In preperation to add another tests which needs the nvmf_wait_for_state
function, move it to the common code base.

Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
 common/nvme    | 27 +++++++++++++++++++++++++++
 tests/nvme/048 | 31 ++-----------------------------
 2 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/common/nvme b/common/nvme
index 04b49c2c1f9edc6de516398b537502fc30a92969..68720ddc20cf3ed5cfe2841f9921321d9899ce0d 100644
--- a/common/nvme
+++ b/common/nvme
@@ -573,6 +573,33 @@ _nvmf_wait_for_ns() {
 	return 0
 }
 
+_nvmf_wait_for_state() {
+	local def_state_timeout=5
+	local subsys_name="$1"
+	local state="$2"
+	local timeout="${3:-$def_state_timeout}"
+	local nvmedev
+	local state_file
+	local start_time
+	local end_time
+
+	nvmedev=$(_find_nvme_dev "${subsys_name}")
+	state_file="/sys/class/nvme-fabrics/ctl/${nvmedev}/state"
+
+	start_time=$(date +%s)
+	while ! grep -q "${state}" "${state_file}"; do
+		sleep 1
+		end_time=$(date +%s)
+		if (( end_time - start_time > timeout )); then
+			echo "expected state \"${state}\" not " \
+				"reached within ${timeout} seconds"
+			return 1
+		fi
+	done
+
+	return 0
+}
+
 _create_nvmet_ns() {
 	local subsysnqn="${def_subsysnqn}"
 	local nsid="${def_nsid}"
diff --git a/tests/nvme/048 b/tests/nvme/048
index bd41faef4e5431957f63184408c147b8ada1a8dd..afd9272c1a31b5a3d2df2e1ce9fe3268de768420 100755
--- a/tests/nvme/048
+++ b/tests/nvme/048
@@ -19,33 +19,6 @@ set_conditions() {
 	_set_nvme_trtype "$@"
 }
 
-nvmf_wait_for_state() {
-	local def_state_timeout=5
-	local subsys_name="$1"
-	local state="$2"
-	local timeout="${3:-$def_state_timeout}"
-	local nvmedev
-	local state_file
-	local start_time
-	local end_time
-
-	nvmedev=$(_find_nvme_dev "${subsys_name}")
-	state_file="/sys/class/nvme-fabrics/ctl/${nvmedev}/state"
-
-	start_time=$(date +%s)
-	while ! grep -q "${state}" "${state_file}"; do
-		sleep 1
-		end_time=$(date +%s)
-		if (( end_time - start_time > timeout )); then
-			echo "expected state \"${state}\" not " \
-				"reached within ${timeout} seconds"
-			return 1
-		fi
-	done
-
-	return 0
-}
-
 nvmf_check_queue_count() {
 	local subsys_name="$1"
 	local queue_count="$2"
@@ -87,7 +60,7 @@ set_qid_max() {
 
 	set_nvmet_attr_qid_max "${subsys_name}" "${qid_max}"
 	nvmf_check_queue_count "${subsys_name}" "${qid_max}" || return 1
-	nvmf_wait_for_state "${subsys_name}" "live" || return 1
+	_nvmf_wait_for_state "${subsys_name}" "live" || return 1
 
 	return 0
 }
@@ -106,7 +79,7 @@ test() {
 		_nvme_connect_subsys --keep-alive-tmo 1 \
 				     --reconnect-delay 2
 
-		if ! nvmf_wait_for_state "${def_subsysnqn}" "live" ; then
+		if ! _nvmf_wait_for_state "${def_subsysnqn}" "live" ; then
 			echo FAIL
 		else
 			set_qid_max "${def_subsysnqn}" 1 || echo FAIL

-- 
2.49.0

[PATCH blktests v2 3/4] nvme/060: add test nvme fabrics target resetting during I/O

[Daniel Wagner]

Newer kernel support to reset the target via the debugfs. Add a new test
case which exercises this interface.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
 tests/nvme/060     | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/060.out |  2 ++
 2 files changed, 64 insertions(+)

diff --git a/tests/nvme/060 b/tests/nvme/060
new file mode 100755
index 0000000000000000000000000000000000000000..293b03cd0ef6a75722c6b86fccbbc931d6f5e3a1
--- /dev/null
+++ b/tests/nvme/060
@@ -0,0 +1,62 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2025 Daniel Wagner, SUSE Labs
+#
+# Test nvme fabrics controller reset/disconnect/reconnect.
+
+. tests/nvme/rc
+
+DESCRIPTION="test nvme fabrics target reset"
+
+requires() {
+	_nvme_requires
+	_have_loop
+	_require_nvme_trtype_is_fabrics
+	_have_kernel_option NVME_TARGET_DEBUGF
+}
+
+set_conditions() {
+	_set_nvme_trtype "$@"
+}
+
+nvmet_debug_trigger_reset() {
+	local nvmet_subsystem="$1"
+	local dfs_path="${NVMET_DFS}/${nvmet_subsystem}"
+
+	find "${dfs_path}" -maxdepth 1 -type d -name 'ctrl*' -exec sh -c 'echo "fatal" > "$1/state"' _ {} \;
+}
+
+nvmet_reset_loop() {
+	while true; do
+		nvmet_debug_trigger_reset "${def_subsysnqn}"
+		sleep 2
+	done
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	_setup_nvmet
+
+	_nvmet_target_setup
+
+	nvmet_reset_loop &
+	reset_loop_pid=$!
+
+	# Reset the host in different states e.g when the host is in the
+	# connected or connecting state.
+	#
+	# The target reset is triggered with an even number timeout, while the
+	# host reconnects with an odd number timeout.
+	for ((i = 0; i <= 5; i++)); do
+		_nvme_connect_subsys --keep-alive-tmo 1 --reconnect-delay 1
+		sleep 3
+		_nvme_disconnect_subsys >> "$FULL" 2>&1
+	done
+
+	{ kill "${reset_loop_pid}"; wait; } &> /dev/null
+
+	_nvmet_target_cleanup
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/060.out b/tests/nvme/060.out
new file mode 100644
index 0000000000000000000000000000000000000000..517ff2dfcfd41c4088991e669af9fef52bde570b
--- /dev/null
+++ b/tests/nvme/060.out
@@ -0,0 +1,2 @@
+Running nvme/060
+Test complete

-- 
2.49.0

Re: [PATCH blktests v2 3/4] nvme/060: add test nvme fabrics target resetting during I/O

[Daniel Wagner]

On Tue, Apr 08, 2025 at 06:25:59PM +0200, Daniel Wagner wrote:
> +requires() {
> +	_nvme_requires
> +	_have_loop
> +	_require_nvme_trtype_is_fabrics
> +	_have_kernel_option NVME_TARGET_DEBUGF

Typo: s/NVME_TARGET_DEBUGF/NVME_TARGET_DEBUGFS/

Re: [PATCH blktests v2 3/4] nvme/060: add test nvme fabrics target resetting during I/O

[Shinichiro Kawasaki]

On Apr 09, 2025 / 19:59, Daniel Wagner wrote:
> On Tue, Apr 08, 2025 at 06:25:59PM +0200, Daniel Wagner wrote:
> > +requires() {
> > +	_nvme_requires
> > +	_have_loop
> > +	_require_nvme_trtype_is_fabrics
> > +	_have_kernel_option NVME_TARGET_DEBUGF
> 
> Typo: s/NVME_TARGET_DEBUGF/NVME_TARGET_DEBUGFS/

I can fold in this change.

BTW, when I ran this test case for tr=loop, I observe the kernel
message reports "invalid parameter" errors as follows:

[  150.907272][ T2312] run blktests nvme/060 at 2025-04-14 20:59:00
[  150.947249][ T2367] loop0: detected capacity change from 0 to 2097152
[  150.959649][ T2370] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
[  151.009708][ T2382] nvme_fabrics: invalid parameter 'reconnect_delay=%d'
[  155.626630][ T2429] nvme_fabrics: invalid parameter 'reconnect_delay=%d'
[  160.158015][ T2476] nvme_fabrics: invalid parameter 'reconnect_delay=%d'
[  164.745460][ T2523] nvme_fabrics: invalid parameter 'reconnect_delay=%d'
[  169.161893][ T2570] nvme_fabrics: invalid parameter 'reconnect_delay=%d'
[  173.766537][ T2619] nvme_fabrics: invalid parameter 'reconnect_delay=%d'

Is it expected?

This test case passes for other transports (rdma, tcp and fc) and they do not
trigger the error message above. My mere guess is that this case might not work
for the loop transport, and may need "_require_nvme_trtype tcp rdma fc" in same
manner as nvme/048.

[PATCH blktests v2 4/4] nvme/061: add test teardown and setup fabrics target during I/O

[Daniel Wagner]

Add a new test case which forcefully removes the target and setup it
again.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
 tests/nvme/061     | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/061.out | 21 +++++++++++++++++
 2 files changed, 87 insertions(+)

diff --git a/tests/nvme/061 b/tests/nvme/061
new file mode 100755
index 0000000000000000000000000000000000000000..3b59d7137932258d06c3d64166db493df176ba4e
--- /dev/null
+++ b/tests/nvme/061
@@ -0,0 +1,66 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2025 Daniel Wagner, SUSE Labs
+#
+# Test if the host keeps running IO when the target is forcefully removed and
+# added back.
+
+. tests/nvme/rc
+
+DESCRIPTION="test fabric target teardown and setup during I/O"
+TIMED=1
+
+requires() {
+	_nvme_requires
+	_have_loop
+	_have_fio
+	_require_nvme_trtype_is_fabrics
+}
+
+set_conditions() {
+	_set_nvme_trtype "$@"
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	_setup_nvmet
+
+	local ns
+
+	_nvmet_target_setup
+
+	_nvme_connect_subsys --keep-alive-tmo 1 --reconnect-delay 1
+
+	ns=$(_find_nvme_ns "${def_subsys_uuid}")
+
+	_run_fio_rand_io --filename="/dev/${ns}" \
+		--group_reporting \
+		--time_based --runtime=1d &> /dev/null &
+	fio_pid=$!
+	sleep 1
+
+	nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
+	state_file="/sys/class/nvme-fabrics/ctl/${nvmedev}/state"
+	for ((i = 0; i <= 5; i++)); do
+		echo "iteration $i"
+
+		_nvmet_target_cleanup
+
+		_nvmf_wait_for_state "${def_subsysnqn}" "connecting" || return 1
+		echo "state: $(cat ${state_file})"
+
+		_nvmet_target_setup
+
+		_nvmf_wait_for_state "${def_subsysnqn}" "live" || return 1
+		echo "state: $(cat ${state_file})"
+	done
+
+	{ kill "${fio_pid}"; wait; } &> /dev/null
+
+	_nvme_disconnect_subsys
+
+	_nvmet_target_cleanup
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/061.out b/tests/nvme/061.out
new file mode 100644
index 0000000000000000000000000000000000000000..75516abdac005854c2be165005c076ef8891c518
--- /dev/null
+++ b/tests/nvme/061.out
@@ -0,0 +1,21 @@
+Running nvme/061
+iteration 0
+state: connecting
+state: live
+iteration 1
+state: connecting
+state: live
+iteration 2
+state: connecting
+state: live
+iteration 3
+state: connecting
+state: live
+iteration 4
+state: connecting
+state: live
+iteration 5
+state: connecting
+state: live
+disconnected 1 controller(s)
+Test complete

-- 
2.49.0

Re: [PATCH blktests v2 4/4] nvme/061: add test teardown and setup fabrics target during I/O

[Shinichiro Kawasaki]

On Apr 08, 2025 / 18:26, Daniel Wagner wrote:
> Add a new test case which forcefully removes the target and setup it
> again.
> 
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
> Signed-off-by: Daniel Wagner <wagi@kernel.org>

When I ran this test case for tr=loop, it fails.

The out file is as follows:

  $ cat ./results/nodev_tr_loop/nvme/061.out.bad
  Running nvme/061
  iteration 0
  grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
  grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
  grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
  grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
  grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
  grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
  expected state "connecting" not  reached within 5 seconds

And kernel reported the "invalid parameter" message:

  [  888.896492][ T3112] run blktests nvme/061 at 2025-04-14 21:11:18
  [  888.937128][ T3158] loop0: detected capacity change from 0 to 2097152
  [  888.949671][ T3161] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
  [  888.997790][ T3170] nvme_fabrics: invalid parameter 'reconnect_delay=%d'

In the v1 series, you noted that your fc-loop fixes will avoid the failure.
But the failure was observed with tr=loop, so I'm not sure fc-loop fixes
will avoids the failure. I'm wondering if this test case is for rdma/tcp/fc
transports only and suspecting it may not be intended for the loop transport.

Also, please find nit comments in line:

> ---
>  tests/nvme/061     | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/nvme/061.out | 21 +++++++++++++++++
>  2 files changed, 87 insertions(+)
> 
> diff --git a/tests/nvme/061 b/tests/nvme/061
> new file mode 100755
> index 0000000000000000000000000000000000000000..3b59d7137932258d06c3d64166db493df176ba4e
> --- /dev/null
> +++ b/tests/nvme/061
> @@ -0,0 +1,66 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2025 Daniel Wagner, SUSE Labs
> +#
> +# Test if the host keeps running IO when the target is forcefully removed and
> +# added back.
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="test fabric target teardown and setup during I/O"
> +TIMED=1
> +
> +requires() {
> +	_nvme_requires
> +	_have_loop
> +	_have_fio
> +	_require_nvme_trtype_is_fabrics
> +}
> +
> +set_conditions() {
> +	_set_nvme_trtype "$@"
> +}
> +
> +test() {
> +	echo "Running ${TEST_NAME}"
> +
> +	_setup_nvmet
> +
> +	local ns
> +
> +	_nvmet_target_setup
> +
> +	_nvme_connect_subsys --keep-alive-tmo 1 --reconnect-delay 1
> +
> +	ns=$(_find_nvme_ns "${def_subsys_uuid}")
> +
> +	_run_fio_rand_io --filename="/dev/${ns}" \
> +		--group_reporting \
> +		--time_based --runtime=1d &> /dev/null &
> +	fio_pid=$!
> +	sleep 1
> +
> +	nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
> +	state_file="/sys/class/nvme-fabrics/ctl/${nvmedev}/state"
> +	for ((i = 0; i <= 5; i++)); do
> +		echo "iteration $i"
> +
> +		_nvmet_target_cleanup
> +
> +		_nvmf_wait_for_state "${def_subsysnqn}" "connecting" || return 1
> +		echo "state: $(cat ${state_file})"

The line above needs one more pair of double quotations to avoid the
shellcheck warn:

		echo "state: $(cat "${state_file}")"

> +
> +		_nvmet_target_setup
> +
> +		_nvmf_wait_for_state "${def_subsysnqn}" "live" || return 1
> +		echo "state: $(cat ${state_file})"

Same here:

		echo "state: $(cat "${state_file}")"

Re: [PATCH blktests v2 4/4] nvme/061: add test teardown and setup fabrics target during I/O

[Daniel Wagner]

On Mon, Apr 14, 2025 at 12:18:05PM +0000, Shinichiro Kawasaki wrote:
> On Apr 08, 2025 / 18:26, Daniel Wagner wrote:
> > Add a new test case which forcefully removes the target and setup it
> > again.
> > 
> > Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
> > Signed-off-by: Daniel Wagner <wagi@kernel.org>
> 
> When I ran this test case for tr=loop, it fails.
> 
> The out file is as follows:
> 
>   $ cat ./results/nodev_tr_loop/nvme/061.out.bad
>   Running nvme/061
>   iteration 0
>   grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
>   grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
>   grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
>   grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
>   grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
>   grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
>   expected state "connecting" not  reached within 5 seconds
> 
> And kernel reported the "invalid parameter" message:
> 
>   [  888.896492][ T3112] run blktests nvme/061 at 2025-04-14 21:11:18
>   [  888.937128][ T3158] loop0: detected capacity change from 0 to 2097152
>   [  888.949671][ T3161] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
>   [  888.997790][ T3170] nvme_fabrics: invalid parameter 'reconnect_delay=%d'
> 
> In the v1 series, you noted that your fc-loop fixes will avoid the
> failure.

Yes, the fcloop fixes will address the fc transport failures. The above
failure is caused by the same issue as in 060: the loop transport
doesn't know about reconnect_delay, thus the requires should list tcp,
rdma and fc as valid transport and not loop.

> But the failure was observed with tr=loop, so I'm not sure fc-loop fixes
> will avoids the failure. I'm wondering if this test case is for rdma/tcp/fc
> transports only and suspecting it may not be intended for the loop
> transport.

Yes, this is correct. My test script listed only tcp, rdma and fc, so I
didn't catch the error with loop.

> > +	nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
> > +	state_file="/sys/class/nvme-fabrics/ctl/${nvmedev}/state"
> > +	for ((i = 0; i <= 5; i++)); do
> > +		echo "iteration $i"
> > +
> > +		_nvmet_target_cleanup
> > +
> > +		_nvmf_wait_for_state "${def_subsysnqn}" "connecting" || return 1
> > +		echo "state: $(cat ${state_file})"
> 
> The line above needs one more pair of double quotations to avoid the
> shellcheck warn:
> 
> 		echo "state: $(cat "${state_file}")"

I didn't know this is actually allowed and even required. Sure, will
update the patches accordingly.

Thanks,
Daniel