DMAC RoCE refacting code

PATCH Applied Leon Romanovsky 2 participants 6 messages Jan 9, 2018 – Jan 15, 2018 1 message not in the archive InfiniBand

lore

Share & tools

lore

Leon Romanovsky

author

2018-01-09 13:24 UTC #0 lore raw

cover DMAC RoCE refacting code

Hi,

The following series is refactoring code for DMAC RoCE code and one fix to perform modify QP on real QP.

Thanks

Parav Pandit (4):
IB/core: Perform modify QP on real one IB/core: Limit DMAC resolution to userspace QPs IB/core: Attempt DMAC resolution for only RoCE IB/core: Limit DMAC resolution to RoCE Connected QPs

 drivers/infiniband/core/verbs.c | 60 ++++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 21 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): 20180109132453.29038-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Leon Romanovsky

author

2018-01-09 13:24 UTC #2 lore raw

PATCH 1/4 IB/core: Perform modify QP on real one

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

Currently qp->port stores the port number whenever IB_QP_PORT QP attribute mask is set (during QP state transition to INIT state). This port number should be stored for the real QP when XRC target QP is used.

Follow the ib_modify_qp() implementation and hide the access to ->real_qp.

Fixes a512c2fbef9c ("IB/core: Introduce modify QP operation with udata")

Signed-off-by Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

Reviewed-by Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/core/verbs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/infiniband/core/verbs.c +3 -2

index bace97c3d986..1436f8c7906a 100644
@@ -1325,7 +1325,7 @@ static int ib_resolve_eth_dmac(struct ib_device *device,
 
 /**
  * ib_modify_qp_with_udata - Modifies the attributes for the specified QP.
- * @qp: The QP to modify.
+ * @ib_qp: The QP to modify.
  * @attr: On input, specifies the QP attributes to modify.  On output,
  *   the current values of selected QP attributes are returned.
  * @attr_mask: A bit-mask used to specify which attributes of the QP

@@ -1334,9 +1334,10 @@ static int ib_resolve_eth_dmac(struct ib_device *device,
  *   are being modified.
  * It returns 0 on success and returns appropriate error code on error.
  */
-int ib_modify_qp_with_udata(struct ib_qp *qp, struct ib_qp_attr *attr,
+int ib_modify_qp_with_udata(struct ib_qp *ib_qp, struct ib_qp_attr *attr,
 			    int attr_mask, struct ib_udata *udata)
 {
+	struct ib_qp *qp = ib_qp->real_qp;
 	u8 port = attr_mask & IB_QP_PORT ? attr->port_num : qp->port;
 	int 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

Leon Romanovsky

author

2018-01-09 13:24 UTC #3 lore raw

PATCH 2/4 IB/core: Limit DMAC resolution to userspace QPs

In reply to a message not in the archive

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

Currently ah_attr is initialized by the ib_cm layer for rdma_cm based applications. For RoCE transport ah_attr.roce.dmac is already initialized by ib_cm, rdma_cm either from wc, path record, route resolve, explicit path record setting depending on active or passive side QP. Therefore avoid resolving DMAC for QP of kernel consumers.

Signed-off-by Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

Reviewed-by Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/core/verbs.c | 48 +++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 19 deletions(-)

drivers/infiniband/core/verbs.c +29 ~~-19~~

index 1436f8c7906a..d8f4f1b56f94 100644
@@ -1324,29 +1324,14 @@ static int ib_resolve_eth_dmac(struct ib_device *device,
 }
 
 /**
- * ib_modify_qp_with_udata - Modifies the attributes for the specified QP.
- * @ib_qp: The QP to modify.
- * @attr: On input, specifies the QP attributes to modify.  On output,
- *   the current values of selected QP attributes are returned.
- * @attr_mask: A bit-mask used to specify which attributes of the QP
- *   are being modified.
- * @udata: pointer to user's input output buffer information
- *   are being modified.
- * It returns 0 on success and returns appropriate error code on error.
+ * IB core internal function to perform QP attributes modification.
  */
-int ib_modify_qp_with_udata(struct ib_qp *ib_qp, struct ib_qp_attr *attr,
-			    int attr_mask, struct ib_udata *udata)
+static int _ib_modify_qp(struct ib_qp *qp, struct ib_qp_attr *attr,
+			 int attr_mask, struct ib_udata *udata)
 {
-	struct ib_qp *qp = ib_qp->real_qp;
 	u8 port = attr_mask & IB_QP_PORT ? attr->port_num : qp->port;
 	int ret;
 
-	if (attr_mask & IB_QP_AV) {
-		ret = ib_resolve_eth_dmac(qp->device, &attr->ah_attr);
-		if (ret)
-			return ret;
-	}
-
 	if (rdma_ib_or_roce(qp->device, port)) {
 		if (attr_mask & IB_QP_RQ_PSN && attr->rq_psn & ~0xffffff) {
 			pr_warn("%s: %s rq_psn overflow, masking to 24 bits\n",

@@ -1367,6 +1352,31 @@ int ib_modify_qp_with_udata(struct ib_qp *ib_qp, struct ib_qp_attr *attr,
 
 	return ret;
 }
+
+/**
+ * ib_modify_qp_with_udata - Modifies the attributes for the specified QP.
+ * @ib_qp: The QP to modify.
+ * @attr: On input, specifies the QP attributes to modify.  On output,
+ *   the current values of selected QP attributes are returned.
+ * @attr_mask: A bit-mask used to specify which attributes of the QP
+ *   are being modified.
+ * @udata: pointer to user's input output buffer information
+ *   are being modified.
+ * It returns 0 on success and returns appropriate error code on error.
+ */
+int ib_modify_qp_with_udata(struct ib_qp *ib_qp, struct ib_qp_attr *attr,
+			    int attr_mask, struct ib_udata *udata)
+{
+	struct ib_qp *qp = ib_qp->real_qp;
+	int ret;
+
+	if (attr_mask & IB_QP_AV) {
+		ret = ib_resolve_eth_dmac(qp->device, &attr->ah_attr);
+		if (ret)
+			return ret;
+	}
+	return _ib_modify_qp(qp, attr, attr_mask, udata);
+}
 EXPORT_SYMBOL(ib_modify_qp_with_udata);
 
 int ib_get_eth_speed(struct ib_device *dev, u8 port_num, u8 *speed, u8 *width)

@@ -1428,7 +1438,7 @@ int ib_modify_qp(struct ib_qp *qp,
 		 struct ib_qp_attr *qp_attr,
 		 int qp_attr_mask)
 {
-	return ib_modify_qp_with_udata(qp, qp_attr, qp_attr_mask, NULL);
+	return _ib_modify_qp(qp->real_qp, qp_attr, qp_attr_mask, NULL);
 }
 EXPORT_SYMBOL(ib_modify_qp);

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

Leon Romanovsky

author

2018-01-09 13:24 UTC #4 lore raw

PATCH 3/4 IB/core: Attempt DMAC resolution for only RoCE

In reply to a message not in the archive

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

Instead of returning 0 (success) for RoCE scenarios where DMAC should not be resolved, avoid such attempt and make code consistent with ib_create_user_ah().

Signed-off-by Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

Reviewed-by Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/core/verbs.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

drivers/infiniband/core/verbs.c +2 -4

index d8f4f1b56f94..019f1d87db46 100644
@@ -1302,9 +1302,6 @@ static int ib_resolve_eth_dmac(struct ib_device *device,
 	if (!rdma_is_port_valid(device, rdma_ah_get_port_num(ah_attr)))
 		return -EINVAL;
 
-	if (ah_attr->type != RDMA_AH_ATTR_TYPE_ROCE)
-		return 0;
-
 	grh = rdma_ah_retrieve_grh(ah_attr);
 
 	if (rdma_is_multicast_addr((struct in6_addr *)ah_attr->grh.dgid.raw)) {

@@ -1370,7 +1367,8 @@ int ib_modify_qp_with_udata(struct ib_qp *ib_qp, struct ib_qp_attr *attr,
 	struct ib_qp *qp = ib_qp->real_qp;
 	int ret;
 
-	if (attr_mask & IB_QP_AV) {
+	if (attr_mask & IB_QP_AV &&
+	    attr->ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE) {
 		ret = ib_resolve_eth_dmac(qp->device, &attr->ah_attr);
 		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

Leon Romanovsky

author

2018-01-09 13:24 UTC #5 lore raw

PATCH 4/4 IB/core: Limit DMAC resolution to RoCE Connected QPs

In reply to a message not in the archive

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

Resolving DMAC for RoCE is applicable to only Connected mode QPs. So resolve DMAC for only for Connected mode QPs.

Signed-off-by Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

Reviewed-by Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/core/verbs.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

drivers/infiniband/core/verbs.c +10 -1

index 019f1d87db46..8a3ef182c72a 100644
@@ -1350,6 +1350,14 @@ static int _ib_modify_qp(struct ib_qp *qp, struct ib_qp_attr *attr,
 	return ret;
 }
 
+static bool is_qp_type_connected(const struct ib_qp *qp)
+{
+	return (qp->qp_type == IB_QPT_UC ||
+		qp->qp_type == IB_QPT_RC ||
+		qp->qp_type == IB_QPT_XRC_INI ||
+		qp->qp_type == IB_QPT_XRC_TGT);
+}
+
 /**
  * ib_modify_qp_with_udata - Modifies the attributes for the specified QP.
  * @ib_qp: The QP to modify.

@@ -1368,7 +1376,8 @@ int ib_modify_qp_with_udata(struct ib_qp *ib_qp, struct ib_qp_attr *attr,
 	int ret;
 
 	if (attr_mask & IB_QP_AV &&
-	    attr->ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE) {
+	    attr->ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE &&
+	    is_qp_type_connected(qp)) {
 		ret = ib_resolve_eth_dmac(qp->device, &attr->ah_attr);
 		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

Jason Gunthorpe

Applied

2018-01-15 23:07 UTC #6 lore raw

In reply to a message not in the archive

Quoted 5 lines · Leon Romanovsky

Hi,
The following series is refactoring code for DMAC RoCE code and one fix to perform modify QP on real QP.

Quoted 7 lines

Thanks
Parav Pandit (4):
IB/core: Perform modify QP on real one IB/core: Limit DMAC resolution to userspace QPs IB/core: Attempt DMAC resolution for only RoCE IB/core: Limit DMAC resolution to RoCE Connected QPs

I applied all of these to for-next

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