Re: [PATCH 3/3] nvme-fabrics: factor out common for auth

[Christoph Hellwig <hch@lst.de>]

On Mon, May 22, 2023 at 06:04:08AM -0700, Chaitanya Kulkarni wrote:
> nvmf_connect_admin_queue and nvmf_connect_io_queue() shares common code
> for post connect command authentication processing that includes,
> returning appropriate NVMe authentication status based on the
> command result, authentication negotiation per qid, waiting on
> negotiation per qid.
> 
> Add a common helper function to reduce the code duplication with
> necessary aruments of qid and result of connect command.
> 
> No functional changes in this patch.
> 
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
>  drivers/nvme/host/fabrics.c | 81 ++++++++++++++++---------------------
>  1 file changed, 35 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
> index 9d3df63eb49a..1d48c0b77cd0 100644
> --- a/drivers/nvme/host/fabrics.c
> +++ b/drivers/nvme/host/fabrics.c
> @@ -433,6 +433,39 @@ static void nvmf_connect_cmd_prep(struct nvme_ctrl *ctrl, u16 qid,
>  		cmd->connect.cattr |= NVME_CONNECT_DISABLE_SQFLOW;
>  }
>  
> +static u16 nvmf_auth_post_queue_connect(struct nvme_ctrl *ctrl, u16 qid,
> +					u32 result)
> +{
> +	int ret;
> +
> +	if (!(result & (NVME_CONNECT_AUTHREQ_ATR | NVME_CONNECT_AUTHREQ_ASCR)))
> +		return NVME_SC_SUCCESS;

While it's a minor duplication, I'd prepfer to keep this check in the
callers.  That makes it very explicit we are only calling into the helper
when authentication is required.

> +	ret = nvme_auth_wait(ctrl, qid);
> +	if (ret) {
> +		dev_warn(ctrl->device, "qid %u: authentication failed\n", qid);
> +		return ret;
> +	}
> +	if (!ret && qid == 0)

ret must be 0 at this point bsed on the check above.