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

[Chao Leng]

On 2022/7/27 16:03, Daniel Wagner wrote:
> 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?
It's really troublesome to make strict judgments. Unexpected repeated connections
in special cases should be acceptable, because the harm is almost negligible and
the probability of actual occurrence is very low.
A bit more strict or relaxing both can be acceptable. but it should be same
for the host_traddr and the host_iface.
> .
>