diff mbox

[rdma-core,1/2] mlx5: Report if kernel allows using MPW in SQ

Message ID 1505130185-32658-2-git-send-email-yishaih@mellanox.com (mailing list archive)
State Accepted
Headers show

Commit Message

Yishai Hadas Sept. 11, 2017, 11:43 a.m. UTC 
From: Bodong Wang <bodong@mellanox.com>

Use flag MLX5DV_CONTEXT_FLAGS_MPW_ALLOWED to indicate hardware
supports multi packet WQE and it's enabled in SQ context.

Flag MLX5DV_CONTEXT_FLAGS_MPW is deprecated, shall not be used
in new applications.

Signed-off-by: Bodong Wang <bodong@mellanox.com>
Reviewed-by: Yishai Hadas <yishaih@mellanox.com>
---
 providers/mlx5/man/mlx5dv_query_device.3 | 3 ++-
 providers/mlx5/mlx5-abi.h                | 5 +++++
 providers/mlx5/mlx5.c                    | 4 ++--
 providers/mlx5/mlx5.h                    | 3 ++-
 providers/mlx5/mlx5dv.h                  | 3 ++-
 providers/mlx5/verbs.c                   | 4 ++--
 6 files changed, 15 insertions(+), 7 deletions(-)

Comments

Jason Gunthorpe Sept. 11, 2017, 4:42 p.m. UTC | #1 
On Mon, Sep 11, 2017 at 02:43:04PM +0300, Yishai Hadas wrote:
>  	MLX5DV_CONTEXT_FLAGS_CQE_V1	= (1 << 0),
> -	MLX5DV_CONTEXT_FLAGS_MPW	= (1 << 1),
> +	MLX5DV_CONTEXT_FLAGS_MPW	= (1 << 1), /* Obsoleted */
> +	MLX5DV_CONTEXT_FLAGS_MPW_ALLOWED = (1 << 2),
>  };
>  
>  enum mlx5dv_cq_init_attr_mask {
> diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c
> index fc63ae9..0e8a9a3 100644
> +++ b/providers/mlx5/verbs.c
> @@ -1965,8 +1965,8 @@ int mlx5_query_device_ex(struct ibv_context *context,
>  	attr->rss_caps.rx_hash_function = resp.rss_caps.rx_hash_function;
>  	attr->packet_pacing_caps = resp.packet_pacing_caps.caps;
>  
> -	if (resp.support_multi_pkt_send_wqe)
> -		mctx->vendor_cap_flags |= MLX5_VENDOR_CAP_FLAGS_MPW;
> +	if (resp.support_multi_pkt_send_wqe & MLX5_ALLOW_MPW)
> +		mctx->vendor_cap_flags |= MLX5_VENDOR_CAP_FLAGS_MPW_ALLOWED;

Er, you can't just drop setting MLX5DV_CONTEXT_FLAGS_MPW? That would
break compat.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yishai Hadas Sept. 12, 2017, 3:13 p.m. UTC | #2 
On 9/11/2017 7:42 PM, Jason Gunthorpe wrote:
> On Mon, Sep 11, 2017 at 02:43:04PM +0300, Yishai Hadas wrote:
>>  	MLX5DV_CONTEXT_FLAGS_CQE_V1	= (1 << 0),
>> -	MLX5DV_CONTEXT_FLAGS_MPW	= (1 << 1),
>> +	MLX5DV_CONTEXT_FLAGS_MPW	= (1 << 1), /* Obsoleted */
>> +	MLX5DV_CONTEXT_FLAGS_MPW_ALLOWED = (1 << 2),
>>  };
>>
>>  enum mlx5dv_cq_init_attr_mask {
>> diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c
>> index fc63ae9..0e8a9a3 100644
>> +++ b/providers/mlx5/verbs.c
>> @@ -1965,8 +1965,8 @@ int mlx5_query_device_ex(struct ibv_context *context,
>>  	attr->rss_caps.rx_hash_function = resp.rss_caps.rx_hash_function;
>>  	attr->packet_pacing_caps = resp.packet_pacing_caps.caps;
>>
>> -	if (resp.support_multi_pkt_send_wqe)
>> -		mctx->vendor_cap_flags |= MLX5_VENDOR_CAP_FLAGS_MPW;
>> +	if (resp.support_multi_pkt_send_wqe & MLX5_ALLOW_MPW)
>> +		mctx->vendor_cap_flags |= MLX5_VENDOR_CAP_FLAGS_MPW_ALLOWED;
>
> Er, you can't just drop setting MLX5DV_CONTEXT_FLAGS_MPW? That would
> break compat.
>

There was a kernel bug in 4.13 that MPW couldn't work properly despite 
the fact that the driver reported to user on its capability.

The bug was fixed in kernel 4.14 and now the feature is supported, the 
legacy value 1 on resp.support_multi_pkt_send_wqe is not reported from 
kernel any more as we can't rely on, and a new bit MLX5_ALLOW_MPW (1 << 
1) was introduced to indicate that feature is supported.

By the above change we want to report feature availability only when the 
new bit was set, legacy applications that use the old bit will see it 
off in both legacy and new kernels with this RDMA CORE as expected.

New applications should move to use the new bit 
MLX5DV_CONTEXT_FLAGS_MPW_ALLOWED as pointed in mlx5dv.h and in the 
updated man page as part of this patch.

Does it clarify the above code change ?



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe Sept. 13, 2017, 6:38 p.m. UTC | #3 
On Tue, Sep 12, 2017 at 06:13:40PM +0300, Yishai Hadas wrote:
> On 9/11/2017 7:42 PM, Jason Gunthorpe wrote:
> >On Mon, Sep 11, 2017 at 02:43:04PM +0300, Yishai Hadas wrote:
> >> 	MLX5DV_CONTEXT_FLAGS_CQE_V1	= (1 << 0),
> >>-	MLX5DV_CONTEXT_FLAGS_MPW	= (1 << 1),
> >>+	MLX5DV_CONTEXT_FLAGS_MPW	= (1 << 1), /* Obsoleted */
> >>+	MLX5DV_CONTEXT_FLAGS_MPW_ALLOWED = (1 << 2),

> The bug was fixed in kernel 4.14 and now the feature is supported, the
> legacy value 1 on resp.support_multi_pkt_send_wqe is not reported from
> kernel any more as we can't rely on, and a new bit MLX5_ALLOW_MPW (1 << 1)
> was introduced to indicate that feature is supported.

That is very messy.. At least delete the MLX5DV_CONTEXT_FLAGS_MPW
constant to create a compile failure for this break.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yishai Hadas Sept. 14, 2017, 1:30 p.m. UTC | #4 
On 9/13/2017 9:38 PM, Jason Gunthorpe wrote:
> On Tue, Sep 12, 2017 at 06:13:40PM +0300, Yishai Hadas wrote:
>> On 9/11/2017 7:42 PM, Jason Gunthorpe wrote:
>>> On Mon, Sep 11, 2017 at 02:43:04PM +0300, Yishai Hadas wrote:
>>>> 	MLX5DV_CONTEXT_FLAGS_CQE_V1	= (1 << 0),
>>>> -	MLX5DV_CONTEXT_FLAGS_MPW	= (1 << 1),
>>>> +	MLX5DV_CONTEXT_FLAGS_MPW	= (1 << 1), /* Obsoleted */
>>>> +	MLX5DV_CONTEXT_FLAGS_MPW_ALLOWED = (1 << 2),
> 
>> The bug was fixed in kernel 4.14 and now the feature is supported, the
>> legacy value 1 on resp.support_multi_pkt_send_wqe is not reported from
>> kernel any more as we can't rely on, and a new bit MLX5_ALLOW_MPW (1 << 1)
>> was introduced to indicate that feature is supported.
> 
> That is very messy.. At least delete the MLX5DV_CONTEXT_FLAGS_MPW
> constant to create a compile failure for this break.

PR was updated as you suggested, series was merged.

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

Patch

diff --git a/providers/mlx5/man/mlx5dv_query_device.3 b/providers/mlx5/man/mlx5dv_query_device.3
index b33a75b..c2f8bf4 100644
--- a/providers/mlx5/man/mlx5dv_query_device.3
+++ b/providers/mlx5/man/mlx5dv_query_device.3
@@ -35,7 +35,8 @@  enum mlx5dv_context_flags {
  * This flag indicates if CQE version 0 or 1 is needed.
  */
  MLX5DV_CONTEXT_FLAGS_CQE_V1 = (1 << 0),
- MLX5DV_CONTEXT_FLAGS_MPW    = (1 << 1), /* Multi packet WQE is supported or not */
+ MLX5DV_CONTEXT_FLAGS_MPW    = (1 << 1), /* Obsoleted */
+ MLX5DV_CONTEXT_FLAGS_MPW_ALLOWED  = (1 << 2), /* Multi packet WQE is allowed */
 .in -8
 };
 .fi
diff --git a/providers/mlx5/mlx5-abi.h b/providers/mlx5/mlx5-abi.h
index d05cb40..c9d3ec2 100644
--- a/providers/mlx5/mlx5-abi.h
+++ b/providers/mlx5/mlx5-abi.h
@@ -273,6 +273,11 @@  struct mlx5_packet_pacing_caps {
 	__u32  reserved;
 };
 
+enum mlx5_mpw_caps {
+	MLX5_MPW_RESERVED	= 1 << 0, /* Obsoleted, don't use */
+	MLX5_ALLOW_MPW		= 1 << 1,
+};
+
 struct mlx5_query_device_ex_resp {
 	struct ibv_query_device_resp_ex ibv_resp;
 	__u32				comp_mask;
diff --git a/providers/mlx5/mlx5.c b/providers/mlx5/mlx5.c
index 0e4d65f..84a67e9 100644
--- a/providers/mlx5/mlx5.c
+++ b/providers/mlx5/mlx5.c
@@ -625,8 +625,8 @@  int mlx5dv_query_device(struct ibv_context *ctx_in,
 	if (mctx->cqe_version == MLX5_CQE_VERSION_V1)
 		attrs_out->flags |= MLX5DV_CONTEXT_FLAGS_CQE_V1;
 
-	if (mctx->vendor_cap_flags & MLX5_VENDOR_CAP_FLAGS_MPW)
-		attrs_out->flags |= MLX5DV_CONTEXT_FLAGS_MPW;
+	if (mctx->vendor_cap_flags & MLX5_VENDOR_CAP_FLAGS_MPW_ALLOWED)
+		attrs_out->flags |= MLX5DV_CONTEXT_FLAGS_MPW_ALLOWED;
 
 	if (attrs_out->comp_mask & MLX5DV_CONTEXT_MASK_CQE_COMPRESION) {
 		attrs_out->cqe_comp_caps = mctx->cqe_comp_caps;
diff --git a/providers/mlx5/mlx5.h b/providers/mlx5/mlx5.h
index ad36cbf..c8d9fe9 100644
--- a/providers/mlx5/mlx5.h
+++ b/providers/mlx5/mlx5.h
@@ -182,7 +182,8 @@  enum {
 };
 
 enum mlx5_vendor_cap_flags {
-	MLX5_VENDOR_CAP_FLAGS_MPW		= 1 << 0,
+	MLX5_VENDOR_CAP_FLAGS_MPW		= 1 << 0, /* Obsoleted */
+	MLX5_VENDOR_CAP_FLAGS_MPW_ALLOWED	= 1 << 1,
 };
 
 enum {
diff --git a/providers/mlx5/mlx5dv.h b/providers/mlx5/mlx5dv.h
index 34b4d27..cf49b63 100644
--- a/providers/mlx5/mlx5dv.h
+++ b/providers/mlx5/mlx5dv.h
@@ -81,7 +81,8 @@  enum mlx5dv_context_flags {
 	 * This flag indicates if CQE version 0 or 1 is needed.
 	 */
 	MLX5DV_CONTEXT_FLAGS_CQE_V1	= (1 << 0),
-	MLX5DV_CONTEXT_FLAGS_MPW	= (1 << 1),
+	MLX5DV_CONTEXT_FLAGS_MPW	= (1 << 1), /* Obsoleted */
+	MLX5DV_CONTEXT_FLAGS_MPW_ALLOWED = (1 << 2),
 };
 
 enum mlx5dv_cq_init_attr_mask {
diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c
index fc63ae9..0e8a9a3 100644
--- a/providers/mlx5/verbs.c
+++ b/providers/mlx5/verbs.c
@@ -1965,8 +1965,8 @@  int mlx5_query_device_ex(struct ibv_context *context,
 	attr->rss_caps.rx_hash_function = resp.rss_caps.rx_hash_function;
 	attr->packet_pacing_caps = resp.packet_pacing_caps.caps;
 
-	if (resp.support_multi_pkt_send_wqe)
-		mctx->vendor_cap_flags |= MLX5_VENDOR_CAP_FLAGS_MPW;
+	if (resp.support_multi_pkt_send_wqe & MLX5_ALLOW_MPW)
+		mctx->vendor_cap_flags |= MLX5_VENDOR_CAP_FLAGS_MPW_ALLOWED;
 
 	mctx->cqe_comp_caps = resp.cqe_comp_caps;