[PATCH rdma-next 0/6] RoCE Path Record refactoring

[PATCH rdma-next 0/6] RoCE Path Record refactoring

[Leon Romanovsky]

Hi,

This series is mostly refactoring code in CMA area as a preparation
to RoCE containers code, however there are two small fixes which is also
targeted to rdma-next.

Thanks

Parav Pandit (6):
  RDMA/cma: Use the net namespace of the rdma_cm_id
  RDMA/cma: Provide function to set RoCE path record L2 parameters
  RDMA/{cma, ucma}: Simplify and rename rdma_set_ib_paths
  RDMA/{cma, ucma}: Refactor to have transport specific checks
  RDMA/cma: Fix setting RoCE specific path record fields
  RDMA/cma: Fix returning correct path record entry for RoCE

 drivers/infiniband/core/cma.c  | 119 +++++++++++++++++++++++++++--------------
 drivers/infiniband/core/ucma.c |  16 ++----
 include/rdma/rdma_cm_ib.h      |   8 +--
 3 files changed, 89 insertions(+), 54 deletions(-)

--
2.15.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 rdma-next 1/6] RDMA/cma: Use the net namespace of the rdma_cm_id

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The net namespace is set during create_rdma_id(), while
the cma_resolve_iboe_route() used init namespace.

The original code was added in commit fa20105e09e9 ("IB/cma: Add support
for network namespaces"), but this path wasn't in use back then.

This patch updates the code to use right namespace, as a preparation
to the following patches.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-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 | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 62fc876169dd..282269f88f89 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2567,7 +2567,8 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
 		goto err2;
 	}
 
-	ndev = dev_get_by_index(&init_net, addr->dev_addr.bound_dev_if);
+	ndev = dev_get_by_index(addr->dev_addr.net,
+				addr->dev_addr.bound_dev_if);
 	if (!ndev) {
 		ret = -ENODEV;
 		goto err2;
@@ -2583,7 +2584,7 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
 		gid_type = ib_network_to_gid_type(addr->dev_addr.network);
 	route->path_rec->rec_type = sa_conv_gid_to_pathrec_type(gid_type);
 
-	sa_path_set_ndev(route->path_rec, &init_net);
+	sa_path_set_ndev(route->path_rec, addr->dev_addr.net);
 	sa_path_set_ifindex(route->path_rec, ndev->ifindex);
 	sa_path_set_dmac(route->path_rec, addr->dev_addr.dst_dev_addr);
 
-- 
2.15.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 rdma-next 1/6] RDMA/cma: Use the net namespace of the rdma_cm_id

[Knut Omang]

On Mon, 2018-01-08 at 17:04 +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> The net namespace is set during create_rdma_id(), while
> the cma_resolve_iboe_route() used init namespace.
> 
> The original code was added in commit fa20105e09e9 ("IB/cma: Add support
> for network namespaces"), but this path wasn't in use back then.
> 
> This patch updates the code to use right namespace, as a preparation
> to the following patches.
> 
> Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-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 | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 62fc876169dd..282269f88f89 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -2567,7 +2567,8 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
>  		goto err2;
>  	}
>  
> -	ndev = dev_get_by_index(&init_net, addr->dev_addr.bound_dev_if);
> +	ndev = dev_get_by_index(addr->dev_addr.net,
> +				addr->dev_addr.bound_dev_if);
>  	if (!ndev) {
>  		ret = -ENODEV;
>  		goto err2;
> @@ -2583,7 +2584,7 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
>  		gid_type = ib_network_to_gid_type(addr->dev_addr.network);
>  	route->path_rec->rec_type = sa_conv_gid_to_pathrec_type(gid_type);
>  
> -	sa_path_set_ndev(route->path_rec, &init_net);
> +	sa_path_set_ndev(route->path_rec, addr->dev_addr.net);
>  	sa_path_set_ifindex(route->path_rec, ndev->ifindex);
>  	sa_path_set_dmac(route->path_rec, addr->dev_addr.dst_dev_addr);
> 

Reviewed-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Knut
--
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 2/6] RDMA/cma: Provide function to set RoCE path record L2 parameters

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Introduce helper function to set path record L2 fields for RoCE.
This includes setting GID type, destination mac address and netdev
ifindex.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-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 | 88 ++++++++++++++++++++++++++-----------------
 1 file changed, 53 insertions(+), 35 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 282269f88f89..f1321baf12d8 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2466,6 +2466,58 @@ static int cma_resolve_ib_route(struct rdma_id_private *id_priv, int timeout_ms)
 	return ret;
 }
 
+static enum ib_gid_type cma_route_gid_type(enum rdma_network_type network_type,
+					   unsigned long supported_gids,
+					   enum ib_gid_type default_gid)
+{
+	if ((network_type == RDMA_NETWORK_IPV4 ||
+	     network_type == RDMA_NETWORK_IPV6) &&
+	    test_bit(IB_GID_TYPE_ROCE_UDP_ENCAP, &supported_gids))
+		return IB_GID_TYPE_ROCE_UDP_ENCAP;
+
+	return default_gid;
+}
+
+/*
+ * cma_iboe_set_path_rec_l2_fields() is helper function which sets
+ * path record type based on GID type.
+ * It also sets up other L2 fields which includes destination mac address
+ * netdev ifindex, of the path record.
+ * It returns the netdev of the bound interface for this path record entry.
+ */
+static struct net_device *
+cma_iboe_set_path_rec_l2_fields(struct rdma_id_private *id_priv)
+{
+	struct rdma_route *route = &id_priv->id.route;
+	enum ib_gid_type gid_type = IB_GID_TYPE_ROCE;
+	struct rdma_addr *addr = &route->addr;
+	unsigned long supported_gids;
+	struct net_device *ndev;
+
+	if (!addr->dev_addr.bound_dev_if)
+		return NULL;
+
+	ndev = dev_get_by_index(addr->dev_addr.net,
+				addr->dev_addr.bound_dev_if);
+	if (!ndev)
+		return NULL;
+
+	supported_gids = roce_gid_type_mask_support(id_priv->id.device,
+						    id_priv->id.port_num);
+	gid_type = cma_route_gid_type(addr->dev_addr.network,
+				      supported_gids,
+				      id_priv->gid_type);
+	/* Use the hint from IP Stack to select GID Type */
+	if (gid_type < ib_network_to_gid_type(addr->dev_addr.network))
+		gid_type = ib_network_to_gid_type(addr->dev_addr.network);
+	route->path_rec->rec_type = sa_conv_gid_to_pathrec_type(gid_type);
+
+	sa_path_set_ndev(route->path_rec, addr->dev_addr.net);
+	sa_path_set_ifindex(route->path_rec, ndev->ifindex);
+	sa_path_set_dmac(route->path_rec, addr->dev_addr.dst_dev_addr);
+	return ndev;
+}
+
 int rdma_set_ib_paths(struct rdma_cm_id *id,
 		      struct sa_path_rec *path_rec, int num_paths)
 {
@@ -2523,18 +2575,6 @@ static int iboe_tos_to_sl(struct net_device *ndev, int tos)
 	return 0;
 }
 
-static enum ib_gid_type cma_route_gid_type(enum rdma_network_type network_type,
-					   unsigned long supported_gids,
-					   enum ib_gid_type default_gid)
-{
-	if ((network_type == RDMA_NETWORK_IPV4 ||
-	     network_type == RDMA_NETWORK_IPV6) &&
-	    test_bit(IB_GID_TYPE_ROCE_UDP_ENCAP, &supported_gids))
-		return IB_GID_TYPE_ROCE_UDP_ENCAP;
-
-	return default_gid;
-}
-
 static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
 {
 	struct rdma_route *route = &id_priv->id.route;
@@ -2542,8 +2582,6 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
 	struct cma_work *work;
 	int ret;
 	struct net_device *ndev;
-	enum ib_gid_type gid_type = IB_GID_TYPE_ROCE;
-	unsigned long supported_gids;
 
 	u8 default_roce_tos = id_priv->cma_dev->default_roce_tos[id_priv->id.port_num -
 					rdma_start_port(id_priv->cma_dev->device)];
@@ -2562,32 +2600,12 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
 
 	route->num_paths = 1;
 
-	if (!addr->dev_addr.bound_dev_if) {
-		ret = -ENODEV;
-		goto err2;
-	}
-
-	ndev = dev_get_by_index(addr->dev_addr.net,
-				addr->dev_addr.bound_dev_if);
+	ndev = cma_iboe_set_path_rec_l2_fields(id_priv);
 	if (!ndev) {
 		ret = -ENODEV;
 		goto err2;
 	}
 
-	supported_gids = roce_gid_type_mask_support(id_priv->id.device,
-						    id_priv->id.port_num);
-	gid_type = cma_route_gid_type(addr->dev_addr.network,
-				      supported_gids,
-				      id_priv->gid_type);
-	/* Use the hint from IP Stack to select GID Type */
-	if (gid_type < ib_network_to_gid_type(addr->dev_addr.network))
-		gid_type = ib_network_to_gid_type(addr->dev_addr.network);
-	route->path_rec->rec_type = sa_conv_gid_to_pathrec_type(gid_type);
-
-	sa_path_set_ndev(route->path_rec, addr->dev_addr.net);
-	sa_path_set_ifindex(route->path_rec, ndev->ifindex);
-	sa_path_set_dmac(route->path_rec, addr->dev_addr.dst_dev_addr);
-
 	rdma_ip2gid((struct sockaddr *)&id_priv->id.route.addr.src_addr,
 		    &route->path_rec->sgid);
 	rdma_ip2gid((struct sockaddr *)&id_priv->id.route.addr.dst_addr,
-- 
2.15.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 rdma-next 3/6] RDMA/{cma, ucma}: Simplify and rename rdma_set_ib_paths

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Since 2006 there has been no user of rdmacm based application to make use
of setting multiple path records using rdma_set_ib_paths API.

Therefore code is simplified to allow setting one path record entry.
Now that it sets only single path, it is renamed to reflect the same.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-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  | 10 +++++-----
 drivers/infiniband/core/ucma.c |  4 ++--
 include/rdma/rdma_cm_ib.h      |  8 ++++----
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index f1321baf12d8..f1e425da926d 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2518,8 +2518,8 @@ cma_iboe_set_path_rec_l2_fields(struct rdma_id_private *id_priv)
 	return ndev;
 }
 
-int rdma_set_ib_paths(struct rdma_cm_id *id,
-		      struct sa_path_rec *path_rec, int num_paths)
+int rdma_set_ib_path(struct rdma_cm_id *id,
+		     struct sa_path_rec *path_rec)
 {
 	struct rdma_id_private *id_priv;
 	int ret;
@@ -2529,20 +2529,20 @@ int rdma_set_ib_paths(struct rdma_cm_id *id,
 			   RDMA_CM_ROUTE_RESOLVED))
 		return -EINVAL;
 
-	id->route.path_rec = kmemdup(path_rec, sizeof *path_rec * num_paths,
+	id->route.path_rec = kmemdup(path_rec, sizeof(*path_rec),
 				     GFP_KERNEL);
 	if (!id->route.path_rec) {
 		ret = -ENOMEM;
 		goto err;
 	}
 
-	id->route.num_paths = num_paths;
+	id->route.num_paths = 1;
 	return 0;
 err:
 	cma_comp_exch(id_priv, RDMA_CM_ROUTE_RESOLVED, RDMA_CM_ADDR_RESOLVED);
 	return ret;
 }
-EXPORT_SYMBOL(rdma_set_ib_paths);
+EXPORT_SYMBOL(rdma_set_ib_path);
 
 static int cma_resolve_iw_route(struct rdma_id_private *id_priv, int timeout_ms)
 {
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index eb85b546e223..6d32af27bac6 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1231,9 +1231,9 @@ static int ucma_set_ib_path(struct ucma_context *ctx,
 		struct sa_path_rec opa;
 
 		sa_convert_path_ib_to_opa(&opa, &sa_path);
-		ret = rdma_set_ib_paths(ctx->cm_id, &opa, 1);
+		ret = rdma_set_ib_path(ctx->cm_id, &opa);
 	} else {
-		ret = rdma_set_ib_paths(ctx->cm_id, &sa_path, 1);
+		ret = rdma_set_ib_path(ctx->cm_id, &sa_path);
 	}
 	if (ret)
 		return ret;
diff --git a/include/rdma/rdma_cm_ib.h b/include/rdma/rdma_cm_ib.h
index 6947a6ba2557..6a69d71a21a5 100644
--- a/include/rdma/rdma_cm_ib.h
+++ b/include/rdma/rdma_cm_ib.h
@@ -36,17 +36,17 @@
 #include <rdma/rdma_cm.h>
 
 /**
- * rdma_set_ib_paths - Manually sets the path records used to establish a
+ * rdma_set_ib_path - Manually sets the path record used to establish a
  *   connection.
  * @id: Connection identifier associated with the request.
  * @path_rec: Reference to the path record
  *
  * This call permits a user to specify routing information for rdma_cm_id's
- * bound to Infiniband devices.  It is called on the client side of a
+ * bound to InfiniBand devices. It is called on the client side of a
  * connection and replaces the call to rdma_resolve_route.
  */
-int rdma_set_ib_paths(struct rdma_cm_id *id,
-		      struct sa_path_rec *path_rec, int num_paths);
+int rdma_set_ib_path(struct rdma_cm_id *id,
+		     struct sa_path_rec *path_rec);
 
 /* Global qkey for UDP QPs and multicast groups. */
 #define RDMA_UDP_QKEY 0x01234567
-- 
2.15.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 rdma-next 4/6] RDMA/{cma, ucma}: Refactor to have transport specific checks

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Code is refactored to perform transport specific checks for OPA, IB,
RoCE to be done in core routine. Wherever possible, it is better to avoid
spreading transport specific code in multiple kernel modules.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-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  | 11 ++++++++++-
 drivers/infiniband/core/ucma.c |  9 +--------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index f1e425da926d..68223bd56b53 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2522,14 +2522,23 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
 		     struct sa_path_rec *path_rec)
 {
 	struct rdma_id_private *id_priv;
+	struct sa_path_rec *in_path_rec;
+	struct sa_path_rec opa;
 	int ret;
 
+	if (rdma_cap_opa_ah(id->device, id->port_num)) {
+		sa_convert_path_ib_to_opa(&opa, path_rec);
+		in_path_rec = &opa;
+	} else {
+		in_path_rec = path_rec;
+	}
+
 	id_priv = container_of(id, struct rdma_id_private, id);
 	if (!cma_comp_exch(id_priv, RDMA_CM_ADDR_RESOLVED,
 			   RDMA_CM_ROUTE_RESOLVED))
 		return -EINVAL;
 
-	id->route.path_rec = kmemdup(path_rec, sizeof(*path_rec),
+	id->route.path_rec = kmemdup(in_path_rec, sizeof(*in_path_rec),
 				     GFP_KERNEL);
 	if (!id->route.path_rec) {
 		ret = -ENOMEM;
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index 6d32af27bac6..7b25226b608e 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1227,14 +1227,7 @@ static int ucma_set_ib_path(struct ucma_context *ctx,
 	sa_path.rec_type = SA_PATH_REC_TYPE_IB;
 	ib_sa_unpack_path(path_data->path_rec, &sa_path);
 
-	if (rdma_cap_opa_ah(ctx->cm_id->device, ctx->cm_id->port_num)) {
-		struct sa_path_rec opa;
-
-		sa_convert_path_ib_to_opa(&opa, &sa_path);
-		ret = rdma_set_ib_path(ctx->cm_id, &opa);
-	} else {
-		ret = rdma_set_ib_path(ctx->cm_id, &sa_path);
-	}
+	ret = rdma_set_ib_path(ctx->cm_id, &sa_path);
 	if (ret)
 		return ret;
 
-- 
2.15.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 rdma-next 4/6] RDMA/{cma, ucma}: Refactor to have transport specific checks

[Jason Gunthorpe]

On Mon, Jan 08, 2018 at 05:04:46PM +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Code is refactored to perform transport specific checks for OPA, IB,
> RoCE to be done in core routine. Wherever possible, it is better to avoid
> spreading transport specific code in multiple kernel modules.
> 
> Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-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  | 11 ++++++++++-
>  drivers/infiniband/core/ucma.c |  9 +--------
>  2 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index f1e425da926d..68223bd56b53 100644
> +++ b/drivers/infiniband/core/cma.c
> @@ -2522,14 +2522,23 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
>  		     struct sa_path_rec *path_rec)
>  {
>  	struct rdma_id_private *id_priv;
> +	struct sa_path_rec *in_path_rec;
> +	struct sa_path_rec opa;
>  	int ret;
>  
> +	if (rdma_cap_opa_ah(id->device, id->port_num)) {
> +		sa_convert_path_ib_to_opa(&opa, path_rec);
> +		in_path_rec = &opa;
> +	} else {
> +		in_path_rec = path_rec;
> +	}
> +
>  	id_priv = container_of(id, struct rdma_id_private, id);
>  	if (!cma_comp_exch(id_priv, RDMA_CM_ADDR_RESOLVED,
>  			   RDMA_CM_ROUTE_RESOLVED))
>  		return -EINVAL;
>  
> -	id->route.path_rec = kmemdup(path_rec, sizeof(*path_rec),
> +	id->route.path_rec = kmemdup(in_path_rec, sizeof(*in_path_rec),
>  				     GFP_KERNEL);
>  	if (!id->route.path_rec) {
>  		ret = -ENOMEM;
> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index 6d32af27bac6..7b25226b608e 100644
> +++ b/drivers/infiniband/core/ucma.c
> @@ -1227,14 +1227,7 @@ static int ucma_set_ib_path(struct ucma_context *ctx,
>  	sa_path.rec_type = SA_PATH_REC_TYPE_IB;
>  	ib_sa_unpack_path(path_data->path_rec, &sa_path);
>  
> -	if (rdma_cap_opa_ah(ctx->cm_id->device, ctx->cm_id->port_num)) {
> -		struct sa_path_rec opa;
> -
> -		sa_convert_path_ib_to_opa(&opa, &sa_path);
> -		ret = rdma_set_ib_path(ctx->cm_id, &opa);
> -	} else {
> -		ret = rdma_set_ib_path(ctx->cm_id, &sa_path);
> -	}
> +	ret = rdma_set_ib_path(ctx->cm_id, &sa_path);


No, this is not right

sa_convert_path_ib_to_opa is misnamed and the usage here is arguably
misplaced.

It is converting the internal OPA path record into a compability path
record. The ONLY place a compatability path record should be used is
at the uapi boundary when copying to userspace.

It is certainly not the right direction to have the core APIs
degrade the path record.

If anything should be fixed here, it is to move the degradation closer
to the actual copy_to_user.

Why was it put here anyhow? I don't see a uapi boundary?

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/6] RDMA/{cma, ucma}: Refactor to have transport specific checks

[Parav Pandit]

> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgg-uk2M96/98Pc@public.gmane.org]
> Sent: Wednesday, January 10, 2018 5:38 PM
> To: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>; RDMA mailing list <linux-
> rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>; Parav Pandit
> <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>; Dasaratharaman Chandramouli
> <dasaratharaman.chandramouli-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>;
> Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Subject: Re: [PATCH rdma-next 4/6] RDMA/{cma, ucma}: Refactor to have
> transport specific checks
> 
> On Mon, Jan 08, 2018 at 05:04:46PM +0200, Leon Romanovsky wrote:
> > From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > Code is refactored to perform transport specific checks for OPA, IB,
> > RoCE to be done in core routine. Wherever possible, it is better to
> > avoid spreading transport specific code in multiple kernel modules.
> >
> > Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Reviewed-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  | 11 ++++++++++-
> > drivers/infiniband/core/ucma.c |  9 +--------
> >  2 files changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/cma.c
> > b/drivers/infiniband/core/cma.c index f1e425da926d..68223bd56b53
> > 100644
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -2522,14 +2522,23 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
> >  		     struct sa_path_rec *path_rec)
> >  {
> >  	struct rdma_id_private *id_priv;
> > +	struct sa_path_rec *in_path_rec;
> > +	struct sa_path_rec opa;
> >  	int ret;
> >
> > +	if (rdma_cap_opa_ah(id->device, id->port_num)) {
> > +		sa_convert_path_ib_to_opa(&opa, path_rec);
> > +		in_path_rec = &opa;
> > +	} else {
> > +		in_path_rec = path_rec;
> > +	}
> > +
> >  	id_priv = container_of(id, struct rdma_id_private, id);
> >  	if (!cma_comp_exch(id_priv, RDMA_CM_ADDR_RESOLVED,
> >  			   RDMA_CM_ROUTE_RESOLVED))
> >  		return -EINVAL;
> >
> > -	id->route.path_rec = kmemdup(path_rec, sizeof(*path_rec),
> > +	id->route.path_rec = kmemdup(in_path_rec, sizeof(*in_path_rec),
> >  				     GFP_KERNEL);
> >  	if (!id->route.path_rec) {
> >  		ret = -ENOMEM;
> > diff --git a/drivers/infiniband/core/ucma.c
> > b/drivers/infiniband/core/ucma.c index 6d32af27bac6..7b25226b608e
> > 100644
> > +++ b/drivers/infiniband/core/ucma.c
> > @@ -1227,14 +1227,7 @@ static int ucma_set_ib_path(struct ucma_context
> *ctx,
> >  	sa_path.rec_type = SA_PATH_REC_TYPE_IB;
> >  	ib_sa_unpack_path(path_data->path_rec, &sa_path);
> >
> > -	if (rdma_cap_opa_ah(ctx->cm_id->device, ctx->cm_id->port_num)) {
> > -		struct sa_path_rec opa;
> > -
> > -		sa_convert_path_ib_to_opa(&opa, &sa_path);
> > -		ret = rdma_set_ib_path(ctx->cm_id, &opa);
> > -	} else {
> > -		ret = rdma_set_ib_path(ctx->cm_id, &sa_path);
> > -	}
> > +	ret = rdma_set_ib_path(ctx->cm_id, &sa_path);
> 
> 
> No, this is not right
> 
> sa_convert_path_ib_to_opa is misnamed and the usage here is arguably
> misplaced.
> 
> It is converting the internal OPA path record into a compability path record. The
> ONLY place a compatability path record should be used is at the uapi boundary
> when copying to userspace.
> 
> It is certainly not the right direction to have the core APIs degrade the path
> record.
> 
> If anything should be fixed here, it is to move the degradation closer to the
> actual copy_to_user.
> 
> Why was it put here anyhow? I don't see a uapi boundary?
> 
>From git blame result, it was put in below patch.
57520751445b837c20a8e658e3dae3a7e7ddf45c
IB/SA: Add OPA path record type
--
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/6] RDMA/{cma, ucma}: Refactor to have transport specific checks

[Leon Romanovsky]

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

On Wed, Jan 10, 2018 at 04:37:48PM -0700, Jason Gunthorpe wrote:
> On Mon, Jan 08, 2018 at 05:04:46PM +0200, Leon Romanovsky wrote:
> > From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > Code is refactored to perform transport specific checks for OPA, IB,
> > RoCE to be done in core routine. Wherever possible, it is better to avoid
> > spreading transport specific code in multiple kernel modules.
> >
> > Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Reviewed-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  | 11 ++++++++++-
> >  drivers/infiniband/core/ucma.c |  9 +--------
> >  2 files changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> > index f1e425da926d..68223bd56b53 100644
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -2522,14 +2522,23 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
> >  		     struct sa_path_rec *path_rec)
> >  {
> >  	struct rdma_id_private *id_priv;
> > +	struct sa_path_rec *in_path_rec;
> > +	struct sa_path_rec opa;
> >  	int ret;
> >
> > +	if (rdma_cap_opa_ah(id->device, id->port_num)) {
> > +		sa_convert_path_ib_to_opa(&opa, path_rec);
> > +		in_path_rec = &opa;
> > +	} else {
> > +		in_path_rec = path_rec;
> > +	}
> > +
> >  	id_priv = container_of(id, struct rdma_id_private, id);
> >  	if (!cma_comp_exch(id_priv, RDMA_CM_ADDR_RESOLVED,
> >  			   RDMA_CM_ROUTE_RESOLVED))
> >  		return -EINVAL;
> >
> > -	id->route.path_rec = kmemdup(path_rec, sizeof(*path_rec),
> > +	id->route.path_rec = kmemdup(in_path_rec, sizeof(*in_path_rec),
> >  				     GFP_KERNEL);
> >  	if (!id->route.path_rec) {
> >  		ret = -ENOMEM;
> > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> > index 6d32af27bac6..7b25226b608e 100644
> > +++ b/drivers/infiniband/core/ucma.c
> > @@ -1227,14 +1227,7 @@ static int ucma_set_ib_path(struct ucma_context *ctx,
> >  	sa_path.rec_type = SA_PATH_REC_TYPE_IB;
> >  	ib_sa_unpack_path(path_data->path_rec, &sa_path);
> >
> > -	if (rdma_cap_opa_ah(ctx->cm_id->device, ctx->cm_id->port_num)) {
> > -		struct sa_path_rec opa;
> > -
> > -		sa_convert_path_ib_to_opa(&opa, &sa_path);
> > -		ret = rdma_set_ib_path(ctx->cm_id, &opa);
> > -	} else {
> > -		ret = rdma_set_ib_path(ctx->cm_id, &sa_path);
> > -	}
> > +	ret = rdma_set_ib_path(ctx->cm_id, &sa_path);
>
>
> No, this is not right
>
> sa_convert_path_ib_to_opa is misnamed and the usage here is arguably
> misplaced.
>
> It is converting the internal OPA path record into a compability path
> record. The ONLY place a compatability path record should be used is
> at the uapi boundary when copying to userspace.
>
> It is certainly not the right direction to have the core APIs
> degrade the path record.
>
> If anything should be fixed here, it is to move the degradation closer
> to the actual copy_to_user.
>
> Why was it put here anyhow? I don't see a uapi boundary?

As Parav wrote, it was added in commit 57520751445b ("IB/SA: Add OPA
path record type") and it looks like this sa_convert_path_ib_to_opa()
can be deleted, because there is the same conversion in ucma_query_path().

Thanks

>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-next 4/6] RDMA/{cma, ucma}: Refactor to have transport specific checks

[Jason Gunthorpe]

On Thu, Jan 11, 2018 at 08:05:37AM +0200, Leon Romanovsky wrote:

> > If anything should be fixed here, it is to move the degradation closer
> > to the actual copy_to_user.
> >
> > Why was it put here anyhow? I don't see a uapi boundary?

Actually, looking again, in the morning, it seems placed OK. This is
about as close to the copy_to_user as possible in this flow..

> As Parav wrote, it was added in commit 57520751445b ("IB/SA: Add OPA
> path record type") and it looks like this sa_convert_path_ib_to_opa()
> can be deleted, because there is the same conversion in ucma_query_path().

ucma_path_query is OK, the sa_convert_path_opa_to_ib is directly before 
the copy_to_user.

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/6] RDMA/{cma, ucma}: Refactor to have transport specific checks

[Leon Romanovsky]

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

On Thu, Jan 11, 2018 at 09:56:22AM -0700, Jason Gunthorpe wrote:
> On Thu, Jan 11, 2018 at 08:05:37AM +0200, Leon Romanovsky wrote:
>
> > > If anything should be fixed here, it is to move the degradation closer
> > > to the actual copy_to_user.
> > >
> > > Why was it put here anyhow? I don't see a uapi boundary?
>
> Actually, looking again, in the morning, it seems placed OK. This is
> about as close to the copy_to_user as possible in this flow..

So, are you going to pick up?

>
> > As Parav wrote, it was added in commit 57520751445b ("IB/SA: Add OPA
> > path record type") and it looks like this sa_convert_path_ib_to_opa()
> > can be deleted, because there is the same conversion in ucma_query_path().
>
> ucma_path_query is OK, the sa_convert_path_opa_to_ib is directly before
> the copy_to_user.
>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-next 4/6] RDMA/{cma, ucma}: Refactor to have transport specific checks

[Jason Gunthorpe]

On Thu, Jan 11, 2018 at 08:50:04PM +0200, Leon Romanovsky wrote:
> On Thu, Jan 11, 2018 at 09:56:22AM -0700, Jason Gunthorpe wrote:
> > On Thu, Jan 11, 2018 at 08:05:37AM +0200, Leon Romanovsky wrote:
> >
> > > > If anything should be fixed here, it is to move the degradation closer
> > > > to the actual copy_to_user.
> > > >
> > > > Why was it put here anyhow? I don't see a uapi boundary?
> >
> > Actually, looking again, in the morning, it seems placed OK. This is
> > about as close to the copy_to_user as possible in this flow..
> 
> So, are you going to pick up?

I ment the current code is OK as is and the sa_covert_path is as close
to the copy_to/from_user as it can be.

What is wrong about this patch is moving the uapi translation further from
the uapi boundary. So still dropped.

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 rdma-next 5/6] RDMA/cma: Fix setting RoCE specific path record fields

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

rdma_set_ib_path() missed setting path record fields for RoCE
transport when RoCE transport support was added.

This results into setting incorrect ndev, destination mac address,
incorrect GID type etc errors. This patch uses recently introduced
helper function to setup such RoCE specific path record fields.

Fixes: 3c86aa70bf67 ("RDMA/cm: Add RDMA CM support for IBoE devices")
Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-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 | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 68223bd56b53..f4c6c2cbc585 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2523,6 +2523,7 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
 {
 	struct rdma_id_private *id_priv;
 	struct sa_path_rec *in_path_rec;
+	struct net_device *ndev;
 	struct sa_path_rec opa;
 	int ret;
 
@@ -2545,6 +2546,18 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
 		goto err;
 	}
 
+	if (rdma_protocol_roce(id->device, id->port_num)) {
+		ndev = cma_iboe_set_path_rec_l2_fields(id_priv);
+		if (!ndev) {
+			ret = -ENODEV;
+			kfree(id->route.path_rec);
+			id->route.path_rec = NULL;
+			goto err;
+		} else {
+			dev_put(ndev);
+		}
+	}
+
 	id->route.num_paths = 1;
 	return 0;
 err:
-- 
2.15.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 rdma-next 5/6] RDMA/cma: Fix setting RoCE specific path record fields

[Jason Gunthorpe]

On Mon, Jan 08, 2018 at 05:04:47PM +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> rdma_set_ib_path() missed setting path record fields for RoCE
> transport when RoCE transport support was added.
> 
> This results into setting incorrect ndev, destination mac address,
> incorrect GID type etc errors. This patch uses recently introduced
> helper function to setup such RoCE specific path record fields.
> 
> Fixes: 3c86aa70bf67 ("RDMA/cm: Add RDMA CM support for IBoE devices")
> Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-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 | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 68223bd56b53..f4c6c2cbc585 100644
> +++ b/drivers/infiniband/core/cma.c
> @@ -2523,6 +2523,7 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
>  {
>  	struct rdma_id_private *id_priv;
>  	struct sa_path_rec *in_path_rec;
> +	struct net_device *ndev;
>  	struct sa_path_rec opa;
>  	int ret;
>  
> @@ -2545,6 +2546,18 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
>  		goto err;
>  	}
>  
> +	if (rdma_protocol_roce(id->device, id->port_num)) {
> +		ndev = cma_iboe_set_path_rec_l2_fields(id_priv);
> +		if (!ndev) {
> +			ret = -ENODEV;
> +			kfree(id->route.path_rec);
> +			id->route.path_rec = NULL;
> +			goto err;
> +		} else {
> +			dev_put(ndev);
> +		}
> +	}

I revised this slightly, I didn't like mixing goto error unwind and
in-lined error unwind in the same function, not the pointless else
block.

Please check my work:

Author: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Date:   Mon Jan 8 17:04:47 2018 +0200

    RDMA/cma: Fix rdma_cm raw IB path setting for RoCE
    
    rdma_set_ib_path() missed setting path record fields for RoCE
    transport when RoCE support was added.
    
    This results in setting incorrect ndev, destination mac address,
    incorrect GID type etc errors when user space attempts to set a raw
    IB path using the roce IB path compatibility mapping from userspace.
    
    Fixes: 3c86aa70bf67 ("RDMA/cm: Add RDMA CM support for IBoE devices")
    Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
    Reviewed-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
    Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
    Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 5c158cda08e31d..30d1c32a816f91 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2521,6 +2521,7 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
 		     struct sa_path_rec *path_rec)
 {
 	struct rdma_id_private *id_priv;
+	struct net_device *ndev;
 	int ret;
 
 	id_priv = container_of(id, struct rdma_id_private, id);
@@ -2535,8 +2536,21 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
 		goto err;
 	}
 
+	if (rdma_protocol_roce(id->device, id->port_num)) {
+		ndev = cma_iboe_set_path_rec_l2_fields(id_priv);
+		if (!ndev) {
+			ret = -ENODEV;
+			goto err_free;
+		}
+		dev_put(ndev);
+	}
+
 	id->route.num_paths = 1;
 	return 0;
+
+err_free:
+	kfree(id->route.path_rec);
+	id->route.path_rec = NULL;
 err:
 	cma_comp_exch(id_priv, RDMA_CM_ROUTE_RESOLVED, RDMA_CM_ADDR_RESOLVED);
 	return ret;
--
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 5/6] RDMA/cma: Fix setting RoCE specific path record fields

[Parav Pandit]

> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgg-uk2M96/98Pc@public.gmane.org]
> Sent: Wednesday, January 10, 2018 11:05 PM
> To: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>; RDMA mailing list <linux-
> rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>; Parav Pandit
> <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Subject: Re: [PATCH rdma-next 5/6] RDMA/cma: Fix setting RoCE specific path
> record fields
> 
> On Mon, Jan 08, 2018 at 05:04:47PM +0200, Leon Romanovsky wrote:
> > From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > rdma_set_ib_path() missed setting path record fields for RoCE
> > transport when RoCE transport support was added.
> >
> > This results into setting incorrect ndev, destination mac address,
> > incorrect GID type etc errors. This patch uses recently introduced
> > helper function to setup such RoCE specific path record fields.
> >
> > Fixes: 3c86aa70bf67 ("RDMA/cm: Add RDMA CM support for IBoE devices")
> > Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Reviewed-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 | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/infiniband/core/cma.c
> > b/drivers/infiniband/core/cma.c index 68223bd56b53..f4c6c2cbc585
> > 100644
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -2523,6 +2523,7 @@ int rdma_set_ib_path(struct rdma_cm_id *id,  {
> >  	struct rdma_id_private *id_priv;
> >  	struct sa_path_rec *in_path_rec;
> > +	struct net_device *ndev;
> >  	struct sa_path_rec opa;
> >  	int ret;
> >
> > @@ -2545,6 +2546,18 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
> >  		goto err;
> >  	}
> >
> > +	if (rdma_protocol_roce(id->device, id->port_num)) {
> > +		ndev = cma_iboe_set_path_rec_l2_fields(id_priv);
> > +		if (!ndev) {
> > +			ret = -ENODEV;
> > +			kfree(id->route.path_rec);
> > +			id->route.path_rec = NULL;
> > +			goto err;
> > +		} else {
> > +			dev_put(ndev);
> > +		}
> > +	}
> 
> I revised this slightly, I didn't like mixing goto error unwind and in-lined error
> unwind in the same function, not the pointless else block.
> 
> Please check my work:
> 
> Author: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Date:   Mon Jan 8 17:04:47 2018 +0200
> 
>     RDMA/cma: Fix rdma_cm raw IB path setting for RoCE
> 
>     rdma_set_ib_path() missed setting path record fields for RoCE
>     transport when RoCE support was added.
> 
>     This results in setting incorrect ndev, destination mac address,
>     incorrect GID type etc errors when user space attempts to set a raw
>     IB path using the roce IB path compatibility mapping from userspace.
> 
>     Fixes: 3c86aa70bf67 ("RDMA/cm: Add RDMA CM support for IBoE devices")
>     Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>     Reviewed-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>     Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>     Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c index
> 5c158cda08e31d..30d1c32a816f91 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -2521,6 +2521,7 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
>  		     struct sa_path_rec *path_rec)
>  {
>  	struct rdma_id_private *id_priv;
> +	struct net_device *ndev;
>  	int ret;
> 
>  	id_priv = container_of(id, struct rdma_id_private, id); @@ -2535,8
> +2536,21 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
>  		goto err;
>  	}
> 
> +	if (rdma_protocol_roce(id->device, id->port_num)) {
> +		ndev = cma_iboe_set_path_rec_l2_fields(id_priv);
> +		if (!ndev) {
> +			ret = -ENODEV;
> +			goto err_free;
> +		}
> +		dev_put(ndev);
> +	}
> +
>  	id->route.num_paths = 1;
>  	return 0;
> +
> +err_free:
> +	kfree(id->route.path_rec);
> +	id->route.path_rec = NULL;
>  err:
>  	cma_comp_exch(id_priv, RDMA_CM_ROUTE_RESOLVED,
> RDMA_CM_ADDR_RESOLVED);
>  	return ret;
Looks good.
--
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 6/6] RDMA/cma: Fix returning correct path record entry for RoCE

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

There are 4 types of path records IB, OPA, RoCEv1, RoCEv2.
Recent change only considered two path records - IB and OPA.
This results into converting RoCE path records as OPA and returning
garbage data in path record entry.
This incorrect decoding broke applications using RoCE transport.
IB, RoCEv1 and RoCEv2 are identical path records to user.
This patch avoids decoding RoCE(v1,v2) path record as OPA path record.

Fixes: 57520751445b ("IB/SA: Add OPA path record type")
Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/ucma.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index 7b25226b608e..29a81e6cfc96 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -904,13 +904,14 @@ static ssize_t ucma_query_path(struct ucma_context *ctx,
 
 		resp->path_data[i].flags = IB_PATH_GMP | IB_PATH_PRIMARY |
 					   IB_PATH_BIDIRECTIONAL;
-		if (rec->rec_type == SA_PATH_REC_TYPE_IB) {
-			ib_sa_pack_path(rec, &resp->path_data[i].path_rec);
-		} else {
+		if (rec->rec_type == SA_PATH_REC_TYPE_OPA) {
 			struct sa_path_rec ib;
 
 			sa_convert_path_opa_to_ib(&ib, rec);
 			ib_sa_pack_path(&ib, &resp->path_data[i].path_rec);
+
+		} else {
+			ib_sa_pack_path(rec, &resp->path_data[i].path_rec);
 		}
 	}
 
-- 
2.15.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 rdma-next 0/6] RoCE Path Record refactoring

[Jason Gunthorpe]

On Mon, Jan 08, 2018 at 05:04:42PM +0200, Leon Romanovsky wrote:
> Hi,
> 
> This series is mostly refactoring code in CMA area as a preparation
> to RoCE containers code, however there are two small fixes which is also
> targeted to rdma-next.
> 
> Thanks
> 
> Parav Pandit (6):
>   RDMA/cma: Use the net namespace of the rdma_cm_id
>   RDMA/cma: Provide function to set RoCE path record L2 parameters
>   RDMA/{cma, ucma}: Simplify and rename rdma_set_ib_paths
>   RDMA/cma: Fix setting RoCE specific path record fields
>   RDMA/cma: Fix returning correct path record entry for RoCE

The above were applied to for-next, thanks. I revised some of the commit
messages.

I dropped this patch:

>   RDMA/{cma, ucma}: Refactor to have transport specific checks

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