From: James Smart <jsmart2021@gmail.com>
To: Daniel Wagner <dwagner@suse.de>, linux-nvme@lists.infradead.org
Cc: linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
Chaitanya Kulkarni <kch@nvidia.com>,
Shin'ichiro Kawasaki <shinichiro@fastmail.com>,
Sagi Grimberg <sagi@grimberg.me>, Hannes Reinecke <hare@suse.de>,
James Smart <jsmart2021@gmail.com>
Subject: Re: [PATCH v2 4/5] nvme-fc: Make initial connect attempt synchronous
Date: Sat, 1 Jul 2023 05:11:11 -0700 [thread overview]
Message-ID: <594f73f2-59b0-bbcb-d7a0-6d89e2446830@gmail.com> (raw)
In-Reply-To: <20230620133711.22840-5-dwagner@suse.de>
On 6/20/2023 6:37 AM, Daniel Wagner wrote:
> Commit 4c984154efa1 ("nvme-fc: change controllers first connect to use
> reconnect path") made the connection attempt asynchronous in order to
> make the connection attempt from autoconnect/boot via udev/systemd up
> case a bit more reliable.
>
> Unfortunately, one side effect of this is that any wrong parameters
> provided from userspace will not be directly reported as invalid, e.g.
> auth keys.
>
> So instead having the policy code inside the kernel it's better to
> address this in userspace, for example in nvme-cli or nvme-stas.
>
> This aligns the fc transport with tcp and rdma.
As much as you want to make this change to make transports "similar", I
am dead set against it unless you are completing a long qualification of
the change on real FC hardware and FC-NVME devices. There is probably
1.5 yrs of testing of different race conditions that drove this change.
You cannot declare success from a simplistic toy tool such as fcloop for
validation.
The original issues exist, probably have even morphed given the time
from the original change, and this will seriously disrupt the transport
and any downstream releases. So I have a very strong NACK on this change.
Yes - things such as the connect failure results are difficult to return
back to nvme-cli. I have had many gripes about the nvme-cli's behavior
over the years, especially on negative cases due to race conditions
which required retries. It still fails this miserably. The async
reconnect path solved many of these issues for fc.
For the auth failure, how do we deal with things if auth fails over time
as reconnects fail due to a credential changes ? I would think
commonality of this behavior drives part of the choice.
-- james
next prev parent reply other threads:[~2023-07-01 12:11 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-20 13:37 [PATCH v2 0/5] nvme-fc: Fix blktests hangers Daniel Wagner
2023-06-20 13:37 ` [PATCH v2 1/5] nvme-fc: Do not wait in vain when unloading module Daniel Wagner
2023-06-20 13:37 ` [PATCH v2 2/5] nvme-fcloop: queue work items correctly Daniel Wagner
2023-06-20 13:37 ` [PATCH v2 3/5] nvmet-fcloop: Remove remote port from list when unlinking Daniel Wagner
2023-06-20 13:37 ` [PATCH v2 4/5] nvme-fc: Make initial connect attempt synchronous Daniel Wagner
2023-06-26 10:59 ` Dan Carpenter
2023-06-26 11:33 ` Dan Carpenter
2023-06-27 6:18 ` Daniel Wagner
2023-06-27 6:39 ` Hannes Reinecke
2023-06-27 6:51 ` Hannes Reinecke
2023-07-01 12:11 ` James Smart [this message]
2023-07-06 12:07 ` Daniel Wagner
2023-07-11 22:47 ` James Smart
2023-07-12 6:50 ` Hannes Reinecke
2023-07-13 20:35 ` Ewan Milne
2023-06-20 13:37 ` [PATCH v2 5/5] nvme-fc: do no free ctrl opts Daniel Wagner
2023-06-30 13:33 ` [PATCH v2 0/5] nvme-fc: Fix blktests hangers Ewan Milne
2023-06-23 17:14 [PATCH v2 4/5] nvme-fc: Make initial connect attempt synchronous kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=594f73f2-59b0-bbcb-d7a0-6d89e2446830@gmail.com \
--to=jsmart2021@gmail.com \
--cc=dwagner@suse.de \
--cc=hare@suse.de \
--cc=kch@nvidia.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=sagi@grimberg.me \
--cc=shinichiro@fastmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.