Re: [PATCH rfc 6/6] nvme-fabrics: expose support for traddr as dns names to userspace

[Sagi Grimberg <sagi@grimberg.me>]

On 8/13/23 16:07, Sagi Grimberg wrote:
> 
>>> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
>>> index fadd25538d1b..6a7e8cbdaf69 100644
>>> --- a/drivers/nvme/host/fabrics.c
>>> +++ b/drivers/nvme/host/fabrics.c
>>> @@ -1331,6 +1331,7 @@ static void __nvmf_concat_opt_tokens(struct 
>>> seq_file *seq_file)
>>>           seq_puts(seq_file, ",");
>>>           seq_puts(seq_file, tok->pattern);
>>>       }
>>> +    seq_puts(seq_file, ",dns_ip_traddr");
>>>       seq_puts(seq_file, "\n");
>>>   }
>>
>> Don't we need to update the parser to accept this option, too?
>> After all, the implication is that all tokens in the output can be 
>> used as valid tokens for the connect string ...
> 
> I struggled with this bit here...
> 
> Today, we simply expose all valid tokens, but in this case
> what we really need is a capability indicator (in this case
> traddr can be a fqdn and not only an ip address).
> 
> So we could make this a parsed token, and make userspace
> actually pass it in case traddr was passed as a fqdn, but
> it is absolutely redundant to do so.
> 
> We could come up with a format that explicitly says this
> is a capability instead of a valid token.
> Like: "cap=dns_ip_traddr" or "format=traddr_as_dns_name" or
> something...
> 
> What we just need, is for userspace to know if it is possible
> to pass traddr in a specific format.
> 
> I tend to think that this is something that we will want for
> other options that can have multiple formats.
> 
> Another option would be to make this a separate token and
> call it ctrl_dns_name that would be separate from traddr, but:
> - that will cause the changes a spray of changes that would accept
>    an alternative to traddr, and validate that at least one exist, but
>    fail if both are given
> - bubble up the usage to userspace, leaving the admin with the
>    exact format of the traddr
> - keep track of this when doing reconnects and stuff.
> 
> So I figured that the current approach would be preferable if
> we accept how to expose that for a given kernel/driver, we can
> accept dns names, or we cannot.
> 
> What do others think? Christoph? Keith? Chaitanya?

Anyone has feedback here?

Seems that we want this, but the question is how to expose
a new format of traddr (now can be a dns name) to userspace?