verbs: Do not block QP attr_masks used by older kernels

PATCH In review Jason Gunthorpe 4 participants 6 messages Nov 2, 2017 – Nov 9, 2017 4 messages not in the archive InfiniBand

lore

Share & tools

lore

Jason Gunthorpe

author

2017-11-02 21:50 UTC #0 lore raw

PATCH 1/1 verbs: Do not block QP attr_masks used by older kernels

At least the RDMA CM in 4.3 will give user space structs with these bits set. Those kernels require user space to pass those set bits back to the kernel. Blocking them causes RDMA CM to be unusable on older kernels.

Fixes 3ca7a1031486 ("ibverbs: Add support for packet pacing")

Reported-by Ram Amrani <Ram.Amrani-YGCgFSpz5w/[email protected]>

Signed-off-by Jason Gunthorpe <jgunthorpe-uk2M96/[email protected]>

---
 libibverbs/cmd.c   | 12 ++++++------
 libibverbs/verbs.h |  9 +++++++++
 2 files changed, 15 insertions(+), 6 deletions(-)

libibverbs/cmd.c +6 -6

index 713a13c0f272b3..fbe822324c9dd5 100644
@@ -1182,10 +1182,10 @@ int ibv_cmd_query_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
 	struct ibv_query_qp_resp resp;
 
 	/*
-	 * Masks over IBV_QP_DEST_QPN are not supported by
-	 * that not extended command.
+	 * Starting with IB_QP_RATE_LIMIT the attribute must go through the
+	 * _ex path.
 	 */
-	if (attr_mask & ~((IBV_QP_DEST_QPN << 1) - 1))
+	if (attr_mask & ~(IBV_QP_RATE_LIMIT - 1))
 		return EOPNOTSUPP;
 
 	IBV_INIT_CMD_RESP(cmd, cmd_size, QUERY_QP, &resp, sizeof resp);

@@ -1352,10 +1352,10 @@ int ibv_cmd_modify_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
 		      struct ibv_modify_qp *cmd, size_t cmd_size)
 {
 	/*
-	 * Masks over IBV_QP_DEST_QPN are only supported by
-	 * ibv_cmd_modify_qp_ex.
+	 * Starting with IB_QP_RATE_LIMIT the attribute must go through the
+	 * _ex path.
 	 */
-	if (attr_mask & ~((IBV_QP_DEST_QPN << 1) - 1))
+	if (attr_mask & ~(IBV_QP_RATE_LIMIT - 1))
 		return EOPNOTSUPP;
 
 	IBV_INIT_CMD(cmd, cmd_size, MODIFY_QP);

libibverbs/verbs.h +9 -0

index 1ab747bb757cf3..2a2264d300e1ca 100644
@@ -909,6 +909,15 @@ enum ibv_qp_attr_mask {
 	IBV_QP_PATH_MIG_STATE		= 1 << 18,
 	IBV_QP_CAP			= 1 << 19,
 	IBV_QP_DEST_QPN			= 1 << 20,
+#if 0
+	/* These bits were supported on older kernels, but never exposed to
+	 * user space:
+	 */
+	_IBV_QP_COMPAT1			= 1 << 21,
+	_IBV_QP_COMPAT2			= 1 << 22,
+	_IBV_QP_COMPAT3			= 1 << 23,
+	_IBV_QP_COMPAT3			= 1 << 24,
+#endif
 	IBV_QP_RATE_LIMIT		= 1 << 25,
 };

Signature · 5 lines

2.7.4
--
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): 20171102215031.GY18874-uk2M96/[email protected]

Leon Romanovsky

2017-11-05 07:10 UTC #2 lore raw

Quoted 51 lines · Jason Gunthorpe

Fixes 3ca7a1031486 ("ibverbs: Add support for packet pacing")

Reported-by Ram Amrani <Ram.Amrani-YGCgFSpz5w/[email protected]>

Signed-off-by Jason Gunthorpe <jgunthorpe-uk2M96/[email protected]>

---
 libibverbs/cmd.c   | 12 ++++++------
 libibverbs/verbs.h |  9 +++++++++
 2 files changed, 15 insertions(+), 6 deletions(-)

libibverbs/cmd.c +6 -6

index 713a13c0f272b3..fbe822324c9dd5 100644
@@ -1182,10 +1182,10 @@ int ibv_cmd_query_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
 	struct ibv_query_qp_resp resp;
 
 	/*
-	 * Masks over IBV_QP_DEST_QPN are not supported by
-	 * that not extended command.
+	 * Starting with IB_QP_RATE_LIMIT the attribute must go through the
+	 * _ex path.
 	 */
-	if (attr_mask & ~((IBV_QP_DEST_QPN << 1) - 1))
+	if (attr_mask & ~(IBV_QP_RATE_LIMIT - 1))
 		return EOPNOTSUPP;
 
 	IBV_INIT_CMD_RESP(cmd, cmd_size, QUERY_QP, &resp, sizeof resp);

@@ -1352,10 +1352,10 @@ int ibv_cmd_modify_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
 		      struct ibv_modify_qp *cmd, size_t cmd_size)
 {
 	/*
-	 * Masks over IBV_QP_DEST_QPN are only supported by
-	 * ibv_cmd_modify_qp_ex.
+	 * Starting with IB_QP_RATE_LIMIT the attribute must go through the
+	 * _ex path.
 	 */
-	if (attr_mask & ~((IBV_QP_DEST_QPN << 1) - 1))
+	if (attr_mask & ~(IBV_QP_RATE_LIMIT - 1))
 		return EOPNOTSUPP;
 
 	IBV_INIT_CMD(cmd, cmd_size, MODIFY_QP);

libibverbs/verbs.h +0 -0

index 1ab747bb757cf3..2a2264d300e1ca 100644
@@ -909,6 +909,15 @@ enum ibv_qp_attr_mask {
 	IBV_QP_PATH_MIG_STATE		= 1 << 18,
 	IBV_QP_CAP			= 1 << 19,

libibverbs/verbs.h enum ibv_qp_attr_mask { · line 920

Quoted 10 lines

libibverbs/verbs.h +9 -0

 	IBV_QP_DEST_QPN			= 1 << 20,
+#if 0
+	/* These bits were supported on older kernels, but never exposed to
+	 * user space:
+	 */
+	_IBV_QP_COMPAT1			= 1 << 21,
+	_IBV_QP_COMPAT2			= 1 << 22,
+	_IBV_QP_COMPAT3			= 1 << 23,
+	_IBV_QP_COMPAT3			= 1 << 24,
+#endif

Why do we need "if 0" in production code (global header file)?

Thanks

Quoted 9 lines

libibverbs/verbs.h +0 -1
 	IBV_QP_RATE_LIMIT		= 1 << 25,
 };
 
--
2.7.4
--
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): 20171105071029.GC31774-U/[email protected]

Jason Gunthorpe

author

2017-11-06 02:28 UTC #4 lore raw

Quoted 7 lines · Leon Romanovsky

Quoted 7 lines

libibverbs/verbs.h +3 -0

@@ -909,6 +909,15 @@ enum ibv_qp_attr_mask {
 	IBV_QP_PATH_MIG_STATE		= 1 << 18,
 	IBV_QP_CAP			= 1 << 19,
 	IBV_QP_DEST_QPN			= 1 << 20,
+#if 0
+	/* These bits were supported on older kernels, but never exposed to
+	 * user space:

Quoted 8 lines

libibverbs/verbs.h enum ibv_qp_attr_mask { · line 920
Quoted 6 lines
libibverbs/verbs.h +6 -0
+	 */
+	_IBV_QP_COMPAT1			= 1 << 21,
+	_IBV_QP_COMPAT2			= 1 << 22,
+	_IBV_QP_COMPAT3			= 1 << 23,
+	_IBV_QP_COMPAT3			= 1 << 24,
+#endif
Why do we need "if 0" in production code (global header file)?

It is really a comment. I don't want to introduce new global symbols just for this, or loose the obvious formatting structure of the enum.

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): 20171106022818.GE26011-uk2M96/[email protected]

Leon Romanovsky

2017-11-06 05:20 UTC #6 lore raw

Quoted 17 lines · Jason Gunthorpe

Quoted 7 lines · Leon Romanovsky
Quoted 7 lines
libibverbs/verbs.h +3 -0
@@ -909,6 +909,15 @@ enum ibv_qp_attr_mask {
 	IBV_QP_PATH_MIG_STATE		= 1 << 18,
 	IBV_QP_CAP			= 1 << 19,
 	IBV_QP_DEST_QPN			= 1 << 20,
+#if 0
+	/* These bits were supported on older kernels, but never exposed to
+	 * user space:
Quoted 8 lines
libibverbs/verbs.h enum ibv_qp_attr_mask { · line 920
Quoted 6 lines
libibverbs/verbs.h +6 -0
+	 */
+	_IBV_QP_COMPAT1			= 1 << 21,
+	_IBV_QP_COMPAT2			= 1 << 22,
+	_IBV_QP_COMPAT3			= 1 << 23,
+	_IBV_QP_COMPAT3			= 1 << 24,
+#endif
Why do we need "if 0" in production code (global header file)?

Quoted 2 lines

It is really a comment. I don't want to introduce new global symbols just for this, or loose the obvious formatting structure of the enum.

Comment will be good enough, no need to add extra:

/*
* .....
* _IBV_QP_COMPAT1 = 1 << 21,
* ......
*/

Thanks

Quoted 2 lines

Jason

Amrani, Ram

2017-11-07 06:21 UTC #7 lore raw

In reply to a message not in the archive

Quoted 8 lines

At least the RDMA CM in 4.3 will give user space structs with these bits set. Those kernels require user space to pass those set bits back to the kernel. Blocking them causes RDMA CM to be unusable on older kernels.
Fixes 3ca7a1031486 ("ibverbs: Add support for packet pacing")
Reported-by Ram Amrani <Ram.Amrani-YGCgFSpz5w/[email protected]>
Signed-off-by Jason Gunthorpe <jgunthorpe-uk2M96/[email protected]>
---

Thank you, Jason.

Ram

--
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): BN3PR07MB2578908EF39AF93B472208ADF8510-EldUQEzkDQfpW3VS/[email protected]

Yishai Hadas

2017-11-09 13:24 UTC #9 lore raw

Quoted 4 lines · Ram

Quoted 4 lines
At least the RDMA CM in 4.3 will give user space structs with these bits set. Those kernels require user space to pass those set bits back to the kernel. Blocking them causes RDMA CM to be unusable on older kernels.

Quoted 9 lines

Quoted 4 lines
Fixes 3ca7a1031486 ("ibverbs: Add support for packet pacing")
Reported-by Ram Amrani <Ram.Amrani-YGCgFSpz5w/[email protected]>
Signed-off-by Jason Gunthorpe <jgunthorpe-uk2M96/[email protected]>
---
Thank you, Jason.
Ram

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