All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication
@ 2022-06-10 11:33 Hannes Reinecke
  2022-06-10 11:33 ` [PATCH 01/10] nvme/rc: do not print error message when no nvme device is found Hannes Reinecke
                   ` (10 more replies)
  0 siblings, 11 replies; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-10 11:33 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke

Hi all,

some people (Hi Sagi!) have complained that nvme in-band authentication
provide far too many configuration options to test it with some easy
commands. So here's a test suite for testing (most of) the various
configuration options and features.

As usual, comments and reviews are welcome.

Changes to v4:
   - Rebase to current HEAD
   - Use return code from 'nvme connect' instead of looking for
     the controller node

Changes to v3:
   - Rebase to current HEAD

Changes to v2:
   - Merge first two tests
   - Add optional arguments to _nvme_connect_subsys()
   - Convert to use _nvme_connect_subsys()

Changes to v1:
  - Move tests to the 'nvme' directory
  - Check for authentication failure on invalid keys

Hannes Reinecke (10):
  nvme/rc: do not print error message when no nvme device is found
  nvme/rc: clear allowed_hosts subdirectory
  nvme/rc: clear hosts directory in _cleanup_nvmet()
  nvme/rc: add functions for in-band authentication
  nvme/rc: add more arguments to _nvme_connect_subsys()
  nvme/040: create authenticated connections
  nvme/041: test dhchap key types for authenticated connections
  nvme/042: test hash and dh group variations for authenticated
    connections
  nvme/043: test bi-directional authentication
  nvme/044: test re-authentication

 tests/nvme/040     |  81 +++++++++++++++++++++++++++++
 tests/nvme/040.out |   7 +++
 tests/nvme/041     |  95 ++++++++++++++++++++++++++++++++++
 tests/nvme/041.out |  16 ++++++
 tests/nvme/042     |  89 ++++++++++++++++++++++++++++++++
 tests/nvme/042.out |  18 +++++++
 tests/nvme/043     | 106 ++++++++++++++++++++++++++++++++++++++
 tests/nvme/043.out |   8 +++
 tests/nvme/044     | 125 +++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/044.out |  12 +++++
 tests/nvme/rc      |  97 +++++++++++++++++++++++++++++++++++
 11 files changed, 654 insertions(+)
 create mode 100644 tests/nvme/040
 create mode 100644 tests/nvme/040.out
 create mode 100644 tests/nvme/041
 create mode 100644 tests/nvme/041.out
 create mode 100644 tests/nvme/042
 create mode 100644 tests/nvme/042.out
 create mode 100644 tests/nvme/043
 create mode 100644 tests/nvme/043.out
 create mode 100644 tests/nvme/044
 create mode 100644 tests/nvme/044.out

-- 
2.26.2



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 01/10] nvme/rc: do not print error message when no nvme device is found
  2022-06-10 11:33 [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Hannes Reinecke
@ 2022-06-10 11:33 ` Hannes Reinecke
  2022-06-10 11:33 ` [PATCH 02/10] nvme/rc: clear allowed_hosts subdirectory Hannes Reinecke
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-10 11:33 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke

_find_nvme_dev() will only print an nvme device node if one is found,
so no point in printing an error message.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
---
 tests/nvme/rc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/nvme/rc b/tests/nvme/rc
index 998b181..ad4324f 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -308,6 +308,7 @@ _find_nvme_dev() {
 	local subsysnqn
 	local dev
 	for dev in /sys/class/nvme/nvme*; do
+		[ -e "$dev" ] || continue
 		dev="$(basename "$dev")"
 		subsysnqn="$(cat "/sys/class/nvme/${dev}/subsysnqn")"
 		if [[ "$subsysnqn" == "$subsys" ]]; then
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 02/10] nvme/rc: clear allowed_hosts subdirectory
  2022-06-10 11:33 [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Hannes Reinecke
  2022-06-10 11:33 ` [PATCH 01/10] nvme/rc: do not print error message when no nvme device is found Hannes Reinecke
@ 2022-06-10 11:33 ` Hannes Reinecke
  2022-06-14  4:12   ` Shinichiro Kawasaki
  2022-06-10 11:33 ` [PATCH 03/10] nvme/rc: clear hosts directory in _cleanup_nvmet() Hannes Reinecke
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-10 11:33 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke

When removing a subsystem we need to clear out the allowed_hosts
subdirectory, otherwise removal will fail.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
---
 tests/nvme/rc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/nvme/rc b/tests/nvme/rc
index ad4324f..454609a 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -264,6 +264,7 @@ _remove_nvmet_subsystem() {
 	local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
 
 	_remove_nvmet_ns "${nvmet_subsystem}" "1"
+	rm "${subsys_path}"/allowed_hosts/*
 	rmdir "${subsys_path}"
 }
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 03/10] nvme/rc: clear hosts directory in _cleanup_nvmet()
  2022-06-10 11:33 [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Hannes Reinecke
  2022-06-10 11:33 ` [PATCH 01/10] nvme/rc: do not print error message when no nvme device is found Hannes Reinecke
  2022-06-10 11:33 ` [PATCH 02/10] nvme/rc: clear allowed_hosts subdirectory Hannes Reinecke
@ 2022-06-10 11:33 ` Hannes Reinecke
  2022-06-10 11:33 ` [PATCH 04/10] nvme/rc: add functions for in-band authentication Hannes Reinecke
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-10 11:33 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke, Hannes Reinecke

From: Hannes Reinecke <hare@suse.com>

When clearing out the configuration we need to clear the 'hosts'
directory, too, as scripts might have added host configurations.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/rc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/nvme/rc b/tests/nvme/rc
index 454609a..cc48802 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -124,6 +124,12 @@ _cleanup_nvmet() {
 		rmdir "${subsys}"
 	done
 
+	for host in "${NVMET_CFS}"/hosts/*; do
+		name=$(basename "${host}")
+		echo "WARNING: Test did not clean up host: ${name}"
+		rmdir "${host}"
+	done
+
 	shopt -u nullglob
 	trap SIGINT
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 04/10] nvme/rc: add functions for in-band authentication
  2022-06-10 11:33 [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Hannes Reinecke
                   ` (2 preceding siblings ...)
  2022-06-10 11:33 ` [PATCH 03/10] nvme/rc: clear hosts directory in _cleanup_nvmet() Hannes Reinecke
@ 2022-06-10 11:33 ` Hannes Reinecke
  2022-06-10 11:33 ` [PATCH 05/10] nvme/rc: add more arguments to _nvme_connect_subsys() Hannes Reinecke
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-10 11:33 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke

Add functions to enable in-band authentication.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/rc | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/tests/nvme/rc b/tests/nvme/rc
index cc48802..2873037 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -75,6 +75,17 @@ _require_nvme_trtype_is_fabrics() {
 	return 0
 }
 
+_require_nvme_cli_auth() {
+	local hostkey
+
+	hostkey="$(nvme gen-dhchap-key -n nvmf-test-subsys 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		SKIP_REASON="nvme gen-dhchap-key command missing"
+		return 1
+	fi
+	return 0
+}
+
 _test_dev_nvme_ctrl() {
 	echo "/dev/char/$(cat "${TEST_DEV_SYSFS}/device/dev")"
 }
@@ -255,6 +266,25 @@ _create_nvmet_subsystem() {
 	_create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}"
 }
 
+_create_nvmet_host() {
+	local nvmet_subsystem="$1"
+	local nvmet_hostnqn="$2"
+	local nvmet_hostkey="$3"
+	local nvmet_ctrlkey="$4"
+	local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
+	local host_path="${NVMET_CFS}/hosts/${nvmet_hostnqn}"
+
+	mkdir "${host_path}"
+	echo 0 > "${cfs_path}/attr_allow_any_host"
+	ln -s "${host_path}" "${cfs_path}/allowed_hosts/${nvmet_hostnqn}"
+	if [[ "${nvmet_hostkey}" ]] ; then
+		echo "${nvmet_hostkey}" > "${host_path}/dhchap_key"
+	fi
+	if [[ "${nvmet_ctrlkey}" ]] ; then
+		echo "${nvmet_ctrlkey}" > "${host_path}/dhchap_ctrl_key"
+	fi
+}
+
 _remove_nvmet_ns() {
 	local nvmet_subsystem="$1"
 	local nsid=$2
@@ -274,6 +304,13 @@ _remove_nvmet_subsystem() {
 	rmdir "${subsys_path}"
 }
 
+_remove_nvmet_host() {
+	local nvmet_host="$1"
+	local host_path="${NVMET_CFS}/hosts/${nvmet_host}"
+
+	rmdir "${host_path}"
+}
+
 _create_nvmet_passthru() {
 	local nvmet_subsystem="$1"
 	local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
@@ -310,6 +347,42 @@ _remove_nvmet_subsystem_from_port() {
 	rm "${NVMET_CFS}/ports/${port}/subsystems/${nvmet_subsystem}"
 }
 
+_set_nvmet_hostkey() {
+	local nvmet_hostnqn="$1"
+	local nvmet_hostkey="$2"
+	local cfs_path="${NVMET_CFS}/hosts/${nvmet_hostnqn}"
+
+	echo "${nvmet_hostkey}" > \
+	     "${cfs_path}/dhchap_key"
+}
+
+_set_nvmet_ctrlkey() {
+	local nvmet_hostnqn="$1"
+	local nvmet_ctrlkey="$2"
+	local cfs_path="${NVMET_CFS}/hosts/${nvmet_hostnqn}"
+
+	echo "${nvmet_ctrlkey}" > \
+	     "${cfs_path}/dhchap_ctrl_key"
+}
+
+_set_nvmet_hash() {
+	local nvmet_hostnqn="$1"
+	local nvmet_hash="$2"
+	local cfs_path="${NVMET_CFS}/hosts/${nvmet_hostnqn}"
+
+	echo "${nvmet_hash}" > \
+	     "${cfs_path}/dhchap_hash"
+}
+
+_set_nvmet_dhgroup() {
+	local nvmet_hostnqn="$1"
+	local nvmet_dhgroup="$2"
+	local cfs_path="${NVMET_CFS}/hosts/${nvmet_hostnqn}"
+
+	echo "${nvmet_dhgroup}" > \
+	     "${cfs_path}/dhchap_dhgroup"
+}
+
 _find_nvme_dev() {
 	local subsys=$1
 	local subsysnqn
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 05/10] nvme/rc: add more arguments to _nvme_connect_subsys()
  2022-06-10 11:33 [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Hannes Reinecke
                   ` (3 preceding siblings ...)
  2022-06-10 11:33 ` [PATCH 04/10] nvme/rc: add functions for in-band authentication Hannes Reinecke
@ 2022-06-10 11:33 ` Hannes Reinecke
  2022-06-14  4:23   ` Shinichiro Kawasaki
  2022-06-10 11:33 ` [PATCH 06/10] nvme/040: create authenticated connections Hannes Reinecke
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-10 11:33 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke, Hannes Reinecke

From: Hannes Reinecke <hare@suse.com>

Add optional arguments for setting 'hostnqn', 'hostid', 'hostkey',
and 'ctrlkey' to _nvme_connect_subsys() to make it usable for
testing nvme in-band authentication.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/rc | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/nvme/rc b/tests/nvme/rc
index 2873037..b7bb3cd 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -190,11 +190,27 @@ _nvme_connect_subsys() {
 	local subsysnqn="$2"
 	local traddr="${3:-$def_traddr}"
 	local trsvcid="${4:-$def_trsvcid}"
+	local hostnqn="${5:-$def_hostnqn}"
+	local hostid="${6:-$def_hostid}"
+	local hostkey="${7}"
+	local ctrlkey="${8}"
 
 	ARGS=(-t "${trtype}" -n "${subsysnqn}")
 	if [[ "${trtype}" != "loop" ]]; then
 		ARGS+=(-a "${traddr}" -s "${trsvcid}")
 	fi
+	if [[ "${hostnqn}" != "$def_hostnqn" ]]; then
+		ARGS+=(--hostnqn="${hostnqn}")
+	fi
+	if [[ "${hostid}" != "$def_hostid" ]]; then
+		ARGS+=(--hostid="${hostid}")
+	fi
+	if [[ -n "${hostkey}" ]]; then
+		ARGS+=(--dhchap-secret="${hostkey}")
+	fi
+	if [[ -n "${ctrlkey}" ]]; then
+		ARGS+=(--dhchap-ctrl-secret="${ctrlkey}")
+	fi
 	nvme connect "${ARGS[@]}"
 }
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 06/10] nvme/040: create authenticated connections
  2022-06-10 11:33 [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Hannes Reinecke
                   ` (4 preceding siblings ...)
  2022-06-10 11:33 ` [PATCH 05/10] nvme/rc: add more arguments to _nvme_connect_subsys() Hannes Reinecke
@ 2022-06-10 11:33 ` Hannes Reinecke
  2022-06-14  4:36   ` Shinichiro Kawasaki
  2022-06-10 11:33 ` [PATCH 07/10] nvme/041: test dhchap key types for " Hannes Reinecke
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-10 11:33 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/040     | 81 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/040.out |  7 ++++
 2 files changed, 88 insertions(+)
 create mode 100644 tests/nvme/040
 create mode 100644 tests/nvme/040.out

diff --git a/tests/nvme/040 b/tests/nvme/040
new file mode 100644
index 0000000..ce6157f
--- /dev/null
+++ b/tests/nvme/040
@@ -0,0 +1,81 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Create authenticated connections
+
+. tests/nvme/rc
+
+DESCRIPTION="Create authenticated connections"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_loop
+	_require_nvme_trtype_is_fabrics
+	_require_nvme_cli_auth
+}
+
+
+test() {
+	local port
+	local subsys_name="blktests-subsystem-1"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local file_path="${TMPDIR}/img"
+	local hostkey
+	local ctrldev
+
+	echo "Running ${TEST_NAME}"
+
+	hostkey="$(nvme gen-dhchap-key -n ${subsys_name} 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	_setup_nvmet
+
+	truncate -s 512M "${file_path}"
+
+	_create_nvmet_subsystem "${subsys_name}" "${file_path}" \
+		"b92842df-a394-44b1-84a4-92ae7d112861"
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+	_add_nvmet_subsys_to_port "${port}" "${subsys_name}"
+	_create_nvmet_host "${subsys_name}" "${hostnqn}" "${hostkey}"
+
+	# Test unauthenticated connection (should fail)
+	echo "Test unauthenticated connection"
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
+			     "${def_traddr}" "${def_trscvid}" \
+			     "${hostnqn}" "${hostid}"
+	if [ $? -eq 0 ] ; then
+		echo "nvme connect succeeded"
+	fi
+
+	_nvme_disconnect_subsys "${subsys_name}"
+
+	# Test authenticated connection
+	echo "Test authenticated connection"
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
+			     "${def_traddr}" "${def_trscvid}" \
+			     "${hostnqn}" "${hostid}" "${hostkey}"
+	if [ $? -ne 0 ] ; then
+		echo "nvme connect failed"
+	fi
+
+	udevadm settle
+
+	_nvme_disconnect_subsys "${subsys_name}"
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
+	_remove_nvmet_subsystem "${subsys_name}"
+
+	_remove_nvmet_port "${port}"
+
+	_remove_nvmet_host "${hostnqn}"
+
+	rm "${file_path}"
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/040.out b/tests/nvme/040.out
new file mode 100644
index 0000000..b5da85f
--- /dev/null
+++ b/tests/nvme/040.out
@@ -0,0 +1,7 @@
+Running nvme/040
+Test unauthenticated connection
+no controller found: failed to write to nvme-fabrics device
+NQN:blktests-subsystem-1 disconnected 0 controller(s)
+Test authenticated connection
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 07/10] nvme/041: test dhchap key types for authenticated connections
  2022-06-10 11:33 [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Hannes Reinecke
                   ` (5 preceding siblings ...)
  2022-06-10 11:33 ` [PATCH 06/10] nvme/040: create authenticated connections Hannes Reinecke
@ 2022-06-10 11:33 ` Hannes Reinecke
  2022-06-14  4:43   ` Shinichiro Kawasaki
  2022-06-10 11:33 ` [PATCH 08/10] nvme/042: test hash and dh group variations " Hannes Reinecke
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-10 11:33 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/041     | 95 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/041.out | 16 ++++++++
 2 files changed, 111 insertions(+)
 create mode 100644 tests/nvme/041
 create mode 100644 tests/nvme/041.out

diff --git a/tests/nvme/041 b/tests/nvme/041
new file mode 100644
index 0000000..2ded9c9
--- /dev/null
+++ b/tests/nvme/041
@@ -0,0 +1,95 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test dhchap key types for authenticated connections
+
+. tests/nvme/rc
+
+DESCRIPTION="Test dhchap key types for authenticated connections"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have loop
+	_require_nvme_trtype_is_fabrics
+	_require_nvme_cli_auth
+}
+
+
+test() {
+	local port
+	local subsys_name="blktests-subsystem-1"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local file_path="${TMPDIR}/img"
+	local hostkey
+	local ctrldev
+
+	echo "Running ${TEST_NAME}"
+
+	_setup_nvmet
+
+	truncate -s 512M "${file_path}"
+
+	_create_nvmet_subsystem "${subsys_name}" "${file_path}"
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+	_add_nvmet_subsys_to_port "${port}" "${subsys_name}"
+	_create_nvmet_host "${subsys_name}" "${hostnqn}"
+
+	for hmac in 0 1 2 3; do
+		echo "Testing hmac ${hmac}"
+		hostkey="$(nvme gen-dhchap-key --hmac=${hmac} -n ${subsys_name} 2> /dev/null)"
+		if [ $? -ne 0 ] ; then
+			echo "couldn't generate host key for hmac ${hmac}"
+			return 1
+		fi
+		_set_nvmet_hostkey "${hostnqn}" "${hostkey}"
+
+		_nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
+				     "${def_traddr}" "${def_trsvcid}" \
+				     "${hostnqn}" "${hostid}" \
+				     "${hostkey}"
+		if [ $? -ne 0 ] ; then
+			echo "nvme connect failed"
+		fi
+
+		udevadm settle
+
+		_nvme_disconnect_subsys "${subsys_name}"
+	done
+
+	for key_len in 32 48 64; do
+		echo "Testing key length ${key_len}"
+		hostkey="$(nvme gen-dhchap-key --key-length=${key_len} -n ${subsys_name} 2> /dev/null)"
+		if [ $? -ne 0 ] ; then
+			echo "couldn't generate host key for length ${key_len}"
+			return 1
+		fi
+		_set_nvmet_hostkey "${hostnqn}" "${hostkey}"
+
+		_nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
+				     "${def_traddr}" "${def_trsvcid}" \
+				     "${hostnqn}" "${hostid}" \
+				     "${hostkey}"
+
+		if [ $? -ne 0 ] ; then
+			echo "nvme connect failed"
+		fi
+
+		udevadm settle
+
+		_nvme_disconnect_subsys "${subsys_name}"
+	done
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
+	_remove_nvmet_subsystem "${subsys_name}"
+
+	_remove_nvmet_port "${port}"
+
+	_remove_nvmet_host "${hostnqn}"
+
+	rm ${file_path}
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/041.out b/tests/nvme/041.out
new file mode 100644
index 0000000..49a56ea
--- /dev/null
+++ b/tests/nvme/041.out
@@ -0,0 +1,16 @@
+Running nvme/041
+Testing hmac 0
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hmac 1
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hmac 2
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hmac 3
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing key length 32
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing key length 48
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing key length 64
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 08/10] nvme/042: test hash and dh group variations for authenticated connections
  2022-06-10 11:33 [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Hannes Reinecke
                   ` (6 preceding siblings ...)
  2022-06-10 11:33 ` [PATCH 07/10] nvme/041: test dhchap key types for " Hannes Reinecke
@ 2022-06-10 11:33 ` Hannes Reinecke
  2022-06-10 11:33 ` [PATCH 09/10] nvme/043: test bi-directional authentication Hannes Reinecke
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-10 11:33 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/042     | 89 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/042.out | 18 ++++++++++
 2 files changed, 107 insertions(+)
 create mode 100644 tests/nvme/042
 create mode 100644 tests/nvme/042.out

diff --git a/tests/nvme/042 b/tests/nvme/042
new file mode 100644
index 0000000..0b9762a
--- /dev/null
+++ b/tests/nvme/042
@@ -0,0 +1,89 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test hash and dh group variations for authenticated connections
+
+. tests/nvme/rc
+
+DESCRIPTION="Test hash and DH group variations for authenticated connections"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_loop
+	_require_nvme_trtype_is_fabrics
+	_require_nvme_cli_auth
+}
+
+
+test() {
+	local port
+	local subsys_name="blktests-subsystem-1"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local file_path="${TMPDIR}/img"
+	local hostkey
+	local ctrldev
+
+	echo "Running ${TEST_NAME}"
+
+	_setup_nvmet
+
+	truncate -s 512M "${file_path}"
+
+	_create_nvmet_subsystem "${subsys_name}" "${file_path}"
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+	_add_nvmet_subsys_to_port "${port}" "${subsys_name}"
+	_create_nvmet_host "${subsys_name}" "${hostnqn}" "${hostkey}"
+
+	for hash in "hmac(sha256)" "hmac(sha384)" "hmac(sha512)" ; do
+
+		echo "Testing hash ${hash}"
+
+		_set_nvmet_hash "${hostnqn}" "${hash}"
+
+		_nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
+				     "${def_traddr}" "${def_trsvcid}" \
+				     "${hostnqn}" "${hostid}" \
+				     "${hostkey}"
+		if [ $? -ne 0 ] ; then
+			echo "nvme connect failed"
+		fi
+
+		udevadm settle
+
+		_nvme_disconnect_subsys "${subsys_name}"
+	done
+
+	for dhgroup in "ffdhe2048" "ffdhe3072" "ffdhe4096" "ffdhe6144" "ffdhe8192" ; do
+
+		echo "Testing DH group ${dhgroup}"
+
+		_set_nvmet_dhgroup "${hostnqn}" "${dhgroup}"
+
+		_nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
+				     "${def_traddr}" "${def_trsvcid}" \
+				     "${hostnqn}" "${hostid}" \
+				     "${hostkey}"
+
+		if [ $? -ne 0 ] ; then
+			echo "nvme connect failed"
+		fi
+
+		udevadm settle
+
+		_nvme_disconnect_subsys "${subsys_name}"
+	done
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
+	_remove_nvmet_subsystem "${subsys_name}"
+
+	_remove_nvmet_port "${port}"
+
+	_remove_nvmet_host "${hostnqn}"
+
+	rm ${file_path}
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/042.out b/tests/nvme/042.out
new file mode 100644
index 0000000..c05aa2f
--- /dev/null
+++ b/tests/nvme/042.out
@@ -0,0 +1,18 @@
+Running nvme/042
+Testing hash hmac(sha256)
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hash hmac(sha384)
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hash hmac(sha512)
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe2048
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe3072
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe4096
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe6144
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe8192
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 09/10] nvme/043: test bi-directional authentication
  2022-06-10 11:33 [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Hannes Reinecke
                   ` (7 preceding siblings ...)
  2022-06-10 11:33 ` [PATCH 08/10] nvme/042: test hash and dh group variations " Hannes Reinecke
@ 2022-06-10 11:33 ` Hannes Reinecke
  2022-06-10 11:33 ` [PATCH 10/10] nvme/044: test re-authentication Hannes Reinecke
  2022-06-14  5:07 ` [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Shinichiro Kawasaki
  10 siblings, 0 replies; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-10 11:33 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/043     | 106 +++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/043.out |   8 ++++
 2 files changed, 114 insertions(+)
 create mode 100644 tests/nvme/043
 create mode 100644 tests/nvme/043.out

diff --git a/tests/nvme/043 b/tests/nvme/043
new file mode 100644
index 0000000..fdccb10
--- /dev/null
+++ b/tests/nvme/043
@@ -0,0 +1,106 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test bi-directional authentication
+
+. tests/nvme/rc
+
+DESCRIPTION="Test bi-directional authentication"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_loop
+	_require_nvme_trtype_is_fabrics
+	_require_nvme_cli_auth
+}
+
+
+test() {
+	local port
+	local subsys_name="blktests-subsystem-1"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local file_path="${TMPDIR}/img"
+	local hostkey
+	local ctrlkey
+	local ctrldev
+
+	echo "Running ${TEST_NAME}"
+
+	hostkey="$(nvme gen-dhchap-key -n ${subsys_name} 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	ctrlkey="$(nvme gen-dhchap-key -n ${subsys_name} 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	_setup_nvmet
+
+	truncate -s 512M "${file_path}"
+
+	_create_nvmet_subsystem "${subsys_name}" "${file_path}"
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+	_add_nvmet_subsys_to_port "${port}" "${subsys_name}"
+	_create_nvmet_host "${subsys_name}" "${hostnqn}" "${hostkey}" "${ctrlkey}"
+
+	_set_nvmet_dhgroup "${hostnqn}" "ffdhe2048"
+
+	# Step 1: Connect with host authentication only
+	echo "Test host authentication"
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
+			     "${def_traddr}" "${def_trsvcid}" \
+			     "${hostnqn}" "${hostid}" \
+			     "${hostkey}"
+	if [ $? -ne 0 ] ; then
+		echo "nvme connect failed"
+	fi
+
+	udevadm settle
+
+	_nvme_disconnect_subsys "${subsys_name}"
+
+	# Step 2: Connect with host authentication
+	# and invalid ctrl authentication
+	echo "Test host authentication and invalid ctrl authentication"
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
+			     "${def_traddr}" "${def_trsvcid}" \
+			     "${hostnqn}" "${hostid}" \
+			     "${hostkey}" "${hostkey}"
+	if [ $? -eq 0 ] ; then
+		echo "nvme connect succeeded (should have failed)"
+		_nvme_disconnect_subsys "${subsys_name}"
+	fi
+
+	# Step 3: Connect with host authentication
+	# and valid ctrl authentication
+	echo "Test host authentication and valid ctrl authentication"
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
+			     "${def_traddr}" "${def_trsvcid}" \
+			     "${hostnqn}" "${hostid}" \
+			     "${hostkey}" "${ctrlkey}"
+	if [ $? -ne 0 ] ; then
+		echo "nvme connect failed"
+	fi
+
+	udevadm settle
+
+	_nvme_disconnect_subsys "${subsys_name}"
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
+	_remove_nvmet_subsystem "${subsys_name}"
+
+	_remove_nvmet_port "${port}"
+
+	_remove_nvmet_host "${hostnqn}"
+
+	rm ${file_path}
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/043.out b/tests/nvme/043.out
new file mode 100644
index 0000000..a49408d
--- /dev/null
+++ b/tests/nvme/043.out
@@ -0,0 +1,8 @@
+Running nvme/043
+Test host authentication
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test host authentication and invalid ctrl authentication
+no controller found: failed to write to nvme-fabrics device
+Test host authentication and valid ctrl authentication
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 10/10] nvme/044: test re-authentication
  2022-06-10 11:33 [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Hannes Reinecke
                   ` (8 preceding siblings ...)
  2022-06-10 11:33 ` [PATCH 09/10] nvme/043: test bi-directional authentication Hannes Reinecke
@ 2022-06-10 11:33 ` Hannes Reinecke
  2022-06-14  5:07 ` [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Shinichiro Kawasaki
  10 siblings, 0 replies; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-10 11:33 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/044     | 125 +++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/044.out |  12 +++++
 2 files changed, 137 insertions(+)
 create mode 100644 tests/nvme/044
 create mode 100644 tests/nvme/044.out

diff --git a/tests/nvme/044 b/tests/nvme/044
new file mode 100644
index 0000000..060194e
--- /dev/null
+++ b/tests/nvme/044
@@ -0,0 +1,125 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test re-authentication
+
+. tests/nvme/rc
+
+DESCRIPTION="Test re-authentication"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_loop
+	_require_nvme_trtype_is_fabrics
+	_require_nvme_cli_auth
+}
+
+
+test() {
+	local port
+	local subsys_name="blktests-subsystem-1"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local file_path="${TMPDIR}/img"
+	local hostkey
+	local ctrlkey
+	local ctrldev
+
+	echo "Running ${TEST_NAME}"
+
+	hostkey="$(nvme gen-dhchap-key -n ${subsys_name} 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	ctrlkey="$(nvme gen-dhchap-key -n ${subsys_name} 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	_setup_nvmet
+
+	truncate -s 512M "${file_path}"
+
+	_create_nvmet_subsystem "${subsys_name}" "${file_path}"
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+	_add_nvmet_subsys_to_port "${port}" "${subsys_name}"
+	_create_nvmet_host "${subsys_name}" "${hostnqn}" "${hostkey}" "${ctrlkey}"
+
+	_set_nvmet_dhgroup "${hostnqn}" "ffdhe2048"
+
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
+			     "${def_traddr}" "${def_trsvcid}" \
+			     "${hostnqn}" "${hostid}" \
+			     "${hostkey}" "${ctrlkey}"
+	if [ $? -ne 0 ] ; then
+		echo "nvme connect failed"
+	fi
+
+	udevadm settle
+
+	echo "Re-authenticate with original host key"
+
+	ctrldev=$(_find_nvme_dev "${subsys_name}")
+	if [ -z "$ctrldev" ] ; then
+		echo "nvme controller not found"
+	fi
+	echo "${hostkey}" > /sys/class/nvme/${ctrldev}/dhchap_secret
+
+	echo "Renew host key on the controller"
+
+	new_hostkey="$(nvme gen-dhchap-key -n ${subsys_name} 2> /dev/null)"
+
+	_set_nvmet_hostkey "${hostnqn}" "${new_hostkey}"
+
+	echo "Re-authenticate with new host key"
+
+	echo "${new_hostkey}" > /sys/class/nvme/${ctrldev}/dhchap_secret
+
+	echo "Renew ctrl key on the controller"
+
+	new_ctrlkey="$(nvme gen-dhchap-key -n ${subsys_name} 2> /dev/null)"
+
+	_set_nvmet_ctrlkey "${hostnqn}" "${new_ctrlkey}"
+
+	echo "Re-authenticate with new ctrl key"
+
+	echo "${new_ctrlkey}" > /sys/class/nvme/${ctrldev}/dhchap_ctrl_secret
+
+	echo "Change DH group to ffdhe8192"
+
+	_set_nvmet_dhgroup "${hostnqn}" "ffdhe8192"
+
+	echo "Re-authenticate with changed DH group"
+
+	echo "${new_hostkey}" > /sys/class/nvme/${ctrldev}/dhchap_secret
+
+	echo "Change hash to hmac(sha512)"
+
+	_set_nvmet_hash "${hostnqn}" "hmac(sha512)"
+
+	echo "Re-authenticate with changed hash"
+
+	echo "${new_hostkey}" > /sys/class/nvme/${ctrldev}/dhchap_secret
+
+	nvmedev=$(_find_nvme_dev "${subsys_name}")
+
+	_run_fio_rand_io --size=8m --filename="/dev/${nvmedev}n1"
+
+	_nvme_disconnect_subsys "${subsys_name}"
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
+	_remove_nvmet_subsystem "${subsys_name}"
+
+	_remove_nvmet_port "${port}"
+
+	_remove_nvmet_host "${hostnqn}"
+
+	rm ${file_path}
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/044.out b/tests/nvme/044.out
new file mode 100644
index 0000000..51a7092
--- /dev/null
+++ b/tests/nvme/044.out
@@ -0,0 +1,12 @@
+Running nvme/044
+Re-authenticate with original host key
+Renew host key on the controller
+Re-authenticate with new host key
+Renew ctrl key on the controller
+Re-authenticate with new ctrl key
+Change DH group to ffdhe8192
+Re-authenticate with changed DH group
+Change hash to hmac(sha512)
+Re-authenticate with changed hash
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 02/10] nvme/rc: clear allowed_hosts subdirectory
  2022-06-10 11:33 ` [PATCH 02/10] nvme/rc: clear allowed_hosts subdirectory Hannes Reinecke
@ 2022-06-14  4:12   ` Shinichiro Kawasaki
  2022-06-20  6:53     ` Hannes Reinecke
  0 siblings, 1 reply; 20+ messages in thread
From: Shinichiro Kawasaki @ 2022-06-14  4:12 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme

On Jun 10, 2022 / 13:33, Hannes Reinecke wrote:
> When removing a subsystem we need to clear out the allowed_hosts
> subdirectory, otherwise removal will fail.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> ---
>  tests/nvme/rc | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index ad4324f..454609a 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -264,6 +264,7 @@ _remove_nvmet_subsystem() {
>  	local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
>  
>  	_remove_nvmet_ns "${nvmet_subsystem}" "1"
> +	rm "${subsys_path}"/allowed_hosts/*

When the allowed_hosts directory is empty, the rm command reports an error,
and it makes nvme/003 fail:

nvme/003 (test if we're sending keep-alives to a discovery controller) [failed]
    runtime  10.481s  ...  10.481s
    --- tests/nvme/003.out      2022-06-02 10:18:53.539739780 +0900
    +++ /home/shin/kts/kernel-test-suite/src/blktests/results/nodev/nvme/003.out.bad    2022-06-14 13:10:22.804354624 +0900
    @@ -1,3 +1,4 @@
     Running nvme/003
     NQN:nqn.2014-08.org.nvmexpress.discovery disconnected 1 controller(s)
    +rm: cannot remove '/sys/kernel/config/nvmet//subsystems/blktests-subsystem-1/allowed_hosts/*': No such file or directory
     Test complete

How about to add -f option to the rm?

-- 
Shin'ichiro Kawasaki

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 05/10] nvme/rc: add more arguments to _nvme_connect_subsys()
  2022-06-10 11:33 ` [PATCH 05/10] nvme/rc: add more arguments to _nvme_connect_subsys() Hannes Reinecke
@ 2022-06-14  4:23   ` Shinichiro Kawasaki
  2022-06-20  6:55     ` Hannes Reinecke
  0 siblings, 1 reply; 20+ messages in thread
From: Shinichiro Kawasaki @ 2022-06-14  4:23 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke

On Jun 10, 2022 / 13:33, Hannes Reinecke wrote:
> From: Hannes Reinecke <hare@suse.com>
> 
> Add optional arguments for setting 'hostnqn', 'hostid', 'hostkey',
> and 'ctrlkey' to _nvme_connect_subsys() to make it usable for
> testing nvme in-band authentication.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  tests/nvme/rc | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index 2873037..b7bb3cd 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -190,11 +190,27 @@ _nvme_connect_subsys() {
>  	local subsysnqn="$2"
>  	local traddr="${3:-$def_traddr}"
>  	local trsvcid="${4:-$def_trsvcid}"
> +	local hostnqn="${5:-$def_hostnqn}"
> +	local hostid="${6:-$def_hostid}"

I can not find def_hostnqn and def_hostid defined. Did they slip out from this
series?

> +	local hostkey="${7}"
> +	local ctrlkey="${8}"
>  
>  	ARGS=(-t "${trtype}" -n "${subsysnqn}")
>  	if [[ "${trtype}" != "loop" ]]; then
>  		ARGS+=(-a "${traddr}" -s "${trsvcid}")
>  	fi
> +	if [[ "${hostnqn}" != "$def_hostnqn" ]]; then
> +		ARGS+=(--hostnqn="${hostnqn}")
> +	fi
> +	if [[ "${hostid}" != "$def_hostid" ]]; then
> +		ARGS+=(--hostid="${hostid}")
> +	fi
> +	if [[ -n "${hostkey}" ]]; then
> +		ARGS+=(--dhchap-secret="${hostkey}")
> +	fi
> +	if [[ -n "${ctrlkey}" ]]; then
> +		ARGS+=(--dhchap-ctrl-secret="${ctrlkey}")
> +	fi
>  	nvme connect "${ARGS[@]}"
>  }
>  
> -- 
> 2.26.2
> 
> 

-- 
Shin'ichiro Kawasaki

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 06/10] nvme/040: create authenticated connections
  2022-06-10 11:33 ` [PATCH 06/10] nvme/040: create authenticated connections Hannes Reinecke
@ 2022-06-14  4:36   ` Shinichiro Kawasaki
  2022-06-20  6:56     ` Hannes Reinecke
  0 siblings, 1 reply; 20+ messages in thread
From: Shinichiro Kawasaki @ 2022-06-14  4:36 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme

On Jun 10, 2022 / 13:33, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  tests/nvme/040     | 81 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/nvme/040.out |  7 ++++
>  2 files changed, 88 insertions(+)
>  create mode 100644 tests/nvme/040
>  create mode 100644 tests/nvme/040.out
> 
> diff --git a/tests/nvme/040 b/tests/nvme/040
> new file mode 100644
> index 0000000..ce6157f
> --- /dev/null
> +++ b/tests/nvme/040
> @@ -0,0 +1,81 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
> +#
> +# Create authenticated connections
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="Create authenticated connections"
> +QUICK=1
> +
> +requires() {
> +	_nvme_requires
> +	_have_loop
> +	_require_nvme_trtype_is_fabrics
> +	_require_nvme_cli_auth
> +}
> +
> +
> +test() {
> +	local port
> +	local subsys_name="blktests-subsystem-1"
> +	local hostid="$(uuidgen)"
> +	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
> +	local file_path="${TMPDIR}/img"
> +	local hostkey
> +	local ctrldev
> +
> +	echo "Running ${TEST_NAME}"
> +
> +	hostkey="$(nvme gen-dhchap-key -n ${subsys_name} 2> /dev/null)"
> +	if [ $? -ne 0 ] ; then
> +		echo "nvme gen-dhchap-key command missing"

This message is a bit confusing. _require_nvme_cli_auth in requries() already
confirmed that nvme gen-dhchap-key command is not missing. I think "nvme
gen-dhchap-key command failed" is a bit better message here.

-- 
Shin'ichiro Kawasaki

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 07/10] nvme/041: test dhchap key types for authenticated connections
  2022-06-10 11:33 ` [PATCH 07/10] nvme/041: test dhchap key types for " Hannes Reinecke
@ 2022-06-14  4:43   ` Shinichiro Kawasaki
  2022-06-20  6:56     ` Hannes Reinecke
  0 siblings, 1 reply; 20+ messages in thread
From: Shinichiro Kawasaki @ 2022-06-14  4:43 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme

On Jun 10, 2022 / 13:33, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  tests/nvme/041     | 95 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/nvme/041.out | 16 ++++++++
>  2 files changed, 111 insertions(+)
>  create mode 100644 tests/nvme/041
>  create mode 100644 tests/nvme/041.out
> 
> diff --git a/tests/nvme/041 b/tests/nvme/041
> new file mode 100644
> index 0000000..2ded9c9
> --- /dev/null
> +++ b/tests/nvme/041
> @@ -0,0 +1,95 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
> +#
> +# Test dhchap key types for authenticated connections
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="Test dhchap key types for authenticated connections"
> +QUICK=1
> +
> +requires() {
> +	_nvme_requires
> +	_have loop

_have_loop?   '_' is missing before loop.

-- 
Shin'ichiro Kawasaki

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication
  2022-06-10 11:33 [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Hannes Reinecke
                   ` (9 preceding siblings ...)
  2022-06-10 11:33 ` [PATCH 10/10] nvme/044: test re-authentication Hannes Reinecke
@ 2022-06-14  5:07 ` Shinichiro Kawasaki
  10 siblings, 0 replies; 20+ messages in thread
From: Shinichiro Kawasaki @ 2022-06-14  5:07 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme

Hi Hannes, thanks for sharing the series. Here are general minor comments on it.

- Newly added test case scripts have file mode 644. I recommend 755 to indicate
  that they are executable and to keep consistency with other test cases.
- Could you please run "make check" and address shellcheck warnings? Shellcheck
  is good to reduce human-error, and I wish to keep zero warning.
- In the series, I observe bash condition expressions with single brackets []
  and double brackets [[ ]] are mixed. Originally double brackets were
  recommended for blktests scripts then I rather prefer double brackets. Having
  said that many single brackets are already used in blktests, so single bracket
  is still ok.

-- 
Shin'ichiro Kawasaki

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 02/10] nvme/rc: clear allowed_hosts subdirectory
  2022-06-14  4:12   ` Shinichiro Kawasaki
@ 2022-06-20  6:53     ` Hannes Reinecke
  0 siblings, 0 replies; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-20  6:53 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme

On 6/14/22 06:12, Shinichiro Kawasaki wrote:
> On Jun 10, 2022 / 13:33, Hannes Reinecke wrote:
>> When removing a subsystem we need to clear out the allowed_hosts
>> subdirectory, otherwise removal will fail.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
>> ---
>>   tests/nvme/rc | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/nvme/rc b/tests/nvme/rc
>> index ad4324f..454609a 100644
>> --- a/tests/nvme/rc
>> +++ b/tests/nvme/rc
>> @@ -264,6 +264,7 @@ _remove_nvmet_subsystem() {
>>   	local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
>>   
>>   	_remove_nvmet_ns "${nvmet_subsystem}" "1"
>> +	rm "${subsys_path}"/allowed_hosts/*
> 
> When the allowed_hosts directory is empty, the rm command reports an error,
> and it makes nvme/003 fail:
> 
> nvme/003 (test if we're sending keep-alives to a discovery controller) [failed]
>      runtime  10.481s  ...  10.481s
>      --- tests/nvme/003.out      2022-06-02 10:18:53.539739780 +0900
>      +++ /home/shin/kts/kernel-test-suite/src/blktests/results/nodev/nvme/003.out.bad    2022-06-14 13:10:22.804354624 +0900
>      @@ -1,3 +1,4 @@
>       Running nvme/003
>       NQN:nqn.2014-08.org.nvmexpress.discovery disconnected 1 controller(s)
>      +rm: cannot remove '/sys/kernel/config/nvmet//subsystems/blktests-subsystem-1/allowed_hosts/*': No such file or directory
>       Test complete
> 
> How about to add -f option to the rm?
> 
Yes, good point.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 05/10] nvme/rc: add more arguments to _nvme_connect_subsys()
  2022-06-14  4:23   ` Shinichiro Kawasaki
@ 2022-06-20  6:55     ` Hannes Reinecke
  0 siblings, 0 replies; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-20  6:55 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke

On 6/14/22 06:23, Shinichiro Kawasaki wrote:
> On Jun 10, 2022 / 13:33, Hannes Reinecke wrote:
>> From: Hannes Reinecke <hare@suse.com>
>>
>> Add optional arguments for setting 'hostnqn', 'hostid', 'hostkey',
>> and 'ctrlkey' to _nvme_connect_subsys() to make it usable for
>> testing nvme in-band authentication.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   tests/nvme/rc | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/tests/nvme/rc b/tests/nvme/rc
>> index 2873037..b7bb3cd 100644
>> --- a/tests/nvme/rc
>> +++ b/tests/nvme/rc
>> @@ -190,11 +190,27 @@ _nvme_connect_subsys() {
>>   	local subsysnqn="$2"
>>   	local traddr="${3:-$def_traddr}"
>>   	local trsvcid="${4:-$def_trsvcid}"
>> +	local hostnqn="${5:-$def_hostnqn}"
>> +	local hostid="${6:-$def_hostid}"
> 
> I can not find def_hostnqn and def_hostid defined. Did they slip out from this
> series?
> 
No, not really; these are optional args anyway.
But I probably should add code to read them in from /etc/nvme/

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 06/10] nvme/040: create authenticated connections
  2022-06-14  4:36   ` Shinichiro Kawasaki
@ 2022-06-20  6:56     ` Hannes Reinecke
  0 siblings, 0 replies; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-20  6:56 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme

On 6/14/22 06:36, Shinichiro Kawasaki wrote:
> On Jun 10, 2022 / 13:33, Hannes Reinecke wrote:
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   tests/nvme/040     | 81 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/nvme/040.out |  7 ++++
>>   2 files changed, 88 insertions(+)
>>   create mode 100644 tests/nvme/040
>>   create mode 100644 tests/nvme/040.out
>>
>> diff --git a/tests/nvme/040 b/tests/nvme/040
>> new file mode 100644
>> index 0000000..ce6157f
>> --- /dev/null
>> +++ b/tests/nvme/040
>> @@ -0,0 +1,81 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
>> +#
>> +# Create authenticated connections
>> +
>> +. tests/nvme/rc
>> +
>> +DESCRIPTION="Create authenticated connections"
>> +QUICK=1
>> +
>> +requires() {
>> +	_nvme_requires
>> +	_have_loop
>> +	_require_nvme_trtype_is_fabrics
>> +	_require_nvme_cli_auth
>> +}
>> +
>> +
>> +test() {
>> +	local port
>> +	local subsys_name="blktests-subsystem-1"
>> +	local hostid="$(uuidgen)"
>> +	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
>> +	local file_path="${TMPDIR}/img"
>> +	local hostkey
>> +	local ctrldev
>> +
>> +	echo "Running ${TEST_NAME}"
>> +
>> +	hostkey="$(nvme gen-dhchap-key -n ${subsys_name} 2> /dev/null)"
>> +	if [ $? -ne 0 ] ; then
>> +		echo "nvme gen-dhchap-key command missing"
> 
> This message is a bit confusing. _require_nvme_cli_auth in requries() already
> confirmed that nvme gen-dhchap-key command is not missing. I think "nvme
> gen-dhchap-key command failed" is a bit better message here.
> 
Okay, will be fixing it up.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 07/10] nvme/041: test dhchap key types for authenticated connections
  2022-06-14  4:43   ` Shinichiro Kawasaki
@ 2022-06-20  6:56     ` Hannes Reinecke
  0 siblings, 0 replies; 20+ messages in thread
From: Hannes Reinecke @ 2022-06-20  6:56 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme

On 6/14/22 06:43, Shinichiro Kawasaki wrote:
> On Jun 10, 2022 / 13:33, Hannes Reinecke wrote:
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   tests/nvme/041     | 95 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/nvme/041.out | 16 ++++++++
>>   2 files changed, 111 insertions(+)
>>   create mode 100644 tests/nvme/041
>>   create mode 100644 tests/nvme/041.out
>>
>> diff --git a/tests/nvme/041 b/tests/nvme/041
>> new file mode 100644
>> index 0000000..2ded9c9
>> --- /dev/null
>> +++ b/tests/nvme/041
>> @@ -0,0 +1,95 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
>> +#
>> +# Test dhchap key types for authenticated connections
>> +
>> +. tests/nvme/rc
>> +
>> +DESCRIPTION="Test dhchap key types for authenticated connections"
>> +QUICK=1
>> +
>> +requires() {
>> +	_nvme_requires
>> +	_have loop
> 
> _have_loop?   '_' is missing before loop.
> 
Correct, will be fixing it up.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2022-06-20  6:56 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-10 11:33 [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Hannes Reinecke
2022-06-10 11:33 ` [PATCH 01/10] nvme/rc: do not print error message when no nvme device is found Hannes Reinecke
2022-06-10 11:33 ` [PATCH 02/10] nvme/rc: clear allowed_hosts subdirectory Hannes Reinecke
2022-06-14  4:12   ` Shinichiro Kawasaki
2022-06-20  6:53     ` Hannes Reinecke
2022-06-10 11:33 ` [PATCH 03/10] nvme/rc: clear hosts directory in _cleanup_nvmet() Hannes Reinecke
2022-06-10 11:33 ` [PATCH 04/10] nvme/rc: add functions for in-band authentication Hannes Reinecke
2022-06-10 11:33 ` [PATCH 05/10] nvme/rc: add more arguments to _nvme_connect_subsys() Hannes Reinecke
2022-06-14  4:23   ` Shinichiro Kawasaki
2022-06-20  6:55     ` Hannes Reinecke
2022-06-10 11:33 ` [PATCH 06/10] nvme/040: create authenticated connections Hannes Reinecke
2022-06-14  4:36   ` Shinichiro Kawasaki
2022-06-20  6:56     ` Hannes Reinecke
2022-06-10 11:33 ` [PATCH 07/10] nvme/041: test dhchap key types for " Hannes Reinecke
2022-06-14  4:43   ` Shinichiro Kawasaki
2022-06-20  6:56     ` Hannes Reinecke
2022-06-10 11:33 ` [PATCH 08/10] nvme/042: test hash and dh group variations " Hannes Reinecke
2022-06-10 11:33 ` [PATCH 09/10] nvme/043: test bi-directional authentication Hannes Reinecke
2022-06-10 11:33 ` [PATCH 10/10] nvme/044: test re-authentication Hannes Reinecke
2022-06-14  5:07 ` [PATCHv5 blktests 00/10] Testsuite for nvme in-band authentication Shinichiro Kawasaki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.