[PATCH 1/2] nvme-rdma: Don't leak uninitialized memory in connect request private data

[PATCH 1/2] nvme-rdma: Don't leak uninitialized memory in connect request private data

[Roland Dreier]

From: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>

Zero out the full nvme_rdma_cm_req structure before sending it.
Otherwise we end up leaking kernel memory in the reserved field, which
might break forward compatibility in the future.

Signed-off-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
 drivers/nvme/host/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 3e3ce2b0424e..b96b88369871 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1269,7 +1269,7 @@ static int nvme_rdma_route_resolved(struct nvme_rdma_queue *queue)
 {
 	struct nvme_rdma_ctrl *ctrl = queue->ctrl;
 	struct rdma_conn_param param = { };
-	struct nvme_rdma_cm_req priv;
+	struct nvme_rdma_cm_req priv = { };
 	int ret;
 
 	param.qp_num = queue->qp->qp_num;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] nvme-rdma: Don't leak uninitialized memory in connect request private data

[Roland Dreier]

From: Roland Dreier <roland@purestorage.com>

Zero out the full nvme_rdma_cm_req structure before sending it.
Otherwise we end up leaking kernel memory in the reserved field, which
might break forward compatibility in the future.

Signed-off-by: Roland Dreier <roland at purestorage.com>
---
 drivers/nvme/host/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 3e3ce2b0424e..b96b88369871 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1269,7 +1269,7 @@ static int nvme_rdma_route_resolved(struct nvme_rdma_queue *queue)
 {
 	struct nvme_rdma_ctrl *ctrl = queue->ctrl;
 	struct rdma_conn_param param = { };
-	struct nvme_rdma_cm_req priv;
+	struct nvme_rdma_cm_req priv = { };
 	int ret;
 
 	param.qp_num = queue->qp->qp_num;
-- 
2.7.4

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Roland Dreier]

From: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>

If a target address does not parse as IPv4, try parsing it as IPv6
(including handling '%<scope-id>' suffixes for link-local addresses).

Signed-off-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
 drivers/nvme/host/rdma.c | 55 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 11 deletions(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index b96b88369871..dd4aa54cd709 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -138,6 +138,7 @@ struct nvme_rdma_ctrl {
 	union {
 		struct sockaddr addr;
 		struct sockaddr_in addr_in;
+		struct sockaddr_in6 addr_in6;
 	};
 
 	struct nvme_ctrl	ctrl;
@@ -1847,19 +1848,51 @@ out_free_io_queues:
 	return ret;
 }
 
-static int nvme_rdma_parse_ipaddr(struct sockaddr_in *in_addr, char *p)
+static int nvme_rdma_parse_ipaddr(struct sockaddr *addr, char *p)
 {
-	u8 *addr = (u8 *)&in_addr->sin_addr.s_addr;
 	size_t buflen = strlen(p);
 
-	/* XXX: handle IPv6 addresses */
+	if (buflen <= INET_ADDRSTRLEN) {
+		struct sockaddr_in *addr4 = (struct sockaddr_in *) addr;
+		if (in4_pton(p, buflen, (u8 *) &addr4->sin_addr.s_addr,
+			     '\0', NULL) > 0) {
+			addr4->sin_family = AF_INET;
+			return 0;
+		}
+	}
 
-	if (buflen > INET_ADDRSTRLEN)
-		return -EINVAL;
-	if (in4_pton(p, buflen, addr, '\0', NULL) == 0)
-		return -EINVAL;
-	in_addr->sin_family = AF_INET;
-	return 0;
+	if (buflen <= INET6_ADDRSTRLEN) {
+		struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *) addr;
+		const char *scope_delim;
+
+		if (in6_pton(p, buflen, (u8 *) &addr6->sin6_addr.s6_addr,
+			     '%', &scope_delim) == 0)
+			return -EINVAL;
+		addr6->sin6_family = AF_INET6;
+
+		if (ipv6_addr_type(&addr6->sin6_addr) & IPV6_ADDR_LINKLOCAL &&
+		    p + buflen != scope_delim && *scope_delim == '%') {
+			char scope_id[16];
+			size_t scope_len = min_t(size_t, sizeof scope_id,
+						 p + buflen - scope_delim - 1);
+			struct net_device *dev;
+
+			memcpy(scope_id, scope_delim + 1, scope_len);
+			scope_id[scope_len] = '\0';
+
+			/* XXX: what network namespace should we use? */
+			dev = dev_get_by_name(&init_net, scope_id);
+			if (dev) {
+				addr6->sin6_scope_id = dev->ifindex;
+				dev_put(dev);
+			} else if (kstrtouint(scope_id, 0, &addr6->sin6_scope_id))
+				return -EINVAL;
+		}
+
+		return 0;
+	}
+
+	return -EINVAL;
 }
 
 static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
@@ -1875,7 +1908,7 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
 	ctrl->ctrl.opts = opts;
 	INIT_LIST_HEAD(&ctrl->list);
 
-	ret = nvme_rdma_parse_ipaddr(&ctrl->addr_in, opts->traddr);
+	ret = nvme_rdma_parse_ipaddr(&ctrl->addr, opts->traddr);
 	if (ret) {
 		pr_err("malformed IP address passed: %s\n", opts->traddr);
 		goto out_free_ctrl;
@@ -1949,7 +1982,7 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
 	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE);
 	WARN_ON_ONCE(!changed);
 
-	dev_info(ctrl->ctrl.device, "new ctrl: NQN \"%s\", addr %pISp\n",
+	dev_info(ctrl->ctrl.device, "new ctrl: NQN \"%s\", addr %pISpcs\n",
 		ctrl->ctrl.opts->subsysnqn, &ctrl->addr);
 
 	kref_get(&ctrl->ctrl.kref);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Roland Dreier]

From: Roland Dreier <roland@purestorage.com>

If a target address does not parse as IPv4, try parsing it as IPv6
(including handling '%<scope-id>' suffixes for link-local addresses).

Signed-off-by: Roland Dreier <roland at purestorage.com>
---
 drivers/nvme/host/rdma.c | 55 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 11 deletions(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index b96b88369871..dd4aa54cd709 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -138,6 +138,7 @@ struct nvme_rdma_ctrl {
 	union {
 		struct sockaddr addr;
 		struct sockaddr_in addr_in;
+		struct sockaddr_in6 addr_in6;
 	};
 
 	struct nvme_ctrl	ctrl;
@@ -1847,19 +1848,51 @@ out_free_io_queues:
 	return ret;
 }
 
-static int nvme_rdma_parse_ipaddr(struct sockaddr_in *in_addr, char *p)
+static int nvme_rdma_parse_ipaddr(struct sockaddr *addr, char *p)
 {
-	u8 *addr = (u8 *)&in_addr->sin_addr.s_addr;
 	size_t buflen = strlen(p);
 
-	/* XXX: handle IPv6 addresses */
+	if (buflen <= INET_ADDRSTRLEN) {
+		struct sockaddr_in *addr4 = (struct sockaddr_in *) addr;
+		if (in4_pton(p, buflen, (u8 *) &addr4->sin_addr.s_addr,
+			     '\0', NULL) > 0) {
+			addr4->sin_family = AF_INET;
+			return 0;
+		}
+	}
 
-	if (buflen > INET_ADDRSTRLEN)
-		return -EINVAL;
-	if (in4_pton(p, buflen, addr, '\0', NULL) == 0)
-		return -EINVAL;
-	in_addr->sin_family = AF_INET;
-	return 0;
+	if (buflen <= INET6_ADDRSTRLEN) {
+		struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *) addr;
+		const char *scope_delim;
+
+		if (in6_pton(p, buflen, (u8 *) &addr6->sin6_addr.s6_addr,
+			     '%', &scope_delim) == 0)
+			return -EINVAL;
+		addr6->sin6_family = AF_INET6;
+
+		if (ipv6_addr_type(&addr6->sin6_addr) & IPV6_ADDR_LINKLOCAL &&
+		    p + buflen != scope_delim && *scope_delim == '%') {
+			char scope_id[16];
+			size_t scope_len = min_t(size_t, sizeof scope_id,
+						 p + buflen - scope_delim - 1);
+			struct net_device *dev;
+
+			memcpy(scope_id, scope_delim + 1, scope_len);
+			scope_id[scope_len] = '\0';
+
+			/* XXX: what network namespace should we use? */
+			dev = dev_get_by_name(&init_net, scope_id);
+			if (dev) {
+				addr6->sin6_scope_id = dev->ifindex;
+				dev_put(dev);
+			} else if (kstrtouint(scope_id, 0, &addr6->sin6_scope_id))
+				return -EINVAL;
+		}
+
+		return 0;
+	}
+
+	return -EINVAL;
 }
 
 static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
@@ -1875,7 +1908,7 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
 	ctrl->ctrl.opts = opts;
 	INIT_LIST_HEAD(&ctrl->list);
 
-	ret = nvme_rdma_parse_ipaddr(&ctrl->addr_in, opts->traddr);
+	ret = nvme_rdma_parse_ipaddr(&ctrl->addr, opts->traddr);
 	if (ret) {
 		pr_err("malformed IP address passed: %s\n", opts->traddr);
 		goto out_free_ctrl;
@@ -1949,7 +1982,7 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
 	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE);
 	WARN_ON_ONCE(!changed);
 
-	dev_info(ctrl->ctrl.device, "new ctrl: NQN \"%s\", addr %pISp\n",
+	dev_info(ctrl->ctrl.device, "new ctrl: NQN \"%s\", addr %pISpcs\n",
 		ctrl->ctrl.opts->subsysnqn, &ctrl->addr);
 
 	kref_get(&ctrl->ctrl.kref);
-- 
2.7.4

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

On 31/07/16 10:27, Roland Dreier wrote:
> From: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
>
> If a target address does not parse as IPv4, try parsing it as IPv6
> (including handling '%<scope-id>' suffixes for link-local addresses).
>
> Signed-off-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>


Nice! thanks Roland,

testing it now...
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

On 31/07/16 10:27, Roland Dreier wrote:
> From: Roland Dreier <roland at purestorage.com>
>
> If a target address does not parse as IPv4, try parsing it as IPv6
> (including handling '%<scope-id>' suffixes for link-local addresses).
>
> Signed-off-by: Roland Dreier <roland at purestorage.com>


Nice! thanks Roland,

testing it now...

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

> On 31/07/16 10:27, Roland Dreier wrote:
>> From: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
>>
>> If a target address does not parse as IPv4, try parsing it as IPv6
>> (including handling '%<scope-id>' suffixes for link-local addresses).
>>
>> Signed-off-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
>
>
> Nice! thanks Roland,
>
> testing it now...

How did you test it btw?

Currently the target driver doesn't support ipv6 addresses.

These complementary patches [1,2] makes the it work e2e! So,

Reviewed-by: Sagi Grimberg <sagi-egDjqUIXVlxBDLzU/O5InQ@public.gmane.org>
and,
Tested-by: Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>


[1]:
--
commit 068d8e511d9a6927721bddfd545ad4976196297d
Author: Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
Date:   Sun Jul 31 13:45:44 2016 +0300

     nvmet-rdma: Add ipv6 support

     Allow the nvme-rdma target accept connections using
     ipv6 address resolution.

     Signed-off-by: Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>

diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 4e83d92d6bdd..347cc6d37dad 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -1336,29 +1336,65 @@ restart:
         mutex_unlock(&nvmet_rdma_queue_mutex);
  }

-static int nvmet_rdma_add_port(struct nvmet_port *port)
+static int nvmet_rdma_set_addr_port(int family, char *ipaddr,
+               char *ipport, struct sockaddr_storage *addr)
  {
-       struct rdma_cm_id *cm_id;
-       struct sockaddr_in addr_in;
-       u16 port_in;
-       int ret;
+       size_t buflen = strlen(ipaddr);
+       struct sockaddr_in *addr4;
+       struct sockaddr_in6 *addr6;
+       u16 port;
+
+       if (kstrtou16(ipport, 0, &port))
+               return -EINVAL;

-       switch (port->disc_addr.adrfam) {
+       switch (family) {
         case NVMF_ADDR_FAMILY_IP4:
+               if (buflen > INET_ADDRSTRLEN)
+                       return -EINVAL;
+
+               addr4 = (struct sockaddr_in *)addr;
+
+               if (in4_pton(ipaddr, buflen, (u8 *)&addr4->sin_addr.s_addr,
+                            '\n', NULL) == 0)
+                       return -EINVAL;
+
+               addr4->sin_family = AF_INET;
+               addr4->sin_port = htons(port);
+               break;
+       case NVMF_ADDR_FAMILY_IP6:
+               if (buflen > INET6_ADDRSTRLEN)
+                       return -EINVAL;
+
+               addr6 = (struct sockaddr_in6 *) addr;
+
+               if (in6_pton(ipaddr, buflen, (u8 
*)&addr6->sin6_addr.s6_addr,
+                            '\n', NULL) == 0)
+                       return -EINVAL;
+
+               addr6->sin6_family = AF_INET6;
+               addr6->sin6_port = htons(port);
                 break;
         default:
-               pr_err("address family %d not supported\n",
-                               port->disc_addr.adrfam);
+               pr_err("address family %d not supported\n", family);
                 return -EINVAL;
         }

-       ret = kstrtou16(port->disc_addr.trsvcid, 0, &port_in);
-       if (ret)
-               return ret;
+       return 0;
+}

-       addr_in.sin_family = AF_INET;
-       addr_in.sin_addr.s_addr = in_aton(port->disc_addr.traddr);
-       addr_in.sin_port = htons(port_in);
+static int nvmet_rdma_add_port(struct nvmet_port *port)
+{
+       struct rdma_cm_id *cm_id;
+       struct sockaddr_storage addr = { };
+       int ret;
+
+       ret = nvmet_rdma_set_addr_port(port->disc_addr.adrfam,
+               port->disc_addr.traddr, port->disc_addr.trsvcid, &addr);
+       if (ret) {
+               pr_err("failed setting addr %s:%s\n",
+               port->disc_addr.traddr, port->disc_addr.trsvcid);
+               return ret;
+       }

         cm_id = rdma_create_id(&init_net, nvmet_rdma_cm_handler, port,
                         RDMA_PS_TCP, IB_QPT_RC);
@@ -1367,20 +1403,33 @@ static int nvmet_rdma_add_port(struct nvmet_port 
*port)
                 return PTR_ERR(cm_id);
         }

-       ret = rdma_bind_addr(cm_id, (struct sockaddr *)&addr_in);
+       /*
+        * Allow both IPv4 and IPv6 sockets to bind a single port
+        * at the same time.
+        */
+       ret = rdma_set_afonly(cm_id, 1);
+       if (ret) {
+               pr_err("rdma_set_afonly failed (%d)\n", ret);
+               goto out_destroy_id;
+       }
+
+       ret = rdma_bind_addr(cm_id, (struct sockaddr *)&addr);
         if (ret) {
-               pr_err("binding CM ID to %pISpc failed (%d)\n", 
&addr_in, ret);
+               pr_err("binding CM ID to %pISpcs failed (%d)\n",
+                       (struct sockaddr *)&addr, ret);
                 goto out_destroy_id;
         }

         ret = rdma_listen(cm_id, 128);
         if (ret) {
-               pr_err("listening to %pISpc failed (%d)\n", &addr_in, ret);
+               pr_err("listening to %pISpcs failed (%d)\n",
+                       (struct sockaddr *)&addr, ret);
                 goto out_destroy_id;
         }

-       pr_info("enabling port %d (%pISpc)\n",
-               le16_to_cpu(port->disc_addr.portid), &addr_in);
+       pr_info("enabling port %d (%pISpcs)\n",
+               le16_to_cpu(port->disc_addr.portid),
+               (struct sockaddr *)&addr);
         port->priv = cm_id;
         return 0;
--

[2]:
--
commit 6d962c76580680e9cd0f5a56f47bcd30b5a39dd7
Author: Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
Date:   Sun Jul 31 13:56:58 2016 +0300

     fabrics: Allow ipv6 address resolution

     Signed-off-by: Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>

diff --git a/fabrics.c b/fabrics.c
index 4bf557b5e672..221e34e5e39b 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -390,7 +390,8 @@ static int connect_ctrl(struct 
nvmf_disc_rsp_page_entry *e)
                 /* we can safely ignore the rest of the entries */
                 break;
         case NVMF_TRTYPE_RDMA:
-               if (e->adrfam != NVMF_ADDR_FAMILY_IP4) {
+               if (e->adrfam != NVMF_ADDR_FAMILY_IP4 &&
+                   e->adrfam != NVMF_ADDR_FAMILY_IP6) {
                         fprintf(stderr, "skipping unsupported adrfam\n");
                         return -EINVAL;
                 }
--
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

> On 31/07/16 10:27, Roland Dreier wrote:
>> From: Roland Dreier <roland at purestorage.com>
>>
>> If a target address does not parse as IPv4, try parsing it as IPv6
>> (including handling '%<scope-id>' suffixes for link-local addresses).
>>
>> Signed-off-by: Roland Dreier <roland at purestorage.com>
>
>
> Nice! thanks Roland,
>
> testing it now...

How did you test it btw?

Currently the target driver doesn't support ipv6 addresses.

These complementary patches [1,2] makes the it work e2e! So,

Reviewed-by: Sagi Grimberg <sagi at grimbeg.me>
and,
Tested-by: Sagi Grimberg <sagi at grimberg.me>


[1]:
--
commit 068d8e511d9a6927721bddfd545ad4976196297d
Author: Sagi Grimberg <sagi at grimberg.me>
Date:   Sun Jul 31 13:45:44 2016 +0300

     nvmet-rdma: Add ipv6 support

     Allow the nvme-rdma target accept connections using
     ipv6 address resolution.

     Signed-off-by: Sagi Grimberg <sagi at grimberg.me>

diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 4e83d92d6bdd..347cc6d37dad 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -1336,29 +1336,65 @@ restart:
         mutex_unlock(&nvmet_rdma_queue_mutex);
  }

-static int nvmet_rdma_add_port(struct nvmet_port *port)
+static int nvmet_rdma_set_addr_port(int family, char *ipaddr,
+               char *ipport, struct sockaddr_storage *addr)
  {
-       struct rdma_cm_id *cm_id;
-       struct sockaddr_in addr_in;
-       u16 port_in;
-       int ret;
+       size_t buflen = strlen(ipaddr);
+       struct sockaddr_in *addr4;
+       struct sockaddr_in6 *addr6;
+       u16 port;
+
+       if (kstrtou16(ipport, 0, &port))
+               return -EINVAL;

-       switch (port->disc_addr.adrfam) {
+       switch (family) {
         case NVMF_ADDR_FAMILY_IP4:
+               if (buflen > INET_ADDRSTRLEN)
+                       return -EINVAL;
+
+               addr4 = (struct sockaddr_in *)addr;
+
+               if (in4_pton(ipaddr, buflen, (u8 *)&addr4->sin_addr.s_addr,
+                            '\n', NULL) == 0)
+                       return -EINVAL;
+
+               addr4->sin_family = AF_INET;
+               addr4->sin_port = htons(port);
+               break;
+       case NVMF_ADDR_FAMILY_IP6:
+               if (buflen > INET6_ADDRSTRLEN)
+                       return -EINVAL;
+
+               addr6 = (struct sockaddr_in6 *) addr;
+
+               if (in6_pton(ipaddr, buflen, (u8 
*)&addr6->sin6_addr.s6_addr,
+                            '\n', NULL) == 0)
+                       return -EINVAL;
+
+               addr6->sin6_family = AF_INET6;
+               addr6->sin6_port = htons(port);
                 break;
         default:
-               pr_err("address family %d not supported\n",
-                               port->disc_addr.adrfam);
+               pr_err("address family %d not supported\n", family);
                 return -EINVAL;
         }

-       ret = kstrtou16(port->disc_addr.trsvcid, 0, &port_in);
-       if (ret)
-               return ret;
+       return 0;
+}

-       addr_in.sin_family = AF_INET;
-       addr_in.sin_addr.s_addr = in_aton(port->disc_addr.traddr);
-       addr_in.sin_port = htons(port_in);
+static int nvmet_rdma_add_port(struct nvmet_port *port)
+{
+       struct rdma_cm_id *cm_id;
+       struct sockaddr_storage addr = { };
+       int ret;
+
+       ret = nvmet_rdma_set_addr_port(port->disc_addr.adrfam,
+               port->disc_addr.traddr, port->disc_addr.trsvcid, &addr);
+       if (ret) {
+               pr_err("failed setting addr %s:%s\n",
+               port->disc_addr.traddr, port->disc_addr.trsvcid);
+               return ret;
+       }

         cm_id = rdma_create_id(&init_net, nvmet_rdma_cm_handler, port,
                         RDMA_PS_TCP, IB_QPT_RC);
@@ -1367,20 +1403,33 @@ static int nvmet_rdma_add_port(struct nvmet_port 
*port)
                 return PTR_ERR(cm_id);
         }

-       ret = rdma_bind_addr(cm_id, (struct sockaddr *)&addr_in);
+       /*
+        * Allow both IPv4 and IPv6 sockets to bind a single port
+        * at the same time.
+        */
+       ret = rdma_set_afonly(cm_id, 1);
+       if (ret) {
+               pr_err("rdma_set_afonly failed (%d)\n", ret);
+               goto out_destroy_id;
+       }
+
+       ret = rdma_bind_addr(cm_id, (struct sockaddr *)&addr);
         if (ret) {
-               pr_err("binding CM ID to %pISpc failed (%d)\n", 
&addr_in, ret);
+               pr_err("binding CM ID to %pISpcs failed (%d)\n",
+                       (struct sockaddr *)&addr, ret);
                 goto out_destroy_id;
         }

         ret = rdma_listen(cm_id, 128);
         if (ret) {
-               pr_err("listening to %pISpc failed (%d)\n", &addr_in, ret);
+               pr_err("listening to %pISpcs failed (%d)\n",
+                       (struct sockaddr *)&addr, ret);
                 goto out_destroy_id;
         }

-       pr_info("enabling port %d (%pISpc)\n",
-               le16_to_cpu(port->disc_addr.portid), &addr_in);
+       pr_info("enabling port %d (%pISpcs)\n",
+               le16_to_cpu(port->disc_addr.portid),
+               (struct sockaddr *)&addr);
         port->priv = cm_id;
         return 0;
--

[2]:
--
commit 6d962c76580680e9cd0f5a56f47bcd30b5a39dd7
Author: Sagi Grimberg <sagi at grimberg.me>
Date:   Sun Jul 31 13:56:58 2016 +0300

     fabrics: Allow ipv6 address resolution

     Signed-off-by: Sagi Grimberg <sagi at grimberg.me>

diff --git a/fabrics.c b/fabrics.c
index 4bf557b5e672..221e34e5e39b 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -390,7 +390,8 @@ static int connect_ctrl(struct 
nvmf_disc_rsp_page_entry *e)
                 /* we can safely ignore the rest of the entries */
                 break;
         case NVMF_TRTYPE_RDMA:
-               if (e->adrfam != NVMF_ADDR_FAMILY_IP4) {
+               if (e->adrfam != NVMF_ADDR_FAMILY_IP4 &&
+                   e->adrfam != NVMF_ADDR_FAMILY_IP6) {
                         fprintf(stderr, "skipping unsupported adrfam\n");
                         return -EINVAL;
                 }
--

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Roland Dreier]

On Sun, Jul 31, 2016 at 3:58 AM, Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org> wrote:
> How did you test it btw?

I have a userspace target :).  Which is also how I caught the
uninitialized private data.

> Currently the target driver doesn't support ipv6 addresses.
>
> These complementary patches [1,2] makes the it work e2e!

That's cool!  Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Roland Dreier]

On Sun, Jul 31, 2016@3:58 AM, Sagi Grimberg <sagi@grimberg.me> wrote:
> How did you test it btw?

I have a userspace target :).  Which is also how I caught the
uninitialized private data.

> Currently the target driver doesn't support ipv6 addresses.
>
> These complementary patches [1,2] makes the it work e2e!

That's cool!  Thanks.

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

>> How did you test it btw?
>
> I have a userspace target :).

Interesting. Is the target public?

> Which is also how I caught the uninitialized private data.

Nice catch!
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

>> How did you test it btw?
>
> I have a userspace target :).

Interesting. Is the target public?

> Which is also how I caught the uninitialized private data.

Nice catch!

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

>>> How did you test it btw?
>>
>> I have a userspace target :).

Is it the spdk one?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

>>> How did you test it btw?
>>
>> I have a userspace target :).

Is it the spdk one?

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Christoph Hellwig]

> Currently the target driver doesn't support ipv6 addresses.
>
> These complementary patches [1,2] makes the it work e2e! So,

I think we should use the same ipv6 addr parsing that Roland used
in the host, and preferably have another module export that as a helper.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Christoph Hellwig]

> Currently the target driver doesn't support ipv6 addresses.
>
> These complementary patches [1,2] makes the it work e2e! So,

I think we should use the same ipv6 addr parsing that Roland used
in the host, and preferably have another module export that as a helper.

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

>> Currently the target driver doesn't support ipv6 addresses.
>>
>> These complementary patches [1,2] makes the it work e2e! So,
>
> I think we should use the same ipv6 addr parsing that Roland used
> in the host, and preferably have another module export that as a helper.

I can try to unify them, but unlike the host that first tries ipv4 and
then fall to ipv6, the target needs to enforce the address family so
it's somewhat different. I kinda think that these are different enough
to stay separate don't you think?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

>> Currently the target driver doesn't support ipv6 addresses.
>>
>> These complementary patches [1,2] makes the it work e2e! So,
>
> I think we should use the same ipv6 addr parsing that Roland used
> in the host, and preferably have another module export that as a helper.

I can try to unify them, but unlike the host that first tries ipv4 and
then fall to ipv6, the target needs to enforce the address family so
it's somewhat different. I kinda think that these are different enough
to stay separate don't you think?

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Christoph Hellwig]

On Mon, Aug 01, 2016 at 02:24:10PM +0300, Sagi Grimberg wrote:
>
>>> Currently the target driver doesn't support ipv6 addresses.
>>>
>>> These complementary patches [1,2] makes the it work e2e! So,
>>
>> I think we should use the same ipv6 addr parsing that Roland used
>> in the host, and preferably have another module export that as a helper.
>
> I can try to unify them, but unlike the host that first tries ipv4 and
> then fall to ipv6, the target needs to enforce the address family so
> it's somewhat different. I kinda think that these are different enough
> to stay separate don't you think?

It'd still need all the scope ID handling similar to what Roland did,
and that's a fair chunk of code.  We have a few options to handle the
different allowed addresses:

 (1) v4/v6 only flags
 (2) having low-level v4/v6 handlers and one that tries these both
 (3) using the try both handler and rejecting the wrong one after
     parsing.

(3) seems easiest, but (2) sounds fine to me.  But I'd really like to
hear from folks on the netdev list what they think of that idea first.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Christoph Hellwig]

On Mon, Aug 01, 2016@02:24:10PM +0300, Sagi Grimberg wrote:
>
>>> Currently the target driver doesn't support ipv6 addresses.
>>>
>>> These complementary patches [1,2] makes the it work e2e! So,
>>
>> I think we should use the same ipv6 addr parsing that Roland used
>> in the host, and preferably have another module export that as a helper.
>
> I can try to unify them, but unlike the host that first tries ipv4 and
> then fall to ipv6, the target needs to enforce the address family so
> it's somewhat different. I kinda think that these are different enough
> to stay separate don't you think?

It'd still need all the scope ID handling similar to what Roland did,
and that's a fair chunk of code.  We have a few options to handle the
different allowed addresses:

 (1) v4/v6 only flags
 (2) having low-level v4/v6 handlers and one that tries these both
 (3) using the try both handler and rejecting the wrong one after
     parsing.

(3) seems easiest, but (2) sounds fine to me.  But I'd really like to
hear from folks on the netdev list what they think of that idea first.

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Roland Dreier]

On Mon, Aug 1, 2016 at 8:50 AM, Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> wrote:
> It'd still need all the scope ID handling similar to what Roland did,
> and that's a fair chunk of code.  We have a few options to handle the
> different allowed addresses:
>
>  (1) v4/v6 only flags
>  (2) having low-level v4/v6 handlers and one that tries these both
>  (3) using the try both handler and rejecting the wrong one after
>      parsing.
>
> (3) seems easiest, but (2) sounds fine to me.  But I'd really like to
> hear from folks on the netdev list what they think of that idea first.

I think adding a new helper that parses both v4 and v6 addresses +
scope ID seems like the best thing for now.  I did a grep for in6_pton
and it looks like at least fs/cifs/netmisc.c and net/sunrpc/addr.c
could use the helper.

What do you think of adding inet_pton_with_scope() to
net/core/utils.c?  I'm open to better ideas on the name.  But I can
code that up and use it in nvme, as well as convert over the two
places I mentioned above.  The first parameter of the function can be
an af, and the caller can pass in AF_UNSPEC, AF_INET, or AF_INET6 to
restrict the parsing to one type of address (or not).

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Roland Dreier]

On Mon, Aug 1, 2016@8:50 AM, Christoph Hellwig <hch@lst.de> wrote:
> It'd still need all the scope ID handling similar to what Roland did,
> and that's a fair chunk of code.  We have a few options to handle the
> different allowed addresses:
>
>  (1) v4/v6 only flags
>  (2) having low-level v4/v6 handlers and one that tries these both
>  (3) using the try both handler and rejecting the wrong one after
>      parsing.
>
> (3) seems easiest, but (2) sounds fine to me.  But I'd really like to
> hear from folks on the netdev list what they think of that idea first.

I think adding a new helper that parses both v4 and v6 addresses +
scope ID seems like the best thing for now.  I did a grep for in6_pton
and it looks like at least fs/cifs/netmisc.c and net/sunrpc/addr.c
could use the helper.

What do you think of adding inet_pton_with_scope() to
net/core/utils.c?  I'm open to better ideas on the name.  But I can
code that up and use it in nvme, as well as convert over the two
places I mentioned above.  The first parameter of the function can be
an af, and the caller can pass in AF_UNSPEC, AF_INET, or AF_INET6 to
restrict the parsing to one type of address (or not).

 - R.

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

>> It'd still need all the scope ID handling similar to what Roland did,
>> and that's a fair chunk of code.  We have a few options to handle the
>> different allowed addresses:
>>
>>  (1) v4/v6 only flags
>>  (2) having low-level v4/v6 handlers and one that tries these both
>>  (3) using the try both handler and rejecting the wrong one after
>>      parsing.
>>
>> (3) seems easiest, but (2) sounds fine to me.  But I'd really like to
>> hear from folks on the netdev list what they think of that idea first.
>
> I think adding a new helper that parses both v4 and v6 addresses +
> scope ID seems like the best thing for now.  I did a grep for in6_pton
> and it looks like at least fs/cifs/netmisc.c and net/sunrpc/addr.c
> could use the helper.

The iscsi target code can use it too.

> What do you think of adding inet_pton_with_scope() to
> net/core/utils.c?  I'm open to better ideas on the name.  But I can
> code that up and use it in nvme, as well as convert over the two
> places I mentioned above.  The first parameter of the function can be
> an af, and the caller can pass in AF_UNSPEC, AF_INET, or AF_INET6 to
> restrict the parsing to one type of address (or not).

Sounds good Roland.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

>> It'd still need all the scope ID handling similar to what Roland did,
>> and that's a fair chunk of code.  We have a few options to handle the
>> different allowed addresses:
>>
>>  (1) v4/v6 only flags
>>  (2) having low-level v4/v6 handlers and one that tries these both
>>  (3) using the try both handler and rejecting the wrong one after
>>      parsing.
>>
>> (3) seems easiest, but (2) sounds fine to me.  But I'd really like to
>> hear from folks on the netdev list what they think of that idea first.
>
> I think adding a new helper that parses both v4 and v6 addresses +
> scope ID seems like the best thing for now.  I did a grep for in6_pton
> and it looks like at least fs/cifs/netmisc.c and net/sunrpc/addr.c
> could use the helper.

The iscsi target code can use it too.

> What do you think of adding inet_pton_with_scope() to
> net/core/utils.c?  I'm open to better ideas on the name.  But I can
> code that up and use it in nvme, as well as convert over the two
> places I mentioned above.  The first parameter of the function can be
> an af, and the caller can pass in AF_UNSPEC, AF_INET, or AF_INET6 to
> restrict the parsing to one type of address (or not).

Sounds good Roland.

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

> I think adding a new helper that parses both v4 and v6 addresses +
> scope ID seems like the best thing for now.  I did a grep for in6_pton
> and it looks like at least fs/cifs/netmisc.c and net/sunrpc/addr.c
> could use the helper.
>
> What do you think of adding inet_pton_with_scope() to
> net/core/utils.c?  I'm open to better ideas on the name.  But I can
> code that up and use it in nvme, as well as convert over the two
> places I mentioned above.  The first parameter of the function can be
> an af, and the caller can pass in AF_UNSPEC, AF_INET, or AF_INET6 to
> restrict the parsing to one type of address (or not).

Hey Roland,

Are you looking into this?

I'd love to get it into 4.9...
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

> I think adding a new helper that parses both v4 and v6 addresses +
> scope ID seems like the best thing for now.  I did a grep for in6_pton
> and it looks like at least fs/cifs/netmisc.c and net/sunrpc/addr.c
> could use the helper.
>
> What do you think of adding inet_pton_with_scope() to
> net/core/utils.c?  I'm open to better ideas on the name.  But I can
> code that up and use it in nvme, as well as convert over the two
> places I mentioned above.  The first parameter of the function can be
> an af, and the caller can pass in AF_UNSPEC, AF_INET, or AF_INET6 to
> restrict the parsing to one type of address (or not).

Hey Roland,

Are you looking into this?

I'd love to get it into 4.9...

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Roland Dreier]

On Thu, Aug 18, 2016 at 12:44 AM, Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org> wrote:
> Are you looking into this?
>
> I'd love to get it into 4.9...

Yeah, me too.  I got distracted about halfway through writing the
patch but I will finish it this week.

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Roland Dreier]

On Thu, Aug 18, 2016@12:44 AM, Sagi Grimberg <sagi@grimberg.me> wrote:
> Are you looking into this?
>
> I'd love to get it into 4.9...

Yeah, me too.  I got distracted about halfway through writing the
patch but I will finish it this week.

 - R.

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

>> Are you looking into this?
>>
>> I'd love to get it into 4.9...
>
> Yeah, me too.  I got distracted about halfway through writing the
> patch but I will finish it this week.

Awesome! thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

>> Are you looking into this?
>>
>> I'd love to get it into 4.9...
>
> Yeah, me too.  I got distracted about halfway through writing the
> patch but I will finish it this week.

Awesome! thanks.

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Christoph Hellwig]

On Mon, Aug 01, 2016 at 09:06:07AM -0700, Roland Dreier wrote:
> On Mon, Aug 1, 2016 at 8:50 AM, Christoph Hellwig <hch@lst.de> wrote:
> > It'd still need all the scope ID handling similar to what Roland did,
> > and that's a fair chunk of code.  We have a few options to handle the
> > different allowed addresses:
> >
> >  (1) v4/v6 only flags
> >  (2) having low-level v4/v6 handlers and one that tries these both
> >  (3) using the try both handler and rejecting the wrong one after
> >      parsing.
> >
> > (3) seems easiest, but (2) sounds fine to me.  But I'd really like to
> > hear from folks on the netdev list what they think of that idea first.
> 
> I think adding a new helper that parses both v4 and v6 addresses +
> scope ID seems like the best thing for now.  I did a grep for in6_pton
> and it looks like at least fs/cifs/netmisc.c and net/sunrpc/addr.c
> could use the helper.
> 
> What do you think of adding inet_pton_with_scope() to
> net/core/utils.c?  I'm open to better ideas on the name.  But I can
> code that up and use it in nvme, as well as convert over the two
> places I mentioned above.  The first parameter of the function can be
> an af, and the caller can pass in AF_UNSPEC, AF_INET, or AF_INET6 to
> restrict the parsing to one type of address (or not).

Sounds fine to me, and I hope the netdev folks (Cc'ed) are fine with
that as well.

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Christoph Hellwig]

On Mon, Aug 01, 2016@09:06:07AM -0700, Roland Dreier wrote:
> On Mon, Aug 1, 2016@8:50 AM, Christoph Hellwig <hch@lst.de> wrote:
> > It'd still need all the scope ID handling similar to what Roland did,
> > and that's a fair chunk of code.  We have a few options to handle the
> > different allowed addresses:
> >
> >  (1) v4/v6 only flags
> >  (2) having low-level v4/v6 handlers and one that tries these both
> >  (3) using the try both handler and rejecting the wrong one after
> >      parsing.
> >
> > (3) seems easiest, but (2) sounds fine to me.  But I'd really like to
> > hear from folks on the netdev list what they think of that idea first.
> 
> I think adding a new helper that parses both v4 and v6 addresses +
> scope ID seems like the best thing for now.  I did a grep for in6_pton
> and it looks like at least fs/cifs/netmisc.c and net/sunrpc/addr.c
> could use the helper.
> 
> What do you think of adding inet_pton_with_scope() to
> net/core/utils.c?  I'm open to better ideas on the name.  But I can
> code that up and use it in nvme, as well as convert over the two
> places I mentioned above.  The first parameter of the function can be
> an af, and the caller can pass in AF_UNSPEC, AF_INET, or AF_INET6 to
> restrict the parsing to one type of address (or not).

Sounds fine to me, and I hope the netdev folks (Cc'ed) are fine with
that as well.

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

>>>> Currently the target driver doesn't support ipv6 addresses.
>>>>
>>>> These complementary patches [1,2] makes the it work e2e! So,
>>>
>>> I think we should use the same ipv6 addr parsing that Roland used
>>> in the host, and preferably have another module export that as a helper.
>>
>> I can try to unify them, but unlike the host that first tries ipv4 and
>> then fall to ipv6, the target needs to enforce the address family so
>> it's somewhat different. I kinda think that these are different enough
>> to stay separate don't you think?
>
> It'd still need all the scope ID handling similar to what Roland did,
> and that's a fair chunk of code.  We have a few options to handle the
> different allowed addresses:
>
>  (1) v4/v6 only flags
>  (2) having low-level v4/v6 handlers and one that tries these both
>  (3) using the try both handler and rejecting the wrong one after
>      parsing.
>
> (3) seems easiest, but (2) sounds fine to me.  But I'd really like to
> hear from folks on the netdev list what they think of that idea first.

(2) sounds fine to me too.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Sagi Grimberg]

>>>> Currently the target driver doesn't support ipv6 addresses.
>>>>
>>>> These complementary patches [1,2] makes the it work e2e! So,
>>>
>>> I think we should use the same ipv6 addr parsing that Roland used
>>> in the host, and preferably have another module export that as a helper.
>>
>> I can try to unify them, but unlike the host that first tries ipv4 and
>> then fall to ipv6, the target needs to enforce the address family so
>> it's somewhat different. I kinda think that these are different enough
>> to stay separate don't you think?
>
> It'd still need all the scope ID handling similar to what Roland did,
> and that's a fair chunk of code.  We have a few options to handle the
> different allowed addresses:
>
>  (1) v4/v6 only flags
>  (2) having low-level v4/v6 handlers and one that tries these both
>  (3) using the try both handler and rejecting the wrong one after
>      parsing.
>
> (3) seems easiest, but (2) sounds fine to me.  But I'd really like to
> hear from folks on the netdev list what they think of that idea first.

(2) sounds fine to me too.

Re: [PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Christoph Hellwig]

Hi Roland,

On Sun, Jul 31, 2016 at 12:27:40AM -0700, Roland Dreier wrote:
> From: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
> 
> If a target address does not parse as IPv4, try parsing it as IPv6
> (including handling '%<scope-id>' suffixes for link-local addresses).

The code below looks fine to me, but is there any chance to add it to
net/ as a generic helper?  It's a little sad this sort of code would
have to live in each driver trying to parse IP addresses.

> Signed-off-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
> ---
>  drivers/nvme/host/rdma.c | 55 ++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 44 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index b96b88369871..dd4aa54cd709 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -138,6 +138,7 @@ struct nvme_rdma_ctrl {
>  	union {
>  		struct sockaddr addr;
>  		struct sockaddr_in addr_in;
> +		struct sockaddr_in6 addr_in6;
>  	};
>  
>  	struct nvme_ctrl	ctrl;
> @@ -1847,19 +1848,51 @@ out_free_io_queues:
>  	return ret;
>  }
>  
> -static int nvme_rdma_parse_ipaddr(struct sockaddr_in *in_addr, char *p)
> +static int nvme_rdma_parse_ipaddr(struct sockaddr *addr, char *p)
>  {
> -	u8 *addr = (u8 *)&in_addr->sin_addr.s_addr;
>  	size_t buflen = strlen(p);
>  
> -	/* XXX: handle IPv6 addresses */
> +	if (buflen <= INET_ADDRSTRLEN) {
> +		struct sockaddr_in *addr4 = (struct sockaddr_in *) addr;
> +		if (in4_pton(p, buflen, (u8 *) &addr4->sin_addr.s_addr,
> +			     '\0', NULL) > 0) {
> +			addr4->sin_family = AF_INET;
> +			return 0;
> +		}
> +	}
>  
> -	if (buflen > INET_ADDRSTRLEN)
> -		return -EINVAL;
> -	if (in4_pton(p, buflen, addr, '\0', NULL) == 0)
> -		return -EINVAL;
> -	in_addr->sin_family = AF_INET;
> -	return 0;
> +	if (buflen <= INET6_ADDRSTRLEN) {
> +		struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *) addr;
> +		const char *scope_delim;
> +
> +		if (in6_pton(p, buflen, (u8 *) &addr6->sin6_addr.s6_addr,
> +			     '%', &scope_delim) == 0)
> +			return -EINVAL;
> +		addr6->sin6_family = AF_INET6;
> +
> +		if (ipv6_addr_type(&addr6->sin6_addr) & IPV6_ADDR_LINKLOCAL &&
> +		    p + buflen != scope_delim && *scope_delim == '%') {
> +			char scope_id[16];
> +			size_t scope_len = min_t(size_t, sizeof scope_id,
> +						 p + buflen - scope_delim - 1);
> +			struct net_device *dev;
> +
> +			memcpy(scope_id, scope_delim + 1, scope_len);
> +			scope_id[scope_len] = '\0';
> +
> +			/* XXX: what network namespace should we use? */
> +			dev = dev_get_by_name(&init_net, scope_id);
> +			if (dev) {
> +				addr6->sin6_scope_id = dev->ifindex;
> +				dev_put(dev);
> +			} else if (kstrtouint(scope_id, 0, &addr6->sin6_scope_id))
> +				return -EINVAL;
> +		}
> +
> +		return 0;
> +	}
> +
> +	return -EINVAL;
>  }
>  
>  static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
> @@ -1875,7 +1908,7 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
>  	ctrl->ctrl.opts = opts;
>  	INIT_LIST_HEAD(&ctrl->list);
>  
> -	ret = nvme_rdma_parse_ipaddr(&ctrl->addr_in, opts->traddr);
> +	ret = nvme_rdma_parse_ipaddr(&ctrl->addr, opts->traddr);
>  	if (ret) {
>  		pr_err("malformed IP address passed: %s\n", opts->traddr);
>  		goto out_free_ctrl;
> @@ -1949,7 +1982,7 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
>  	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE);
>  	WARN_ON_ONCE(!changed);
>  
> -	dev_info(ctrl->ctrl.device, "new ctrl: NQN \"%s\", addr %pISp\n",
> +	dev_info(ctrl->ctrl.device, "new ctrl: NQN \"%s\", addr %pISpcs\n",
>  		ctrl->ctrl.opts->subsysnqn, &ctrl->addr);
>  
>  	kref_get(&ctrl->ctrl.kref);
> -- 
> 2.7.4
---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] nvme-rdma: Add handling for connecting to IPv6 targets

[Christoph Hellwig]

Hi Roland,

On Sun, Jul 31, 2016@12:27:40AM -0700, Roland Dreier wrote:
> From: Roland Dreier <roland at purestorage.com>
> 
> If a target address does not parse as IPv4, try parsing it as IPv6
> (including handling '%<scope-id>' suffixes for link-local addresses).

The code below looks fine to me, but is there any chance to add it to
net/ as a generic helper?  It's a little sad this sort of code would
have to live in each driver trying to parse IP addresses.

> Signed-off-by: Roland Dreier <roland at purestorage.com>
> ---
>  drivers/nvme/host/rdma.c | 55 ++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 44 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index b96b88369871..dd4aa54cd709 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -138,6 +138,7 @@ struct nvme_rdma_ctrl {
>  	union {
>  		struct sockaddr addr;
>  		struct sockaddr_in addr_in;
> +		struct sockaddr_in6 addr_in6;
>  	};
>  
>  	struct nvme_ctrl	ctrl;
> @@ -1847,19 +1848,51 @@ out_free_io_queues:
>  	return ret;
>  }
>  
> -static int nvme_rdma_parse_ipaddr(struct sockaddr_in *in_addr, char *p)
> +static int nvme_rdma_parse_ipaddr(struct sockaddr *addr, char *p)
>  {
> -	u8 *addr = (u8 *)&in_addr->sin_addr.s_addr;
>  	size_t buflen = strlen(p);
>  
> -	/* XXX: handle IPv6 addresses */
> +	if (buflen <= INET_ADDRSTRLEN) {
> +		struct sockaddr_in *addr4 = (struct sockaddr_in *) addr;
> +		if (in4_pton(p, buflen, (u8 *) &addr4->sin_addr.s_addr,
> +			     '\0', NULL) > 0) {
> +			addr4->sin_family = AF_INET;
> +			return 0;
> +		}
> +	}
>  
> -	if (buflen > INET_ADDRSTRLEN)
> -		return -EINVAL;
> -	if (in4_pton(p, buflen, addr, '\0', NULL) == 0)
> -		return -EINVAL;
> -	in_addr->sin_family = AF_INET;
> -	return 0;
> +	if (buflen <= INET6_ADDRSTRLEN) {
> +		struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *) addr;
> +		const char *scope_delim;
> +
> +		if (in6_pton(p, buflen, (u8 *) &addr6->sin6_addr.s6_addr,
> +			     '%', &scope_delim) == 0)
> +			return -EINVAL;
> +		addr6->sin6_family = AF_INET6;
> +
> +		if (ipv6_addr_type(&addr6->sin6_addr) & IPV6_ADDR_LINKLOCAL &&
> +		    p + buflen != scope_delim && *scope_delim == '%') {
> +			char scope_id[16];
> +			size_t scope_len = min_t(size_t, sizeof scope_id,
> +						 p + buflen - scope_delim - 1);
> +			struct net_device *dev;
> +
> +			memcpy(scope_id, scope_delim + 1, scope_len);
> +			scope_id[scope_len] = '\0';
> +
> +			/* XXX: what network namespace should we use? */
> +			dev = dev_get_by_name(&init_net, scope_id);
> +			if (dev) {
> +				addr6->sin6_scope_id = dev->ifindex;
> +				dev_put(dev);
> +			} else if (kstrtouint(scope_id, 0, &addr6->sin6_scope_id))
> +				return -EINVAL;
> +		}
> +
> +		return 0;
> +	}
> +
> +	return -EINVAL;
>  }
>  
>  static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
> @@ -1875,7 +1908,7 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
>  	ctrl->ctrl.opts = opts;
>  	INIT_LIST_HEAD(&ctrl->list);
>  
> -	ret = nvme_rdma_parse_ipaddr(&ctrl->addr_in, opts->traddr);
> +	ret = nvme_rdma_parse_ipaddr(&ctrl->addr, opts->traddr);
>  	if (ret) {
>  		pr_err("malformed IP address passed: %s\n", opts->traddr);
>  		goto out_free_ctrl;
> @@ -1949,7 +1982,7 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
>  	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE);
>  	WARN_ON_ONCE(!changed);
>  
> -	dev_info(ctrl->ctrl.device, "new ctrl: NQN \"%s\", addr %pISp\n",
> +	dev_info(ctrl->ctrl.device, "new ctrl: NQN \"%s\", addr %pISpcs\n",
>  		ctrl->ctrl.opts->subsysnqn, &ctrl->addr);
>  
>  	kref_get(&ctrl->ctrl.kref);
> -- 
> 2.7.4
---end quoted text---

Re: [PATCH 1/2] nvme-rdma: Don't leak uninitialized memory in connect request private data

[Sagi Grimberg]

Looks fine,

Acked-by: Sagi Grimberg <sai-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] nvme-rdma: Don't leak uninitialized memory in connect request private data

[Sagi Grimberg]

Looks fine,

Acked-by: Sagi Grimberg <sai at grimberg.me>

Re: [PATCH 1/2] nvme-rdma: Don't leak uninitialized memory in connect request private data

[Christoph Hellwig]

On Sun, Jul 31, 2016 at 12:27:39AM -0700, Roland Dreier wrote:
> From: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
> 
> Zero out the full nvme_rdma_cm_req structure before sending it.
> Otherwise we end up leaking kernel memory in the reserved field, which
> might break forward compatibility in the future.
> 
> Signed-off-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>

Looks fine,

Reviewed-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] nvme-rdma: Don't leak uninitialized memory in connect request private data

[Christoph Hellwig]

On Sun, Jul 31, 2016@12:27:39AM -0700, Roland Dreier wrote:
> From: Roland Dreier <roland at purestorage.com>
> 
> Zero out the full nvme_rdma_cm_req structure before sending it.
> Otherwise we end up leaking kernel memory in the reserved field, which
> might break forward compatibility in the future.
> 
> Signed-off-by: Roland Dreier <roland at purestorage.com>

Looks fine,

Reviewed-by: Christoph Hellwig <hch at lst.de>