* [PATCH rdma-core 1/4] verbs: Introduce ibv_query_qp_data_in_order() verb
2021-06-09 15:59 [PATCH rdma-core 0/4] verbs: Introduce ibv_query_qp_data_in_order() verb Yishai Hadas
@ 2021-06-09 15:59 ` Yishai Hadas
2021-06-09 15:59 ` [PATCH rdma-core 2/4] mlx5: Implement " Yishai Hadas
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Yishai Hadas @ 2021-06-09 15:59 UTC (permalink / raw)
To: linux-rdma; +Cc: jgg, yishaih, maorg, phaddad, Maor Gottlieb, Yishai Hadas
From: Patrisious Haddad <phaddad@nvidia.com>
Introduce ibv_query_qp_data_in_order() verb, this enables an application
to check whether the receiving data of the local QP is guaranteed to be
in order for a given operation within its WQE.
Once true, it allows user to poll for data instead of poll for
completion.
A detailed man page was added as well.
Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
Reviewed-by: Maor Gottlieb <maorg@nvidia.com>
Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
---
debian/libibverbs1.symbols | 2 +
libibverbs/CMakeLists.txt | 2 +-
libibverbs/driver.h | 2 +
libibverbs/dummy_ops.c | 8 ++++
libibverbs/libibverbs.map.in | 5 +++
libibverbs/man/CMakeLists.txt | 1 +
libibverbs/man/ibv_query_qp_data_in_order.3.md | 62 ++++++++++++++++++++++++++
libibverbs/verbs.c | 6 +++
libibverbs/verbs.h | 14 ++++++
9 files changed, 101 insertions(+), 1 deletion(-)
create mode 100644 libibverbs/man/ibv_query_qp_data_in_order.3.md
diff --git a/debian/libibverbs1.symbols b/debian/libibverbs1.symbols
index e7453b5..7b6bc8f 100644
--- a/debian/libibverbs1.symbols
+++ b/debian/libibverbs1.symbols
@@ -11,6 +11,7 @@ libibverbs.so.1 libibverbs1 #MINVER#
IBVERBS_1.11@IBVERBS_1.11 32
IBVERBS_1.12@IBVERBS_1.12 34
IBVERBS_1.13@IBVERBS_1.13 35
+ IBVERBS_1.14@IBVERBS_1.14 36
(symver)IBVERBS_PRIVATE_34 34
_ibv_query_gid_ex@IBVERBS_1.11 32
_ibv_query_gid_table@IBVERBS_1.11 32
@@ -98,6 +99,7 @@ libibverbs.so.1 libibverbs1 #MINVER#
ibv_query_port@IBVERBS_1.1 1.1.6
ibv_query_qp@IBVERBS_1.0 1.1.6
ibv_query_qp@IBVERBS_1.1 1.1.6
+ ibv_query_qp_data_in_order@IBVERBS_1.14 36
ibv_query_srq@IBVERBS_1.0 1.1.6
ibv_query_srq@IBVERBS_1.1 1.1.6
ibv_rate_to_mbps@IBVERBS_1.1 1.1.8
diff --git a/libibverbs/CMakeLists.txt b/libibverbs/CMakeLists.txt
index 16df2c5..3c486b9 100644
--- a/libibverbs/CMakeLists.txt
+++ b/libibverbs/CMakeLists.txt
@@ -21,7 +21,7 @@ configure_file("libibverbs.map.in"
rdma_library(ibverbs "${CMAKE_CURRENT_BINARY_DIR}/libibverbs.map"
# See Documentation/versioning.md
- 1 1.13.${PACKAGE_VERSION}
+ 1 1.14.${PACKAGE_VERSION}
all_providers.c
cmd.c
cmd_ah.c
diff --git a/libibverbs/driver.h b/libibverbs/driver.h
index 926023b..8b2e045 100644
--- a/libibverbs/driver.h
+++ b/libibverbs/driver.h
@@ -370,6 +370,8 @@ struct verbs_context_ops {
struct ibv_port_attr *port_attr);
int (*query_qp)(struct ibv_qp *qp, struct ibv_qp_attr *attr,
int attr_mask, struct ibv_qp_init_attr *init_attr);
+ int (*query_qp_data_in_order)(struct ibv_qp *qp, enum ibv_wr_opcode op,
+ uint32_t flags);
int (*query_rt_values)(struct ibv_context *context,
struct ibv_values_ex *values);
int (*query_srq)(struct ibv_srq *srq, struct ibv_srq_attr *srq_attr);
diff --git a/libibverbs/dummy_ops.c b/libibverbs/dummy_ops.c
index 5ed67bf..bf70775 100644
--- a/libibverbs/dummy_ops.c
+++ b/libibverbs/dummy_ops.c
@@ -397,6 +397,12 @@ static int query_ece(struct ibv_qp *qp, struct ibv_ece *ece)
return EOPNOTSUPP;
}
+static int query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
+ uint32_t flags)
+{
+ return 0;
+}
+
static int query_port(struct ibv_context *context, uint8_t port_num,
struct ibv_port_attr *port_attr)
{
@@ -559,6 +565,7 @@ const struct verbs_context_ops verbs_dummy_ops = {
query_ece,
query_port,
query_qp,
+ query_qp_data_in_order,
query_rt_values,
query_srq,
read_counters,
@@ -683,6 +690,7 @@ void verbs_set_ops(struct verbs_context *vctx,
SET_PRIV_OP_IC(vctx, query_ece);
SET_PRIV_OP_IC(ctx, query_port);
SET_PRIV_OP(ctx, query_qp);
+ SET_PRIV_OP_IC(ctx, query_qp_data_in_order);
SET_OP(vctx, query_rt_values);
SET_OP(vctx, read_counters);
SET_PRIV_OP(ctx, query_srq);
diff --git a/libibverbs/libibverbs.map.in b/libibverbs/libibverbs.map.in
index 7c0fb6a..0e39428 100644
--- a/libibverbs/libibverbs.map.in
+++ b/libibverbs/libibverbs.map.in
@@ -160,6 +160,11 @@ IBVERBS_1.13 {
ibv_unimport_dm;
} IBVERBS_1.12;
+IBVERBS_1.14 {
+ global:
+ ibv_query_qp_data_in_order;
+} IBVERBS_1.13;
+
/* If any symbols in this stanza change ABI then the entire staza gets a new symbol
version. See the top level CMakeLists.txt for this setting. */
diff --git a/libibverbs/man/CMakeLists.txt b/libibverbs/man/CMakeLists.txt
index 4d96e80..712a30f 100644
--- a/libibverbs/man/CMakeLists.txt
+++ b/libibverbs/man/CMakeLists.txt
@@ -64,6 +64,7 @@ rdma_man_pages(
ibv_query_pkey.3.md
ibv_query_port.3
ibv_query_qp.3
+ ibv_query_qp_data_in_order.3.md
ibv_query_rt_values_ex.3
ibv_query_srq.3
ibv_rate_to_mbps.3.md
diff --git a/libibverbs/man/ibv_query_qp_data_in_order.3.md b/libibverbs/man/ibv_query_qp_data_in_order.3.md
new file mode 100644
index 0000000..7558a92
--- /dev/null
+++ b/libibverbs/man/ibv_query_qp_data_in_order.3.md
@@ -0,0 +1,62 @@
+---
+date: 2020-3-3
+footer: libibverbs
+header: "Libibverbs Programmer's Manual"
+layout: page
+license: 'Licensed under the OpenIB.org BSD license (FreeBSD Variant) - See COPYING.md'
+section: 3
+title: ibv_query_qp_data_in_order
+---
+
+# NAME
+
+ibv_query_qp_data_in_order - check if qp data is guaranteed to be in order.
+
+# SYNOPSIS
+
+```c
+#include <infiniband/verbs.h>
+
+int ibv_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op, uint32_t flags);
+
+```
+
+
+# DESCRIPTION
+
+**ibv_query_qp_data_in_order()** Checks whether WQE data is guaranteed to be
+written in-order, and thus reader may poll for data instead of poll for completion.
+This function indicates data is written in-order within each WQE, but cannot be used to determine ordering between separate WQEs.
+This function describes ordering at the receiving side of the QP, not the sending side.
+
+# ARGUMENTS
+*qp*
+: The local queue pair (QP) to query.
+
+*op*
+: The operation type to query about. Different operation types may write data in a different order.
+ For RDMA read operations: describes ordering of RDMA reads posted on this local QP.
+ For RDMA write operations: describes ordering of remote RDMA writes being done into this local QP.
+ For RDMA send operations: describes ordering of remote RDMA sends being done into this local QP.
+ This function should not be used to determine ordering of other operation types.
+
+*flags*
+: Extra field for future input. For now must be 0.
+
+# RETURN VALUE
+
+**ibv_query_qp_data_in_order()** Returns 1 if the data is guaranteed to be written in-order, 0 otherwise.
+
+# NOTES
+
+Return value is valid only when the data is read by the CPU and relaxed ordering MR is not the target of the transfer.
+
+# SEE ALSO
+
+**ibv_query_qp**(3)
+
+# AUTHOR
+
+Patrisious Haddad <phaddad@nvidia.com>
+
+Yochai Cohen <yochai@nvidia.com>
diff --git a/libibverbs/verbs.c b/libibverbs/verbs.c
index b2cba3f..19ff12f 100644
--- a/libibverbs/verbs.c
+++ b/libibverbs/verbs.c
@@ -684,6 +684,12 @@ LATEST_SYMVER_FUNC(ibv_query_qp, 1_1, "IBVERBS_1.1",
return 0;
}
+int ibv_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
+ uint32_t flags)
+{
+ return get_ops(qp->context)->query_qp_data_in_order(qp, op, flags);
+}
+
LATEST_SYMVER_FUNC(ibv_modify_qp, 1_1, "IBVERBS_1.1",
int,
struct ibv_qp *qp, struct ibv_qp_attr *attr,
diff --git a/libibverbs/verbs.h b/libibverbs/verbs.h
index 7d42095..3dd9a79 100644
--- a/libibverbs/verbs.h
+++ b/libibverbs/verbs.h
@@ -3148,6 +3148,20 @@ ibv_modify_qp_rate_limit(struct ibv_qp *qp,
}
/**
+ * ibv_query_qp_data_in_order - Checks whether the data is guaranteed to be
+ * written in-order.
+ * @qp: The QP to query.
+ * @op: Operation type.
+ * @flags: Extra field for future input. For now must be 0.
+ *
+ * Return Value
+ * ibv_query_qp_data_in_order() returns 1 if the data is guaranteed to be
+ * written in-order, 0 otherwise.
+ */
+int ibv_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
+ uint32_t flags);
+
+/**
* ibv_query_qp - Returns the attribute list and current values for the
* specified QP.
* @qp: The QP to query.
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH rdma-core 2/4] mlx5: Implement ibv_query_qp_data_in_order() verb
2021-06-09 15:59 [PATCH rdma-core 0/4] verbs: Introduce ibv_query_qp_data_in_order() verb Yishai Hadas
2021-06-09 15:59 ` [PATCH rdma-core 1/4] " Yishai Hadas
@ 2021-06-09 15:59 ` Yishai Hadas
2021-06-10 5:50 ` Leon Romanovsky
2021-06-09 15:59 ` [PATCH rdma-core 3/4] pyverbs: Add query QP data in order support Yishai Hadas
2021-06-09 15:59 ` [PATCH rdma-core 4/4] tests: Add query QP data in order coverage Yishai Hadas
3 siblings, 1 reply; 10+ messages in thread
From: Yishai Hadas @ 2021-06-09 15:59 UTC (permalink / raw)
To: linux-rdma; +Cc: jgg, yishaih, maorg, phaddad, Maor Gottlieb, Yishai Hadas
From: Patrisious Haddad <phaddad@nvidia.com>
Implement the ibv_query_qp_data_in_order() verb by using DEVX to read
from firmware the 'in_order_data' capability.
Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
Reviewed-by: Maor Gottlieb <maorg@nvidia.com>
Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
---
providers/mlx5/mlx5.c | 1 +
providers/mlx5/mlx5.h | 3 +++
providers/mlx5/mlx5_ifc.h | 39 +++++++++++++++++++++++++++++++--
providers/mlx5/verbs.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 96 insertions(+), 2 deletions(-)
diff --git a/providers/mlx5/mlx5.c b/providers/mlx5/mlx5.c
index 8ef305e..75ffde9 100644
--- a/providers/mlx5/mlx5.c
+++ b/providers/mlx5/mlx5.c
@@ -167,6 +167,7 @@ static const struct verbs_context_ops mlx5_ctx_common_ops = {
.unimport_dm = mlx5_unimport_dm,
.unimport_mr = mlx5_unimport_mr,
.unimport_pd = mlx5_unimport_pd,
+ .query_qp_data_in_order = mlx5_query_qp_data_in_order,
};
static const struct verbs_context_ops mlx5_ctx_cqev1_ops = {
diff --git a/providers/mlx5/mlx5.h b/providers/mlx5/mlx5.h
index f6c74df..bfa66ab 100644
--- a/providers/mlx5/mlx5.h
+++ b/providers/mlx5/mlx5.h
@@ -398,6 +398,7 @@ struct mlx5_context {
struct mlx5_bf *nc_uar;
void *cq_uar_reg;
struct mlx5_reserved_qpns reserved_qpns;
+ uint8_t qp_data_in_order_cap:1;
};
struct mlx5_bitmap {
@@ -1088,6 +1089,8 @@ struct ibv_qp *mlx5_create_qp(struct ibv_pd *pd, struct ibv_qp_init_attr *attr);
int mlx5_query_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
int attr_mask,
struct ibv_qp_init_attr *init_attr);
+int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
+ uint32_t flags);
int mlx5_modify_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
int attr_mask);
int mlx5_modify_qp_rate_limit(struct ibv_qp *qp,
diff --git a/providers/mlx5/mlx5_ifc.h b/providers/mlx5/mlx5_ifc.h
index 35be5ba..302788f 100644
--- a/providers/mlx5/mlx5_ifc.h
+++ b/providers/mlx5/mlx5_ifc.h
@@ -51,6 +51,7 @@ enum {
MLX5_CMD_OP_INIT2INIT_QP = 0x50e,
MLX5_CMD_OP_CREATE_PSV = 0x600,
MLX5_CMD_OP_DESTROY_PSV = 0x601,
+ MLX5_CMD_OP_QUERY_DCT = 0x713,
MLX5_CMD_OP_QUERY_ESW_VPORT_CONTEXT = 0x752,
MLX5_CMD_OP_QUERY_NIC_VPORT_CONTEXT = 0x754,
MLX5_CMD_OP_QUERY_ROCE_ADDRESS = 0x760,
@@ -641,7 +642,11 @@ struct mlx5_ifc_cmd_hca_cap_bits {
u8 reserved_at_21[0xf];
u8 vhca_id[0x10];
- u8 reserved_at_40[0x40];
+ u8 reserved_at_40[0x20];
+
+ u8 reserved_at_60[0x2];
+ u8 qp_data_in_order[0x1];
+ u8 reserved_at_63[0x1d];
u8 log_max_srq_sz[0x8];
u8 log_max_qp_sz[0x8];
@@ -2654,6 +2659,12 @@ enum {
MLX5_ACTION_IN_FIELD_OUT_GTPU_TEID = 0x6E,
};
+struct mlx5_ifc_dctc_bits {
+ u8 reserved_at_0[0x1d];
+ u8 data_in_order[0x1];
+ u8 reserved_at_1e[0x362];
+};
+
struct mlx5_ifc_packet_reformat_context_in_bits {
u8 reserved_at_0[0x5];
u8 reformat_type[0x3];
@@ -3040,7 +3051,7 @@ struct mlx5_ifc_qpc_bits {
u8 log_sq_size[0x4];
u8 reserved_at_55[0x3];
u8 ts_format[0x2];
- u8 reserved_at_5a[0x1];
+ u8 data_in_order[0x1];
u8 rlky[0x1];
u8 ulp_stateless_offload_mode[0x4];
@@ -3409,6 +3420,30 @@ struct mlx5_ifc_query_qp_in_bits {
u8 reserved_at_60[0x20];
};
+struct mlx5_ifc_query_dct_out_bits {
+ u8 status[0x8];
+ u8 reserved_at_8[0x18];
+
+ u8 syndrome[0x20];
+
+ u8 reserved_at_40[0x40];
+
+ struct mlx5_ifc_dctc_bits dctc;
+};
+
+struct mlx5_ifc_query_dct_in_bits {
+ u8 opcode[0x10];
+ u8 reserved_at_10[0x10];
+
+ u8 reserved_at_20[0x10];
+ u8 op_mod[0x10];
+
+ u8 reserved_at_40[0x8];
+ u8 dctn[0x18];
+
+ u8 reserved_at_60[0x20];
+};
+
struct mlx5_ifc_tisc_bits {
u8 strict_lag_tx_port_affinity[0x1];
u8 tls_en[0x1];
diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c
index 8fcef62..88f4c3c 100644
--- a/providers/mlx5/verbs.c
+++ b/providers/mlx5/verbs.c
@@ -2674,6 +2674,57 @@ free:
return 0;
}
+static int query_dct_in_order(struct ibv_qp *qp)
+{
+ uint32_t in_dct[DEVX_ST_SZ_DW(query_dct_in)] = {};
+ uint32_t out_dct[DEVX_ST_SZ_DW(query_dct_out)] = {};
+ int ret;
+
+ DEVX_SET(query_dct_in, in_dct, opcode, MLX5_CMD_OP_QUERY_DCT);
+ DEVX_SET(query_dct_in, in_dct, dctn, qp->qp_num);
+ ret = mlx5dv_devx_qp_query(qp, in_dct, sizeof(in_dct), out_dct,
+ sizeof(out_dct));
+ if (ret)
+ return 0;
+
+ return DEVX_GET(query_dct_out, out_dct, dctc.data_in_order);
+}
+
+int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
+ uint32_t flags)
+{
+ uint32_t in_qp[DEVX_ST_SZ_DW(query_qp_in)] = {};
+ uint32_t out_qp[DEVX_ST_SZ_DW(query_qp_out)] = {};
+ struct mlx5_context *mctx = to_mctx(qp->context);
+ struct mlx5_qp *mqp = to_mqp(qp);
+ int ret;
+
+/* Currently this API is only supported for x86 architectures since most
+ * non-x86 platforms are known to be OOO and need to do a per-platform study.
+ */
+#if !defined(__i386__) && !defined(__x86_64__)
+ return 0;
+#endif
+
+ if (flags || !mctx->qp_data_in_order_cap)
+ return 0;
+
+ if (mqp->dc_type == MLX5DV_DCTYPE_DCT)
+ return query_dct_in_order(qp);
+
+ if (qp->state != IBV_QPS_RTS)
+ return 0;
+
+ DEVX_SET(query_qp_in, in_qp, opcode, MLX5_CMD_OP_QUERY_QP);
+ DEVX_SET(query_qp_in, in_qp, qpn, qp->qp_num);
+ ret = mlx5dv_devx_qp_query(qp, in_qp, sizeof(in_qp), out_qp,
+ sizeof(out_qp));
+ if (ret)
+ return 0;
+
+ return DEVX_GET(query_qp_out, out_qp, qpc.data_in_order);
+}
+
int mlx5_query_qp(struct ibv_qp *ibqp, struct ibv_qp_attr *attr,
int attr_mask, struct ibv_qp_init_attr *init_attr)
{
@@ -3584,6 +3635,10 @@ static void get_hca_general_caps(struct mlx5_context *mctx)
if (ret)
return;
+ mctx->qp_data_in_order_cap =
+ DEVX_GET(query_hca_cap_out, out,
+ capability.cmd_hca_cap.qp_data_in_order);
+
mctx->entropy_caps.num_lag_ports =
DEVX_GET(query_hca_cap_out, out,
capability.cmd_hca_cap.num_lag_ports);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH rdma-core 2/4] mlx5: Implement ibv_query_qp_data_in_order() verb
2021-06-09 15:59 ` [PATCH rdma-core 2/4] mlx5: Implement " Yishai Hadas
@ 2021-06-10 5:50 ` Leon Romanovsky
2021-06-10 11:42 ` Jason Gunthorpe
0 siblings, 1 reply; 10+ messages in thread
From: Leon Romanovsky @ 2021-06-10 5:50 UTC (permalink / raw)
To: Yishai Hadas; +Cc: linux-rdma, jgg, yishaih, maorg, phaddad, Maor Gottlieb
On Wed, Jun 09, 2021 at 06:59:30PM +0300, Yishai Hadas wrote:
> From: Patrisious Haddad <phaddad@nvidia.com>
>
> Implement the ibv_query_qp_data_in_order() verb by using DEVX to read
> from firmware the 'in_order_data' capability.
>
> Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
> Reviewed-by: Maor Gottlieb <maorg@nvidia.com>
> Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
> ---
> providers/mlx5/mlx5.c | 1 +
> providers/mlx5/mlx5.h | 3 +++
> providers/mlx5/mlx5_ifc.h | 39 +++++++++++++++++++++++++++++++--
> providers/mlx5/verbs.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 96 insertions(+), 2 deletions(-)
<...>
> +int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> + uint32_t flags)
> +{
> + uint32_t in_qp[DEVX_ST_SZ_DW(query_qp_in)] = {};
> + uint32_t out_qp[DEVX_ST_SZ_DW(query_qp_out)] = {};
> + struct mlx5_context *mctx = to_mctx(qp->context);
> + struct mlx5_qp *mqp = to_mqp(qp);
> + int ret;
> +
> +/* Currently this API is only supported for x86 architectures since most
> + * non-x86 platforms are known to be OOO and need to do a per-platform study.
> + */
> +#if !defined(__i386__) && !defined(__x86_64__)
> + return 0;
> +#endif
Does it compile without warnings/errors on such platforms?
You have "return 0;" in the middle of function, so the right thing to do
it is to write with "#if ..." over function or inside like below, as
long as "#else" exists.
int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
uint32_t flags)
{
#if !defined(__i386__) && !defined(__x86_64__)
/* Currently this API is only supported for x86 architectures since most
* non-x86 platforms are known to be OOO and need to do a per-platform study.
*/
return 0;
#else
.....
#endif
> +
> + if (flags || !mctx->qp_data_in_order_cap)
> + return 0;
> +
> + if (mqp->dc_type == MLX5DV_DCTYPE_DCT)
> + return query_dct_in_order(qp);
> +
> + if (qp->state != IBV_QPS_RTS)
> + return 0;
> +
> + DEVX_SET(query_qp_in, in_qp, opcode, MLX5_CMD_OP_QUERY_QP);
> + DEVX_SET(query_qp_in, in_qp, qpn, qp->qp_num);
> + ret = mlx5dv_devx_qp_query(qp, in_qp, sizeof(in_qp), out_qp,
> + sizeof(out_qp));
> + if (ret)
> + return 0;
> +
> + return DEVX_GET(query_qp_out, out_qp, qpc.data_in_order);
> +}
> +
Thanks
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH rdma-core 2/4] mlx5: Implement ibv_query_qp_data_in_order() verb
2021-06-10 5:50 ` Leon Romanovsky
@ 2021-06-10 11:42 ` Jason Gunthorpe
2021-06-10 11:50 ` Leon Romanovsky
0 siblings, 1 reply; 10+ messages in thread
From: Jason Gunthorpe @ 2021-06-10 11:42 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Yishai Hadas, linux-rdma, yishaih, maorg, phaddad, Maor Gottlieb
On Thu, Jun 10, 2021 at 08:50:59AM +0300, Leon Romanovsky wrote:
> On Wed, Jun 09, 2021 at 06:59:30PM +0300, Yishai Hadas wrote:
> > From: Patrisious Haddad <phaddad@nvidia.com>
> >
> > Implement the ibv_query_qp_data_in_order() verb by using DEVX to read
> > from firmware the 'in_order_data' capability.
> >
> > Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
> > Reviewed-by: Maor Gottlieb <maorg@nvidia.com>
> > Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
> > providers/mlx5/mlx5.c | 1 +
> > providers/mlx5/mlx5.h | 3 +++
> > providers/mlx5/mlx5_ifc.h | 39 +++++++++++++++++++++++++++++++--
> > providers/mlx5/verbs.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 96 insertions(+), 2 deletions(-)
>
> <...>
>
> > +int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> > + uint32_t flags)
> > +{
> > + uint32_t in_qp[DEVX_ST_SZ_DW(query_qp_in)] = {};
> > + uint32_t out_qp[DEVX_ST_SZ_DW(query_qp_out)] = {};
> > + struct mlx5_context *mctx = to_mctx(qp->context);
> > + struct mlx5_qp *mqp = to_mqp(qp);
> > + int ret;
> > +
> > +/* Currently this API is only supported for x86 architectures since most
> > + * non-x86 platforms are known to be OOO and need to do a per-platform study.
> > + */
> > +#if !defined(__i386__) && !defined(__x86_64__)
> > + return 0;
> > +#endif
>
> Does it compile without warnings/errors on such platforms?
> You have "return 0;" in the middle of function, so the right thing to do
> it is to write with "#if ..." over function or inside like below, as
> long as "#else" exists.
>
> int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> uint32_t flags)
> {
> #if !defined(__i386__) && !defined(__x86_64__)
> /* Currently this API is only supported for x86 architectures since most
> * non-x86 platforms are known to be OOO and need to do a per-platform study.
> */
> return 0;
> #else
> .....
> #endif
We should probably put the above in the core code anyhow
Jason
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH rdma-core 2/4] mlx5: Implement ibv_query_qp_data_in_order() verb
2021-06-10 11:42 ` Jason Gunthorpe
@ 2021-06-10 11:50 ` Leon Romanovsky
2021-06-10 12:51 ` Yishai Hadas
0 siblings, 1 reply; 10+ messages in thread
From: Leon Romanovsky @ 2021-06-10 11:50 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Yishai Hadas, linux-rdma, yishaih, maorg, phaddad, Maor Gottlieb
On Thu, Jun 10, 2021 at 08:42:24AM -0300, Jason Gunthorpe wrote:
> On Thu, Jun 10, 2021 at 08:50:59AM +0300, Leon Romanovsky wrote:
> > On Wed, Jun 09, 2021 at 06:59:30PM +0300, Yishai Hadas wrote:
> > > From: Patrisious Haddad <phaddad@nvidia.com>
> > >
> > > Implement the ibv_query_qp_data_in_order() verb by using DEVX to read
> > > from firmware the 'in_order_data' capability.
> > >
> > > Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
> > > Reviewed-by: Maor Gottlieb <maorg@nvidia.com>
> > > Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
> > > providers/mlx5/mlx5.c | 1 +
> > > providers/mlx5/mlx5.h | 3 +++
> > > providers/mlx5/mlx5_ifc.h | 39 +++++++++++++++++++++++++++++++--
> > > providers/mlx5/verbs.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 96 insertions(+), 2 deletions(-)
> >
> > <...>
> >
> > > +int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> > > + uint32_t flags)
> > > +{
> > > + uint32_t in_qp[DEVX_ST_SZ_DW(query_qp_in)] = {};
> > > + uint32_t out_qp[DEVX_ST_SZ_DW(query_qp_out)] = {};
> > > + struct mlx5_context *mctx = to_mctx(qp->context);
> > > + struct mlx5_qp *mqp = to_mqp(qp);
> > > + int ret;
> > > +
> > > +/* Currently this API is only supported for x86 architectures since most
> > > + * non-x86 platforms are known to be OOO and need to do a per-platform study.
> > > + */
> > > +#if !defined(__i386__) && !defined(__x86_64__)
> > > + return 0;
> > > +#endif
> >
> > Does it compile without warnings/errors on such platforms?
> > You have "return 0;" in the middle of function, so the right thing to do
> > it is to write with "#if ..." over function or inside like below, as
> > long as "#else" exists.
> >
> > int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> > uint32_t flags)
> > {
> > #if !defined(__i386__) && !defined(__x86_64__)
> > /* Currently this API is only supported for x86 architectures since most
> > * non-x86 platforms are known to be OOO and need to do a per-platform study.
> > */
> > return 0;
> > #else
> > .....
> > #endif
>
> We should probably put the above in the core code anyhow
Agree, it makes sense.
Thanks
>
> Jason
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH rdma-core 3/4] pyverbs: Add query QP data in order support
2021-06-09 15:59 [PATCH rdma-core 0/4] verbs: Introduce ibv_query_qp_data_in_order() verb Yishai Hadas
2021-06-09 15:59 ` [PATCH rdma-core 1/4] " Yishai Hadas
2021-06-09 15:59 ` [PATCH rdma-core 2/4] mlx5: Implement " Yishai Hadas
@ 2021-06-09 15:59 ` Yishai Hadas
2021-06-09 15:59 ` [PATCH rdma-core 4/4] tests: Add query QP data in order coverage Yishai Hadas
3 siblings, 0 replies; 10+ messages in thread
From: Yishai Hadas @ 2021-06-09 15:59 UTC (permalink / raw)
To: linux-rdma
Cc: jgg, yishaih, maorg, phaddad, Shachar Kagan, Ido Kalir, Edward Srouji
From: Shachar Kagan <skagan@nvidia.com>
Add the ability to check if QP data is guaranteed to be in order.
Signed-off-by: Shachar Kagan <skagan@nvidia.com>
Reviewed-by: Ido Kalir <idok@nvidia.com>
Signed-off-by: Edward Srouji <edwards@nvidia.com>
---
pyverbs/libibverbs.pxd | 1 +
pyverbs/qp.pyx | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/pyverbs/libibverbs.pxd b/pyverbs/libibverbs.pxd
index 35e47aa..4c84c18 100644
--- a/pyverbs/libibverbs.pxd
+++ b/pyverbs/libibverbs.pxd
@@ -723,6 +723,7 @@ cdef extern from 'infiniband/verbs.h':
int ibv_query_rt_values_ex(ibv_context *context, ibv_values_ex *values)
int ibv_get_async_event(ibv_context *context, ibv_async_event *event)
void ibv_ack_async_event(ibv_async_event *event)
+ int ibv_query_qp_data_in_order(ibv_qp *qp, ibv_wr_opcode op, uint32_t flags)
cdef extern from 'infiniband/driver.h':
diff --git a/pyverbs/qp.pyx b/pyverbs/qp.pyx
index 0b117df..88516f8 100644
--- a/pyverbs/qp.pyx
+++ b/pyverbs/qp.pyx
@@ -1229,6 +1229,15 @@ cdef class QP(PyverbsCM):
if rc != 0:
raise PyverbsRDMAError('Failed to Bind MW', rc)
+ def query_data_in_order(self, op, flags=0):
+ """
+ Query if QP data is guaranteed to be in order.
+ :param op: Operation type.
+ :param flags: Extra field for future input. For now must be 0.
+ :return: 1 in case the data is guaranteed to be in order, 0 otherwise.
+ """
+ return v.ibv_query_qp_data_in_order(self.qp, op, flags)
+
@property
def qp_type(self):
return self.qp.qp_type
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH rdma-core 4/4] tests: Add query QP data in order coverage
2021-06-09 15:59 [PATCH rdma-core 0/4] verbs: Introduce ibv_query_qp_data_in_order() verb Yishai Hadas
` (2 preceding siblings ...)
2021-06-09 15:59 ` [PATCH rdma-core 3/4] pyverbs: Add query QP data in order support Yishai Hadas
@ 2021-06-09 15:59 ` Yishai Hadas
3 siblings, 0 replies; 10+ messages in thread
From: Yishai Hadas @ 2021-06-09 15:59 UTC (permalink / raw)
To: linux-rdma
Cc: jgg, yishaih, maorg, phaddad, Shachar Kagan, Ido Kalir, Edward Srouji
From: Shachar Kagan <skagan@nvidia.com>
Add a test which queries a QP in RTS state and check if data is
guaranteed to be in order.
Signed-off-by: Shachar Kagan <skagan@nvidia.com>
Reviewed-by: Ido Kalir <idok@nvidia.com>
Signed-off-by: Edward Srouji <edwards@nvidia.com>
---
tests/test_qp.py | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/tests/test_qp.py b/tests/test_qp.py
index 53b1d32..a623cb3 100644
--- a/tests/test_qp.py
+++ b/tests/test_qp.py
@@ -228,6 +228,19 @@ class QPTest(PyverbsAPITestCase):
"""
self.query_qp_common_test(e.IBV_QPT_RAW_PACKET)
+ def test_query_data_in_order(self):
+ """
+ Queries an UD QP data in order after moving it to RTS state.
+ Verifies that the result from the query is valid.
+ """
+ with PD(self.ctx) as pd:
+ with CQ(self.ctx, 100, None, None, 0) as cq:
+ qia = u.get_qp_init_attr(cq, self.attr)
+ qia.qp_type = e.IBV_QPT_UD
+ qp = self.create_qp(pd, qia, False, True, self.ib_port)
+ is_data_in_order = qp.query_data_in_order(e.IBV_WR_SEND)
+ self.assertIn(is_data_in_order, [0, 1], 'Data in order result is not valid')
+
def test_modify_ud_qp(self):
"""
Queries a UD QP after calling modify(). Verifies that its properties are
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread