[ewg] Re: [ofa-general] [PATCH] librdmacm: provide wrapper functions to extract src/dst addresses

[Pete Wyckoff]

sean.hefty at intel.com wrote on Wed, 17 Oct 2007 11:39 -0700:
> Provide wrapper functions to retrieve the source and destination
> addresses.  This is based on feedback from Doug Ledford.
[..]
> +static inline struct sockaddr *rdma_get_src_addr(struct rdma_cm_id *id)
> +{
> +	return &id->route.addr.src_addr;
> +}
> +
> +static inline struct sockaddr *rdma_get_dst_addr(struct rdma_cm_id *id)
> +{
> +	return &id->route.addr.dst_addr;
> +}

I like the idea of making these fields easier to use, but find the
naming a bit confusing for one particular example.  Server process
does rdma_bind_addr(), rdma_listen().  Gets RDMA_CM_EVENT_CONNECT_REQUEST.
Inspects the rdma_cm_event's event->id using your new function calls
above.  The question the server asks is, "shall I permit this remote
node to connect to me, based on IP address?"  If yes, it will call
rdma_accept().

In sockets, we use getpeername(), which is natural:  the other side
is the peer.  With this new RDMA interface, one may be tempted to
use rdma_get_src_addr(), since the connecting peer was the "source"
of the connect request message, but no, rdma_get_dst_addr() is what
is required to find out the peer address.

In general, these RDMA connections are bidirectional just like
sockets, so the concept of src and dst are not clear.  You have made
the identification of "src" == "me" and "dst" == "peer".  I'd
suggest using the more natural getsockname() and getpeername()
approach from sockets, or similar.  How about

    rdma_get_local_addr(id) { return src_addr; }
    rdma_get_peer_addr(id) { return dst_addr; }

instead?

		-- Pete