Re: [PATCH 11/18] nvme-tcp: enable TLS handshake upcall

Sagi Grimberg
2023-04-18T10:12:41Z

>>   --- a/drivers/nvme/host/fabrics.c
>>> +++ b/drivers/nvme/host/fabrics.c
>>> @@ -609,6 +609,7 @@ static const match_table_t opt_tokens = {
>>>       { NVMF_OPT_DISCOVERY,        "discovery"        },
>>>       { NVMF_OPT_DHCHAP_SECRET,    "dhchap_secret=%s"    },
>>>       { NVMF_OPT_DHCHAP_CTRL_SECRET,    "dhchap_ctrl_secret=%s"    },
>>> +    { NVMF_OPT_TLS,            "tls"            },
>>>       { NVMF_OPT_ERR,            NULL            }
>>>   };
>>
>> Coming back to your previous discussion about this. 'tls' is always
>> announced
>> but not always supported. This renders the unsupported option
>> filtering added to
>> libnvme useles:
>>
>>    https://github.com/linux-nvme/libnvme/issues/612
>>
>> Let's assume we go ahead with this patch. The user set explicit the
>> tls option
>> via 'nvme connect ... --tls' and the kernel will return EINVAL, but
>> userland
>> can't really know what it was and drop --tls and retry again (or print
>> some
>> useful information). It could be any other parameter provided by the
>> user.
>>
>> How is userland supposed to do any feature detection?
>>
> And that is a good question.
> I've just removed most of the #ifdef annotations on request from Sagi.
> If we were going with your suggestion I would need to bring them back.
>
> So, Sagi: What should we do?
> Re-clutter and allow nvme-cli to figure out if TLS is supported?
> And have it always enabled and invent other means for nvme-cli?

I don't understand the issue.

userspace already has a way to know if a feature is supported by reading
the misc device.