Message ID | 20240221132404.6311-3-dwagner@suse.de |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-74782-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:693c:2685:b0:108:e6aa:91d0 with SMTP id mn5csp1032275dyc; Wed, 21 Feb 2024 05:25:18 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCX9jKZNaWxvCSV0krzCUDVoLK4sB3vfMIV4I+yPlYJzLxqS4E5wlkYvR5Svs3bQ9fREqJEiZktiF7zHoviapqcYOuQ6Gg== X-Google-Smtp-Source: AGHT+IFKqiPlLYZJaGQ1tzgPnVL0JWv67tLjI/6ChG+/ZkoSCdG1BvJh+1UUHAZFzGMdO38H78XJ X-Received: by 2002:a17:906:bce5:b0:a3e:c5c3:cb70 with SMTP id op5-20020a170906bce500b00a3ec5c3cb70mr4463163ejb.11.1708521918156; Wed, 21 Feb 2024 05:25:18 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1708521918; cv=pass; d=google.com; s=arc-20160816; b=Wm1vzjOpn1PQEthVb4cwloECUvtpr+u6KG9E3gbdIhmvUiyvKItNdntP8tg9mNkcXU K2QBcgz39T3wKpQj7hWvglHCZHhPvgJBc3RlKf4+sWFfPGVYoKmKsz9aenpMTVBsMmbe 3VmQ9cA81FDdxOovWKx+fyCCtzhOAXfyuUzJPWYZL4IGB+8d3g/s/q6fYNzrsTHu77Y/ v1fkffse87meNRSK+VSrlhb7BVsDhYvQx5TudrU3B3GMVuvMyISVgRDUOo44PMBQi/o2 9Gzltv1FoBFT3sMSjeCVVrq4zT5YSCDm2+wYaEQ3x51nzwFbhw+3vSsxc5A3XIpbuikr Z5TQ== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:in-reply-to:message-id :date:subject:cc:to:from:dkim-signature:dkim-signature :dkim-signature:dkim-signature; bh=zU7+H6imK6PdglzKKo++O8AYMoRco75nFqgEhuv3i/Y=; fh=RymfTjZCcjoLZ4QyuoC8uFkgj4QNYM/VblY/3wFaCcc=; b=ZHs2HG1lRFYOraCjDptrQoUODql9nY/Iz0nDL/wNm0X5ff55adhyKQpOqyoOZtwf5J jZp2/7aA5G2x870+YkLMRjIF+lCMwmEwjRIdXVkt3JGHsFanonKdYSEueUld//3uXDV7 sSREdYE3iD8EIp2LM9GBeIhltvId4V3+AwMckkKLa2Z62D4rWNnQc7aSa6mF2m2uXLHS 7i/0B7+j/1zjsrtTnEkF1Cnc/I81PXFCf3hFNR8vkpfORucBdUBAstYBk6q6ghRjA2kW ADhfbR2WdWejxGg+HyAc9Acby5DqkyWGmEGDWgDG9qiXiVue4Mm7T84uwwRpjAAcCUht jjJA==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@suse.de header.s=susede2_rsa header.b=w5Fx9Fz5; dkim=neutral (no key) header.i=@suse.de header.s=susede2_ed25519; dkim=pass header.i=@suse.de header.s=susede2_rsa header.b=Oldx6DBU; dkim=neutral (no key) header.i=@suse.de header.s=susede2_ed25519; arc=pass (i=1 spf=pass spfdomain=suse.de dkim=pass dkdomain=suse.de dkim=pass dkdomain=suse.de dmarc=pass fromdomain=suse.de); spf=pass (google.com: domain of linux-kernel+bounces-74782-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-74782-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=suse.de Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [2604:1380:4601:e00::3]) by mx.google.com with ESMTPS id e16-20020a170906081000b00a3f65131487si22207ejd.632.2024.02.21.05.25.17 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Feb 2024 05:25:18 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-74782-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3; Authentication-Results: mx.google.com; dkim=pass header.i=@suse.de header.s=susede2_rsa header.b=w5Fx9Fz5; dkim=neutral (no key) header.i=@suse.de header.s=susede2_ed25519; dkim=pass header.i=@suse.de header.s=susede2_rsa header.b=Oldx6DBU; dkim=neutral (no key) header.i=@suse.de header.s=susede2_ed25519; arc=pass (i=1 spf=pass spfdomain=suse.de dkim=pass dkdomain=suse.de dkim=pass dkdomain=suse.de dmarc=pass fromdomain=suse.de); spf=pass (google.com: domain of linux-kernel+bounces-74782-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-74782-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=suse.de Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 9B7811F23675 for <ouuuleilei@gmail.com>; Wed, 21 Feb 2024 13:25:17 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id DC3E87CF03; Wed, 21 Feb 2024 13:24:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="w5Fx9Fz5"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="wikyw4A4"; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="Oldx6DBU"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="HNlrwYsn" Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 13D247866C for <linux-kernel@vger.kernel.org>; Wed, 21 Feb 2024 13:24:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=195.135.223.131 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708521857; cv=none; b=em9uwlN5j06xR0wBbNjfSje5MrmHL8lPS5ToKZFkckkBkjzQhbbG0h+q8TnibJIvSRO0cVZRYuFcSCQEFGm4VSZXF6GnIA8ohWpN41Naz/mGAJ7+OrACXsG/0xC2iWx4Yi+uluH6uN6W73qldsXCzXEElbUY54Dwaw8G8D+NEEI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708521857; c=relaxed/simple; bh=Ym7TFu6r6aw+U9CNds3HvZg6pH5a78LLvgHTOEOcQfs=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=CqrAkR09H9gQOBQGS3e+u09FBUBhV1J+aHYRsbpmeVesZkHO0c8aXf/JyaRz7YtKGf9YUcs7kYIR1cxY8IsGGmG8KkDDROq47e2RTRPDMgsrsHwANh10rt5A+pP1noPsmR0dpnC9UdikEzXFDLwQpmVLJ+klYmhjm9e/KRllapo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=suse.de; spf=pass smtp.mailfrom=suse.de; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b=w5Fx9Fz5; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b=wikyw4A4; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b=Oldx6DBU; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b=HNlrwYsn; arc=none smtp.client-ip=195.135.223.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.de Received: from imap2.dmz-prg2.suse.org (imap2.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:98]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id BAFE61FB61; Wed, 21 Feb 2024 13:24:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1708521854; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zU7+H6imK6PdglzKKo++O8AYMoRco75nFqgEhuv3i/Y=; b=w5Fx9Fz5qYRbrm7sF5DA/BeiBibRfI4JepUlrYc+xzdPzhB0BHL8mb3VWdQ0Ub/QwXu+IJ SPe2/wHblk44TlusZdYDevbcGJy+k4V1/UHQDV/Qn3qmzw/++5QoQNvN6w982l32N8FLaV hnhesFx7jmXt8uRXD/gd1v/zNiqzVoY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1708521854; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zU7+H6imK6PdglzKKo++O8AYMoRco75nFqgEhuv3i/Y=; b=wikyw4A4sNr+V/eA2NM9fo0ArKcCRREKSH+JmyCDqUacibWJ0xpJ3LARycW60zFzmEGvl5 Gd59FOqncoKYD8AA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1708521853; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zU7+H6imK6PdglzKKo++O8AYMoRco75nFqgEhuv3i/Y=; b=Oldx6DBUt1bx2wzCuiBkGHXJ1WMBORMOA/Vq+yBMwicQthLQmi+ss4iqLb9oTfDqp9x9O6 lcYjV2nTVeC8jE5OvB62o+8z+y7XVC0Tb45Kg3AicJzLt7eggfsgdxExs/zcWmkuJgpBe3 Wf2lZHc/VvWr5OpWUF5r1O9UGxuuOcA= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1708521853; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zU7+H6imK6PdglzKKo++O8AYMoRco75nFqgEhuv3i/Y=; b=HNlrwYsn0sBlrkqQLpdXSPXPq9uk48zyG3xZJyPqMU7It49GBfZpo0TpBdG83X+s+XJddZ uYoTzx26bZxm41AA== Received: from imap2.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap2.dmz-prg2.suse.org (Postfix) with ESMTPS id A589113A42; Wed, 21 Feb 2024 13:24:13 +0000 (UTC) Received: from dovecot-director2.suse.de ([10.150.64.162]) by imap2.dmz-prg2.suse.org with ESMTPSA id xjDoJn351WUXPQAAn2gu4w (envelope-from <dwagner@suse.de>); Wed, 21 Feb 2024 13:24:13 +0000 From: Daniel Wagner <dwagner@suse.de> To: James Smart <james.smart@broadcom.com> Cc: Keith Busch <kbusch@kernel.org>, Christoph Hellwig <hch@lst.de>, Sagi Grimberg <sagi@grimberg.me>, Hannes Reinecke <hare@suse.de>, linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org, Daniel Wagner <dwagner@suse.de> Subject: [PATCH v2 2/5] nvme-fc: do not retry when auth fails or connection is refused Date: Wed, 21 Feb 2024 14:24:01 +0100 Message-ID: <20240221132404.6311-3-dwagner@suse.de> X-Mailer: git-send-email 2.43.1 In-Reply-To: <20240221132404.6311-1-dwagner@suse.de> References: <20240221132404.6311-1-dwagner@suse.de> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Authentication-Results: smtp-out2.suse.de; dkim=pass header.d=suse.de header.s=susede2_rsa header.b=Oldx6DBU; dkim=pass header.d=suse.de header.s=susede2_ed25519 header.b=HNlrwYsn X-Spamd-Result: default: False [3.05 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; R_DKIM_ALLOW(-0.20)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; SPAMHAUS_XBL(0.00)[2a07:de40:b281:104:10:150:64:98:from]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; R_MISSING_CHARSET(2.50)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; BROKEN_CONTENT_TYPE(1.50)[]; RCVD_COUNT_THREE(0.00)[3]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; DKIM_TRACE(0.00)[suse.de:+]; MX_GOOD(-0.01)[]; RCPT_COUNT_SEVEN(0.00)[8]; MID_CONTAINS_FROM(1.00)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[lst.de:email,suse.de:dkim,suse.de:email]; FUZZY_BLOCKED(0.00)[rspamd.com]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_TLS_ALL(0.00)[]; BAYES_HAM(-1.64)[92.74%] X-Rspamd-Server: rspamd1.dmz-prg2.suse.org X-Spam-Score: 3.05 X-Rspamd-Queue-Id: BAFE61FB61 X-Spam-Level: *** X-Spam-Flag: NO X-Spamd-Bar: +++ X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1791515079053402417 X-GMAIL-MSGID: 1791515079053402417 |
Series |
nvme-fc: fix blktests nvme/041
|
|
Commit Message
Daniel Wagner
Feb. 21, 2024, 1:24 p.m. UTC
There is no point in retrying to connect if the authentication fails. Connection refused is also issued from the authentication path, thus also do not retry. Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Daniel Wagner <dwagner@suse.de> --- drivers/nvme/host/fc.c | 2 ++ 1 file changed, 2 insertions(+)
Comments
On 2/21/24 14:24, Daniel Wagner wrote: > There is no point in retrying to connect if the authentication fails. > > Connection refused is also issued from the authentication path, thus > also do not retry. > > Reviewed-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > drivers/nvme/host/fc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c > index a5b29e9ad342..b81046c9f171 100644 > --- a/drivers/nvme/host/fc.c > +++ b/drivers/nvme/host/fc.c > @@ -3312,6 +3312,8 @@ nvme_fc_reconnect_or_delete(struct nvme_fc_ctrl *ctrl, int status) > ctrl->cnum, status); > if (status > 0 && (status & NVME_SC_DNR)) > recon = false; > + if (status == NVME_SC_AUTH_REQUIRED || status == -ECONNREFUSED) > + recon = false; > } else if (time_after_eq(jiffies, rport->dev_loss_end)) > recon = false; > Is this still required after the patchset to retry authentication errors? Cheers, Hannes
On Wed, Feb 21, 2024 at 04:53:44PM +0100, Hannes Reinecke wrote: > On 2/21/24 14:24, Daniel Wagner wrote: > > There is no point in retrying to connect if the authentication fails. > > > > Connection refused is also issued from the authentication path, thus > > also do not retry. > > > > Reviewed-by: Christoph Hellwig <hch@lst.de> > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > > --- > > drivers/nvme/host/fc.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c > > index a5b29e9ad342..b81046c9f171 100644 > > --- a/drivers/nvme/host/fc.c > > +++ b/drivers/nvme/host/fc.c > > @@ -3312,6 +3312,8 @@ nvme_fc_reconnect_or_delete(struct nvme_fc_ctrl *ctrl, int status) > > ctrl->cnum, status); > > if (status > 0 && (status & NVME_SC_DNR)) > > recon = false; > > + if (status == NVME_SC_AUTH_REQUIRED || status == -ECONNREFUSED) > > + recon = false; > > } else if (time_after_eq(jiffies, rport->dev_loss_end)) > > recon = false; > Is this still required after the patchset to retry authentication errors? Do you mean 48dae46676d1 ("nvme: enable retries for authentication commands") ? In this case yes, I've tested on top of this patch. This breaks the loop where the provided key is invalid or is missing. The loop would happy retry until reaching max of retries.
On 2/21/24 17:37, Daniel Wagner wrote: > On Wed, Feb 21, 2024 at 04:53:44PM +0100, Hannes Reinecke wrote: >> On 2/21/24 14:24, Daniel Wagner wrote: >>> There is no point in retrying to connect if the authentication fails. >>> >>> Connection refused is also issued from the authentication path, thus >>> also do not retry. >>> >>> Reviewed-by: Christoph Hellwig <hch@lst.de> >>> Signed-off-by: Daniel Wagner <dwagner@suse.de> >>> --- >>> drivers/nvme/host/fc.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c >>> index a5b29e9ad342..b81046c9f171 100644 >>> --- a/drivers/nvme/host/fc.c >>> +++ b/drivers/nvme/host/fc.c >>> @@ -3312,6 +3312,8 @@ nvme_fc_reconnect_or_delete(struct nvme_fc_ctrl *ctrl, int status) >>> ctrl->cnum, status); >>> if (status > 0 && (status & NVME_SC_DNR)) >>> recon = false; >>> + if (status == NVME_SC_AUTH_REQUIRED || status == -ECONNREFUSED) >>> + recon = false; >>> } else if (time_after_eq(jiffies, rport->dev_loss_end)) >>> recon = false; >> Is this still required after the patchset to retry authentication errors? > > Do you mean > > 48dae46676d1 ("nvme: enable retries for authentication commands") > > ? Yes. > > In this case yes, I've tested on top of this patch. This breaks the loop > where the provided key is invalid or is missing. The loop would happy > retry until reaching max of retries. But that's to be expected, no? After all, that's _precisely_ what NVME_SC_DNR is for; if you shouldn't retry, that bit is set. If it's not set, you should. So why is NVME_SC_AUTH_REQUIRED an exception? Cheers, Hannes
On Thu, Feb 22, 2024 at 07:46:12AM +0100, Hannes Reinecke wrote: > On 2/21/24 17:37, Daniel Wagner wrote: > > On Wed, Feb 21, 2024 at 04:53:44PM +0100, Hannes Reinecke wrote: > > In this case yes, I've tested on top of this patch. This breaks the loop > > where the provided key is invalid or is missing. The loop would happy > > retry until reaching max of retries. > > But that's to be expected, no? Why? If the key is wrong/missing it will be likely wrong/missing the next retry again. So what's the point in retrying? > After all, that's _precisely_ what > NVME_SC_DNR is for; > if you shouldn't retry, that bit is set. > If it's not set, you should. Okay, in this case there is a bug in the auth code somewhere. > So why is NVME_SC_AUTH_REQUIRED an exception? status is either NVME_SC_AUTH_REQUIRED or -ECONNREFUSED for the first part of the nvme/041 (no key set). In this case DNR bit is not set. Note, when -ECONNREFUSED is return if (status > 0 && (status & NVME_SC_DNR)) will not catch it either. Glad we have these tests now.
On Thu, Feb 22, 2024 at 08:45:04AM +0100, Daniel Wagner wrote: > On Thu, Feb 22, 2024 at 07:46:12AM +0100, Hannes Reinecke wrote: > > On 2/21/24 17:37, Daniel Wagner wrote: > > > On Wed, Feb 21, 2024 at 04:53:44PM +0100, Hannes Reinecke wrote: > > > In this case yes, I've tested on top of this patch. This breaks the loop > > > where the provided key is invalid or is missing. The loop would happy > > > retry until reaching max of retries. > > > > But that's to be expected, no? > > Why? If the key is wrong/missing it will be likely wrong/missing the > next retry again. So what's the point in retrying? > > > After all, that's _precisely_ what > > NVME_SC_DNR is for; > > if you shouldn't retry, that bit is set. > > If it's not set, you should. > > Okay, in this case there is a bug in the auth code somewhere. With the change below nvme/041 also passes: modified drivers/nvme/host/fabrics.c @@ -467,7 +467,7 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl) if (result & NVME_CONNECT_AUTHREQ_ASCR) { dev_warn(ctrl->device, "qid 0: secure concatenation is not supported\n"); - ret = NVME_SC_AUTH_REQUIRED; + ret = NVME_SC_AUTH_REQUIRED | NVME_SC_DNR; goto out_free_data; } /* Authentication required */ @@ -475,7 +475,7 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl) if (ret) { dev_warn(ctrl->device, "qid 0: authentication setup failed\n"); - ret = NVME_SC_AUTH_REQUIRED; + ret = NVME_SC_AUTH_REQUIRED | NVME_SC_DNR; goto out_free_data; } ret = nvme_auth_wait(ctrl, 0); @@ -540,8 +540,9 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid) /* Secure concatenation is not implemented */ if (result & NVME_CONNECT_AUTHREQ_ASCR) { dev_warn(ctrl->device, - "qid 0: secure concatenation is not supported\n"); - ret = NVME_SC_AUTH_REQUIRED; + "qid %d: secure concatenation is not supported\n", + qid); + ret = NVME_SC_AUTH_REQUIRED | NVME_SC_DNR; goto out_free_data; } /* Authentication required */ @@ -549,7 +550,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid) if (ret) { dev_warn(ctrl->device, "qid %d: authentication setup failed\n", qid); - ret = NVME_SC_AUTH_REQUIRED; + ret = NVME_SC_AUTH_REQUIRED | NVME_SC_DNR; } else { ret = nvme_auth_wait(ctrl, qid); if (ret) Is this what you had in mind?
On 2/22/24 18:02, Daniel Wagner wrote: > On Thu, Feb 22, 2024 at 08:45:04AM +0100, Daniel Wagner wrote: >> On Thu, Feb 22, 2024 at 07:46:12AM +0100, Hannes Reinecke wrote: >>> On 2/21/24 17:37, Daniel Wagner wrote: >>>> On Wed, Feb 21, 2024 at 04:53:44PM +0100, Hannes Reinecke wrote: >>>> In this case yes, I've tested on top of this patch. This breaks the loop >>>> where the provided key is invalid or is missing. The loop would happy >>>> retry until reaching max of retries. >>> >>> But that's to be expected, no? >> >> Why? If the key is wrong/missing it will be likely wrong/missing the >> next retry again. So what's the point in retrying? >> >>> After all, that's _precisely_ what >>> NVME_SC_DNR is for; >>> if you shouldn't retry, that bit is set. >>> If it's not set, you should. >> >> Okay, in this case there is a bug in the auth code somewhere. > > With the change below nvme/041 also passes: > > modified drivers/nvme/host/fabrics.c > @@ -467,7 +467,7 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl) > if (result & NVME_CONNECT_AUTHREQ_ASCR) { > dev_warn(ctrl->device, > "qid 0: secure concatenation is not supported\n"); > - ret = NVME_SC_AUTH_REQUIRED; > + ret = NVME_SC_AUTH_REQUIRED | NVME_SC_DNR; > goto out_free_data; > } > /* Authentication required */ > @@ -475,7 +475,7 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl) > if (ret) { > dev_warn(ctrl->device, > "qid 0: authentication setup failed\n"); > - ret = NVME_SC_AUTH_REQUIRED; > + ret = NVME_SC_AUTH_REQUIRED | NVME_SC_DNR; > goto out_free_data; > } > ret = nvme_auth_wait(ctrl, 0); > @@ -540,8 +540,9 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid) > /* Secure concatenation is not implemented */ > if (result & NVME_CONNECT_AUTHREQ_ASCR) { > dev_warn(ctrl->device, > - "qid 0: secure concatenation is not supported\n"); > - ret = NVME_SC_AUTH_REQUIRED; > + "qid %d: secure concatenation is not supported\n", > + qid); > + ret = NVME_SC_AUTH_REQUIRED | NVME_SC_DNR; > goto out_free_data; > } > /* Authentication required */ > @@ -549,7 +550,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid) > if (ret) { > dev_warn(ctrl->device, > "qid %d: authentication setup failed\n", qid); > - ret = NVME_SC_AUTH_REQUIRED; > + ret = NVME_SC_AUTH_REQUIRED | NVME_SC_DNR; > } else { > ret = nvme_auth_wait(ctrl, qid); > if (ret) > > Is this what you had in mind? Which, incidentally, is basically the patch I just posted. Cheers, Hannes
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c index a5b29e9ad342..b81046c9f171 100644 --- a/drivers/nvme/host/fc.c +++ b/drivers/nvme/host/fc.c @@ -3312,6 +3312,8 @@ nvme_fc_reconnect_or_delete(struct nvme_fc_ctrl *ctrl, int status) ctrl->cnum, status); if (status > 0 && (status & NVME_SC_DNR)) recon = false; + if (status == NVME_SC_AUTH_REQUIRED || status == -ECONNREFUSED) + recon = false; } else if (time_after_eq(jiffies, rport->dev_loss_end)) recon = false;