Skip to contentSkip to conversation

RoCE Path Record refactoring

PATCH In review Leon Romanovsky 4 participants 17 messages Jan 8, 2018 – Jan 19, 2018 9 messages not in the archive InfiniBand

lore
Share & tools
lore

Leon Romanovsky

author
#0 lore raw

cover RoCE Path Record refactoring

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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20180108150448.29069-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

author
#2 lore raw

PATCH 1/6 RDMA/cma: Use the net namespace of the rdma_cm_id

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/core/cma.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/infiniband/core/cma.c +3 -2
index 62fc876169dd..282269f88f89 100644
@@ -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);
Signature · 6 lines
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20180108150448.29069-2-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Reviewed-by
#4 lore raw

Re: [PATCH rdma-next 1/6] RDMA/cma: Use the net namespace of the rdma_cm_id

Quoted 33 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/core/cma.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/infiniband/core/cma.c +2 -1
index 62fc876169dd..282269f88f89 100644
@@ -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;

drivers/infiniband/core/cma.c static int cma_resolve_iboe_route(struct rdma_id_private *id_priv) · line 2590

Quoted 8 lines
drivers/infiniband/core/cma.c +1 -1
@@ -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 <[email protected]>

Knut
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#5 lore raw

PATCH 2/6 RDMA/cma: Provide function to set RoCE path record L2 parameters

In reply to a message not in the archive

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/core/cma.c | 88 ++++++++++++++++++++++++++-----------------
 1 file changed, 53 insertions(+), 35 deletions(-)
drivers/infiniband/core/cma.c +53 -35
index 282269f88f89..f1321baf12d8 100644
@@ -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,
Signature · 6 lines
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#6 lore raw

PATCH 3/6 RDMA/{cma, ucma}: Simplify and rename rdma_set_ib_paths

In reply to a message not in the archive

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 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(-)
drivers/infiniband/core/cma.c +5 -5
index f1321baf12d8..f1e425da926d 100644
@@ -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)
 {
drivers/infiniband/core/ucma.c +2 -2
index eb85b546e223..6d32af27bac6 100644
@@ -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;
include/rdma/rdma_cm_ib.h +4 -4
index 6947a6ba2557..6a69d71a21a5 100644
@@ -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
Signature · 6 lines
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#7 lore raw

PATCH 4/6 RDMA/{cma, ucma}: Refactor to have transport specific checks

In reply to a message not in the archive

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/core/cma.c  | 11 ++++++++++-
 drivers/infiniband/core/ucma.c |  9 +--------
 2 files changed, 11 insertions(+), 9 deletions(-)
drivers/infiniband/core/cma.c +10 -1
index f1e425da926d..68223bd56b53 100644
@@ -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;
drivers/infiniband/core/ucma.c +1 -8
index 6d32af27bac6..7b25226b608e 100644
@@ -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;
Signature · 6 lines
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20180108150448.29069-5-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

#9 lore raw

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

Quoted 47 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
 drivers/infiniband/core/cma.c  | 11 ++++++++++-
 drivers/infiniband/core/ucma.c |  9 +--------
 2 files changed, 11 insertions(+), 9 deletions(-)
drivers/infiniband/core/cma.c +11 -1
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;
drivers/infiniband/core/ucma.c +1 -0
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);

drivers/infiniband/core/ucma.c static int ucma_set_ib_path(struct ucma_context *ctx, · line 1230

Quoted 10 lines
drivers/infiniband/core/ucma.c +1 -8
 
-	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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20180110233748.GS4518-uk2M96/[email protected]

#11 lore raw

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

Quoted 89 lines

-----Original Message-----
From: Jason Gunthorpe [mailto:jgg-uk2M96/[email protected]]
Sent: Wednesday, January 10, 2018 5:38 PM
To: Leon Romanovsky <[email protected]>
Cc: Doug Ledford <[email protected]>; RDMA mailing list <linux- [email protected]>; Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>; Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>; Dasaratharaman Chandramouli <dasaratharaman.chandramouli-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; Don Hiatt <[email protected]>; Ira Weiny <[email protected]>
Subject: Re: [PATCH rdma-next 4/6] RDMA/{cma, ucma}: Refactor to have
transport specific checks

Review static int ucma_set_ib_path(struct ucma_context

Quoted 47 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>

drivers/infiniband/core/cma.c | 11 ++++++++++-
drivers/infiniband/core/ucma.c | 9 +--------

 2 files changed, 11 insertions(+), 9 deletions(-)
(quoted diff) +0 -0

b/drivers/infiniband/core/cma.c index f1e425da926d..68223bd56b53 100644
+++ b/drivers/infiniband/core/cma.c

drivers/infiniband/core/cma.c +10 -1
@@ -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;
(quoted diff) +0 -0

b/drivers/infiniband/core/ucma.c index 6d32af27bac6..7b25226b608e 100644
+++ b/drivers/infiniband/core/ucma.c

(quoted diff) +0 -0
@@ -1227,14 +1227,7 @@ static int ucma_set_ib_path(struct ucma_context

*ctx,

drivers/infiniband/core/ucma.c static int ucma_set_ib_path(struct ucma_context *ctx, · line 1230

Quoted 12 lines
drivers/infiniband/core/ucma.c +1 -8
 	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.

Quoted 6 lines

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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#12 lore raw

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

In reply to a message not in the archive
Quoted 69 lines · Jason Gunthorpe
Quoted 47 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
 drivers/infiniband/core/cma.c  | 11 ++++++++++-
 drivers/infiniband/core/ucma.c |  9 +--------
 2 files changed, 11 insertions(+), 9 deletions(-)
drivers/infiniband/core/cma.c +11 -1
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;
drivers/infiniband/core/ucma.c +1 -0
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);

drivers/infiniband/core/ucma.c static int ucma_set_ib_path(struct ucma_context *ctx, · line 1230

Quoted 10 lines
drivers/infiniband/core/ucma.c +1 -8
 
-	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.

Quoted 7 lines

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

Quoted 2 lines

Jason

A message that is not in the archive (sent off-list or not yet received): 20180111060537.GO7368-U/[email protected]

#14 lore raw

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

Quoted 4 lines · Leon Romanovsky
Quoted 4 lines

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

Quoted 3 lines

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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20180111165622.GB1309-uk2M96/[email protected]

author
#16 lore raw

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

Quoted 9 lines · Jason Gunthorpe
Quoted 4 lines · Leon Romanovsky
Quoted 4 lines

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?

Quoted 9 lines
Quoted 4 lines

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

A message that is not in the archive (sent off-list or not yet received): 20180111185004.GD15760-U/[email protected]

#18 lore raw

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

Quoted 6 lines · Leon Romanovsky
Quoted 5 lines · Jason Gunthorpe
Quoted 3 lines · Leon Romanovsky
Quoted 3 lines

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

Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line

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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#19 lore raw

PATCH 5/6 RDMA/cma: Fix setting RoCE specific path record fields

In reply to a message not in the archive

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/core/cma.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
drivers/infiniband/core/cma.c +13 -0
index 68223bd56b53..f4c6c2cbc585 100644
@@ -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:
Signature · 6 lines
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20180108150448.29069-6-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Reviewed-by Mark Bloch
#21 lore raw

Re: [PATCH rdma-next 5/6] RDMA/cma: Fix setting RoCE specific path record fields

Quoted 32 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
 drivers/infiniband/core/cma.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
drivers/infiniband/core/cma.c +3 -0
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)) {

drivers/infiniband/core/cma.c int rdma_set_ib_path(struct rdma_cm_id *id, · line 2559

Quoted 10 lines
drivers/infiniband/core/cma.c +10 -0
+		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/[email protected]>
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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
Signed-off-by Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/[email protected]>
drivers/infiniband/core/cma.c +14 -0
index 5c158cda08e31d..30d1c32a816f91 100644
@@ -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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20180111050453.GB16668-uk2M96/[email protected]

#23 lore raw

RE: [PATCH rdma-next 5/6] RDMA/cma: Fix setting RoCE specific path record fields

Quoted 112 lines

-----Original Message-----
From: Jason Gunthorpe [mailto:jgg-uk2M96/[email protected]]
Sent: Wednesday, January 10, 2018 11:05 PM
To: Leon Romanovsky <[email protected]>
Cc: Doug Ledford <[email protected]>; RDMA mailing list <linux- [email protected]>; Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>; Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>
Subject: Re: [PATCH rdma-next 5/6] RDMA/cma: Fix setting RoCE specific path
record fields

Quoted 32 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>

drivers/infiniband/core/cma.c | 13 +++++++++++++

 1 file changed, 13 insertions(+)
(quoted diff) +0 -0

b/drivers/infiniband/core/cma.c index 68223bd56b53..f4c6c2cbc585 100644
+++ b/drivers/infiniband/core/cma.c

drivers/infiniband/core/cma.c +2 -0
@@ -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)) {

drivers/infiniband/core/cma.c int rdma_set_ib_path(struct rdma_cm_id *id, · line 2559

Quoted 10 lines
drivers/infiniband/core/cma.c +10 -0
+		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/[email protected]>
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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
Signed-off-by Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/[email protected]>
drivers/infiniband/core/cma.c index renamed from drivers/infiniband/core/cma.c +0 -0

5c158cda08e31d..30d1c32a816f91 100644

drivers/infiniband/core/cma.c +15 -0
@@ -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,
Quoted 2 lines

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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#24 lore raw

PATCH 6/6 RDMA/cma: Fix returning correct path record entry for RoCE

In reply to a message not in the archive

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Reviewed-by Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/core/ucma.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/infiniband/core/ucma.c +4 -3
index 7b25226b608e..29a81e6cfc96 100644
@@ -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);
 		}
 	}
Signature · 6 lines
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#25 lore raw
In reply to a message not in the archive
Quoted 6 lines · 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.

Quoted 8 lines

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:

Quoted 1 line

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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html