[PATCH V1 rdma-core 0/2] Optimize RoCE address handle creation for userspace

[PATCH V1 rdma-core 0/2] Optimize RoCE address handle creation for userspace

[Yishai Hadas]

Sending V1 to address a note coming from Jason re symbol versioning,
details below.

This patch set from Moni is the supplementary part of the kernel series that
was lastly sent upstream.

Creating a UD address handler when link layer is Ethernet requires resolving
the remote L3 address (GID) to a L2 address (MAC/VLAN).

Until now the way to resolve GID (which is the remote IP or a function of it)
to a MAC was with an interface supplied by libnl. The implementation of this
interface is heavy and fails on large load of requests to create an address
handle.

This series of patches enables user drivers that care for it to optimize the
resolution of L3 to L2 addresses with uverbs interface.

The series was tested successfully with CX4 in both kernel which doesn't have
this support and with a kernel which includes the above support.

Pull request was sent:
https://github.com/linux-rdma/rdma-core/pull/42

Yishai

Changes from v0:
ibv_cmd_create_ah symbol version was changed to a newer version to prevent
legacy providers from being loaded when they are working with new libibverbs.
This behavior is better than getting some random error upon calling to
ibv_cmd_create_ah from a legacy provider which is out of the consolidate rdma
tree.

Test results:
------------------------------------------------------------------------------
Upon testing the above use case the provider couldn't be loaded as expected,
"libibverbs: Warning: couldn't load driver 'libmlx5-rdmav2.so':
libmlx5-rdmav2.so: cannot open shared object file: No such file or directory"

Run with strace, can see that as part of the dynamic linking loader flow (i.e.
dlopen) libmlx5-rdmav2.so was opened successfully from its default location
(i.e. /usr/lib64/libibverbs/), however, it was just later closed, apparently as
of the missing symbol, as expected. Later on the linker looked for other
locations for same library (e.g. /lib64/tls/x86_64, /lib64/tls) but didn't
find, so the flow ended up with the above error message from libibverbs.

>From strace:
open("/usr/lib64/libibverbs/libmlx5-rdmav2.so", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\20(\0\0\0\0\0\0"., 832)
     = 832
fstat(3, {st_mode=S_IFREG|0755, st_size=555006, ...}) = 0
mmap(NULL, 2192400, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0)
     = 0x7f910ba21000
mprotect(0x7f910ba38000, 2093056, PROT_NONE) = 0
mmap(0x7f910bc37000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|
				MAP_DENYWRITE, 3, 0x16000) = 0x7f910bc37000
close(3)                                = 0
munmap(0x7f910ba21000, 2192400)         = 0
open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=143158, ...}) = 0
mmap(NULL, 143158, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f910e5d0000
close(3)                                = 0
open("/lib64/tls/x86_64/libmlx5-rdmav2.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT
     (No such file or directory)

...
-------------------------------------------------------------------------------

Moni Shoua (2):
  ibverbs: Allow vendor data response in create_ah command
  mlx5: Create and destroy address handle with kernel assistance

 libibverbs/cmd.c                | 11 ++++++-----
 libibverbs/driver.h             |  4 +++-
 libibverbs/libibverbs.map       |  6 +++++-
 providers/hfi1verbs/verbs.c     |  4 +++-
 providers/ipathverbs/verbs.c    |  4 +++-
 providers/mlx5/mlx5-abi.h       |  7 +++++++
 providers/mlx5/mlx5.h           |  2 ++
 providers/mlx5/verbs.c          | 28 +++++++++++++++++++++++-----
 providers/ocrdma/ocrdma_verbs.c |  4 +++-
 providers/rxe/rxe.c             |  4 +++-
 10 files changed, 58 insertions(+), 16 deletions(-)

-- 
1.8.3.1

--
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 V1 rdma-core 1/2] ibverbs: Allow vendor data response in create_ah command

[Yishai Hadas]

From: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Let caller pass response buffer and response size to ibv_cmd_create_ah
to enable kernel driver to return some private vendor data.

Signed-off-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 libibverbs/cmd.c                | 11 ++++++-----
 libibverbs/driver.h             |  4 +++-
 libibverbs/libibverbs.map       |  6 +++++-
 providers/hfi1verbs/verbs.c     |  4 +++-
 providers/ipathverbs/verbs.c    |  4 +++-
 providers/ocrdma/ocrdma_verbs.c |  4 +++-
 providers/rxe/rxe.c             |  4 +++-
 7 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/libibverbs/cmd.c b/libibverbs/cmd.c
index 06a017b..eaf7164 100644
--- a/libibverbs/cmd.c
+++ b/libibverbs/cmd.c
@@ -1492,12 +1492,13 @@ int ibv_cmd_post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *wr,
 }
 
 int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
-		      struct ibv_ah_attr *attr)
+		      struct ibv_ah_attr *attr,
+		      struct ibv_create_ah_resp *resp,
+		      size_t resp_size)
 {
 	struct ibv_create_ah      cmd;
-	struct ibv_create_ah_resp resp;
 
-	IBV_INIT_CMD_RESP(&cmd, sizeof cmd, CREATE_AH, &resp, sizeof resp);
+	IBV_INIT_CMD_RESP(&cmd, sizeof cmd, CREATE_AH, resp, resp_size);
 	cmd.user_handle            = (uintptr_t) ah;
 	cmd.pd_handle              = pd->handle;
 	cmd.attr.dlid              = attr->dlid;
@@ -1515,9 +1516,9 @@ int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
 	if (write(pd->context->cmd_fd, &cmd, sizeof cmd) != sizeof cmd)
 		return errno;
 
-	(void) VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
+	(void) VALGRIND_MAKE_MEM_DEFINED(resp, resp_size);
 
-	ah->handle  = resp.handle;
+	ah->handle  = resp->handle;
 	ah->context = pd->context;
 
 	return 0;
diff --git a/libibverbs/driver.h b/libibverbs/driver.h
index ea3dade..c170169 100644
--- a/libibverbs/driver.h
+++ b/libibverbs/driver.h
@@ -233,7 +233,9 @@ int ibv_cmd_post_recv(struct ibv_qp *ibqp, struct ibv_recv_wr *wr,
 int ibv_cmd_post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *wr,
 			  struct ibv_recv_wr **bad_wr);
 int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
-		      struct ibv_ah_attr *attr);
+		      struct ibv_ah_attr *attr,
+		      struct ibv_create_ah_resp *resp,
+		      size_t resp_size);
 int ibv_cmd_destroy_ah(struct ibv_ah *ah);
 int ibv_cmd_attach_mcast(struct ibv_qp *qp, const union ibv_gid *gid, uint16_t lid);
 int ibv_cmd_detach_mcast(struct ibv_qp *qp, const union ibv_gid *gid, uint16_t lid);
diff --git a/libibverbs/libibverbs.map b/libibverbs/libibverbs.map
index 33cd6b6..7858e19 100644
--- a/libibverbs/libibverbs.map
+++ b/libibverbs/libibverbs.map
@@ -64,7 +64,6 @@ IBVERBS_1.0 {
 		ibv_cmd_post_send;
 		ibv_cmd_post_recv;
 		ibv_cmd_post_srq_recv;
-		ibv_cmd_create_ah;
 		ibv_cmd_destroy_ah;
 		ibv_cmd_attach_mcast;
 		ibv_cmd_detach_mcast;
@@ -131,3 +130,8 @@ IBVERBS_1.3 {
 		ibv_cmd_destroy_rwq_ind_table;
 		ibv_query_gid_type;
 } IBVERBS_1.1;
+
+IBVERBS_1.4 {
+	global:
+		ibv_cmd_create_ah;
+} IBVERBS_1.3;
diff --git a/providers/hfi1verbs/verbs.c b/providers/hfi1verbs/verbs.c
index 06ddbb7..d39a4d0 100644
--- a/providers/hfi1verbs/verbs.c
+++ b/providers/hfi1verbs/verbs.c
@@ -678,12 +678,14 @@ int hfi1_post_srq_recv(struct ibv_srq *ibsrq, struct ibv_recv_wr *wr,
 struct ibv_ah *hfi1_create_ah(struct ibv_pd *pd, struct ibv_ah_attr *attr)
 {
 	struct ibv_ah *ah;
+	struct ibv_create_ah_resp resp;
 
 	ah = malloc(sizeof *ah);
 	if (ah == NULL)
 		return NULL;
 
-	if (ibv_cmd_create_ah(pd, ah, attr)) {
+	memset(&resp, 0, sizeof(resp));
+	if (ibv_cmd_create_ah(pd, ah, attr, &resp, sizeof(resp))) {
 		free(ah);
 		return NULL;
 	}
diff --git a/providers/ipathverbs/verbs.c b/providers/ipathverbs/verbs.c
index 35b2162..2145b15 100644
--- a/providers/ipathverbs/verbs.c
+++ b/providers/ipathverbs/verbs.c
@@ -653,12 +653,14 @@ int ipath_post_srq_recv(struct ibv_srq *ibsrq, struct ibv_recv_wr *wr,
 struct ibv_ah *ipath_create_ah(struct ibv_pd *pd, struct ibv_ah_attr *attr)
 {
 	struct ibv_ah *ah;
+	struct ibv_create_ah_resp resp;
 
 	ah = malloc(sizeof *ah);
 	if (ah == NULL)
 		return NULL;
 
-	if (ibv_cmd_create_ah(pd, ah, attr)) {
+	memset(&resp, 0, sizeof(resp));
+	if (ibv_cmd_create_ah(pd, ah, attr, &resp, sizeof(resp))) {
 		free(ah);
 		return NULL;
 	}
diff --git a/providers/ocrdma/ocrdma_verbs.c b/providers/ocrdma/ocrdma_verbs.c
index 7e066dc..dce833b 100644
--- a/providers/ocrdma/ocrdma_verbs.c
+++ b/providers/ocrdma/ocrdma_verbs.c
@@ -2099,6 +2099,7 @@ struct ibv_ah *ocrdma_create_ah(struct ibv_pd *ibpd, struct ibv_ah_attr *attr)
 	int ahtbl_idx;
 	struct ocrdma_pd *pd;
 	struct ocrdma_ah *ah;
+	struct ibv_create_ah_resp resp;
 
 	pd = get_ocrdma_pd(ibpd);
 	ah = malloc(sizeof *ah);
@@ -2111,7 +2112,8 @@ struct ibv_ah *ocrdma_create_ah(struct ibv_pd *ibpd, struct ibv_ah_attr *attr)
 	if (ahtbl_idx < 0)
 		goto tbl_err;
 	attr->dlid = ahtbl_idx;
-	status = ibv_cmd_create_ah(ibpd, &ah->ibv_ah, attr);
+	memset(&resp, 0, sizeof(resp));
+	status = ibv_cmd_create_ah(ibpd, &ah->ibv_ah, attr, &resp, sizeof(resp));
 	if (status)
 		goto cmd_err;
 
diff --git a/providers/rxe/rxe.c b/providers/rxe/rxe.c
index d23ef3d..1dd30ea 100644
--- a/providers/rxe/rxe.c
+++ b/providers/rxe/rxe.c
@@ -781,6 +781,7 @@ static struct ibv_ah *rxe_create_ah(struct ibv_pd *pd, struct ibv_ah_attr *attr)
 	struct rxe_ah *ah;
 	struct rxe_av *av;
 	union ibv_gid sgid;
+	struct ibv_create_ah_resp resp;
 
 	err = ibv_query_gid(pd->context, attr->port_num, attr->grh.sgid_index,
 			    &sgid);
@@ -803,7 +804,8 @@ static struct ibv_ah *rxe_create_ah(struct ibv_pd *pd, struct ibv_ah_attr *attr)
 	rdma_gid2ip(&av->sgid_addr._sockaddr, &sgid);
 	rdma_gid2ip(&av->dgid_addr._sockaddr, &attr->grh.dgid);
 
-	if (ibv_cmd_create_ah(pd, &ah->ibv_ah, attr)) {
+	memset(&resp, 0, sizeof(resp));
+	if (ibv_cmd_create_ah(pd, &ah->ibv_ah, attr, &resp, sizeof(resp))) {
 		free(ah);
 		return NULL;
 	}
-- 
1.8.3.1

--
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 V1 rdma-core 2/2] mlx5: Create and destroy address handle with kernel assistance

[Yishai Hadas]

From: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Depending on kernel support and when link layer is Ethernet, call kernel
when asked to create an address handle. Kernel will return destination
mac which is required to create the address handle. The alternative,
when kernel support is missing, is to use netlink which is less
efficient.  Destroying an address handle also needs kernel assistance to
clean resources that were allocated during address handle creation.

Signed-off-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 providers/mlx5/mlx5-abi.h |  7 +++++++
 providers/mlx5/mlx5.h     |  2 ++
 providers/mlx5/verbs.c    | 28 +++++++++++++++++++++++-----
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/providers/mlx5/mlx5-abi.h b/providers/mlx5/mlx5-abi.h
index 4b5b577..800915d 100644
--- a/providers/mlx5/mlx5-abi.h
+++ b/providers/mlx5/mlx5-abi.h
@@ -91,6 +91,13 @@ struct mlx5_alloc_ucontext_resp {
 	__u64				hca_core_clock_offset;
 };
 
+struct mlx5_create_ah_resp {
+	struct ibv_create_ah_resp	ibv_resp;
+	__u32				response_length;
+	__u8				dmac[ETHERNET_LL_SIZE];
+	__u8				reserved[6];
+};
+
 struct mlx5_alloc_pd_resp {
 	struct ibv_alloc_pd_resp	ibv_resp;
 	__u32				pdn;
diff --git a/providers/mlx5/mlx5.h b/providers/mlx5/mlx5.h
index cb65429..cf314fc 100644
--- a/providers/mlx5/mlx5.h
+++ b/providers/mlx5/mlx5.h
@@ -217,6 +217,7 @@ enum mlx5_rsc_type {
 
 enum {
 	MLX5_USER_CMDS_SUPP_UHW_QUERY_DEVICE = 1 << 0,
+	MLX5_USER_CMDS_SUPP_UHW_CREATE_AH    = 1 << 1,
 };
 
 struct mlx5_resource {
@@ -470,6 +471,7 @@ struct mlx5_av {
 struct mlx5_ah {
 	struct ibv_ah			ibv_ah;
 	struct mlx5_av			av;
+	bool				kern_ah;
 };
 
 struct mlx5_rwq {
diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c
index e288ebf..7107a5b 100644
--- a/providers/mlx5/verbs.c
+++ b/providers/mlx5/verbs.c
@@ -1636,11 +1636,21 @@ struct ibv_ah *mlx5_create_ah(struct ibv_pd *pd, struct ibv_ah_attr *attr)
 	}
 
 	if (is_eth) {
-		uint16_t vid;
+		if (ctx->cmds_supp_uhw & MLX5_USER_CMDS_SUPP_UHW_CREATE_AH) {
+			struct mlx5_create_ah_resp resp = {};
 
-		if (ibv_resolve_eth_l2_from_gid(pd->context, attr, ah->av.rmac,
-						&vid))
-			goto err;
+			if (ibv_cmd_create_ah(pd, &ah->ibv_ah, attr, &resp.ibv_resp, sizeof(resp)))
+				goto err;
+
+			ah->kern_ah = true;
+			memcpy(ah->av.rmac, resp.dmac, ETHERNET_LL_SIZE);
+		} else {
+			uint16_t vid;
+
+			if (ibv_resolve_eth_l2_from_gid(pd->context, attr,
+							ah->av.rmac, &vid))
+				goto err;
+		}
 	}
 
 	return &ah->ibv_ah;
@@ -1651,8 +1661,16 @@ err:
 
 int mlx5_destroy_ah(struct ibv_ah *ah)
 {
-	free(to_mah(ah));
+	struct mlx5_ah *mah = to_mah(ah);
+	int err;
+
+	if (mah->kern_ah) {
+		err = ibv_cmd_destroy_ah(ah);
+		if (err)
+			return err;
+	}
 
+	free(mah);
 	return 0;
 }
 
-- 
1.8.3.1

--
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 V1 rdma-core 0/2] Optimize RoCE address handle creation for userspace

[Jason Gunthorpe]

On Sun, Dec 04, 2016 at 05:53:24PM +0200, Yishai Hadas wrote:

> Test results:
> Upon testing the above use case the provider couldn't be loaded as expected,
> "libibverbs: Warning: couldn't load driver 'libmlx5-rdmav2.so':
> libmlx5-rdmav2.so: cannot open shared object file: No such file or directory"

The dynmic linker didn't print something too? That is surprising..

I'm not sure where we are in the release cycle right now.. Doug - if
we didn't make a release with the 1.3 symbol tag (this was created
just before rdma-core) then this should go into 1.3 when it gets
merged..

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 V1 rdma-core 0/2] Optimize RoCE address handle creation for userspace

[Yishai Hadas]

On 12/5/2016 7:07 PM, Jason Gunthorpe wrote:
> On Sun, Dec 04, 2016 at 05:53:24PM +0200, Yishai Hadas wrote:
>
>> Test results:
>> Upon testing the above use case the provider couldn't be loaded as expected,
>> "libibverbs: Warning: couldn't load driver 'libmlx5-rdmav2.so':
>> libmlx5-rdmav2.so: cannot open shared object file: No such file or directory"
>
> The dynmic linker didn't print something too? That is surprising..

No more output, I was surprised as well about the error message, that's 
why I used strace to follow the system calls. Looking at the man page of 
dlopen can see that upon an error the most recent error is returned 
which was in that case ENOENT as the linker continued searching in other 
locations.

Just run with LD_LIBRARY_PATH=/usr/lib64/libibverbs which based on man 
page will cause the linker to look only in that location, I get below 
error as we expect it.

libibverbs: Warning: couldn't load driver 'libmlx5-rdmav2.so': 
/usr/lib64/libibverbs/libmlx5-rdmav2.so: symbol ibv_cmd_create_ah, 
version IBVERBS_1.0 not defined in file libibverbs.so.1 with link time 
reference

> I'm not sure where we are in the release cycle right now.. Doug - if
> we didn't make a release with the 1.3 symbol tag (this was created
> just before rdma-core) then this should go into 1.3 when it gets
> merged..

Jason,
You pointed to use 1.4, the series followed that.

Doug,
Can you please comment here ? we should have a clear definition what 
defined to be a "release" which requires a new number.
In case we want to use 1.3 please let me know if I need to re-spin or 
you will change as part of the merge.
--
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 V1 rdma-core 0/2] Optimize RoCE address handle creation for userspace

[Jason Gunthorpe]

On Tue, Dec 06, 2016 at 12:01:13AM +0200, Yishai Hadas wrote:
> >I'm not sure where we are in the release cycle right now.. Doug - if
> >we didn't make a release with the 1.3 symbol tag (this was created
> >just before rdma-core) then this should go into 1.3 when it gets
> >merged..
> 
> Jason,
> You pointed to use 1.4, the series followed that.

Yes, but I realized we may not have made a ibverbs release before
starting rdma-core.

> Doug,
> Can you please comment here ? we should have a clear definition what defined
> to be a "release" which requires a new number.
> In case we want to use 1.3 please let me know if I need to re-spin or you
> will change as part of the merge.

Doug/Leon/You have to check this stuff before merging any series that
touches a map file, beacuse it depends on when in the cycle the patch
gets merged.

After the first rdma-core release is done we should denote the map
stanzas with a comment indicating which release number they are for so
it is easier to keep track.

eg we are nearing the release for 4.9, so presumably Doug will tag a
rdma-core relase soon. If this series does not make that cutoff 1.4
would be correct.

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