[PATCH 26/37] librdmacm: set src_addr in rdma_getaddrinfo

[PATCH 26/37] librdmacm: set src_addr in rdma_getaddrinfo

[Sean Hefty]

RDMA requires the user to allocate hardware resources before
establishing a connection.  To support this, the user must know
the source address that the connection will use to reach the
remote endpoint.  Modify rdma_getaddrinfo to determine an
appropriate source address based on the specified destination,
when a source address is not given.

Signed-off-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---

 src/addrinfo.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/src/addrinfo.c b/src/addrinfo.c
index 15ae071..dfaf9d5 100644
--- a/src/addrinfo.c
+++ b/src/addrinfo.c
@@ -39,6 +39,7 @@
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <netdb.h>
+#include <unistd.h>
 
 #include "cma.h"
 #include <rdma/rdma_cma.h>
@@ -129,6 +130,48 @@ static int ucma_convert_to_rai(struct rdma_addrinfo *rai, struct addrinfo *ai)
 	return 0;
 }
 
+static int ucma_resolve_src(struct rdma_addrinfo *rai)
+{
+	struct sockaddr *addr;
+	socklen_t len;
+	int ret, s;
+
+	s = socket(rai->ai_family, SOCK_DGRAM, IPPROTO_UDP);
+	if (s < 0)
+		return s;
+
+	ret = connect(s, rai->ai_dst_addr, rai->ai_dst_len);
+	if (ret)
+		goto err1;
+
+	addr = zalloc(rai->ai_dst_len);
+	if (!addr) {
+		ret = ERR(ENOMEM);
+		goto err1;
+	}
+
+	len = rai->ai_dst_len;
+	ret = getsockname(s, addr, &len);
+	if (ret)
+		goto err2;
+
+	if (addr->sa_family == AF_INET)
+		((struct sockaddr_in *) addr)->sin_port = 0;
+	else
+		((struct sockaddr_in6 *) addr)->sin6_port = 0;
+	rai->ai_src_addr = addr;
+	rai->ai_src_len = len;
+
+	close(s);
+	return 0;
+
+err2:
+	free(addr);
+err1:
+	close(s);
+	return ret;
+}
+
 int rdma_getaddrinfo(char *node, char *service,
 		     struct rdma_addrinfo *hints,
 		     struct rdma_addrinfo **res)
@@ -159,6 +202,23 @@ int rdma_getaddrinfo(char *node, char *service,
 	if (ret)
 		goto err2;
 
+	if (!rai->ai_src_len) {
+		if (hints && hints->ai_src_len) {
+			rai->ai_src_addr = zalloc(hints->ai_src_len);
+			if (!rai->ai_src_addr) {
+				ret = ERR(ENOMEM);
+				goto err2;
+			}
+			memcpy(rai->ai_src_addr, hints->ai_src_addr,
+			       hints->ai_src_len);
+			rai->ai_src_len = hints->ai_src_len;
+		} else {
+			ret = ucma_resolve_src(rai);
+			if (ret)
+				goto err2;
+		}
+	}
+
 	freeaddrinfo(ai);
 	*res = rai;
 	return 0;



--
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 26/37] librdmacm: set src_addr in rdma_getaddrinfo

[Jason Gunthorpe]

On Wed, Apr 07, 2010 at 10:12:43AM -0700, Sean Hefty wrote:
> RDMA requires the user to allocate hardware resources before
> establishing a connection.  To support this, the user must know
> the source address that the connection will use to reach the
> remote endpoint.  Modify rdma_getaddrinfo to determine an
> appropriate source address based on the specified destination,
> when a source address is not given.

I haven't looked through everything you posted to make a suggestion
here, but this bothers me..

The resources should be allocated after the rdma_bind syscall, prior to
listen/accept or connect, IMHO.

How does tha rai->ai_src_addr get used to allocate resources anyhow?

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 26/37] librdmacm: set src_addr in rdma_getaddrinfo

[Sean Hefty]

>I haven't looked through everything you posted to make a suggestion
>here, but this bothers me..
>
>The resources should be allocated after the rdma_bind syscall, prior to
>listen/accept or connect, IMHO.
>
>How does tha rai->ai_src_addr get used to allocate resources anyhow?

Maybe the patch description is off.

All this does (in a very non-sexy way) is set ai_src_addr.  It does not allocate
any hardware resources.  A user can provide ai_src_addr as input into rdma_bind
or rdma_resolve_addr.

The motivation is twofold.  First, the user can select the rdma_addrinfo for a
connection by examining the src/dst address pair.  This may be desired for
failover or performance reasons.  Second, route resolution may require knowing
both the source and destination addresses.  For example, IB ACM requires both
addresses as input.

- Sean

--
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 26/37] librdmacm: set src_addr in rdma_getaddrinfo

[Jason Gunthorpe]

On Wed, Apr 07, 2010 at 12:54:56PM -0700, Sean Hefty wrote:
> >I haven't looked through everything you posted to make a suggestion
> >here, but this bothers me..
> >
> >The resources should be allocated after the rdma_bind syscall, prior to
> >listen/accept or connect, IMHO.
> >
> >How does tha rai->ai_src_addr get used to allocate resources anyhow?
> 
> Maybe the patch description is off.
> 
> All this does (in a very non-sexy way) is set ai_src_addr.  It does
> not allocate any hardware resources.  A user can provide ai_src_addr
> as input into rdma_bind or rdma_resolve_addr.
> 
> The motivation is twofold.  First, the user can select the
> rdma_addrinfo for a connection by examining the src/dst address
> pair.  This may be desired for failover or performance reasons.
> Second, route resolution may require knowing both the source and
> destination addresses.  For example, IB ACM requires both addresses
> as input.

Huumm

I don't have a problem with ai_src_addr being set, when necessary, but
setting it unconditionally seems wrong to me. In most cases the kernel
should select the source during route resolution, not be forced to
something in userspace.

Certainly for AF_INET/6 I don't think this should be done..

Apps doing complex things for failover should supply a source address
in the hints and call rdma_getaddrinfo for each adaptor.

AF_IB has the scope ID in the destination to specify the adaptor for
link-local GIDs, so the source should not often be needed.

Not sure what you mean that ACM requires it? Doesn't ACM plug in at
the rdma_getaddrinfo stage? If so it can get the source on its own
like you did in this patch. I agree that ACM should always return
results with the source set, because it is providing path records
relative to a specific adaptor.

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 26/37] librdmacm: set src_addr in rdma_getaddrinfo

[Sean Hefty]

>I don't have a problem with ai_src_addr being set, when necessary, but
>setting it unconditionally seems wrong to me. In most cases the kernel
>should select the source during route resolution, not be forced to
>something in userspace.

Just to be precise, the source is selected during address resolution, and the
existing APIs allow the user to indicate that a specific source should be used.

This is a requirement of some applications.

>Not sure what you mean that ACM requires it? Doesn't ACM plug in at
>the rdma_getaddrinfo stage? If so it can get the source on its own
>like you did in this patch. I agree that ACM should always return
>results with the source set, because it is providing path records
>relative to a specific adaptor.

Yes - the code to set the source could move from librdmacm into ACM.

I can change rdma_getaddrinfo to only set the source address if either the user
provides one through a hint, or if resolved through ACM.

- Sean

--
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 26/37] librdmacm: set src_addr in rdma_getaddrinfo

[Jason Gunthorpe]

On Wed, Apr 07, 2010 at 03:10:36PM -0700, Sean Hefty wrote:

> >Not sure what you mean that ACM requires it? Doesn't ACM plug in at
> >the rdma_getaddrinfo stage? If so it can get the source on its own
> >like you did in this patch. I agree that ACM should always return
> >results with the source set, because it is providing path records
> >relative to a specific adaptor.
> 
> Yes - the code to set the source could move from librdmacm into ACM.
> 
> I can change rdma_getaddrinfo to only set the source address if
> either the user provides one through a hint, or if resolved through
> ACM.

That would be my preference. I think the kernel calls should use a
null source address in the common case and a set source should be an
exceptional case. This matches sockets very well.

I'd see two cases for setting a source address, an app that wants to
control the bind port - this is similar to socket cases, and is
generally an exceptional case.

The other is that an app wants the connection to be usable with a
certain PD. This is more like the DAPL case, as far as I understand it
(ie resources have been allocated against a PD prior to the addresses
being known). This would be best served by having the hints include a
PD and have rdma_getaddrinfo generate a source address that works with
that PD. A PD is more general than a source address - in single HCA
cases a PD will be usable with all ports.

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

[PATCH 26/37 v2] librdmacm: set src_addr in rdma_getaddrinfo

[Sean Hefty]

If the user provides a source address as a hint, copy
the address to rdma_addrinfo ai_src_addr.

Signed-off-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
Changes from v1:
Only copy the source address if one is provided by the user though the hints.
Do not assign a source address otherwise.  The kernel will resolve the
source address for us via rdma_resolve_addr().

 src/addrinfo.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/src/addrinfo.c b/src/addrinfo.c
index 15ae071..f5f86a0 100644
--- a/src/addrinfo.c
+++ b/src/addrinfo.c
@@ -39,6 +39,7 @@
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <netdb.h>
+#include <unistd.h>
 
 #include "cma.h"
 #include <rdma/rdma_cma.h>
@@ -159,6 +160,17 @@ int rdma_getaddrinfo(char *node, char *service,
 	if (ret)
 		goto err2;
 
+	if (!rai->ai_src_len && hints && hints->ai_src_len) {
+		rai->ai_src_addr = calloc(1, hints->ai_src_len);
+		if (!rai->ai_src_addr) {
+			ret = ERR(ENOMEM);
+			goto err2;
+		}
+		memcpy(rai->ai_src_addr, hints->ai_src_addr,
+		       hints->ai_src_len);
+		rai->ai_src_len = hints->ai_src_len;
+	}
+
 	freeaddrinfo(ai);
 	*res = rai;
 	return 0;



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