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

[Max Gurtovoy]

Hi Sagi,

On 23/01/2023 14:33, Sagi Grimberg 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
>
> Maybe do nvmf_existing_ctrl() that iterates with device_for_each_child 
> and filters by transport?
>
do you propose some incremental logic to this series or to change some 
logic in the existing patches ?


> This will eliminate the identical code from rdma/tcp and potentially
> from fc?

We agreed that FC will use this common code after some cleanup/changes.

We can't hold improvements to the subsystem if some specific transport 
has to do some preparations/re-designs to fit common code.