[PATCHv2] nvme: print out valid arguments when reading from /dev/nvme-fabrics

[PATCHv2] nvme: print out valid arguments when reading from /dev/nvme-fabrics

[Hannes Reinecke]

Currently applications have a hard time figuring out which
nvme-over-fabrics arguments are supported for any given kernel;
the ioctl will return an error code on failure, and the application
has to guess whether this was due to an invalid argument or due
to a connection or controller error.
With this patch applications can read a list of supported
arguments by simply reading from /dev/nvme-fabrics, allowing
them to validate the connection string.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/host/fabrics.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 282d54117e0a..7ae041e2b3fb 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -1069,6 +1069,26 @@ static ssize_t nvmf_dev_write(struct file *file, const char __user *ubuf,
 	return ret ? ret : count;
 }
 
+static void __nvmf_concat_opt_tokens(struct seq_file *seq_file)
+{
+	const struct match_token *tok;
+	int idx;
+
+	/*
+	 * Add dummy entries for instance and cntlid to
+	 * signal an invalid/non-existing controller
+	 */
+	seq_puts(seq_file, "instance=-1,cntlid=-1");
+	for (idx = 0; idx < ARRAY_SIZE(opt_tokens); idx++) {
+		tok = &opt_tokens[idx];
+		if (tok->token == NVMF_OPT_ERR)
+			continue;
+		seq_puts(seq_file, ",");
+		seq_puts(seq_file, tok->pattern);
+	}
+	seq_puts(seq_file, "\n");
+}
+
 static int nvmf_dev_show(struct seq_file *seq_file, void *private)
 {
 	struct nvme_ctrl *ctrl;
@@ -1077,7 +1097,7 @@ static int nvmf_dev_show(struct seq_file *seq_file, void *private)
 	mutex_lock(&nvmf_dev_mutex);
 	ctrl = seq_file->private;
 	if (!ctrl) {
-		ret = -EINVAL;
+		__nvmf_concat_opt_tokens(seq_file);
 		goto out_unlock;
 	}
 
-- 
2.29.2

Re: [PATCHv2] nvme: print out valid arguments when reading from /dev/nvme-fabrics

[Chaitanya Kulkarni]

On 12/7/21 5:55 AM, Hannes Reinecke wrote:
> Currently applications have a hard time figuring out which
> nvme-over-fabrics arguments are supported for any given kernel;
> the ioctl will return an error code on failure, and the application
> has to guess whether this was due to an invalid argument or due
> to a connection or controller error.
> With this patch applications can read a list of supported
> arguments by simply reading from /dev/nvme-fabrics, allowing
> them to validate the connection string.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   drivers/nvme/host/fabrics.c | 22 +++++++++++++++++++++-
>   1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
> index 282d54117e0a..7ae041e2b3fb 100644
> --- a/drivers/nvme/host/fabrics.c
> +++ b/drivers/nvme/host/fabrics.c
> @@ -1069,6 +1069,26 @@ static ssize_t nvmf_dev_write(struct file *file, const char __user *ubuf,
>   	return ret ? ret : count;
>   }
>   
> +static void __nvmf_concat_opt_tokens(struct seq_file *seq_file)
> +{
> +	const struct match_token *tok;
> +	int idx;
> +
> +	/*
> +	 * Add dummy entries for instance and cntlid to
> +	 * signal an invalid/non-existing controller
> +	 */
> +	seq_puts(seq_file, "instance=-1,cntlid=-1");
> +	for (idx = 0; idx < ARRAY_SIZE(opt_tokens); idx++) {
> +		tok = &opt_tokens[idx];
> +		if (tok->token == NVMF_OPT_ERR)
> +			continue;
> +		seq_puts(seq_file, ",");

Can we use "\n" instead of "," ?
with that change it looks :-

instance=-1,cntlid=-1
transport=%s
traddr=%s
trsvcid=%s
nqn=%s
queue_size=%d
nr_io_queues=%d
reconnect_delay=%d
ctrl_loss_tmo=%d
keep_alive_tmo=%d
hostnqn=%s
host_traddr=%s
host_iface=%s
hostid=%s
duplicate_connect
disable_sqflow
hdr_digest
data_digest
nr_write_queues=%d
nr_poll_queues=%d
tos=%d
fast_io_fail_tmo=%d
discovery

Re: [PATCHv2] nvme: print out valid arguments when reading from /dev/nvme-fabrics

[Hannes Reinecke]

On 12/13/21 9:06 AM, Chaitanya Kulkarni wrote:
> On 12/7/21 5:55 AM, Hannes Reinecke wrote:
>> Currently applications have a hard time figuring out which
>> nvme-over-fabrics arguments are supported for any given kernel;
>> the ioctl will return an error code on failure, and the application
>> has to guess whether this was due to an invalid argument or due
>> to a connection or controller error.
>> With this patch applications can read a list of supported
>> arguments by simply reading from /dev/nvme-fabrics, allowing
>> them to validate the connection string.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>    drivers/nvme/host/fabrics.c | 22 +++++++++++++++++++++-
>>    1 file changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
>> index 282d54117e0a..7ae041e2b3fb 100644
>> --- a/drivers/nvme/host/fabrics.c
>> +++ b/drivers/nvme/host/fabrics.c
>> @@ -1069,6 +1069,26 @@ static ssize_t nvmf_dev_write(struct file *file, const char __user *ubuf,
>>    	return ret ? ret : count;
>>    }
>>    
>> +static void __nvmf_concat_opt_tokens(struct seq_file *seq_file)
>> +{
>> +	const struct match_token *tok;
>> +	int idx;
>> +
>> +	/*
>> +	 * Add dummy entries for instance and cntlid to
>> +	 * signal an invalid/non-existing controller
>> +	 */
>> +	seq_puts(seq_file, "instance=-1,cntlid=-1");
>> +	for (idx = 0; idx < ARRAY_SIZE(opt_tokens); idx++) {
>> +		tok = &opt_tokens[idx];
>> +		if (tok->token == NVMF_OPT_ERR)
>> +			continue;
>> +		seq_puts(seq_file, ",");
> 
> Can we use "\n" instead of "," ?
> with that change it looks :-
> 
> instance=-1,cntlid=-1
> transport=%s
> traddr=%s
> trsvcid=%s
> nqn=%s
> queue_size=%d
> nr_io_queues=%d
> reconnect_delay=%d
> ctrl_loss_tmo=%d
> keep_alive_tmo=%d
> hostnqn=%s
> host_traddr=%s
> host_iface=%s
> hostid=%s
> duplicate_connect
> disable_sqflow
> hdr_digest
> data_digest
> nr_write_queues=%d
> nr_poll_queues=%d
> tos=%d
> fast_io_fail_tmo=%d
> discovery
> 
The ',' format is being used for a successful connect, so I'd rather 
stay with that to make parsing easier.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCHv2] nvme: print out valid arguments when reading from /dev/nvme-fabrics

[Chaitanya Kulkarni]

  Can we use "\n" instead of "," ?
>> with that change it looks :-
>>
>> instance=-1,cntlid=-1
>> transport=%s
>> traddr=%s
>> trsvcid=%s
>> nqn=%s
>> queue_size=%d
>> nr_io_queues=%d
>> reconnect_delay=%d
>> ctrl_loss_tmo=%d
>> keep_alive_tmo=%d
>> hostnqn=%s
>> host_traddr=%s
>> host_iface=%s
>> hostid=%s
>> duplicate_connect
>> disable_sqflow
>> hdr_digest
>> data_digest
>> nr_write_queues=%d
>> nr_poll_queues=%d
>> tos=%d
>> fast_io_fail_tmo=%d
>> discovery
>>
> The ',' format is being used for a successful connect, so I'd rather
> stay with that to make parsing easier.
> 

hmmm, looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

Re: [PATCHv2] nvme: print out valid arguments when reading from /dev/nvme-fabrics

[Christoph Hellwig]

Thanks,

applied to nvme-5.17.