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

Hannes Reinecke
2023-04-18T09:38:40Z

On 4/18/23 07:52, Daniel Wagner wrote:
> On Mon, Apr 17, 2023 at 03:02:55PM +0200, Hannes Reinecke wrote:
> --- 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?

Cheers,

Hannes