* [PATCH] nvme-fabrics: fix I/O connect error handling
@ 2024-02-08 11:31 Chaitanya Kulkarni
2024-02-13 5:53 ` Christoph Hellwig
2024-02-13 10:32 ` Hannes Reinecke
0 siblings, 2 replies; 3+ messages in thread
From: Chaitanya Kulkarni @ 2024-02-08 11:31 UTC (permalink / raw)
To: linux-nvme; +Cc: kbusch, hch, sagi, Chaitanya Kulkarni
In nvmf_connect_io_queue(), if connect I/O command fails, we log the
error and continue for authentication. This overrides error captured
from __nvme_submit_sync_cmd(), causing wrong return value.
Add goto out_free_data after logging connect error to fix the issue.
Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
blktests is passing with this patch :-
blktests (master) # ./check nvme
nvme/002 (create many subsystems and test discovery) [passed]
runtime 24.281s ... 24.481s
nvme/003 (test if we're sending keep-alives to a discovery controller) [passed]
runtime 11.262s ... 11.275s
nvme/004 (test nvme and nvmet UUID NS descriptors) [passed]
runtime 0.492s ... 0.496s
nvme/005 (reset local loopback target) [passed]
runtime 0.810s ... 0.807s
nvme/006 (create an NVMeOF target with a block device-backed ns) [passed]
runtime 0.093s ... 0.091s
nvme/007 (create an NVMeOF target with a file-backed ns) [passed]
runtime 0.063s ... 0.064s
nvme/008 (create an NVMeOF host with a block device-backed ns) [passed]
runtime 0.490s ... 0.491s
nvme/009 (create an NVMeOF host with a file-backed ns) [passed]
runtime 0.478s ... 0.466s
nvme/010 (run data verification fio job on NVMeOF block device-backed ns) [passed]
runtime 51.736s ... 41.236s
nvme/011 (run data verification fio job on NVMeOF file-backed ns) [passed]
runtime 203.890s ... 141.790s
nvme/012 (run mkfs and data verification fio job on NVMeOF block device-backed ns) [passed]
runtime 45.403s ... 33.713s
nvme/013 (run mkfs and data verification fio job on NVMeOF file-backed ns) [passed]
runtime 113.799s ... 119.578s
nvme/014 (flush a NVMeOF block device-backed ns) [passed]
runtime 8.669s ... 8.488s
nvme/015 (unit test for NVMe flush for file backed ns) [passed]
runtime 7.303s ... 6.619s
nvme/016 (create/delete many NVMeOF block device-backed ns and test discovery) [passed]
runtime 12.945s ... 13.004s
nvme/017 (create/delete many file-ns and test discovery) [passed]
runtime 14.377s ... 14.490s
nvme/018 (unit test NVMe-oF out of range access on a file backend) [passed]
runtime 0.491s ... 0.472s
nvme/019 (test NVMe DSM Discard command on NVMeOF block-device ns) [passed]
runtime 0.488s ... 0.505s
nvme/020 (test NVMe DSM Discard command on NVMeOF file-backed ns) [passed]
runtime 0.502s ... 0.472s
nvme/021 (test NVMe list command on NVMeOF file-backed ns) [passed]
runtime 0.472s ... 0.499s
nvme/022 (test NVMe reset command on NVMeOF file-backed ns) [passed]
runtime 0.802s ... 0.812s
nvme/023 (test NVMe smart-log command on NVMeOF block-device ns) [passed]
runtime 0.505s ... 0.488s
nvme/024 (test NVMe smart-log command on NVMeOF file-backed ns) [passed]
runtime 0.474s ... 0.481s
nvme/025 (test NVMe effects-log command on NVMeOF file-backed ns) [passed]
runtime 0.484s ... 0.470s
nvme/026 (test NVMe ns-descs command on NVMeOF file-backed ns) [passed]
runtime 0.482s ... 0.479s
nvme/027 (test NVMe ns-rescan command on NVMeOF file-backed ns) [passed]
runtime 0.473s ... 0.493s
nvme/028 (test NVMe list-subsys command on NVMeOF file-backed ns) [passed]
runtime 0.492s ... 0.474s
nvme/029 (test userspace IO via nvme-cli read/write interface) [passed]
runtime 0.671s ... 0.667s
nvme/030 (ensure the discovery generation counter is updated appropriately) [passed]
runtime 0.244s ... 0.241s
nvme/031 (test deletion of NVMeOF controllers immediately after setup) [passed]
runtime 4.329s ... 4.257s
nvme/038 (test deletion of NVMeOF subsystem without enabling) [passed]
runtime 0.019s ... 0.020s
nvme/040 (test nvme fabrics controller reset/disconnect operation during I/O) [passed]
runtime 7.225s ... 7.154s
nvme/041 (Create authenticated connections) [passed]
runtime 1.946s ... 1.938s
nvme/042 (Test dhchap key types for authenticated connections) [passed]
runtime 5.804s ... 5.733s
nvme/043 (Test hash and DH group variations for authenticated connections) [passed]
runtime 25.909s ... 21.684s
nvme/044 (Test bi-directional authentication) [passed]
runtime 4.080s ... 4.091s
nvme/045 (Test re-authentication) [passed]
runtime 2.213s ... 1.706s
nvme/047 (test different queue types for fabric transports) [not run]
nvme_trtype=loop is not supported in this test
nvme/048 (Test queue count changes on reconnect) [not run]
nvme_trtype=loop is not supported in this test
blktests (master) #
drivers/nvme/host/fabrics.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 3499acbf6a82..495c171daead 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -534,6 +534,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)
if (ret) {
nvmf_log_connect_error(ctrl, ret, le32_to_cpu(res.u32),
&cmd, data);
+ goto out_free_data;
}
result = le32_to_cpu(res.u32);
if (result & (NVME_CONNECT_AUTHREQ_ATR | NVME_CONNECT_AUTHREQ_ASCR)) {
--
2.40.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] nvme-fabrics: fix I/O connect error handling
2024-02-08 11:31 [PATCH] nvme-fabrics: fix I/O connect error handling Chaitanya Kulkarni
@ 2024-02-13 5:53 ` Christoph Hellwig
2024-02-13 10:32 ` Hannes Reinecke
1 sibling, 0 replies; 3+ messages in thread
From: Christoph Hellwig @ 2024-02-13 5:53 UTC (permalink / raw)
To: Chaitanya Kulkarni; +Cc: linux-nvme, kbusch, hch, sagi
On Thu, Feb 08, 2024 at 03:31:25AM -0800, Chaitanya Kulkarni wrote:
> In nvmf_connect_io_queue(), if connect I/O command fails, we log the
> error and continue for authentication. This overrides error captured
> from __nvme_submit_sync_cmd(), causing wrong return value.
>
> Add goto out_free_data after logging connect error to fix the issue.
>
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
>
> blktests is passing with this patch :-
Does this imply it didn't before? In general a Fixes tag would be
more useful than a test log..
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] nvme-fabrics: fix I/O connect error handling
2024-02-08 11:31 [PATCH] nvme-fabrics: fix I/O connect error handling Chaitanya Kulkarni
2024-02-13 5:53 ` Christoph Hellwig
@ 2024-02-13 10:32 ` Hannes Reinecke
1 sibling, 0 replies; 3+ messages in thread
From: Hannes Reinecke @ 2024-02-13 10:32 UTC (permalink / raw)
To: Chaitanya Kulkarni, linux-nvme; +Cc: kbusch, hch, sagi
On 2/8/24 12:31, Chaitanya Kulkarni wrote:
> In nvmf_connect_io_queue(), if connect I/O command fails, we log the
> error and continue for authentication. This overrides error captured
> from __nvme_submit_sync_cmd(), causing wrong return value.
>
> Add goto out_free_data after logging connect error to fix the issue.
>
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
>
> blktests is passing with this patch :-
>
> blktests (master) # ./check nvme
> nvme/002 (create many subsystems and test discovery) [passed]
> runtime 24.281s ... 24.481s
> nvme/003 (test if we're sending keep-alives to a discovery controller) [passed]
> runtime 11.262s ... 11.275s
> nvme/004 (test nvme and nvmet UUID NS descriptors) [passed]
> runtime 0.492s ... 0.496s
> nvme/005 (reset local loopback target) [passed]
> runtime 0.810s ... 0.807s
> nvme/006 (create an NVMeOF target with a block device-backed ns) [passed]
> runtime 0.093s ... 0.091s
> nvme/007 (create an NVMeOF target with a file-backed ns) [passed]
> runtime 0.063s ... 0.064s
> nvme/008 (create an NVMeOF host with a block device-backed ns) [passed]
> runtime 0.490s ... 0.491s
> nvme/009 (create an NVMeOF host with a file-backed ns) [passed]
> runtime 0.478s ... 0.466s
> nvme/010 (run data verification fio job on NVMeOF block device-backed ns) [passed]
> runtime 51.736s ... 41.236s
> nvme/011 (run data verification fio job on NVMeOF file-backed ns) [passed]
> runtime 203.890s ... 141.790s
> nvme/012 (run mkfs and data verification fio job on NVMeOF block device-backed ns) [passed]
> runtime 45.403s ... 33.713s
> nvme/013 (run mkfs and data verification fio job on NVMeOF file-backed ns) [passed]
> runtime 113.799s ... 119.578s
> nvme/014 (flush a NVMeOF block device-backed ns) [passed]
> runtime 8.669s ... 8.488s
> nvme/015 (unit test for NVMe flush for file backed ns) [passed]
> runtime 7.303s ... 6.619s
> nvme/016 (create/delete many NVMeOF block device-backed ns and test discovery) [passed]
> runtime 12.945s ... 13.004s
> nvme/017 (create/delete many file-ns and test discovery) [passed]
> runtime 14.377s ... 14.490s
> nvme/018 (unit test NVMe-oF out of range access on a file backend) [passed]
> runtime 0.491s ... 0.472s
> nvme/019 (test NVMe DSM Discard command on NVMeOF block-device ns) [passed]
> runtime 0.488s ... 0.505s
> nvme/020 (test NVMe DSM Discard command on NVMeOF file-backed ns) [passed]
> runtime 0.502s ... 0.472s
> nvme/021 (test NVMe list command on NVMeOF file-backed ns) [passed]
> runtime 0.472s ... 0.499s
> nvme/022 (test NVMe reset command on NVMeOF file-backed ns) [passed]
> runtime 0.802s ... 0.812s
> nvme/023 (test NVMe smart-log command on NVMeOF block-device ns) [passed]
> runtime 0.505s ... 0.488s
> nvme/024 (test NVMe smart-log command on NVMeOF file-backed ns) [passed]
> runtime 0.474s ... 0.481s
> nvme/025 (test NVMe effects-log command on NVMeOF file-backed ns) [passed]
> runtime 0.484s ... 0.470s
> nvme/026 (test NVMe ns-descs command on NVMeOF file-backed ns) [passed]
> runtime 0.482s ... 0.479s
> nvme/027 (test NVMe ns-rescan command on NVMeOF file-backed ns) [passed]
> runtime 0.473s ... 0.493s
> nvme/028 (test NVMe list-subsys command on NVMeOF file-backed ns) [passed]
> runtime 0.492s ... 0.474s
> nvme/029 (test userspace IO via nvme-cli read/write interface) [passed]
> runtime 0.671s ... 0.667s
> nvme/030 (ensure the discovery generation counter is updated appropriately) [passed]
> runtime 0.244s ... 0.241s
> nvme/031 (test deletion of NVMeOF controllers immediately after setup) [passed]
> runtime 4.329s ... 4.257s
> nvme/038 (test deletion of NVMeOF subsystem without enabling) [passed]
> runtime 0.019s ... 0.020s
> nvme/040 (test nvme fabrics controller reset/disconnect operation during I/O) [passed]
> runtime 7.225s ... 7.154s
> nvme/041 (Create authenticated connections) [passed]
> runtime 1.946s ... 1.938s
> nvme/042 (Test dhchap key types for authenticated connections) [passed]
> runtime 5.804s ... 5.733s
> nvme/043 (Test hash and DH group variations for authenticated connections) [passed]
> runtime 25.909s ... 21.684s
> nvme/044 (Test bi-directional authentication) [passed]
> runtime 4.080s ... 4.091s
> nvme/045 (Test re-authentication) [passed]
> runtime 2.213s ... 1.706s
> nvme/047 (test different queue types for fabric transports) [not run]
> nvme_trtype=loop is not supported in this test
> nvme/048 (Test queue count changes on reconnect) [not run]
> nvme_trtype=loop is not supported in this test
> blktests (master) #
>
> drivers/nvme/host/fabrics.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
> index 3499acbf6a82..495c171daead 100644
> --- a/drivers/nvme/host/fabrics.c
> +++ b/drivers/nvme/host/fabrics.c
> @@ -534,6 +534,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)
> if (ret) {
> nvmf_log_connect_error(ctrl, ret, le32_to_cpu(res.u32),
> &cmd, data);
> + goto out_free_data;
> }
> result = le32_to_cpu(res.u32);
> if (result & (NVME_CONNECT_AUTHREQ_ATR | NVME_CONNECT_AUTHREQ_ASCR)) {
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-13 10:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-08 11:31 [PATCH] nvme-fabrics: fix I/O connect error handling Chaitanya Kulkarni
2024-02-13 5:53 ` Christoph Hellwig
2024-02-13 10:32 ` Hannes Reinecke
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox