[PATCH rdma-next 0/4] IB/core: Add InfiniBand router support

[PATCH rdma-next 0/4] IB/core: Add InfiniBand router support

[Leon Romanovsky]

InfiniBand has gone a long way in providing efficient large-scale
high performance connectivity. IB subnets have shown to scale to
tens of thousands of nodes, both in raw capacity and in management.

As demand for computing capacity increases, future clusters sizes might
exceed the number of addressable endpoints in a single IB subnet (around
40K nodes).

To accommodate such clusters, a routing layer with the same latencies
and bandwidth characteristics as switches is required. In addition, as
data center deployments evolve, it becomes beneficial to consolidate
resources across multiple clusters.

There are multiple applications for this technology such as computing
clusters which require access to a common storage infrastructure.
Routers enable such connectivity while reducing management complexity
and isolating intra-subnet faults.

In this patch set the forwarding between the IB subnets is performed
by including a GRH header.
The IB router’s basic functionality includes:
 * Removal of current L2 LRH (local routing header)
 * Routing table lookup – using GID from GRH
 * Building new LRH according to the destination based on the routing table

In order to retrieve the destination GID, new resolution method was needed.
There is an assumption that rdmacm is used only between nodes in the same
IB subnet, this is why ARP resolution can be used to turn IP to GID in rdmacm.

When dealing with IB communication between subnets this assumption
is no longer valid. ARP resolution will get us the next hop device
address and not the peer node's device address.

To solve this issue, we will check user space if it can provide the
GID of the peer node, and fail if not. For doing so, we added a new
RDMA local service operation (IP to GID resolution) and a sequence number
to identify each request. The client request would include the ifindex
of the outgoing interface and attributes which indicate the destination IP.
The local service would answer with the requested destination GID.

Available in the "topic/ib-router" topic branch of this git repo:

git://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git

Or for browsing:

https://git.kernel.org/cgit/linux/kernel/git/leon/linux-rdma.git/log/?h=topic/ib-router

It is tested after applying topic/fix-core and topic/ipoib-device-address.

Thanks

Mark Bloch (4):
  IB/netlink: Make ib_netlink a standalone module
  IB/netlink: Allow multiple clients to register under the same family
  IB/netlink: Add new local service operation
  IB/core: Add IP to GID netlink offload

 drivers/infiniband/core/Makefile   |   6 +-
 drivers/infiniband/core/addr.c     | 225 +++++++++++++++++++++++++++++++++----
 drivers/infiniband/core/cma.c      |   3 +-
 drivers/infiniband/core/device.c   |   9 --
 drivers/infiniband/core/iwcm.c     |   3 +-
 drivers/infiniband/core/netlink.c  |  46 ++++++--
 drivers/infiniband/core/sa_query.c |   2 +-
 include/rdma/rdma_netlink.h        |   7 +-
 include/uapi/rdma/rdma_netlink.h   |  10 ++
 9 files changed, 259 insertions(+), 52 deletions(-)

-- 
2.1.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 rdma-next 1/4] IB/netlink: Make ib_netlink a standalone module

[Leon Romanovsky]

From: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

ib_netlink is initialized by ib_core, which means that any module
ib_core depends on (ib_addr for example) can't use ib_netlink.

Let's turn ib_netlink to be standalone module, so it could be
used by any IB component.

Signed-off-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/Makefile  | 6 ++++--
 drivers/infiniband/core/device.c  | 9 ---------
 drivers/infiniband/core/netlink.c | 9 ++++++++-
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/core/Makefile b/drivers/infiniband/core/Makefile
index f818538..73a044b 100644
--- a/drivers/infiniband/core/Makefile
+++ b/drivers/infiniband/core/Makefile
@@ -3,13 +3,13 @@ user_access-$(CONFIG_INFINIBAND_ADDR_TRANS)	:= rdma_ucm.o
 
 obj-$(CONFIG_INFINIBAND) +=		ib_core.o ib_mad.o ib_sa.o \
 					ib_cm.o iw_cm.o ib_addr.o \
-					$(infiniband-y)
+					ib_netlink.o $(infiniband-y)
 obj-$(CONFIG_INFINIBAND_USER_MAD) +=	ib_umad.o
 obj-$(CONFIG_INFINIBAND_USER_ACCESS) +=	ib_uverbs.o ib_ucm.o \
 					$(user_access-y)
 
 ib_core-y :=			packer.o ud_header.o verbs.o cq.o sysfs.o \
-				device.o fmr_pool.o cache.o netlink.o \
+				device.o fmr_pool.o cache.o \
 				roce_gid_mgmt.o
 ib_core-$(CONFIG_INFINIBAND_USER_MEM) += umem.o
 ib_core-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += umem_odp.o umem_rbtree.o
@@ -18,6 +18,8 @@ ib_mad-y :=			mad.o smi.o agent.o mad_rmpp.o
 
 ib_sa-y :=			sa_query.o multicast.o
 
+ib_netlink-y :=			netlink.o
+
 ib_cm-y :=			cm.o
 
 iw_cm-y :=			iwcm.o iwpm_util.o iwpm_msg.o
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 1097984..e482164 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -977,18 +977,10 @@ static int __init ib_core_init(void)
 		goto err_comp;
 	}
 
-	ret = ibnl_init();
-	if (ret) {
-		pr_warn("Couldn't init IB netlink interface\n");
-		goto err_sysfs;
-	}
-
 	ib_cache_setup();
 
 	return 0;
 
-err_sysfs:
-	class_unregister(&ib_class);
 err_comp:
 	destroy_workqueue(ib_comp_wq);
 err:
@@ -999,7 +991,6 @@ err:
 static void __exit ib_core_cleanup(void)
 {
 	ib_cache_cleanup();
-	ibnl_cleanup();
 	class_unregister(&ib_class);
 	destroy_workqueue(ib_comp_wq);
 	/* Make sure that any pending umem accounting work is done. */
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index aa491f7..ff2dcbb 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -38,6 +38,10 @@
 #include <net/sock.h>
 #include <rdma/rdma_netlink.h>
 
+MODULE_AUTHOR("Roland Dreier");
+MODULE_DESCRIPTION("RDMA netlink infrastructure");
+MODULE_LICENSE("Dual BSD/GPL");
+
 struct ibnl_client {
 	struct list_head		list;
 	int				index;
@@ -255,7 +259,7 @@ int __init ibnl_init(void)
 	return 0;
 }
 
-void ibnl_cleanup(void)
+void __exit ibnl_cleanup(void)
 {
 	struct ibnl_client *cur, *next;
 
@@ -268,3 +272,6 @@ void ibnl_cleanup(void)
 
 	netlink_kernel_release(nls);
 }
+
+module_init(ibnl_init);
+module_exit(ibnl_cleanup);
-- 
2.1.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

Re: [PATCH rdma-next 1/4] IB/netlink: Make ib_netlink a standalone module

[Jason Gunthorpe]

On Wed, May 04, 2016 at 06:41:55PM +0300, Leon Romanovsky wrote:
> From: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> ib_netlink is initialized by ib_core, which means that any module
> ib_core depends on (ib_addr for example) can't use ib_netlink.

What is the downside of merging ib_addr into ib_core?

It seems like we have too many modules already.

Jason
--
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

Re: [PATCH rdma-next 1/4] IB/netlink: Make ib_netlink a standalone module

[Ira Weiny]

On Wed, May 04, 2016 at 12:46:52PM -0600, Jason Gunthorpe wrote:
> On Wed, May 04, 2016 at 06:41:55PM +0300, Leon Romanovsky wrote:
> > From: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > 
> > ib_netlink is initialized by ib_core, which means that any module
> > ib_core depends on (ib_addr for example) can't use ib_netlink.
> 
> What is the downside of merging ib_addr into ib_core?
> 
> It seems like we have too many modules already.

Agreed.

> 
> Jason
> --
> 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
--
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

Re: [PATCH rdma-next 1/4] IB/netlink: Make ib_netlink a standalone module

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

On Wed, May 04, 2016 at 04:53:25PM -0400, Ira Weiny wrote:
> On Wed, May 04, 2016 at 12:46:52PM -0600, Jason Gunthorpe wrote:
> > On Wed, May 04, 2016 at 06:41:55PM +0300, Leon Romanovsky wrote:
> > > From: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > 
> > > ib_netlink is initialized by ib_core, which means that any module
> > > ib_core depends on (ib_addr for example) can't use ib_netlink.
> > 
> > What is the downside of merging ib_addr into ib_core?
> > 
> > It seems like we have too many modules already.
> 
> Agreed.

Thanks,
I'll move v1 of this patch to my "trivial fixes" series.

> 
> > 
> > Jason
> > --
> > 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
> --
> 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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH rdma-next 2/4] IB/netlink: Allow multiple clients to register under the same family

[Leon Romanovsky]

From: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

This commit adds the ability for multiple clients to register to the same
family. We will allow this only if there isn't an overlap between them.
If there is an overlap, we will fail the latest client registration.

Signed-off-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cma.c      |  3 ++-
 drivers/infiniband/core/iwcm.c     |  3 ++-
 drivers/infiniband/core/netlink.c  | 37 ++++++++++++++++++++++++++-----------
 drivers/infiniband/core/sa_query.c |  2 +-
 include/rdma/rdma_netlink.h        |  7 +++++--
 5 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 09a0243..8d93ab1 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -4312,7 +4312,8 @@ err_wq:
 static void __exit cma_cleanup(void)
 {
 	cma_configfs_exit();
-	ibnl_remove_client(RDMA_NL_RDMA_CM);
+	ibnl_remove_client(RDMA_NL_RDMA_CM,
+			   RDMA_NL_RDMA_CM_NUM_OPS, cma_cb_table);
 	ib_unregister_client(&cma_client);
 	unregister_netdevice_notifier(&cma_nb);
 	rdma_addr_unregister_client(&addr_client);
diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c
index 5011ecf..0efa1e7 100644
--- a/drivers/infiniband/core/iwcm.c
+++ b/drivers/infiniband/core/iwcm.c
@@ -1199,7 +1199,8 @@ static void __exit iw_cm_cleanup(void)
 {
 	unregister_net_sysctl_table(iwcm_ctl_table_hdr);
 	destroy_workqueue(iwcm_wq);
-	ibnl_remove_client(RDMA_NL_IWCM);
+	ibnl_remove_client(RDMA_NL_IWCM, RDMA_NL_IWPM_NUM_OPS,
+			   iwcm_nl_cb_table);
 	iwpm_exit(RDMA_NL_IWCM);
 }
 
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index ff2dcbb..06e9de0 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -66,6 +66,7 @@ int ibnl_add_client(int index, int nops,
 {
 	struct ibnl_client *cur;
 	struct ibnl_client *nl_client;
+	int i;
 
 	nl_client = kmalloc(sizeof *nl_client, GFP_KERNEL);
 	if (!nl_client)
@@ -79,10 +80,15 @@ int ibnl_add_client(int index, int nops,
 
 	list_for_each_entry(cur, &client_list, list) {
 		if (cur->index == index) {
-			pr_warn("Client for %d already exists\n", index);
-			mutex_unlock(&ibnl_mutex);
-			kfree(nl_client);
-			return -EINVAL;
+			for (i = 0; i < min(nops, cur->nops); i++) {
+				if (cur->cb_table[i].dump &&
+				    cb_table[i].dump) {
+					pr_warn("Client for %d already exists\n", index);
+					mutex_unlock(&ibnl_mutex);
+					kfree(nl_client);
+					return -EINVAL;
+				}
+			}
 		}
 	}
 
@@ -94,17 +100,24 @@ int ibnl_add_client(int index, int nops,
 }
 EXPORT_SYMBOL(ibnl_add_client);
 
-int ibnl_remove_client(int index)
+int ibnl_remove_client(int index, int nops,
+		       const struct ibnl_client_cbs cb_table[])
 {
 	struct ibnl_client *cur, *next;
+	int i;
 
 	mutex_lock(&ibnl_mutex);
 	list_for_each_entry_safe(cur, next, &client_list, list) {
-		if (cur->index == index) {
-			list_del(&(cur->list));
-			mutex_unlock(&ibnl_mutex);
-			kfree(cur);
-			return 0;
+		if (cur->index == index && cur->nops == nops) {
+			for (i = 0; i < nops; i++) {
+				if (cb_table[i].dump &&
+				    cur->cb_table[i].dump) {
+					list_del(&cur->list);
+					mutex_unlock(&ibnl_mutex);
+					kfree(cur);
+					return 0;
+				}
+			}
 		}
 	}
 	pr_warn("Can't remove callback for client idx %d. Not found\n", index);
@@ -159,9 +172,11 @@ static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 
 	list_for_each_entry(client, &client_list, list) {
 		if (client->index == index) {
-			if (op >= client->nops || !client->cb_table[op].dump)
+			if (op >= client->nops)
 				return -EINVAL;
 
+			if (!client->cb_table[op].dump)
+				continue;
 			/*
 			 * For response or local service set_timeout request,
 			 * there is no need to use netlink_dump_start.
diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index 5050c61..62dc8dd 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -1841,7 +1841,7 @@ err1:
 
 static void __exit ib_sa_cleanup(void)
 {
-	ibnl_remove_client(RDMA_NL_LS);
+	ibnl_remove_client(RDMA_NL_LS, RDMA_NL_LS_NUM_OPS, ib_sa_cb_table);
 	cancel_delayed_work(&ib_nl_timed_work);
 	flush_workqueue(ib_nl_wq);
 	destroy_workqueue(ib_nl_wq);
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index 5852661..c8aef66 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -16,7 +16,7 @@ void ibnl_cleanup(void);
 /**
  * Add a a client to the list of IB netlink exporters.
  * @index: Index of the added client
- * @nops: Number of supported ops by the added client.
+ * @nops: Number of max supported ops by the added client.
  * @cb_table: A table for op->callback
  *
  * Returns 0 on success or a negative error code.
@@ -27,10 +27,13 @@ int ibnl_add_client(int index, int nops,
 /**
  * Remove a client from IB netlink.
  * @index: Index of the removed IB client.
+ * @nops: Number of max supported ops by the added client.
+ * @cb_table: A table for op->callback
  *
  * Returns 0 on success or a negative error code.
  */
-int ibnl_remove_client(int index);
+int ibnl_remove_client(int index, int nops,
+		       const struct ibnl_client_cbs cb_table[]);
 
 /**
  * Put a new message in a supplied skb.
-- 
2.1.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

Re: [PATCH rdma-next 2/4] IB/netlink: Allow multiple clients to register under the same family

[Dennis Dalessandro]

On Wed, May 04, 2016 at 06:41:56PM +0300, Leon Romanovsky wrote:
>From: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
>This commit adds the ability for multiple clients to register to the same
>family. We will allow this only if there isn't an overlap between them.
>If there is an overlap, we will fail the latest client registration.

Can you expand on why we need to do this and what you mean by "overlap"?

Maybe I'm not clear on the terminology. I thought the "family" was something 
like NETLINK_RDMA, which we already allow multiple "clients" to register with.

These things:

enum {
        RDMA_NL_RDMA_CM = 1,
        RDMA_NL_IWCM,
        RDMA_NL_RSVD,
        RDMA_NL_LS,     /* RDMA Local Services */
        RDMA_NL_I40IW,
        RDMA_NL_NUM_CLIENTS
};

It seems what you are trying to do is allow the same "client" to register 
multiple times, but with differing numbers of ops?

Is there a reason to not just have another client index?

-Denny
--
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 rdma-next 3/4] IB/netlink: Add new local service operation

[Leon Romanovsky]

From: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

This commits adds new RDMA local service operation:
- IP to GID resolution.

The client request will include the ifindex of the
outgoing interface and will place in an attribute
(LS_NLA_TYPE_IPV4 or LS_NLA_TYPE_IPV6) the destination IP.

The local service will answer with a message that
has the following attribute:
- LS_NLA_TYPE_DGID - The destination GID.

Signed-off-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/uapi/rdma/rdma_netlink.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index 6e373d1..17bbea1 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -135,10 +135,12 @@ enum {
  * Local service operations:
  *   RESOLVE - The client requests the local service to resolve a path.
  *   SET_TIMEOUT - The local service requests the client to set the timeout.
+ *   IP_RESOLVE - The client requests the local servie to resolve an ip to gid.
  */
 enum {
 	RDMA_NL_LS_OP_RESOLVE = 0,
 	RDMA_NL_LS_OP_SET_TIMEOUT,
+	RDMA_NL_LS_OP_IP_RESOLVE,
 	RDMA_NL_LS_NUM_OPS
 };
 
@@ -176,6 +178,10 @@ struct rdma_ls_resolve_header {
 	__u8 path_use;
 };
 
+struct rdma_ls_ip_resolve_header {
+	__u32 ifindex;
+};
+
 /* Local service attribute type */
 #define RDMA_NLA_F_MANDATORY	(1 << 13)
 #define RDMA_NLA_TYPE_MASK	(~(NLA_F_NESTED | NLA_F_NET_BYTEORDER | \
@@ -193,6 +199,8 @@ struct rdma_ls_resolve_header {
  *   TCLASS          u8
  *   PKEY            u16                        cpu
  *   QOS_CLASS       u16                        cpu
+ *   IPV4            u32                        BE
+ *   IPV6            u8[16]                     BE
  */
 enum {
 	LS_NLA_TYPE_UNSPEC = 0,
@@ -204,6 +212,8 @@ enum {
 	LS_NLA_TYPE_TCLASS,
 	LS_NLA_TYPE_PKEY,
 	LS_NLA_TYPE_QOS_CLASS,
+	LS_NLA_TYPE_IPV4,
+	LS_NLA_TYPE_IPV6,
 	LS_NLA_TYPE_MAX
 };
 
-- 
2.1.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 rdma-next 4/4] IB/core: Add IP to GID netlink offload

[Leon Romanovsky]

From: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

There is an assumption that rdmacm is used only between nodes
in the same IB subnet, this why ARP resolution can be used to turn
IP to GID in rdmacm.

When dealing with IB communication between subnets this assumption
is no longer valid. ARP resolution will get us the next hop device
address and not the peer node's device address.

To overcome this limitation, let's check user space if it can
provide the GID of the peer node, and fail if not.

We add a sequence number to identify each request and fill in the GID
upon answer from user space.

Signed-off-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/addr.c | 225 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 201 insertions(+), 24 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 337353d..e1a3ef6 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -46,6 +46,8 @@
 #include <net/ip6_route.h>
 #include <rdma/ib_addr.h>
 #include <rdma/ib.h>
+#include <rdma/rdma_netlink.h>
+#include <net/netlink.h>
 
 MODULE_AUTHOR("Sean Hefty");
 MODULE_DESCRIPTION("IB Address Translation");
@@ -62,8 +64,11 @@ struct addr_req {
 			 struct rdma_dev_addr *addr, void *context);
 	unsigned long timeout;
 	int status;
+	u32 seq;
 };
 
+static atomic_t ib_nl_addr_request_seq;
+
 static void process_req(struct work_struct *work);
 
 static DEFINE_MUTEX(lock);
@@ -71,6 +76,130 @@ static LIST_HEAD(req_list);
 static DECLARE_DELAYED_WORK(work, process_req);
 static struct workqueue_struct *addr_wq;
 
+static const struct nla_policy ib_nl_addr_policy[LS_NLA_TYPE_MAX] = {
+	[LS_NLA_TYPE_DGID]		= {.type = NLA_BINARY,
+		.len = sizeof(struct rdma_nla_ls_gid)},
+};
+
+static inline int ib_nl_is_good_ip_resp(const struct nlmsghdr *nlh)
+{
+	struct nlattr *tb[LS_NLA_TYPE_MAX] = {};
+	int ret;
+
+	if (nlh->nlmsg_flags & RDMA_NL_LS_F_ERR)
+		return 0;
+
+	ret = nla_parse(tb, LS_NLA_TYPE_MAX - 1, nlmsg_data(nlh),
+			nlmsg_len(nlh), ib_nl_addr_policy);
+	if (ret)
+		return 0;
+
+	return 1;
+}
+
+static void ib_nl_process_good_ip_rsep(const struct nlmsghdr *nlh)
+{
+	const struct nlattr *head, *curr;
+	union ib_gid gid;
+	struct addr_req *req;
+	int len, rem;
+	int found = 0;
+
+	head = (const struct nlattr *)nlmsg_data(nlh);
+	len = nlmsg_len(nlh);
+
+	nla_for_each_attr(curr, head, len, rem) {
+		if (curr->nla_type == LS_NLA_TYPE_DGID)
+			memcpy(&gid, nla_data(curr), nla_len(curr));
+	}
+
+	mutex_lock(&lock);
+	list_for_each_entry(req, &req_list, list) {
+		if (nlh->nlmsg_seq != req->seq)
+			continue;
+		/* We set the DGID part, the rest was set earlier */
+		rdma_addr_set_dgid(req->addr, &gid);
+		req->status = 0;
+		found = 1;
+		break;
+	}
+	mutex_unlock(&lock);
+
+	if (!found)
+		pr_info("Couldn't find request waiting for DGID: %pI6\n",
+			&gid);
+}
+
+static int ib_nl_handle_ip_res_resp(struct sk_buff *skb,
+				    struct netlink_callback *cb)
+{
+	const struct nlmsghdr *nlh = (struct nlmsghdr *)cb->nlh;
+
+	if ((nlh->nlmsg_flags & NLM_F_REQUEST) ||
+	    !(NETLINK_CB(skb).sk) ||
+	    !netlink_capable(skb, CAP_NET_ADMIN))
+		return -EPERM;
+
+	if (ib_nl_is_good_ip_resp(nlh))
+		ib_nl_process_good_ip_rsep(nlh);
+
+	return skb->len;
+}
+
+static struct ibnl_client_cbs ib_addr_cb_table[RDMA_NL_LS_NUM_OPS] = {
+	[RDMA_NL_LS_OP_IP_RESOLVE] = {
+		.dump = ib_nl_handle_ip_res_resp,
+		.module = THIS_MODULE },
+};
+
+static int ib_nl_ip_send_msg(struct rdma_dev_addr *dev_addr,
+			     const void *daddr,
+			     u32 seq, u16 family)
+{
+	struct sk_buff *skb = NULL;
+	struct nlmsghdr *nlh;
+	struct rdma_ls_ip_resolve_header *header;
+	void *data;
+	size_t size;
+	int attrtype;
+	int len;
+
+	if (family == AF_INET) {
+		size = sizeof(struct in_addr);
+		attrtype = RDMA_NLA_F_MANDATORY | LS_NLA_TYPE_IPV4;
+	} else {
+		size = sizeof(struct in6_addr);
+		attrtype = RDMA_NLA_F_MANDATORY | LS_NLA_TYPE_IPV6;
+	}
+
+	len = nla_total_size(sizeof(size));
+	len += NLMSG_ALIGN(sizeof(*header));
+
+	skb = nlmsg_new(len, GFP_KERNEL);
+
+	data = ibnl_put_msg(skb, &nlh, seq, 0, RDMA_NL_LS,
+			    RDMA_NL_LS_OP_IP_RESOLVE, NLM_F_REQUEST);
+	if (!data) {
+		nlmsg_free(skb);
+		return -ENODATA;
+	}
+
+	/* Construct the family header first */
+	header = (struct rdma_ls_ip_resolve_header *)
+		skb_put(skb, NLMSG_ALIGN(sizeof(*header)));
+	header->ifindex = dev_addr->bound_dev_if;
+	nla_put(skb, attrtype, size, daddr);
+
+	/* Repair the nlmsg header length */
+	nlmsg_end(skb, nlh);
+	ibnl_multicast(skb, nlh, RDMA_NL_GROUP_LS, GFP_KERNEL);
+
+	/* Make the request retry, so when we get the response from userspace
+	 * we will have something.
+	 */
+	return -ENODATA;
+}
+
 int rdma_addr_size(struct sockaddr *addr)
 {
 	switch (addr->sa_family) {
@@ -199,6 +328,17 @@ static void queue_req(struct addr_req *req)
 	mutex_unlock(&lock);
 }
 
+static int ib_nl_fetch_ha(struct dst_entry *dst, struct rdma_dev_addr *dev_addr,
+			  const void *daddr, u32 seq, u16 family)
+{
+	if (ibnl_chk_listeners(RDMA_NL_GROUP_LS))
+		return -EADDRNOTAVAIL;
+
+	/* We fill in what we can, the response will fill the rest */
+	rdma_copy_addr(dev_addr, dst->dev, NULL);
+	return ib_nl_ip_send_msg(dev_addr, daddr, seq, family);
+}
+
 static int dst_fetch_ha(struct dst_entry *dst, struct rdma_dev_addr *dev_addr,
 			const void *daddr)
 {
@@ -223,6 +363,39 @@ static int dst_fetch_ha(struct dst_entry *dst, struct rdma_dev_addr *dev_addr,
 	return ret;
 }
 
+static bool has_gateway(struct dst_entry *dst, sa_family_t family)
+{
+	struct rtable *rt;
+	struct rt6_info *rt6;
+
+	if (family == AF_INET) {
+		rt = container_of(dst, struct rtable, dst);
+		return rt->rt_uses_gateway;
+	}
+
+	rt6 = container_of(dst, struct rt6_info, dst);
+	return rt6->rt6i_flags & RTF_GATEWAY;
+}
+
+static int fetch_ha(struct dst_entry *dst, struct rdma_dev_addr *dev_addr,
+		    const struct sockaddr *dst_in, u32 seq)
+{
+	const struct sockaddr_in *dst_in4 =
+		(const struct sockaddr_in *)dst_in;
+	const struct sockaddr_in6 *dst_in6 =
+		(const struct sockaddr_in6 *)dst_in;
+	const void *daddr = (dst_in->sa_family == AF_INET) ?
+		(const void *)&dst_in4->sin_addr.s_addr :
+		(const void *)&dst_in6->sin6_addr;
+	sa_family_t family = dst_in->sa_family;
+
+	/* Gateway + ARPHRD_INFINIBAND -> IB router */
+	if (has_gateway(dst, family) && dst->dev->type == ARPHRD_INFINIBAND)
+		return ib_nl_fetch_ha(dst, dev_addr, daddr, seq, family);
+	else
+		return dst_fetch_ha(dst, dev_addr, daddr);
+}
+
 static int addr4_resolve(struct sockaddr_in *src_in,
 			 const struct sockaddr_in *dst_in,
 			 struct rdma_dev_addr *addr,
@@ -246,10 +419,11 @@ static int addr4_resolve(struct sockaddr_in *src_in,
 	src_in->sin_family = AF_INET;
 	src_in->sin_addr.s_addr = fl4.saddr;
 
-	/* If there's a gateway, we're definitely in RoCE v2 (as RoCE v1 isn't
-	 * routable) and we could set the network type accordingly.
+	/* If there's a gateway and type of device not ARPHRD_INFINIBAND, we're
+	 * definitely in RoCE v2 (as RoCE v1 isn't routable) set the network
+	 * type accordingly.
 	 */
-	if (rt->rt_uses_gateway)
+	if (rt->rt_uses_gateway && rt->dst.dev->type != ARPHRD_INFINIBAND)
 		addr->network = RDMA_NETWORK_IPV4;
 
 	addr->hoplimit = ip4_dst_hoplimit(&rt->dst);
@@ -291,10 +465,12 @@ static int addr6_resolve(struct sockaddr_in6 *src_in,
 		src_in->sin6_addr = fl6.saddr;
 	}
 
-	/* If there's a gateway, we're definitely in RoCE v2 (as RoCE v1 isn't
-	 * routable) and we could set the network type accordingly.
+	/* If there's a gateway and type of device not ARPHRD_INFINIBAND, we're
+	 * definitely in RoCE v2 (as RoCE v1 isn't routable) set the network
+	 * type accordingly.
 	 */
-	if (rt->rt6i_flags & RTF_GATEWAY)
+	if (rt->rt6i_flags & RTF_GATEWAY &&
+	    ip6_dst_idev(dst)->dev->type != ARPHRD_INFINIBAND)
 		addr->network = RDMA_NETWORK_IPV6;
 
 	addr->hoplimit = ip6_dst_hoplimit(dst);
@@ -317,7 +493,8 @@ static int addr6_resolve(struct sockaddr_in6 *src_in,
 
 static int addr_resolve_neigh(struct dst_entry *dst,
 			      const struct sockaddr *dst_in,
-			      struct rdma_dev_addr *addr)
+			      struct rdma_dev_addr *addr,
+			      u32 seq)
 {
 	if (dst->dev->flags & IFF_LOOPBACK) {
 		int ret;
@@ -331,17 +508,8 @@ static int addr_resolve_neigh(struct dst_entry *dst,
 	}
 
 	/* If the device doesn't do ARP internally */
-	if (!(dst->dev->flags & IFF_NOARP)) {
-		const struct sockaddr_in *dst_in4 =
-			(const struct sockaddr_in *)dst_in;
-		const struct sockaddr_in6 *dst_in6 =
-			(const struct sockaddr_in6 *)dst_in;
-
-		return dst_fetch_ha(dst, addr,
-				    dst_in->sa_family == AF_INET ?
-				    (const void *)&dst_in4->sin_addr.s_addr :
-				    (const void *)&dst_in6->sin6_addr);
-	}
+	if (!(dst->dev->flags & IFF_NOARP))
+		return fetch_ha(dst, addr, dst_in, seq);
 
 	return rdma_copy_addr(addr, dst->dev, NULL);
 }
@@ -349,7 +517,8 @@ static int addr_resolve_neigh(struct dst_entry *dst,
 static int addr_resolve(struct sockaddr *src_in,
 			const struct sockaddr *dst_in,
 			struct rdma_dev_addr *addr,
-			bool resolve_neigh)
+			bool resolve_neigh,
+			u32 seq)
 {
 	struct net_device *ndev;
 	struct dst_entry *dst;
@@ -366,7 +535,7 @@ static int addr_resolve(struct sockaddr *src_in,
 			return ret;
 
 		if (resolve_neigh)
-			ret = addr_resolve_neigh(&rt->dst, dst_in, addr);
+			ret = addr_resolve_neigh(&rt->dst, dst_in, addr, seq);
 
 		ndev = rt->dst.dev;
 		dev_hold(ndev);
@@ -383,7 +552,7 @@ static int addr_resolve(struct sockaddr *src_in,
 			return ret;
 
 		if (resolve_neigh)
-			ret = addr_resolve_neigh(dst, dst_in, addr);
+			ret = addr_resolve_neigh(dst, dst_in, addr, seq);
 
 		ndev = dst->dev;
 		dev_hold(ndev);
@@ -412,7 +581,7 @@ static void process_req(struct work_struct *work)
 			src_in = (struct sockaddr *) &req->src_addr;
 			dst_in = (struct sockaddr *) &req->dst_addr;
 			req->status = addr_resolve(src_in, dst_in, req->addr,
-						   true);
+						   true, req->seq);
 			if (req->status && time_after_eq(jiffies, req->timeout))
 				req->status = -ETIMEDOUT;
 			else if (req->status == -ENODATA)
@@ -471,8 +640,9 @@ int rdma_resolve_ip(struct rdma_addr_client *client,
 	req->context = context;
 	req->client = client;
 	atomic_inc(&client->refcount);
+	req->seq = (u32)atomic_inc_return(&ib_nl_addr_request_seq);
 
-	req->status = addr_resolve(src_in, dst_in, addr, true);
+	req->status = addr_resolve(src_in, dst_in, addr, true, req->seq);
 	switch (req->status) {
 	case 0:
 		req->timeout = jiffies;
@@ -510,7 +680,7 @@ int rdma_resolve_ip_route(struct sockaddr *src_addr,
 		src_in->sa_family = dst_addr->sa_family;
 	}
 
-	return addr_resolve(src_in, dst_addr, addr, false);
+	return addr_resolve(src_in, dst_addr, addr, false, 0);
 }
 EXPORT_SYMBOL(rdma_resolve_ip_route);
 
@@ -640,13 +810,20 @@ static int __init addr_init(void)
 	if (!addr_wq)
 		return -ENOMEM;
 
+	atomic_set(&ib_nl_addr_request_seq, 0);
 	register_netevent_notifier(&nb);
 	rdma_addr_register_client(&self);
+	if (ibnl_add_client(RDMA_NL_LS, RDMA_NL_LS_NUM_OPS,
+			    ib_addr_cb_table)) {
+		pr_warn("RDMA ADDR: failed to add netlink callback\n");
+	}
+
 	return 0;
 }
 
 static void __exit addr_cleanup(void)
 {
+	ibnl_remove_client(RDMA_NL_LS, RDMA_NL_LS_NUM_OPS, ib_addr_cb_table);
 	rdma_addr_unregister_client(&self);
 	unregister_netevent_notifier(&nb);
 	destroy_workqueue(addr_wq);
-- 
2.1.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

Re: [PATCH rdma-next 4/4] IB/core: Add IP to GID netlink offload

[Jason Gunthorpe]

On Wed, May 04, 2016 at 06:41:58PM +0300, Leon Romanovsky wrote:
> From: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> There is an assumption that rdmacm is used only between nodes
> in the same IB subnet, this why ARP resolution can be used to turn
> IP to GID in rdmacm.
> 
> When dealing with IB communication between subnets this assumption
> is no longer valid. ARP resolution will get us the next hop device
> address and not the peer node's device address.
> 
> To overcome this limitation, let's check user space if it can
> provide the GID of the peer node, and fail if not.
> 
> We add a sequence number to identify each request and fill in the GID
> upon answer from user space.

This description doesn't describe what this patch is trying to do.

This patch is delegating IP to GID translation to user space if there
is a route table entry for the destination.

I have to say, I really don't like this at all. If we want to have
proper routing support then the translation needs to be done somehow
in-band. What is user space supposed to do?

Jason
--
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

RE: [PATCH rdma-next 4/4] IB/core: Add IP to GID netlink offload

[Mark Bloch]

Sorry it took me so long to response, I had personal matters I had to attend
and was without internet access.

> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org]
> Sent: Wednesday, May 04, 2016 9:53 PM
> To: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Mark Bloch
> <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>; Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>; Matan
> Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Subject: Re: [PATCH rdma-next 4/4] IB/core: Add IP to GID netlink offload
> 
> On Wed, May 04, 2016 at 06:41:58PM +0300, Leon Romanovsky wrote:
> > From: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > There is an assumption that rdmacm is used only between nodes
> > in the same IB subnet, this why ARP resolution can be used to turn
> > IP to GID in rdmacm.
> >
> > When dealing with IB communication between subnets this assumption
> > is no longer valid. ARP resolution will get us the next hop device
> > address and not the peer node's device address.
> >
> > To overcome this limitation, let's check user space if it can
> > provide the GID of the peer node, and fail if not.
> >
> > We add a sequence number to identify each request and fill in the GID
> > upon answer from user space.
> 
> This description doesn't describe what this patch is trying to do.
> 
> This patch is delegating IP to GID translation to user space if there
> is a route table entry for the destination.
> 
> I have to say, I really don't like this at all. If we want to have
> proper routing support then the translation needs to be done somehow
> in-band. What is user space supposed to do?
I agree that in the long term an in-band kernel based solution is a better option.
As it stands today, I don't know of any standard way to achieve that.

I'll add a better description to the next version, but in a nutshell:
We are using ibacm to answer those requests, we envision two ways to do that,
The easier one is to load Ibacm with a file that contains ip->gid of the entire fabric.
The more tricky one is feed live information, so if a new node becomes active, update
The entire fabric about its IP and GID and remove the information if a node goes down.

That's also the reason why I've added the RDMA_NL_LS_OP_IP_RESOLVE under 
Local service operations. This way ibacm can still listen on one netlink socket
For both SA and IP->GID queries.

> Jason
--
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

Re: [PATCH rdma-next 4/4] IB/core: Add IP to GID netlink offload

[Ira Weiny]

On Wed, May 04, 2016 at 06:41:58PM +0300, Leon Romanovsky wrote:
> From: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> There is an assumption that rdmacm is used only between nodes
> in the same IB subnet, this why ARP resolution can be used to turn
> IP to GID in rdmacm.
> 
> When dealing with IB communication between subnets this assumption
> is no longer valid. ARP resolution will get us the next hop device
> address and not the peer node's device address.
> 
> To overcome this limitation, let's check user space if it can
> provide the GID of the peer node, and fail if not.
> 
> We add a sequence number to identify each request and fill in the GID
> upon answer from user space.
> 
> Signed-off-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/infiniband/core/addr.c | 225 ++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 201 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
> index 337353d..e1a3ef6 100644
> --- a/drivers/infiniband/core/addr.c
> +++ b/drivers/infiniband/core/addr.c
> @@ -46,6 +46,8 @@
>  #include <net/ip6_route.h>
>  #include <rdma/ib_addr.h>
>  #include <rdma/ib.h>
> +#include <rdma/rdma_netlink.h>
> +#include <net/netlink.h>
>  
>  MODULE_AUTHOR("Sean Hefty");
>  MODULE_DESCRIPTION("IB Address Translation");
> @@ -62,8 +64,11 @@ struct addr_req {
>  			 struct rdma_dev_addr *addr, void *context);
>  	unsigned long timeout;
>  	int status;
> +	u32 seq;
>  };
>  
> +static atomic_t ib_nl_addr_request_seq;
> +
>  static void process_req(struct work_struct *work);
>  
>  static DEFINE_MUTEX(lock);
> @@ -71,6 +76,130 @@ static LIST_HEAD(req_list);
>  static DECLARE_DELAYED_WORK(work, process_req);
>  static struct workqueue_struct *addr_wq;
>  
> +static const struct nla_policy ib_nl_addr_policy[LS_NLA_TYPE_MAX] = {
> +	[LS_NLA_TYPE_DGID]		= {.type = NLA_BINARY,
> +		.len = sizeof(struct rdma_nla_ls_gid)},
> +};
> +
> +static inline int ib_nl_is_good_ip_resp(const struct nlmsghdr *nlh)
> +{
> +	struct nlattr *tb[LS_NLA_TYPE_MAX] = {};
> +	int ret;
> +
> +	if (nlh->nlmsg_flags & RDMA_NL_LS_F_ERR)
> +		return 0;
> +
> +	ret = nla_parse(tb, LS_NLA_TYPE_MAX - 1, nlmsg_data(nlh),
> +			nlmsg_len(nlh), ib_nl_addr_policy);
> +	if (ret)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static void ib_nl_process_good_ip_rsep(const struct nlmsghdr *nlh)
> +{
> +	const struct nlattr *head, *curr;
> +	union ib_gid gid;
> +	struct addr_req *req;
> +	int len, rem;
> +	int found = 0;
> +
> +	head = (const struct nlattr *)nlmsg_data(nlh);
> +	len = nlmsg_len(nlh);
> +
> +	nla_for_each_attr(curr, head, len, rem) {
> +		if (curr->nla_type == LS_NLA_TYPE_DGID)
> +			memcpy(&gid, nla_data(curr), nla_len(curr));
> +	}
> +
> +	mutex_lock(&lock);
> +	list_for_each_entry(req, &req_list, list) {
> +		if (nlh->nlmsg_seq != req->seq)
> +			continue;
> +		/* We set the DGID part, the rest was set earlier */
> +		rdma_addr_set_dgid(req->addr, &gid);
> +		req->status = 0;
> +		found = 1;
> +		break;
> +	}
> +	mutex_unlock(&lock);
> +
> +	if (!found)
> +		pr_info("Couldn't find request waiting for DGID: %pI6\n",
> +			&gid);
> +}
> +
> +static int ib_nl_handle_ip_res_resp(struct sk_buff *skb,
> +				    struct netlink_callback *cb)
> +{
> +	const struct nlmsghdr *nlh = (struct nlmsghdr *)cb->nlh;
> +
> +	if ((nlh->nlmsg_flags & NLM_F_REQUEST) ||
> +	    !(NETLINK_CB(skb).sk) ||
> +	    !netlink_capable(skb, CAP_NET_ADMIN))
> +		return -EPERM;
> +
> +	if (ib_nl_is_good_ip_resp(nlh))
> +		ib_nl_process_good_ip_rsep(nlh);
> +
> +	return skb->len;
> +}
> +
> +static struct ibnl_client_cbs ib_addr_cb_table[RDMA_NL_LS_NUM_OPS] = {
> +	[RDMA_NL_LS_OP_IP_RESOLVE] = {
> +		.dump = ib_nl_handle_ip_res_resp,
> +		.module = THIS_MODULE },
> +};
> +
> +static int ib_nl_ip_send_msg(struct rdma_dev_addr *dev_addr,
> +			     const void *daddr,
> +			     u32 seq, u16 family)
> +{
> +	struct sk_buff *skb = NULL;
> +	struct nlmsghdr *nlh;
> +	struct rdma_ls_ip_resolve_header *header;
> +	void *data;
> +	size_t size;
> +	int attrtype;
> +	int len;
> +
> +	if (family == AF_INET) {
> +		size = sizeof(struct in_addr);
> +		attrtype = RDMA_NLA_F_MANDATORY | LS_NLA_TYPE_IPV4;
> +	} else {
> +		size = sizeof(struct in6_addr);
> +		attrtype = RDMA_NLA_F_MANDATORY | LS_NLA_TYPE_IPV6;
> +	}
> +
> +	len = nla_total_size(sizeof(size));
> +	len += NLMSG_ALIGN(sizeof(*header));
> +
> +	skb = nlmsg_new(len, GFP_KERNEL);
> +
> +	data = ibnl_put_msg(skb, &nlh, seq, 0, RDMA_NL_LS,
> +			    RDMA_NL_LS_OP_IP_RESOLVE, NLM_F_REQUEST);
> +	if (!data) {
> +		nlmsg_free(skb);
> +		return -ENODATA;
> +	}
> +
> +	/* Construct the family header first */
> +	header = (struct rdma_ls_ip_resolve_header *)
> +		skb_put(skb, NLMSG_ALIGN(sizeof(*header)));
> +	header->ifindex = dev_addr->bound_dev_if;
> +	nla_put(skb, attrtype, size, daddr);
> +
> +	/* Repair the nlmsg header length */
> +	nlmsg_end(skb, nlh);
> +	ibnl_multicast(skb, nlh, RDMA_NL_GROUP_LS, GFP_KERNEL);
> +
> +	/* Make the request retry, so when we get the response from userspace
> +	 * we will have something.
> +	 */
> +	return -ENODATA;
> +}
> +
>  int rdma_addr_size(struct sockaddr *addr)
>  {
>  	switch (addr->sa_family) {
> @@ -199,6 +328,17 @@ static void queue_req(struct addr_req *req)
>  	mutex_unlock(&lock);
>  }
>  
> +static int ib_nl_fetch_ha(struct dst_entry *dst, struct rdma_dev_addr *dev_addr,
> +			  const void *daddr, u32 seq, u16 family)
> +{
> +	if (ibnl_chk_listeners(RDMA_NL_GROUP_LS))
> +		return -EADDRNOTAVAIL;
> +
> +	/* We fill in what we can, the response will fill the rest */
> +	rdma_copy_addr(dev_addr, dst->dev, NULL);
> +	return ib_nl_ip_send_msg(dev_addr, daddr, seq, family);
> +}
> +
>  static int dst_fetch_ha(struct dst_entry *dst, struct rdma_dev_addr *dev_addr,
>  			const void *daddr)
>  {
> @@ -223,6 +363,39 @@ static int dst_fetch_ha(struct dst_entry *dst, struct rdma_dev_addr *dev_addr,
>  	return ret;
>  }
>  
> +static bool has_gateway(struct dst_entry *dst, sa_family_t family)
> +{
> +	struct rtable *rt;
> +	struct rt6_info *rt6;
> +
> +	if (family == AF_INET) {
> +		rt = container_of(dst, struct rtable, dst);
> +		return rt->rt_uses_gateway;
> +	}
> +
> +	rt6 = container_of(dst, struct rt6_info, dst);
> +	return rt6->rt6i_flags & RTF_GATEWAY;
> +}
> +
> +static int fetch_ha(struct dst_entry *dst, struct rdma_dev_addr *dev_addr,
> +		    const struct sockaddr *dst_in, u32 seq)
> +{
> +	const struct sockaddr_in *dst_in4 =
> +		(const struct sockaddr_in *)dst_in;
> +	const struct sockaddr_in6 *dst_in6 =
> +		(const struct sockaddr_in6 *)dst_in;
> +	const void *daddr = (dst_in->sa_family == AF_INET) ?
> +		(const void *)&dst_in4->sin_addr.s_addr :
> +		(const void *)&dst_in6->sin6_addr;
> +	sa_family_t family = dst_in->sa_family;
> +
> +	/* Gateway + ARPHRD_INFINIBAND -> IB router */
> +	if (has_gateway(dst, family) && dst->dev->type == ARPHRD_INFINIBAND)
> +		return ib_nl_fetch_ha(dst, dev_addr, daddr, seq, family);
> +	else
> +		return dst_fetch_ha(dst, dev_addr, daddr);
> +}
> +
>  static int addr4_resolve(struct sockaddr_in *src_in,
>  			 const struct sockaddr_in *dst_in,
>  			 struct rdma_dev_addr *addr,
> @@ -246,10 +419,11 @@ static int addr4_resolve(struct sockaddr_in *src_in,
>  	src_in->sin_family = AF_INET;
>  	src_in->sin_addr.s_addr = fl4.saddr;
>  
> -	/* If there's a gateway, we're definitely in RoCE v2 (as RoCE v1 isn't
> -	 * routable) and we could set the network type accordingly.
> +	/* If there's a gateway and type of device not ARPHRD_INFINIBAND, we're
> +	 * definitely in RoCE v2 (as RoCE v1 isn't routable) set the network
> +	 * type accordingly.
>  	 */
> -	if (rt->rt_uses_gateway)
> +	if (rt->rt_uses_gateway && rt->dst.dev->type != ARPHRD_INFINIBAND)
>  		addr->network = RDMA_NETWORK_IPV4;
>  
>  	addr->hoplimit = ip4_dst_hoplimit(&rt->dst);
> @@ -291,10 +465,12 @@ static int addr6_resolve(struct sockaddr_in6 *src_in,
>  		src_in->sin6_addr = fl6.saddr;
>  	}
>  
> -	/* If there's a gateway, we're definitely in RoCE v2 (as RoCE v1 isn't
> -	 * routable) and we could set the network type accordingly.
> +	/* If there's a gateway and type of device not ARPHRD_INFINIBAND, we're
> +	 * definitely in RoCE v2 (as RoCE v1 isn't routable) set the network
> +	 * type accordingly.
>  	 */
> -	if (rt->rt6i_flags & RTF_GATEWAY)
> +	if (rt->rt6i_flags & RTF_GATEWAY &&
> +	    ip6_dst_idev(dst)->dev->type != ARPHRD_INFINIBAND)
>  		addr->network = RDMA_NETWORK_IPV6;
>  
>  	addr->hoplimit = ip6_dst_hoplimit(dst);
> @@ -317,7 +493,8 @@ static int addr6_resolve(struct sockaddr_in6 *src_in,
>  
>  static int addr_resolve_neigh(struct dst_entry *dst,
>  			      const struct sockaddr *dst_in,
> -			      struct rdma_dev_addr *addr)
> +			      struct rdma_dev_addr *addr,
> +			      u32 seq)
>  {
>  	if (dst->dev->flags & IFF_LOOPBACK) {
>  		int ret;
> @@ -331,17 +508,8 @@ static int addr_resolve_neigh(struct dst_entry *dst,
>  	}
>  
>  	/* If the device doesn't do ARP internally */
> -	if (!(dst->dev->flags & IFF_NOARP)) {
> -		const struct sockaddr_in *dst_in4 =
> -			(const struct sockaddr_in *)dst_in;
> -		const struct sockaddr_in6 *dst_in6 =
> -			(const struct sockaddr_in6 *)dst_in;
> -
> -		return dst_fetch_ha(dst, addr,
> -				    dst_in->sa_family == AF_INET ?
> -				    (const void *)&dst_in4->sin_addr.s_addr :
> -				    (const void *)&dst_in6->sin6_addr);
> -	}
> +	if (!(dst->dev->flags & IFF_NOARP))
> +		return fetch_ha(dst, addr, dst_in, seq);
>  
>  	return rdma_copy_addr(addr, dst->dev, NULL);
>  }
> @@ -349,7 +517,8 @@ static int addr_resolve_neigh(struct dst_entry *dst,
>  static int addr_resolve(struct sockaddr *src_in,
>  			const struct sockaddr *dst_in,
>  			struct rdma_dev_addr *addr,
> -			bool resolve_neigh)
> +			bool resolve_neigh,
> +			u32 seq)
>  {
>  	struct net_device *ndev;
>  	struct dst_entry *dst;
> @@ -366,7 +535,7 @@ static int addr_resolve(struct sockaddr *src_in,
>  			return ret;
>  
>  		if (resolve_neigh)
> -			ret = addr_resolve_neigh(&rt->dst, dst_in, addr);
> +			ret = addr_resolve_neigh(&rt->dst, dst_in, addr, seq);
>  
>  		ndev = rt->dst.dev;
>  		dev_hold(ndev);
> @@ -383,7 +552,7 @@ static int addr_resolve(struct sockaddr *src_in,
>  			return ret;
>  
>  		if (resolve_neigh)
> -			ret = addr_resolve_neigh(dst, dst_in, addr);
> +			ret = addr_resolve_neigh(dst, dst_in, addr, seq);
>  
>  		ndev = dst->dev;
>  		dev_hold(ndev);
> @@ -412,7 +581,7 @@ static void process_req(struct work_struct *work)
>  			src_in = (struct sockaddr *) &req->src_addr;
>  			dst_in = (struct sockaddr *) &req->dst_addr;
>  			req->status = addr_resolve(src_in, dst_in, req->addr,
> -						   true);
> +						   true, req->seq);
>  			if (req->status && time_after_eq(jiffies, req->timeout))
>  				req->status = -ETIMEDOUT;
>  			else if (req->status == -ENODATA)
> @@ -471,8 +640,9 @@ int rdma_resolve_ip(struct rdma_addr_client *client,
>  	req->context = context;
>  	req->client = client;
>  	atomic_inc(&client->refcount);
> +	req->seq = (u32)atomic_inc_return(&ib_nl_addr_request_seq);
>  
> -	req->status = addr_resolve(src_in, dst_in, addr, true);
> +	req->status = addr_resolve(src_in, dst_in, addr, true, req->seq);
>  	switch (req->status) {
>  	case 0:
>  		req->timeout = jiffies;
> @@ -510,7 +680,7 @@ int rdma_resolve_ip_route(struct sockaddr *src_addr,
>  		src_in->sa_family = dst_addr->sa_family;
>  	}
>  
> -	return addr_resolve(src_in, dst_addr, addr, false);
> +	return addr_resolve(src_in, dst_addr, addr, false, 0);
>  }
>  EXPORT_SYMBOL(rdma_resolve_ip_route);
>  
> @@ -640,13 +810,20 @@ static int __init addr_init(void)
>  	if (!addr_wq)
>  		return -ENOMEM;
>  
> +	atomic_set(&ib_nl_addr_request_seq, 0);
>  	register_netevent_notifier(&nb);
>  	rdma_addr_register_client(&self);
> +	if (ibnl_add_client(RDMA_NL_LS, RDMA_NL_LS_NUM_OPS,

                                        ^^^^^^^^^^^^^^^^^^^
This seems odd that we have to pass RDMA_NL_LS_NUM_OPS in every add_client call
for the same client from different modules trying to use this "client".

Previously this was a "register this client with a static array of operations
of size X".  Now we have an array of operations which may or may not be
in use...

I think you are breaking the design pattern of the netlink code and this will
cause confusion in the future.

I think to do what you want (which I'm not objecting to) we need to change the
way the registration works in netlink.

Perhaps it would be better to have an "add_operation" call which adds
support for the operation to the client.  If the client is not registered then
it will need to be.  Who is responsible for the client registration and removal
is the question.  IMO it is odd that the client registration is dynamic at
all because we have an enum defining them.

???

Ira

> +			    ib_addr_cb_table)) {
> +		pr_warn("RDMA ADDR: failed to add netlink callback\n");
> +	}
> +
>  	return 0;
>  }
>  
>  static void __exit addr_cleanup(void)
>  {
> +	ibnl_remove_client(RDMA_NL_LS, RDMA_NL_LS_NUM_OPS, ib_addr_cb_table);
>  	rdma_addr_unregister_client(&self);
>  	unregister_netevent_notifier(&nb);
>  	destroy_workqueue(addr_wq);
> -- 
> 2.1.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
--
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

Re: [PATCH rdma-next 4/4] IB/core: Add IP to GID netlink offload

[Jason Gunthorpe]

On Wed, May 04, 2016 at 05:32:12PM -0400, Ira Weiny wrote:

> support for the operation to the client.  If the client is not registered then
> it will need to be.  Who is responsible for the client registration and removal
> is the question.  IMO it is odd that the client registration is dynamic at
> all because we have an enum defining them.

Indeed, this dynamic stuff seems to be a side effect of too many
modules.

Jason
--
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