[PATCH v2] nvme: consider also host_iface when checking ip options

[Daniel Wagner]

On Wed, Jul 27, 2022 at 02:52:25PM +0800, Chao Leng wrote:
> host_iface is different with traddr and trsvcid.
> traddr is required opt, trsvcid can be specified or the default.
> host_iface is NULL if without specified. It is same with host_tradr.
> 
> For example:
> nvme connect -t tcp -n nvmet-test -a 192.168.19.54 -s 4420 -f eth0
> nvme connect -t tcp -n nvmet-test -a 192.168.19.54 -s 4420
> According to your patch, the second will be treated as a duplicate.

Correct. And incidentally this is a duplicate :)

> It is similar like this:
> nvme connect -t tcp -n nvmet-test -a 192.168.19.54 -s 4420 -w 192.168.154.50
> nvme connect -t tcp -n nvmet-test -a 192.168.19.54 -s 4420
> This will be treated as a different new.

Yes, though if in this case eth0 has only IP 192.168.154.50, it would be
a duplicate.

> host_iface and host_tradr are similar opts related to host outgoing.
> host_iface and host_tradr may or may not be specified.
> The check logic should be consistent.

I see. The comment nvmf_ip_options_match() on this topic says the whole
logic is a bit 'rough'. Bringing iface_host into this picture makes the
whole logic even more complex. it's possible to use iface_host and
host_traddr combined. That's why I thought it would be a good idea to
make it more restrict.

I suspect the correct solution would be to inference each option set
which actual configuration is used. Not sure if it is worth the
hassle. Is there any real harm with duplicate connections, except
performance?

Even with this patch, it's possible to do something like this here:

# nvme list-subsys
nvme-subsys0 - NQN=nvmet-test
\
 +- nvme0 tcp traddr=192.168.19.54,trsvcid=4420,host_iface=eth0 live optimized
 +- nvme1 tcp traddr=192.168.19.54,trsvcid=4420,host_traddr=192.168.154.50 live optimized

It is a duplicate connection...

I don't mind relaxing the check but the question is should it combined
with the host_traddr logic or stay separate?