| Message ID | 20201106230122.17411-3-rpearson@hpe.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Provider/rxe: Implement extended verbs APIs | expand |
On Fri, Nov 06, 2020 at 05:01:20PM -0600, Bob Pearson wrote: > Implement ibv_query_device_ex verb. Make it depend on a RXE_CAP_CMD_EX > capability bit supported by both provider and driver. > > Signed-off-by: Bob Pearson <rpearson@hpe.com> > kernel-headers/rdma/rdma_user_rxe.h | 1 + > providers/rxe/rxe.c | 35 +++++++++++++++++++++++++++++ > providers/rxe/rxe.h | 2 +- > 3 files changed, 37 insertions(+), 1 deletion(-) > > diff --git a/kernel-headers/rdma/rdma_user_rxe.h b/kernel-headers/rdma/rdma_user_rxe.h > index 70ac031e..a31465e2 100644 > +++ b/kernel-headers/rdma/rdma_user_rxe.h > @@ -160,6 +160,7 @@ struct rxe_recv_wqe { > > enum rxe_capabilities { > RXE_CAP_NONE = 0, > + RXE_CAP_CMD_EX = 1ULL << 0, > }; All the kernel-headers/ changes need to be in one patch at the start, the kernel-headers/update script will make the required commit. Keeping this 100% in sync with the kernel is important > struct rxe_alloc_context_cmd { > diff --git a/providers/rxe/rxe.c b/providers/rxe/rxe.c > index c29b7de5..b1fa2f42 100644 > +++ b/providers/rxe/rxe.c > @@ -87,6 +87,34 @@ static int rxe_query_device(struct ibv_context *context, > return 0; > } > > +static int rxe_query_device_ex(struct ibv_context *context, > + const struct ibv_query_device_ex_input *input, > + struct ibv_device_attr_ex *attr, > + size_t attr_size) > +{ > + int ret; > + uint64_t raw_fw_ver; > + unsigned int major, minor, sub_minor; > + struct ibv_query_device_ex cmd = {}; > + struct ib_uverbs_ex_query_device_resp resp = {}; > + > + fprintf(stderr, "%s: called\n", __func__); Don't send debugging prints in patches > + ret = ibv_cmd_query_device_ex(context, input, attr, sizeof(*attr), > + &raw_fw_ver, &cmd, sizeof(cmd), > + &resp, sizeof(resp)); > + if (ret) > + return ret; > + > + major = (raw_fw_ver >> 32) & 0xffff; > + minor = (raw_fw_ver >> 16) & 0xffff; > + sub_minor = raw_fw_ver & 0xffff; > + > + snprintf(attr->orig_attr.fw_ver, sizeof(attr->orig_attr.fw_ver), > + "%d.%d.%d", major, minor, sub_minor); > + > + return 0; > +} > + > static int rxe_query_port(struct ibv_context *context, uint8_t port, > struct ibv_port_attr *attr) > { > @@ -860,6 +888,10 @@ static const struct verbs_context_ops rxe_ctx_ops = { > .free_context = rxe_free_context, > }; > > +static const struct verbs_context_ops rxe_ctx_ops_cmd_ex = { > + .query_device_ex = rxe_query_device_ex, > +}; > + > static struct verbs_context *rxe_alloc_context(struct ibv_device *ibdev, > int cmd_fd, > void *private_data) > @@ -883,6 +915,9 @@ static struct verbs_context *rxe_alloc_context(struct ibv_device *ibdev, > > verbs_set_ops(&context->ibv_ctx, &rxe_ctx_ops); > > + if (context->capabilities & RXE_CAP_CMD_EX) > + verbs_set_ops(&context->ibv_ctx, &rxe_ctx_ops_cmd_ex); This isn't needed, we know if ibv_cmd_query_device_ex() is not supported because the kernel returns -EOPNOTSUP when we call it. What is needed is to just call the fallback like dummy ops does: if (ret == -EOPNOTSUPP) { if (input && input->comp_mask) return EINVAL; if (attr_size < sizeof(attr->orig_attr)) return EOPNOTSUPP; memset(attr, 0, attr_size); return ibv_query_device(context, &attr->orig_attr); } And I wonder if we should make ibv_cmd_query_device_ex() just do this? This whole thing is a mess now that the kernel always supports ibv_cmd_query_device_ex() on all drivers. I don't have time to fix it all properly, so I suggest you just use the above fragment for now instead of the RXE_CAP_CMD_EX Jason
diff --git a/kernel-headers/rdma/rdma_user_rxe.h b/kernel-headers/rdma/rdma_user_rxe.h index 70ac031e..a31465e2 100644 --- a/kernel-headers/rdma/rdma_user_rxe.h +++ b/kernel-headers/rdma/rdma_user_rxe.h @@ -160,6 +160,7 @@ struct rxe_recv_wqe { enum rxe_capabilities { RXE_CAP_NONE = 0, + RXE_CAP_CMD_EX = 1ULL << 0, }; struct rxe_alloc_context_cmd { diff --git a/providers/rxe/rxe.c b/providers/rxe/rxe.c index c29b7de5..b1fa2f42 100644 --- a/providers/rxe/rxe.c +++ b/providers/rxe/rxe.c @@ -87,6 +87,34 @@ static int rxe_query_device(struct ibv_context *context, return 0; } +static int rxe_query_device_ex(struct ibv_context *context, + const struct ibv_query_device_ex_input *input, + struct ibv_device_attr_ex *attr, + size_t attr_size) +{ + int ret; + uint64_t raw_fw_ver; + unsigned int major, minor, sub_minor; + struct ibv_query_device_ex cmd = {}; + struct ib_uverbs_ex_query_device_resp resp = {}; + + fprintf(stderr, "%s: called\n", __func__); + ret = ibv_cmd_query_device_ex(context, input, attr, sizeof(*attr), + &raw_fw_ver, &cmd, sizeof(cmd), + &resp, sizeof(resp)); + if (ret) + return ret; + + major = (raw_fw_ver >> 32) & 0xffff; + minor = (raw_fw_ver >> 16) & 0xffff; + sub_minor = raw_fw_ver & 0xffff; + + snprintf(attr->orig_attr.fw_ver, sizeof(attr->orig_attr.fw_ver), + "%d.%d.%d", major, minor, sub_minor); + + return 0; +} + static int rxe_query_port(struct ibv_context *context, uint8_t port, struct ibv_port_attr *attr) { @@ -860,6 +888,10 @@ static const struct verbs_context_ops rxe_ctx_ops = { .free_context = rxe_free_context, }; +static const struct verbs_context_ops rxe_ctx_ops_cmd_ex = { + .query_device_ex = rxe_query_device_ex, +}; + static struct verbs_context *rxe_alloc_context(struct ibv_device *ibdev, int cmd_fd, void *private_data) @@ -883,6 +915,9 @@ static struct verbs_context *rxe_alloc_context(struct ibv_device *ibdev, verbs_set_ops(&context->ibv_ctx, &rxe_ctx_ops); + if (context->capabilities & RXE_CAP_CMD_EX) + verbs_set_ops(&context->ibv_ctx, &rxe_ctx_ops_cmd_ex); + return &context->ibv_ctx; out: diff --git a/providers/rxe/rxe.h b/providers/rxe/rxe.h index 736cc30e..f9cae315 100644 --- a/providers/rxe/rxe.h +++ b/providers/rxe/rxe.h @@ -49,7 +49,7 @@ enum rdma_network_type { }; enum rxe_provider_cap { - RXE_PROVIDER_CAP = RXE_CAP_NONE, + RXE_PROVIDER_CAP = RXE_CAP_CMD_EX, }; struct rxe_device {
Implement ibv_query_device_ex verb. Make it depend on a RXE_CAP_CMD_EX capability bit supported by both provider and driver. Signed-off-by: Bob Pearson <rpearson@hpe.com> --- kernel-headers/rdma/rdma_user_rxe.h | 1 + providers/rxe/rxe.c | 35 +++++++++++++++++++++++++++++ providers/rxe/rxe.h | 2 +- 3 files changed, 37 insertions(+), 1 deletion(-)