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

[Chao Leng]

On 2022/7/26 15:18, Daniel Wagner wrote:
> On Tue, Jul 26, 2022 at 10:11:41AM +0800, Chao Leng wrote:
>> Checking host_iface is similar to host_tradr, so some suggestions:
>> First, move these codes near checking host_traddr.
> 
> We have several places where we have the same sequence as above, that is
> traddr, trsvcid and then host_iface, e.g. nvme_class_uevent,
> nvmf_free_options, nvmf_get_address. So I'd say we should stick to this
> and not start introducing a new ordering scheme.
> 
>> Second, the checking logic should be the same as that of host_tradr.
> 
> What do you mean with this? We compare the host_iface with strcmp. It is
> exactly the same thing we do with traddr and trsvcid.
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.
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.

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.
> 
>> It might be better to add a helper to implement host port attribute
>> checking. Of course, this can be considered as a separate patch.
> 
> As far I can tell Christoph is not a big fan of small one-line helpers
> especially if there is only one call place. But as you said, that's his
> call :)
> .
>