| Message ID | 20231115055220.2656965-3-shinichiro.kawasaki@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nvme: require kernel config NVME_HOST_AUTH | expand |
On 11/15/23 06:52, Shin'ichiro Kawasaki wrote: > The kernel commit d68006348288 ("nvme: rework NVME_AUTH Kconfig > selection") in v6.7-rc1 introduced a new kernel config option > NVME_HOST_AUTH. The nvme test cases from 041 to 045 fail when the option > is disabled. Check the requirement of the test cases. > > Reported-by: Yi Zhang <yi.zhang@redhat.com> > Link: https://lore.kernel.org/linux-nvme/CAHj4cs_Lprbh1zWsJ2yT6+qhNoqjrGDrBgx+XOYvU9SLpmLTtw@mail.gmail.com/ > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> > --- > tests/nvme/041 | 1 + > tests/nvme/042 | 1 + > tests/nvme/043 | 1 + > tests/nvme/044 | 1 + > tests/nvme/045 | 1 + > 5 files changed, 5 insertions(+) > > diff --git a/tests/nvme/041 b/tests/nvme/041 > index d23f10a..a7a5b38 100755 > --- a/tests/nvme/041 > +++ b/tests/nvme/041 > @@ -14,6 +14,7 @@ requires() { > _have_loop > _have_kernel_option NVME_AUTH > _have_kernel_option NVME_TARGET_AUTH > + _kver_gt_or_eq 6 7 && _have_kernel_option NVME_HOST_AUTH > _require_nvme_trtype_is_fabrics > _require_nvme_cli_auth > } > diff --git a/tests/nvme/042 b/tests/nvme/042 > index 9fda681..50d56d6 100755 > --- a/tests/nvme/042 > +++ b/tests/nvme/042 > @@ -14,6 +14,7 @@ requires() { > _have_loop > _have_kernel_option NVME_AUTH > _have_kernel_option NVME_TARGET_AUTH > + _kver_gt_or_eq 6 7 && _have_kernel_option NVME_HOST_AUTH > _require_nvme_trtype_is_fabrics > _require_nvme_cli_auth > } > diff --git a/tests/nvme/043 b/tests/nvme/043 > index c6a0aa0..b5f10bc 100755 > --- a/tests/nvme/043 > +++ b/tests/nvme/043 > @@ -14,6 +14,7 @@ requires() { > _have_loop > _have_kernel_option NVME_AUTH > _have_kernel_option NVME_TARGET_AUTH > + _kver_gt_or_eq 6 7 && _have_kernel_option NVME_HOST_AUTH > _require_nvme_trtype_is_fabrics > _require_nvme_cli_auth > _have_driver dh_generic > diff --git a/tests/nvme/044 b/tests/nvme/044 > index 7bd43f3..06e17d1 100755 > --- a/tests/nvme/044 > +++ b/tests/nvme/044 > @@ -14,6 +14,7 @@ requires() { > _have_loop > _have_kernel_option NVME_AUTH > _have_kernel_option NVME_TARGET_AUTH > + _kver_gt_or_eq 6 7 && _have_kernel_option NVME_HOST_AUTH > _require_nvme_trtype_is_fabrics > _require_nvme_cli_auth > _have_driver dh_generic > diff --git a/tests/nvme/045 b/tests/nvme/045 > index 1eb1032..126060c 100755 > --- a/tests/nvme/045 > +++ b/tests/nvme/045 > @@ -15,6 +15,7 @@ requires() { > _have_loop > _have_kernel_option NVME_AUTH > _have_kernel_option NVME_TARGET_AUTH > + _kver_gt_or_eq 6 7 && _have_kernel_option NVME_HOST_AUTH > _require_nvme_trtype_is_fabrics > _require_nvme_cli_auth > _have_driver dh_generic Why do we need to check for the kernel version? Any kernel not having the NVME_HOST_AUTH config symbol clearly will have it unset, no? I'd rather update _have_kernel_option to handle the case where a config symbol is not present, treating it as unset. That way we can drop the dependency on the kernel version (which, btw, is kinda pointless for the development branches anyway). Cheers, Hannes
On Nov 15, 2023 / 06:59, Hannes Reinecke wrote: [...] > > diff --git a/tests/nvme/045 b/tests/nvme/045 > > index 1eb1032..126060c 100755 > > --- a/tests/nvme/045 > > +++ b/tests/nvme/045 > > @@ -15,6 +15,7 @@ requires() { > > _have_loop > > _have_kernel_option NVME_AUTH > > _have_kernel_option NVME_TARGET_AUTH > > + _kver_gt_or_eq 6 7 && _have_kernel_option NVME_HOST_AUTH > > _require_nvme_trtype_is_fabrics > > _require_nvme_cli_auth > > _have_driver dh_generic > > Why do we need to check for the kernel version? > Any kernel not having the NVME_HOST_AUTH config symbol clearly will > have it unset, no? > I'd rather update _have_kernel_option to handle the case where > a config symbol is not present, treating it as unset. At this point, _have_kernel_option() already handles NVME_HOST_AUTH as unset when the kernel does not have the config symbol. > That way we can drop the dependency on the kernel version (which, btw, is > kinda pointless for the development branches anyway). For the newer kernel, the test cases require NVME_HOST_AUTH is set. In other words, the test cases are skipped when NVME_HOST_AUTH is unset. If we follow your idea and drop the kernel version dependncy, the test cases will be skipped on older kernels which do not have NVME_HOST_AUTH symbol. I wanted to allow running the test cases on older kernels, then added the kernel version check. I agree that kernel version dependency is not the best. As another solution, I considered introducing a helper function _kernel_option_exists() which checks if one of strings "# CONFIG_NVME_HOST_AUTH is not set" or "# CONFIG_NVME_HOST_AUTH=[ym]" exists in kernel config files. With this, we can do as follows: _kernel_option_exists NVME_HOST_AUTH && _have_kernel_option NVME_HOST_AUTH This assumes that one of the strings always exists in kernel configs. I was not sure about the assumption, then chose the way to check kernel version. (Any advice on this assumption will be appreciated...)
On 11/15/23 08:46, Shinichiro Kawasaki wrote: > On Nov 15, 2023 / 06:59, Hannes Reinecke wrote: > [...] >>> diff --git a/tests/nvme/045 b/tests/nvme/045 >>> index 1eb1032..126060c 100755 >>> --- a/tests/nvme/045 >>> +++ b/tests/nvme/045 >>> @@ -15,6 +15,7 @@ requires() { >>> _have_loop >>> _have_kernel_option NVME_AUTH >>> _have_kernel_option NVME_TARGET_AUTH >>> + _kver_gt_or_eq 6 7 && _have_kernel_option NVME_HOST_AUTH >>> _require_nvme_trtype_is_fabrics >>> _require_nvme_cli_auth >>> _have_driver dh_generic >> >> Why do we need to check for the kernel version? >> Any kernel not having the NVME_HOST_AUTH config symbol clearly will >> have it unset, no? >> I'd rather update _have_kernel_option to handle the case where >> a config symbol is not present, treating it as unset. > > At this point, _have_kernel_option() already handles NVME_HOST_AUTH as unset > when the kernel does not have the config symbol. > >> That way we can drop the dependency on the kernel version (which, btw, is >> kinda pointless for the development branches anyway). > > For the newer kernel, the test cases require NVME_HOST_AUTH is set. In other > words, the test cases are skipped when NVME_HOST_AUTH is unset. If we follow > your idea and drop the kernel version dependncy, the test cases will be skipped > on older kernels which do not have NVME_HOST_AUTH symbol. I wanted to allow > running the test cases on older kernels, then added the kernel version check. > > I agree that kernel version dependency is not the best. As another solution, > I considered introducing a helper function _kernel_option_exists() which > checks if one of strings "# CONFIG_NVME_HOST_AUTH is not set" or > "# CONFIG_NVME_HOST_AUTH=[ym]" exists in kernel config files. With this, we > can do as follows: > > _kernel_option_exists NVME_HOST_AUTH && _have_kernel_option NVME_HOST_AUTH > > This assumes that one of the strings always exists in kernel configs. I was not > sure about the assumption, then chose the way to check kernel version. (Any > advice on this assumption will be appreciated...) None of this is really a good solution. Probably we should strive to make nvme-cli handling this situation correctly; after all, it would know if the fabrics option is supported or not. Daniel? Cheers, Hannes
On Wed, Nov 15, 2023 at 09:20:28AM +0100, Hannes Reinecke wrote: > > I agree that kernel version dependency is not the best. As another solution, > > I considered introducing a helper function _kernel_option_exists() which > > checks if one of strings "# CONFIG_NVME_HOST_AUTH is not set" or > > "# CONFIG_NVME_HOST_AUTH=[ym]" exists in kernel config files. With this, we > > can do as follows: > > > > _kernel_option_exists NVME_HOST_AUTH && _have_kernel_option NVME_HOST_AUTH > > > > This assumes that one of the strings always exists in kernel configs. I was not > > sure about the assumption, then chose the way to check kernel version. (Any > > advice on this assumption will be appreciated...) > > None of this is really a good solution. Probably we should strive to make > nvme-cli handling this situation correctly; after all, it would know if the > fabrics option is supported or not. nvme-cli is detecting if a feature is present by reading /dev/nvme-fabrics. I think we should do something similar in blktest but not by calling nvme-cli in the _require() block. Though we don't have to rely on nvme-cli. We can do something like this: diff --git a/tests/nvme/045 b/tests/nvme/045 index 1eb1032a3b93..954f96bedd5a 100755 --- a/tests/nvme/045 +++ b/tests/nvme/045 @@ -17,6 +17,7 @@ requires() { _have_kernel_option NVME_TARGET_AUTH _require_nvme_trtype_is_fabrics _require_nvme_cli_auth + _require_kernel_nvme_feature dhchap_ctrl_secret _have_driver dh_generic } diff --git a/tests/nvme/rc b/tests/nvme/rc index 1cff522d8543..67b812cf0c66 100644 --- a/tests/nvme/rc +++ b/tests/nvme/rc @@ -155,6 +155,17 @@ _require_nvme_cli_auth() { return 0 } +_require_kernel_nvme_feature() { + local feature="$1" + + if ! [ -f /dev/nvme-fabrics ]; then + return 1; + fi + + grep "${feature}" /dev/nvme-fabrics + return $? +} + _test_dev_nvme_ctrl() { echo "/dev/char/$(cat "${TEST_DEV_SYSFS}/device/dev")" }
On 11/15/23 15:18, Daniel Wagner wrote: > On Wed, Nov 15, 2023 at 09:20:28AM +0100, Hannes Reinecke wrote: >>> I agree that kernel version dependency is not the best. As another solution, >>> I considered introducing a helper function _kernel_option_exists() which >>> checks if one of strings "# CONFIG_NVME_HOST_AUTH is not set" or >>> "# CONFIG_NVME_HOST_AUTH=[ym]" exists in kernel config files. With this, we >>> can do as follows: >>> >>> _kernel_option_exists NVME_HOST_AUTH && _have_kernel_option NVME_HOST_AUTH >>> >>> This assumes that one of the strings always exists in kernel configs. I was not >>> sure about the assumption, then chose the way to check kernel version. (Any >>> advice on this assumption will be appreciated...) >> >> None of this is really a good solution. Probably we should strive to make >> nvme-cli handling this situation correctly; after all, it would know if the >> fabrics option is supported or not. > > nvme-cli is detecting if a feature is present by reading > /dev/nvme-fabrics. I think we should do something similar in blktest > but not by calling nvme-cli in the _require() block. Though we don't > have to rely on nvme-cli. We can do something like this: > > > diff --git a/tests/nvme/045 b/tests/nvme/045 > index 1eb1032a3b93..954f96bedd5a 100755 > --- a/tests/nvme/045 > +++ b/tests/nvme/045 > @@ -17,6 +17,7 @@ requires() { > _have_kernel_option NVME_TARGET_AUTH > _require_nvme_trtype_is_fabrics > _require_nvme_cli_auth > + _require_kernel_nvme_feature dhchap_ctrl_secret > _have_driver dh_generic > } > > diff --git a/tests/nvme/rc b/tests/nvme/rc > index 1cff522d8543..67b812cf0c66 100644 > --- a/tests/nvme/rc > +++ b/tests/nvme/rc > @@ -155,6 +155,17 @@ _require_nvme_cli_auth() { > return 0 > } > > +_require_kernel_nvme_feature() { > + local feature="$1" > + > + if ! [ -f /dev/nvme-fabrics ]; then > + return 1; > + fi > + > + grep "${feature}" /dev/nvme-fabrics > + return $? > +} > + > _test_dev_nvme_ctrl() { > echo "/dev/char/$(cat "${TEST_DEV_SYSFS}/device/dev")" > } > Exactly. Far better :-) Cheers, Hannes
On Nov 15, 2023 / 15:32, Hannes Reinecke wrote: > On 11/15/23 15:18, Daniel Wagner wrote: > > On Wed, Nov 15, 2023 at 09:20:28AM +0100, Hannes Reinecke wrote: > > > > I agree that kernel version dependency is not the best. As another solution, > > > > I considered introducing a helper function _kernel_option_exists() which > > > > checks if one of strings "# CONFIG_NVME_HOST_AUTH is not set" or > > > > "# CONFIG_NVME_HOST_AUTH=[ym]" exists in kernel config files. With this, we > > > > can do as follows: > > > > > > > > _kernel_option_exists NVME_HOST_AUTH && _have_kernel_option NVME_HOST_AUTH > > > > > > > > This assumes that one of the strings always exists in kernel configs. I was not > > > > sure about the assumption, then chose the way to check kernel version. (Any > > > > advice on this assumption will be appreciated...) > > > > > > None of this is really a good solution. Probably we should strive to make > > > nvme-cli handling this situation correctly; after all, it would know if the > > > fabrics option is supported or not. > > > > nvme-cli is detecting if a feature is present by reading > > /dev/nvme-fabrics. I think we should do something similar in blktest > > but not by calling nvme-cli in the _require() block. Though we don't > > have to rely on nvme-cli. We can do something like this: > > > > > > diff --git a/tests/nvme/045 b/tests/nvme/045 > > index 1eb1032a3b93..954f96bedd5a 100755 > > --- a/tests/nvme/045 > > +++ b/tests/nvme/045 > > @@ -17,6 +17,7 @@ requires() { > > _have_kernel_option NVME_TARGET_AUTH > > _require_nvme_trtype_is_fabrics > > _require_nvme_cli_auth > > + _require_kernel_nvme_feature dhchap_ctrl_secret The idea looked good and I checked /dev/nvme-fabrics content on kernel v6.7- rc1. But unfortunately, I found that /dev/nvme-fabrics content is same regardless of the kernel config NVME_HOST_AUTH. I checked opt_tokens in drivers/nvme/host/fabrics.c, and saw that "dhchap_ctrl_secret=%s" is not surrounded with #ifdef CONFIG_NVME_HOST_AUTH. Should we add the #ifdef? I tried to find out other differences that NVME_HOST_AUTH makes and visible from userland. I found ctrl_dhchap_secret sysfs attribute of nvme devices is in #ifdef CONFIG_HOST_AUTH. But to find the attribute, it looks "nvme connect" needs to happen before-hand. So the attribute does not look usable. Hmm. > > _have_driver dh_generic > > } > > > > diff --git a/tests/nvme/rc b/tests/nvme/rc > > index 1cff522d8543..67b812cf0c66 100644 > > --- a/tests/nvme/rc > > +++ b/tests/nvme/rc > > @@ -155,6 +155,17 @@ _require_nvme_cli_auth() { > > return 0 > > } > > > > +_require_kernel_nvme_feature() { > > + local feature="$1" > > + > > + if ! [ -f /dev/nvme-fabrics ]; then > > + return 1; > > + fi > > + > > + grep "${feature}" /dev/nvme-fabrics > > + return $? > > +} > > + > > _test_dev_nvme_ctrl() { > > echo "/dev/char/$(cat "${TEST_DEV_SYSFS}/device/dev")" > > } > > > Exactly. > Far better :-) > > Cheers, > > Hannes > -- > Dr. Hannes Reinecke Kernel Storage Architect > hare@suse.de +49 911 74053 688 > SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg > Managing Directors: I. Totev, A. Myers, A. McDonald, M. B. Moerman > (HRB 36809, AG Nürnberg) >
On Nov 16, 2023 / 02:09, Shinichiro Kawasaki wrote: > On Nov 15, 2023 / 15:32, Hannes Reinecke wrote: > > On 11/15/23 15:18, Daniel Wagner wrote: [...] > > > diff --git a/tests/nvme/045 b/tests/nvme/045 > > > index 1eb1032a3b93..954f96bedd5a 100755 > > > --- a/tests/nvme/045 > > > +++ b/tests/nvme/045 > > > @@ -17,6 +17,7 @@ requires() { > > > _have_kernel_option NVME_TARGET_AUTH > > > _require_nvme_trtype_is_fabrics > > > _require_nvme_cli_auth > > > + _require_kernel_nvme_feature dhchap_ctrl_secret > > The idea looked good and I checked /dev/nvme-fabrics content on kernel v6.7- > rc1. But unfortunately, I found that /dev/nvme-fabrics content is same > regardless of the kernel config NVME_HOST_AUTH. I checked opt_tokens in > drivers/nvme/host/fabrics.c, and saw that "dhchap_ctrl_secret=%s" is not > surrounded with #ifdef CONFIG_NVME_HOST_AUTH. Should we add the #ifdef? > > I tried to find out other differences that NVME_HOST_AUTH makes and visible > from userland. I found ctrl_dhchap_secret sysfs attribute of nvme devices is > in #ifdef CONFIG_HOST_AUTH. But to find the attribute, it looks "nvme connect" > needs to happen before-hand. So the attribute does not look usable. Hmm. I rethought about the ctrl_dhchap_secret sysfs attribute, and came up with an idea to set up nvme target without host key and do "nvme connect". (With host key, nvme connect fails). Then check if the sysfs attributes exists or not. I quickly created a patch below, and it looks working. The check creates a nvme target and affects the test system, then I think it should be done in test() rather than requires(). If there is no better idea, we can take this solution. diff --git a/tests/nvme/041 b/tests/nvme/041 index d23f10a..28322e4 100755 --- a/tests/nvme/041 +++ b/tests/nvme/041 @@ -27,6 +27,10 @@ test() { local hostkey local ctrldev + if ! _nvme_host_supports_dhchap_ctrl_secret; then + return 1 + fi + hostkey="$(nvme gen-dhchap-key -n ${def_subsysnqn} 2> /dev/null)" if [ -z "$hostkey" ] ; then echo "nvme gen-dhchap-key failed" diff --git a/tests/nvme/rc b/tests/nvme/rc index 1cff522..9e77d7a 100644 --- a/tests/nvme/rc +++ b/tests/nvme/rc @@ -1010,3 +1010,21 @@ _nvme_reset_ctrl() { _nvme_delete_ctrl() { echo 1 > /sys/class/nvme/"$1"/delete_controller } + +# Set up nvme target without hostkey and see if dhchap_ctrl_secret exists. +_nvme_host_supports_dhchap_ctrl_secret() { + local ctrldev + local ret=0 + + _nvmet_target_setup --hostkey "" + _nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}" + cdev=$(_find_nvme_dev "${def_subsysnqn}") + if [[ -z $cdev || ! -e "/sys/class/nvme/${cdev}/dhchap_ctrl_secret" ]]; then + ret=1 + SKIP_REASONS+=("dhchap_ctrl_secret is not enabled (check CONFIG_NVME_HOST_AUTH)") + fi + _nvme_disconnect_subsys "${def_subsysnqn}" > /dev/null 2>&1 + _nvmet_target_cleanup + + return $ret +}
On Thu, Nov 16, 2023 at 03:03:20AM +0000, Shinichiro Kawasaki wrote: > > The idea looked good and I checked /dev/nvme-fabrics content on kernel v6.7- > > rc1. But unfortunately, I found that /dev/nvme-fabrics content is same > > regardless of the kernel config NVME_HOST_AUTH. I checked opt_tokens in > > drivers/nvme/host/fabrics.c, and saw that "dhchap_ctrl_secret=%s" is not > > surrounded with #ifdef CONFIG_NVME_HOST_AUTH. Should we add the > > #ifdef? Yes. The whole point of adding the features to /dev/nvme-fabrics is that we can figure out easily what is supported. If dhchap_ctrl_secret is not working due missing CONFIG_NVME_HOST_AUTH, then it should not be in the list. > > I tried to find out other differences that NVME_HOST_AUTH makes and visible > > from userland. I found ctrl_dhchap_secret sysfs attribute of nvme devices is > > in #ifdef CONFIG_HOST_AUTH. But to find the attribute, it looks "nvme connect" > > needs to happen before-hand. So the attribute does not look usable. Hmm. > > I rethought about the ctrl_dhchap_secret sysfs attribute, and came up with an > idea to set up nvme target without host key and do "nvme connect". (With host > key, nvme connect fails). Then check if the sysfs attributes exists or not. > > I quickly created a patch below, and it looks working. The check creates a nvme > target and affects the test system, then I think it should be done in test() > rather than requires(). If there is no better idea, we can take this solution. I'd rather see this fixed as pointed out above. This is a lot of overhead to figure out if something is supported. Thanks, Daniel
On 11/16/23 04:03, Shinichiro Kawasaki wrote: > On Nov 16, 2023 / 02:09, Shinichiro Kawasaki wrote: >> On Nov 15, 2023 / 15:32, Hannes Reinecke wrote: >>> On 11/15/23 15:18, Daniel Wagner wrote: > [...] >>>> diff --git a/tests/nvme/045 b/tests/nvme/045 >>>> index 1eb1032a3b93..954f96bedd5a 100755 >>>> --- a/tests/nvme/045 >>>> +++ b/tests/nvme/045 >>>> @@ -17,6 +17,7 @@ requires() { >>>> _have_kernel_option NVME_TARGET_AUTH >>>> _require_nvme_trtype_is_fabrics >>>> _require_nvme_cli_auth >>>> + _require_kernel_nvme_feature dhchap_ctrl_secret >> >> The idea looked good and I checked /dev/nvme-fabrics content on kernel v6.7- >> rc1. But unfortunately, I found that /dev/nvme-fabrics content is same >> regardless of the kernel config NVME_HOST_AUTH. I checked opt_tokens in >> drivers/nvme/host/fabrics.c, and saw that "dhchap_ctrl_secret=%s" is not >> surrounded with #ifdef CONFIG_NVME_HOST_AUTH. Should we add the #ifdef? >> >> I tried to find out other differences that NVME_HOST_AUTH makes and visible >> from userland. I found ctrl_dhchap_secret sysfs attribute of nvme devices is >> in #ifdef CONFIG_HOST_AUTH. But to find the attribute, it looks "nvme connect" >> needs to happen before-hand. So the attribute does not look usable. Hmm. > > I rethought about the ctrl_dhchap_secret sysfs attribute, and came up with an > idea to set up nvme target without host key and do "nvme connect". (With host > key, nvme connect fails). Then check if the sysfs attributes exists or not. > > I quickly created a patch below, and it looks working. The check creates a nvme > target and affects the test system, then I think it should be done in test() > rather than requires(). If there is no better idea, we can take this solution. > > diff --git a/tests/nvme/041 b/tests/nvme/041 > index d23f10a..28322e4 100755 > --- a/tests/nvme/041 > +++ b/tests/nvme/041 > @@ -27,6 +27,10 @@ test() { > local hostkey > local ctrldev > > + if ! _nvme_host_supports_dhchap_ctrl_secret; then > + return 1 > + fi > + > hostkey="$(nvme gen-dhchap-key -n ${def_subsysnqn} 2> /dev/null)" > if [ -z "$hostkey" ] ; then > echo "nvme gen-dhchap-key failed" > diff --git a/tests/nvme/rc b/tests/nvme/rc > index 1cff522..9e77d7a 100644 > --- a/tests/nvme/rc > +++ b/tests/nvme/rc > @@ -1010,3 +1010,21 @@ _nvme_reset_ctrl() { > _nvme_delete_ctrl() { > echo 1 > /sys/class/nvme/"$1"/delete_controller > } > + > +# Set up nvme target without hostkey and see if dhchap_ctrl_secret exists. > +_nvme_host_supports_dhchap_ctrl_secret() { > + local ctrldev > + local ret=0 > + > + _nvmet_target_setup --hostkey "" > + _nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}" > + cdev=$(_find_nvme_dev "${def_subsysnqn}") > + if [[ -z $cdev || ! -e "/sys/class/nvme/${cdev}/dhchap_ctrl_secret" ]]; then > + ret=1 > + SKIP_REASONS+=("dhchap_ctrl_secret is not enabled (check CONFIG_NVME_HOST_AUTH)") > + fi > + _nvme_disconnect_subsys "${def_subsysnqn}" > /dev/null 2>&1 > + _nvmet_target_cleanup > + > + return $ret > +} Errm. Not quite what I had in mind. To step back a bit: why again do you want to run these tests on older kernels? If authentication is not present it's _quite_ pointless running them... So what's the rationale? Cheers, Hannes
On Nov 16, 2023 / 08:32, Hannes Reinecke wrote: [...] > To step back a bit: why again do you want to run these tests on older > kernels? If authentication is not present it's _quite_ pointless running > them... > So what's the rationale? In this context, LTS kernel 6.1.y is in my mind as the older kernels. According to the git log, nvme authentication was introduced with kernel v6.0. I have just built the kernel v6.1.62 and confirmed the it supports nvme authentication (CONFIG_NVME_AUTH) and the test cases from nvme/041 to nvme/045 passes on it.
diff --git a/tests/nvme/041 b/tests/nvme/041 index d23f10a..a7a5b38 100755 --- a/tests/nvme/041 +++ b/tests/nvme/041 @@ -14,6 +14,7 @@ requires() { _have_loop _have_kernel_option NVME_AUTH _have_kernel_option NVME_TARGET_AUTH + _kver_gt_or_eq 6 7 && _have_kernel_option NVME_HOST_AUTH _require_nvme_trtype_is_fabrics _require_nvme_cli_auth } diff --git a/tests/nvme/042 b/tests/nvme/042 index 9fda681..50d56d6 100755 --- a/tests/nvme/042 +++ b/tests/nvme/042 @@ -14,6 +14,7 @@ requires() { _have_loop _have_kernel_option NVME_AUTH _have_kernel_option NVME_TARGET_AUTH + _kver_gt_or_eq 6 7 && _have_kernel_option NVME_HOST_AUTH _require_nvme_trtype_is_fabrics _require_nvme_cli_auth } diff --git a/tests/nvme/043 b/tests/nvme/043 index c6a0aa0..b5f10bc 100755 --- a/tests/nvme/043 +++ b/tests/nvme/043 @@ -14,6 +14,7 @@ requires() { _have_loop _have_kernel_option NVME_AUTH _have_kernel_option NVME_TARGET_AUTH + _kver_gt_or_eq 6 7 && _have_kernel_option NVME_HOST_AUTH _require_nvme_trtype_is_fabrics _require_nvme_cli_auth _have_driver dh_generic diff --git a/tests/nvme/044 b/tests/nvme/044 index 7bd43f3..06e17d1 100755 --- a/tests/nvme/044 +++ b/tests/nvme/044 @@ -14,6 +14,7 @@ requires() { _have_loop _have_kernel_option NVME_AUTH _have_kernel_option NVME_TARGET_AUTH + _kver_gt_or_eq 6 7 && _have_kernel_option NVME_HOST_AUTH _require_nvme_trtype_is_fabrics _require_nvme_cli_auth _have_driver dh_generic diff --git a/tests/nvme/045 b/tests/nvme/045 index 1eb1032..126060c 100755 --- a/tests/nvme/045 +++ b/tests/nvme/045 @@ -15,6 +15,7 @@ requires() { _have_loop _have_kernel_option NVME_AUTH _have_kernel_option NVME_TARGET_AUTH + _kver_gt_or_eq 6 7 && _have_kernel_option NVME_HOST_AUTH _require_nvme_trtype_is_fabrics _require_nvme_cli_auth _have_driver dh_generic
The kernel commit d68006348288 ("nvme: rework NVME_AUTH Kconfig selection") in v6.7-rc1 introduced a new kernel config option NVME_HOST_AUTH. The nvme test cases from 041 to 045 fail when the option is disabled. Check the requirement of the test cases. Reported-by: Yi Zhang <yi.zhang@redhat.com> Link: https://lore.kernel.org/linux-nvme/CAHj4cs_Lprbh1zWsJ2yT6+qhNoqjrGDrBgx+XOYvU9SLpmLTtw@mail.gmail.com/ Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> --- tests/nvme/041 | 1 + tests/nvme/042 | 1 + tests/nvme/043 | 1 + tests/nvme/044 | 1 + tests/nvme/045 | 1 + 5 files changed, 5 insertions(+)