diff mbox series

[blktests,v1,2/2] nvme/048: add reconnect after ctrl key change

Message ID 20240304161303.19681-3-dwagner@suse.de (mailing list archive)
State New, archived
Headers show
Series extend nvme/045 to reconnect with invalid key | expand

Commit Message

Daniel Wagner March 4, 2024, 4:13 p.m. UTC 
The re-authentication is a soft state, meaning unless the host has to
reconnect a key change on the target side is not observed. Extend the
current test with a forced reconnect after a key change. This
exercises the DNR handling code of the host.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 tests/nvme/045     | 72 +++++++++++++++++++++++++++++++++++++++++++++-
 tests/nvme/045.out |  3 +-
 2 files changed, 73 insertions(+), 2 deletions(-)

Comments

Shin'ichiro Kawasaki March 5, 2024, 9:38 a.m. UTC | #1 
On Mar 04, 2024 / 17:13, Daniel Wagner wrote:
> The re-authentication is a soft state, meaning unless the host has to
> reconnect a key change on the target side is not observed. Extend the
> current test with a forced reconnect after a key change. This
> exercises the DNR handling code of the host.
> 
> Signed-off-by: Daniel Wagner <dwagner@suse.de>

This looks a good improvement of the test case. I have two comments:

- The commit title says nvme/048, but it should be nvme/045.
- The helper functions nvmf_wait_for_state() and set_nvmet_attr_qid_max()
  are exactly same as those in nvme/048, aren't they?
  Probably, it's the better to move them to nvme/rc, as a preparation patch.
Daniel Wagner March 5, 2024, 11:08 a.m. UTC | #2 
On Tue, Mar 05, 2024 at 09:38:41AM +0000, Shinichiro Kawasaki wrote:
> On Mar 04, 2024 / 17:13, Daniel Wagner wrote:
> > The re-authentication is a soft state, meaning unless the host has to
> > reconnect a key change on the target side is not observed. Extend the
> > current test with a forced reconnect after a key change. This
> > exercises the DNR handling code of the host.
> > 
> > Signed-off-by: Daniel Wagner <dwagner@suse.de>
> 
> This looks a good improvement of the test case. I have two comments:
> 
> - The commit title says nvme/048, but it should be nvme/045.

Indeed :)

> - The helper functions nvmf_wait_for_state() and set_nvmet_attr_qid_max()
>   are exactly same as those in nvme/048, aren't they?
>   Probably, it's the better to move them to nvme/rc, as a preparation patch.

I forgot that we already had those two helpers. Sure, will do.
diff mbox series

Patch

diff --git a/tests/nvme/045 b/tests/nvme/045
index be408b629771..b0527685f61f 100755
--- a/tests/nvme/045
+++ b/tests/nvme/045
@@ -21,6 +21,62 @@  requires() {
 	_have_driver dh_generic
 }
 
+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_wait_for_ctrl_delete() {
+	local def_state_timeout=5
+	local nvmedev="$1"
+	local timeout="${2:-$def_state_timeout}"
+	local ctrl="/sys/class/nvme-fabrics/ctl/${nvmedev}/state"
+	local start_time
+	local end_time
+
+	start_time=$(date +%s)
+	while [ -f "${ctrl}" ]; do
+		sleep 1
+		end_time=$(date +%s)
+		if (( end_time - start_time > timeout )); then
+			echo "controller \"${nvmedev}\" not deleted" \
+				"within ${timeout} seconds"
+			return 1
+		fi
+	done
+
+	return 0
+}
+
+set_nvmet_attr_qid_max() {
+	local nvmet_subsystem="$1"
+	local qid_max="$2"
+	local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
+
+	echo "${qid_max}" > "${cfs_path}/attr_qid_max"
+}
 
 test() {
 	echo "Running ${TEST_NAME}"
@@ -55,7 +111,11 @@  test() {
 			     --hostnqn "${def_hostnqn}" \
 			     --hostid "${def_hostid}" \
 			     --dhchap-secret "${hostkey}" \
-			     --dhchap-ctrl-secret "${ctrlkey}"
+			     --dhchap-ctrl-secret "${ctrlkey}" \
+			     --reconnect-delay 1
+
+	nvmf_wait_for_state "${def_subsysnqn}" "live"
+	nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
 
 	echo "Re-authenticate with original host key"
 
@@ -108,6 +168,16 @@  test() {
 	rand_io_size="$(_nvme_calc_rand_io_size 4m)"
 	_run_fio_rand_io --size="${rand_io_size}" --filename="/dev/${nvmedev}n1"
 
+	echo "Renew host key on the controller and force reconnect"
+
+	new_hostkey="$(nvme gen-dhchap-key -n ${def_subsysnqn} 2> /dev/null)"
+
+	_set_nvmet_hostkey "${def_hostnqn}" "${new_hostkey}"
+
+	# Force a reconnect
+	set_nvmet_attr_qid_max "${def_subsysnqn}" 1
+	nvmf_wait_for_ctrl_delete "${nvmedev}"
+
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
 	_nvmet_target_cleanup
diff --git a/tests/nvme/045.out b/tests/nvme/045.out
index 84a69ef5c4c6..3331304ef71a 100644
--- a/tests/nvme/045.out
+++ b/tests/nvme/045.out
@@ -8,5 +8,6 @@  Change DH group to ffdhe8192
 Re-authenticate with changed DH group
 Change hash to hmac(sha512)
 Re-authenticate with changed hash
-disconnected 1 controller(s)
+Renew host key on the controller and force reconnect
+disconnected 0 controller(s)
 Test complete