On 5/3/21 6:59 PM, Belanger, Martin wrote:

Hi Hannes,

I just noticed there were in-line comments. to answer you questions:

This is the recommended style when working with linux patches; please do 
not top-post.

Q1) Why not simply 'host_iface' ? 'triface' is a bit awkward.
A1) I used TRIFACE to keep consistency with all other transport options: traddr, trsvcid, host_traddr.
I will rename to host_iface at your suggestion.

Q2) Is this valid for all transports? I guess it would only work for 'tcp', and maybe 'rdma' if one
would be running ROCE. Shouldn't we error out on other transports like 'fc' or 'loop'?
A2) This is only for TCP, and we do check that this option is only allowed for TCP by specifying it
in the "allowed_opts" as follows (see file tcp.c):

static struct nvmf_transport_ops nvme_tcp_transport = {
         .name           = "tcp",
         .module         = THIS_MODULE,
         .required_opts  = NVMF_OPT_TRADDR,
         .allowed_opts   = NVMF_OPT_TRSVCID | NVMF_OPT_RECONNECT_DELAY |
                           NVMF_OPT_HOST_TRADDR | NVMF_OPT_CTRL_LOSS_TMO |
                           NVMF_OPT_HDR_DIGEST | NVMF_OPT_DATA_DIGEST |
                           NVMF_OPT_NR_WRITE_QUEUES | NVMF_OPT_NR_POLL_QUEUES |
                           NVMF_OPT_TOS | NVMF_OPT_HOST_TRIFACE,
         .create_ctrl    = nvme_tcp_create_ctrl,
};

Right, okay.

Q3) Normally the options are just parts of the 'address' string; why didn't
you use that approach here?
A3) I don't understand what you mean?

The 'address' string contains all transport arguments (without the 
transport type), concatenated by a ','.
So the 'host_iface' value should just be added to the address string 
itself, and not added as separate argument to the sysfs printf() call.

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

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme