[ofa-general] RE: [PATCH RFC] RDMA/CMA: Allocate PS_TCP ports from the host TCP port space.

[Sean Hefty]

>The correct solution in my mind is to use the host stack's TCP port
>space for _all_ RDMA_PS_TCP port allocations.   The patch below is a
>minimal delta to unify the port spaces bay using the kernel stack to
>bind ports.  This is done by allocating a kernel socket and binding to
>the appropriate local addr/port.  It also allows the kernel stack to
>pick ephemeral ports by virtue of just passing in port 0 on the kernel
>bind operation.

I'm not thrilled with the idea of overlapping port spaces, and I can't come up
with a solution that works for all situations.  I understand the overlapping
port space problem, but I consider the ability to use the same port number for
both RDMA and sockets a feature.

What if MPI used a similar mechanism as SDP?  That is, if it gets a port number
from sockets, it reserves that same RDMA port number, or vice-versa.  The
rdma_cm advertises separate port spaces from TCP/UDP, so IMO any assumption
otherwise, at this point, is a bug in the user's code.

Before merging the port spaces, I'd like a way for an application to use a
single well-known port number that works over both RDMA and sockets.

>RDMA/CMA: Allocate PS_TCP ports from the host TCP port space.

Is there any reason to limit this behavior to TCP only, or would we also include
UDP?

>diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
>index 9e0ab04..e4d2d7f 100644
>--- a/drivers/infiniband/core/cma.c
>+++ b/drivers/infiniband/core/cma.c
>@@ -111,6 +111,7 @@ struct rdma_id_private {
>  	struct rdma_cm_id	id;
>
>  	struct rdma_bind_list	*bind_list;
>+	struct socket		*sock;

This points off to a rather largish structure...

>  	struct hlist_node	node;
>  	struct list_head	list;
>  	struct list_head	listen_list;
>@@ -695,6 +696,8 @@ static void cma_release_port(struct rdma
>  		kfree(bind_list);
>  	}
>  	mutex_unlock(&lock);
>+	if (id_priv->sock)
>+		sock_release(id_priv->sock);
>  }
>
>  void rdma_destroy_id(struct rdma_cm_id *id)
>@@ -1790,6 +1793,25 @@ static int cma_use_port(struct idr *ps,
>  	return 0;
>  }
>
>+static int cma_get_tcp_port(struct rdma_id_private *id_priv)
>+{
>+	int ret;
>+	struct socket *sock;
>+
>+	ret = sock_create_kern(AF_INET, SOCK_STREAM, IPPROTO_TCP, &sock);
>+	if (ret)
>+		return ret;
>+	ret = sock->ops->bind(sock,
>+			  (struct socketaddr *)&id_priv->id.route.addr.src_addr,
>+			  ip_addr_size(&id_priv->id.route.addr.src_addr));
>+	if (ret) {
>+		sock_release(sock);
>+		return ret;
>+	}
>+	id_priv->sock = sock;
>+	return 0;
>+}
>+
>  static int cma_get_port(struct rdma_id_private *id_priv)
>  {
>  	struct idr *ps;
>@@ -1801,6 +1823,9 @@ static int cma_get_port(struct rdma_id_p
>  		break;
>  	case RDMA_PS_TCP:
>  		ps = &tcp_ps;
>+		ret = cma_get_tcp_port(id_priv); /* Synch with native stack */
>+		if (ret)
>+			goto out;

Would we need tcp_ps (and udp_ps) anymore?  Also, I think SDP maps into the TCP
port space already, so changes to SDP will be needed as well, which may
eliminate its port space.

- Sean