Re: [PATCH 03/19] nvme-fabrics: reconnect if the association is gone

[Christoph Hellwig <hch@lst.de>]

On Thu, Oct 29, 2026 at 08:52:36AM -0700, Keith Busch wrote:
>  	if (ret) {
> -		nvmf_log_connect_error(ctrl, ret, le32_to_cpu(res.u32),
> -				       &cmd, data);
> +		result = le32_to_cpu(res.u32);

Nit on the result assignment:  This can just be moved before the if
and shared between the success and failure cases.

> +		nvmf_log_connect_error(ctrl, ret, result, &cmd, data);
> +		/*
> +		 * The controller ID we got from the admin queue's Connect is
> +		 * not known any more: the association has gone since. DNR is
> +		 * right about this command and says nothing about the next
> +		 * association. Don't let nvmf_should_reconnect() see it.
> +		 */
> +		if (ret > 0 &&
> +		    (ret & NVME_SCT_SC_MASK) == NVME_SC_CONNECT_INVALID_PARAM &&
> +		    result >> 16 && (result & 0xffff) ==
> +				offsetof(struct nvmf_connect_data, cntlid))
> +			ret = -ENOTCONN;

I really don't like this.  What real problem is this fixing?  Do we
really see setups where the the controller goes away before I/O command
connect, and the caller expects retries?  In that case we'll need to
describe this scenario.

And the condition above should move into a helper clearly documenting
it, and the decoding of the result should use common helpers with
nvmf_log_connect_error.