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

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

[Daniel Wagner]

It's perfectly fine to use the same traddr and trsvcid more than once
as long we use different host interface. This is used in setups where
the host has more than one interface but the target exposes only one
traddr/trsvcid combination.

Use the same acceptance rules for host_iface as we have for
host_traddr.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
---

tw:~ # nvme connect -t tcp -n nvmet-test -a 192.168.19.54 -s 4420 -f enp1s0
tw:~ # nvme connect -t tcp -n nvmet-test -a 192.168.19.54 -s 4420 -f enp1s0
Failed to write to /dev/nvme-fabrics: Operation already in progress
no controller found: failed to write to nvme-fabrics device
tw:~ # nvme connect -t tcp -n nvmet-test -a 192.168.19.54 -s 4420 
tw:~ # nvme connect -t tcp -n nvmet-test -a 192.168.19.54 -s 4420 -f enp7s0
tw:~ # nvme list-subsys
nvme-subsys0 - NQN=nvmet-test
\
 +- nvme0 tcp traddr=192.168.19.54,trsvcid=4420,host_iface=enp1s0 live optimized
 +- nvme1 tcp traddr=192.168.19.54,trsvcid=4420 live optimized
 +- nvme2 tcp traddr=192.168.19.54,trsvcid=4420,host_iface=enp7s0 live optimized

changes:
v3: same check rules as for local address
v2: added NVMF_OPT_HOST_TRADDR checks but to strict
v1: initial version

 drivers/nvme/host/fabrics.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 5207a2348257..2d948bcc31e0 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -965,13 +965,17 @@ bool nvmf_ip_options_match(struct nvme_ctrl *ctrl,
 		return false;
 
 	/*
-	 * Checking the local address is rough. In most cases, none is specified
-	 * and the host port is selected by the stack.
+	 * Checking the local address or host interfaces is rough.
+	 *
+	 * In most cases, none is specified and the host port or
+	 * host interface is selected by the stack.
 	 *
 	 * Assume no match if:
-	 * -  local address is specified and address is not the same
-	 * -  local address is not specified but remote is, or vice versa
-	 *    (admin using specific host_traddr when it matters).
+	 * -  local address or host interface is specified and address
+	 *    or host interface is not the same
+	 * -  local address or host interface is not specified but
+	 *    remote is, or vice versa (admin using specific
+	 *    host_traddr/host_iface when it matters).
 	 */
 	if ((opts->mask & NVMF_OPT_HOST_TRADDR) &&
 	    (ctrl->opts->mask & NVMF_OPT_HOST_TRADDR)) {
@@ -982,6 +986,15 @@ bool nvmf_ip_options_match(struct nvme_ctrl *ctrl,
 		return false;
 	}
 
+	if ((opts->mask & NVMF_OPT_HOST_IFACE) &&
+	    (ctrl->opts->mask & NVMF_OPT_HOST_IFACE)) {
+		if (strcmp(opts->host_iface, ctrl->opts->host_iface))
+			return false;
+	} else if ((opts->mask & NVMF_OPT_HOST_IFACE) ||
+		   (ctrl->opts->mask & NVMF_OPT_HOST_IFACE)) {
+		return false;
+	}
+
 	return true;
 }
 EXPORT_SYMBOL_GPL(nvmf_ip_options_match);
-- 
2.37.1

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

[Chao Leng]

Looks good to me.

Reviewed-by: Chao Leng <lengchao@huawei.com>

On 2022/7/29 22:26, Daniel Wagner wrote:
> It's perfectly fine to use the same traddr and trsvcid more than once
> as long we use different host interface. This is used in setups where
> the host has more than one interface but the target exposes only one
> traddr/trsvcid combination.
> 
> Use the same acceptance rules for host_iface as we have for
> host_traddr.
> 
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
> 
> tw:~ # nvme connect -t tcp -n nvmet-test -a 192.168.19.54 -s 4420 -f enp1s0
> tw:~ # nvme connect -t tcp -n nvmet-test -a 192.168.19.54 -s 4420 -f enp1s0
> Failed to write to /dev/nvme-fabrics: Operation already in progress
> no controller found: failed to write to nvme-fabrics device
> tw:~ # nvme connect -t tcp -n nvmet-test -a 192.168.19.54 -s 4420
> tw:~ # nvme connect -t tcp -n nvmet-test -a 192.168.19.54 -s 4420 -f enp7s0
> tw:~ # nvme list-subsys
> nvme-subsys0 - NQN=nvmet-test
> \
>   +- nvme0 tcp traddr=192.168.19.54,trsvcid=4420,host_iface=enp1s0 live optimized
>   +- nvme1 tcp traddr=192.168.19.54,trsvcid=4420 live optimized
>   +- nvme2 tcp traddr=192.168.19.54,trsvcid=4420,host_iface=enp7s0 live optimized
> 
> changes:
> v3: same check rules as for local address
> v2: added NVMF_OPT_HOST_TRADDR checks but to strict
> v1: initial version
> 
>   drivers/nvme/host/fabrics.c | 23 ++++++++++++++++++-----
>   1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
> index 5207a2348257..2d948bcc31e0 100644
> --- a/drivers/nvme/host/fabrics.c
> +++ b/drivers/nvme/host/fabrics.c
> @@ -965,13 +965,17 @@ bool nvmf_ip_options_match(struct nvme_ctrl *ctrl,
>   		return false;
>   
>   	/*
> -	 * Checking the local address is rough. In most cases, none is specified
> -	 * and the host port is selected by the stack.
> +	 * Checking the local address or host interfaces is rough.
> +	 *
> +	 * In most cases, none is specified and the host port or
> +	 * host interface is selected by the stack.
>   	 *
>   	 * Assume no match if:
> -	 * -  local address is specified and address is not the same
> -	 * -  local address is not specified but remote is, or vice versa
> -	 *    (admin using specific host_traddr when it matters).
> +	 * -  local address or host interface is specified and address
> +	 *    or host interface is not the same
> +	 * -  local address or host interface is not specified but
> +	 *    remote is, or vice versa (admin using specific
> +	 *    host_traddr/host_iface when it matters).
>   	 */
>   	if ((opts->mask & NVMF_OPT_HOST_TRADDR) &&
>   	    (ctrl->opts->mask & NVMF_OPT_HOST_TRADDR)) {
> @@ -982,6 +986,15 @@ bool nvmf_ip_options_match(struct nvme_ctrl *ctrl,
>   		return false;
>   	}
>   
> +	if ((opts->mask & NVMF_OPT_HOST_IFACE) &&
> +	    (ctrl->opts->mask & NVMF_OPT_HOST_IFACE)) {
> +		if (strcmp(opts->host_iface, ctrl->opts->host_iface))
> +			return false;
> +	} else if ((opts->mask & NVMF_OPT_HOST_IFACE) ||
> +		   (ctrl->opts->mask & NVMF_OPT_HOST_IFACE)) {
> +		return false;
> +	}
> +
>   	return true;
>   }
>   EXPORT_SYMBOL_GPL(nvmf_ip_options_match);
> 

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

[Daniel Wagner]

On Sat, Jul 30, 2022 at 08:41:51AM +0800, Chao Leng wrote:
> Looks good to me.
> 
> Reviewed-by: Chao Leng <lengchao@huawei.com>

ping

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

[Christoph Hellwig]

Thanks,

applied to nvme-6.1.