[PATCH 1/1] nvme-fabrics: add ENOKEY to no retry criteria for authentication failures

[PATCH 1/1] nvme-fabrics: add ENOKEY to no retry criteria for authentication failures

[Justin Tee]

With authentication, in addition to EKEYREJECTED there is also no point in
retrying reconnects when status is ENOKEY.  Thus, add -ENOKEY as another
criteria to determine when to stop retries.

Cc: Daniel Wagner <wagi@kernel.org>
Cc: Hannes Reinecke <hare@suse.de>
Closes: https://lore.kernel.org/linux-nvme/20250829-nvme-fc-sync-v3-0-d69c87e63aee@kernel.org/
Signed-off-by: Justin Tee <justintee8345@gmail.com>
---
 drivers/nvme/host/fabrics.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 3d4d6d8e88c4..fb7765bf0cdf 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -594,7 +594,7 @@ bool nvmf_should_reconnect(struct nvme_ctrl *ctrl, int status)
 	if (status > 0 && (status & NVME_STATUS_DNR))
 		return false;
 
-	if (status == -EKEYREJECTED)
+	if (status == -EKEYREJECTED || status == -ENOKEY)
 		return false;
 
 	if (ctrl->opts->max_reconnects == -1 ||
-- 
2.38.0

Re: [PATCH 1/1] nvme-fabrics: add ENOKEY to no retry criteria for authentication failures

[Hannes Reinecke]

On 11/17/25 19:43, Justin Tee wrote:
> With authentication, in addition to EKEYREJECTED there is also no point in
> retrying reconnects when status is ENOKEY.  Thus, add -ENOKEY as another
> criteria to determine when to stop retries.
> 
> Cc: Daniel Wagner <wagi@kernel.org>
> Cc: Hannes Reinecke <hare@suse.de>
> Closes: https://lore.kernel.org/linux-nvme/20250829-nvme-fc-sync-v3-0-d69c87e63aee@kernel.org/
> Signed-off-by: Justin Tee <justintee8345@gmail.com>
> ---
>   drivers/nvme/host/fabrics.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
> index 3d4d6d8e88c4..fb7765bf0cdf 100644
> --- a/drivers/nvme/host/fabrics.c
> +++ b/drivers/nvme/host/fabrics.c
> @@ -594,7 +594,7 @@ bool nvmf_should_reconnect(struct nvme_ctrl *ctrl, int status)
>   	if (status > 0 && (status & NVME_STATUS_DNR))
>   		return false;
>   
> -	if (status == -EKEYREJECTED)
> +	if (status == -EKEYREJECTED || status == -ENOKEY)
>   		return false;
>   
>   	if (ctrl->opts->max_reconnects == -1 ||

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH 1/1] nvme-fabrics: add ENOKEY to no retry criteria for authentication failures

[Daniel Wagner]

Hi Justin,

Sorry for the late response. I was on vacation.

On Tue, Nov 18, 2025 at 08:01:10AM +0100, Hannes Reinecke wrote:
> On 11/17/25 19:43, Justin Tee wrote:
> > With authentication, in addition to EKEYREJECTED there is also no point in
> > retrying reconnects when status is ENOKEY.  Thus, add -ENOKEY as another
> > criteria to determine when to stop retries.
> > 
> > Cc: Daniel Wagner <wagi@kernel.org>
> > Cc: Hannes Reinecke <hare@suse.de>
> > Closes: https://lore.kernel.org/linux-nvme/20250829-nvme-fc-sync-v3-0-d69c87e63aee@kernel.org/
> > Signed-off-by: Justin Tee <justintee8345@gmail.com>

The patch itself is okay, nothing against it. But it wont close the bug
report. blktests nvme/041 just passes because we don't check the return
code from 'nvme connect' yet. I wanted to add this for a while, time to
do it.

Furthermore, the test now races with the kernel. If the worker thread in
the kernel is delayed, the 'nvme disconnect' might still see a controller:

     Test authenticated connection
    -disconnected 1 controller(s)
    +disconnected 0 controller(s)

Anyway, I am fine with this change just without the 'Closes' tag.

Tested-by: Daniel Wagner <wagi@kernel.org>
Reviewed-by: Daniel Wagner <wagi@kernel.org>

Re: [PATCH 1/1] nvme-fabrics: add ENOKEY to no retry criteria for authentication failures

[Justin Tee]

Hi Daniel,

> The patch itself is okay, nothing against it. But it wont close the bug
> report. blktests nvme/041 just passes because we don't check the return
> code from 'nvme connect' yet. I wanted to add this for a while, time to
> do it.
>
> Furthermore, the test now races with the kernel. If the worker thread in
> the kernel is delayed, the 'nvme disconnect' might still see a controller:
>
>      Test authenticated connection
>     -disconnected 1 controller(s)
>     +disconnected 0 controller(s)
>
> Anyway, I am fine with this change just without the 'Closes' tag.
>
> Tested-by: Daniel Wagner <wagi@kernel.org>
> Reviewed-by: Daniel Wagner <wagi@kernel.org>

Was under the impression that the issue with 041 is that there is a
lingering reconnect attempt leftover from the “Test unauthenticated
connection (should fail)” portion that interferes with the next “Test
authenticated connection” portion of the blktest.

Through correcting the nvmf_should_reconnect routine, which stops
retrying reconnect when -ENOKEY, there would be no interference in the
second part of the test when nvme connect is with an actual key.  So,
there will be a controller to nvme disconnect, as expected, for the
“Test authenticated connection” portion of the test.

Regards,
Justin

Re: [PATCH 1/1] nvme-fabrics: add ENOKEY to no retry criteria for authentication failures

[Daniel Wagner]

Hi Justin,

On Mon, Nov 24, 2025 at 04:57:29PM -0800, Justin Tee wrote:
> Was under the impression that the issue with 041 is that there is a
> lingering reconnect attempt leftover from the “Test unauthenticated
> connection (should fail)” portion that interferes with the next “Test
> authenticated connection” portion of the blktest.

Yes, that was one part of the problem. FC keept continuing reconnecting
with an invalid key. The other transport would also try to reconnect
with an invalid key after the first initial connection. This is a long
standing problem and your patch fixes it for all transport.

As I said your fix is good and addresses the reconnect with an invalid
key problem. Though it doesn't address the issue that 'nvme connect'
will still return success.

> Through correcting the nvmf_should_reconnect routine, which stops
> retrying reconnect when -ENOKEY, there would be no interference in the
> second part of the test when nvme connect is with an actual key.  So,
> there will be a controller to nvme disconnect, as expected, for the
> “Test authenticated connection” portion of the test.

All the nvme tests which use 'nvme connect' do not check the return code
yet. Something I am going to add now. This should ensure that all the
transport behave from userpoint of view the same. There is no exception
for a transport to behave differently IMO. On the other hand we can't
break existing APIs, so we have to come up with an opt-in solution here.

Hope this makes it a bit more clearer.

Thanks a lot,
Daniel

Re: [PATCH 1/1] nvme-fabrics: add ENOKEY to no retry criteria for authentication failures

[Daniel Wagner]

On Tue, Nov 25, 2025 at 11:34:40AM +0100, Daniel Wagner wrote:
> As I said your fix is good and addresses the reconnect with an invalid
> key problem. Though it doesn't address the issue that 'nvme connect'
> will still return success.

To my surprise this is not happening. nvme/041 is behaving for all
transport the same. I've extended blktests to check the return code for
'nvme connect'

  https://github.com/linux-blktests/blktests/pull/216

So your patch fixes this problem completely. Nice!

Thanks,
Daniel

Re: [PATCH 1/1] nvme-fabrics: add ENOKEY to no retry criteria for authentication failures

[Keith Busch]

On Mon, Nov 17, 2025 at 10:43:43AM -0800, Justin Tee wrote:
> With authentication, in addition to EKEYREJECTED there is also no point in
> retrying reconnects when status is ENOKEY.  Thus, add -ENOKEY as another
> criteria to determine when to stop retries.

Thanks, applied to nvme-6.19.

Re: [PATCH 1/1] nvme-fabrics: add ENOKEY to no retry criteria for authentication failures

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>