[PATCH 1/3] nvme-fabrics: introduce existing_ctrl operation

[James Smart]

On 1/15/2023 2:03 AM, Max Gurtovoy wrote:
> The check if the connection request matches an existing controller is
> duplicated in several transports (such as RDMA and TCP). Move it to
> common code as optional operation.
> 
> Also, this check can be done before creating the controller to simplify
> the flow.
> 
> Reviewed-by: Israel Rukshin <israelr at nvidia.com>
> Signed-off-by: Max Gurtovoy <mgurtovoy at nvidia.com>
> ---
>   drivers/nvme/host/fabrics.c | 7 +++++++
>   drivers/nvme/host/fabrics.h | 3 +++
>   2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
> index ce27276f552d..3d5035331b9f 100644
> --- a/drivers/nvme/host/fabrics.c
> +++ b/drivers/nvme/host/fabrics.c
> @@ -1100,6 +1100,13 @@ nvmf_create_ctrl(struct device *dev, const char *buf)
>   	if (ret)
>   		goto out_module_put;
>   
> +	if (ops->existing_ctrl) {
> +		if (!opts->duplicate_connect && ops->existing_ctrl(opts)) {
> +			ret = -EALREADY;
> +			goto out_module_put;
> +		}
> +	}
> +
>   	ctrl = ops->create_ctrl(dev, opts);
>   	if (IS_ERR(ctrl)) {
>   		ret = PTR_ERR(ctrl); > diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h
> index a6e22116e139..587b92575a9b 100644
> --- a/drivers/nvme/host/fabrics.h
> +++ b/drivers/nvme/host/fabrics.h
> @@ -153,6 +153,8 @@ struct nvmf_ctrl_options {
>    *			that would go into starting up that fabric
>    *			for the purpose of conneciton to an NVMe controller
>    *			using that fabric technology.
> + * @existing_ctrl():	optional function pointer that checks if a connection
> + * 			request matches an existing controller.
>    *
>    * Notes:
>    *	1. At minimum, 'required_opts' and 'allowed_opts' should
> @@ -170,6 +172,7 @@ struct nvmf_transport_ops {
>   	int			allowed_opts;
>   	struct nvme_ctrl	*(*create_ctrl)(struct device *dev,
>   					struct nvmf_ctrl_options *opts);
> +	bool			(*existing_ctrl)(struct nvmf_ctrl_options *opts);
>   };
>   
>   static inline bool

Reviewed-by: James Smart <jsmart2021 at gmail.com>

A bit disheartening we're "commonizing" in manner that FC can't 
participate.   I looked at FC supporting this, and although it could, 
it's a bunch of code duplication, more code than leaving rdma/tcp code 
in w/o optimization, and FC still has to repeat what it does in 
controller create as there are other checks to be made. It's more 
optimal to leave things as is on FC.

Max: mind pinging me ahead of time when creating these common items so 
we can get the 3rd transport participating if possible ?

-- james