[PATCH] nvme-fabrics: fix I/O connect error handling

[Hannes Reinecke]

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 at 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 at suse.de>

Cheers,

Hannes