[8/9] verbs: Remove dead code

Commit Message

Jason Gunthorpe Nov. 16, 2020, 8:23 p.m. UTC

Remove the old query_device support code, it is now replaced by
ibv_cmd_query_device_any()

Signed-off-by: Jason Gunthorpe <[e-mail]>
---
 libibverbs/cmd.c             | 99 ------------------------------------
 libibverbs/driver.h          |  8 ---
 libibverbs/libibverbs.map.in |  1 -
 3 files changed, 108 deletions(-)

Comments

Gal Pressman Nov. 18, 2020, 12:46 p.m. UTC | #1

On 16/11/2020 22:23, Jason Gunthorpe wrote:
> Remove the old query_device support code, it is now replaced by
> ibv_cmd_query_device_any()
> 
> Signed-off-by: Jason Gunthorpe <[e-mail]>

Shouldn't the legacy fallback in ibv_query_device_ex() be removed as well?

/**
 * ibv_query_device_ex - Get extended device properties
 */
static inline int
ibv_query_device_ex(struct ibv_context *context,
		    const struct ibv_query_device_ex_input *input,
		    struct ibv_device_attr_ex *attr)
{
	struct verbs_context *vctx;
	int ret;

	if (input && input->comp_mask)
		return EINVAL;

	vctx = verbs_get_ctx_op(context, query_device_ex);
	if (!vctx)
		goto legacy;

	ret = vctx->query_device_ex(context, input, attr, sizeof(*attr));
	if (ret == EOPNOTSUPP || ret == ENOSYS)
		goto legacy;

	return ret;

legacy:
	memset(attr, 0, sizeof(*attr));
	ret = ibv_query_device(context, &attr->orig_attr);

	return ret;
}

Jason Gunthorpe Nov. 18, 2020, 12:53 p.m. UTC | #2

On Wed, Nov 18, 2020 at 02:46:05PM +0200, Gal Pressman wrote:
> On 16/11/2020 22:23, Jason Gunthorpe wrote:
> > Remove the old query_device support code, it is now replaced by
> > ibv_cmd_query_device_any()
> > 
> > Signed-off-by: Jason Gunthorpe <[e-mail]>
> 
> Shouldn't the legacy fallback in ibv_query_device_ex() be removed as well?

I thought about it, and yes we could convert that into a real function
call and remove all the junk

But if we do that it causes a symbol ver dependencies and if we are
going that far then we should really just do all of the
verbs_get_ctx_op() inlines and be done with that mess entirely.

This would mean anything linked with a new rdma-core would not be able
to load on any of the older ones..

I've been saving that gem for some future adventure since it seems to
cause some general pain. It would be good to couple it with some other
verbs.h clean up as well

Jason

Patch

diff --git a/libibverbs/cmd.c b/libibverbs/cmd.c
index a439f8c06481dd..ec9750e7c04eb4 100644
--- a/libibverbs/cmd.c
+++ b/libibverbs/cmd.c
@@ -44,7 +44,6 @@ 
 #include <infiniband/cmd_write.h>
 #include "ibverbs.h"
 #include <ccan/minmax.h>
-#include <util/util.h>
 
 bool verbs_allow_disassociate_destroy;
 
@@ -113,104 +112,6 @@  int ibv_cmd_query_device(struct ibv_context *context,
 	return 0;
 }
 
-int ibv_cmd_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,
-			    uint64_t *raw_fw_ver,
-			    struct ibv_query_device_ex *cmd,
-			    size_t cmd_size,
-			    struct ib_uverbs_ex_query_device_resp *resp,
-			    size_t resp_size)
-{
-	int err;
-
-	if (input && input->comp_mask)
-		return EINVAL;
-
-	if (attr_size < offsetof(struct ibv_device_attr_ex, comp_mask) +
-			sizeof(attr->comp_mask))
-		return EINVAL;
-
-	cmd->comp_mask = 0;
-	cmd->reserved = 0;
-	memset(attr->orig_attr.fw_ver, 0, sizeof(attr->orig_attr.fw_ver));
-	memset(&attr->comp_mask, 0, attr_size - sizeof(attr->orig_attr));
-
-	err = execute_cmd_write_ex(context, IB_USER_VERBS_EX_CMD_QUERY_DEVICE,
-				   cmd, cmd_size, resp, resp_size);
-	if (err)
-		return err;
-
-	copy_query_dev_fields(&attr->orig_attr, &resp->base, raw_fw_ver);
-	/* Report back supported comp_mask bits. For now no comp_mask bit is
-	 * defined */
-	attr->comp_mask = resp->comp_mask & 0;
-
-#define CAN_COPY(_ibv_attr, _uverbs_attr)                                      \
-	(attr_size >= offsetofend(struct ibv_device_attr_ex, _ibv_attr) &&     \
-	 resp->response_length >=                                              \
-		 offsetofend(struct ib_uverbs_ex_query_device_resp,            \
-			     _uverbs_attr))
-
-	if (CAN_COPY(odp_caps, odp_caps)) {
-		attr->odp_caps.general_caps = resp->odp_caps.general_caps;
-		attr->odp_caps.per_transport_caps.rc_odp_caps =
-			resp->odp_caps.per_transport_caps.rc_odp_caps;
-		attr->odp_caps.per_transport_caps.uc_odp_caps =
-			resp->odp_caps.per_transport_caps.uc_odp_caps;
-		attr->odp_caps.per_transport_caps.ud_odp_caps =
-			resp->odp_caps.per_transport_caps.ud_odp_caps;
-	}
-
-	if (CAN_COPY(completion_timestamp_mask, timestamp_mask))
-		attr->completion_timestamp_mask = resp->timestamp_mask;
-
-	if (CAN_COPY(hca_core_clock, hca_core_clock))
-		attr->hca_core_clock = resp->hca_core_clock;
-
-	if (CAN_COPY(device_cap_flags_ex, device_cap_flags_ex))
-		attr->device_cap_flags_ex = resp->device_cap_flags_ex;
-
-	if (CAN_COPY(rss_caps, rss_caps)) {
-		attr->rss_caps.supported_qpts = resp->rss_caps.supported_qpts;
-		attr->rss_caps.max_rwq_indirection_tables =
-			resp->rss_caps.max_rwq_indirection_tables;
-		attr->rss_caps.max_rwq_indirection_table_size =
-			resp->rss_caps.max_rwq_indirection_table_size;
-	}
-
-	if (CAN_COPY(max_wq_type_rq, max_wq_type_rq))
-		attr->max_wq_type_rq = resp->max_wq_type_rq;
-
-	if (CAN_COPY(raw_packet_caps, raw_packet_caps))
-		attr->raw_packet_caps = resp->raw_packet_caps;
-
-	if (CAN_COPY(tm_caps, tm_caps)) {
-		attr->tm_caps.max_rndv_hdr_size =
-			resp->tm_caps.max_rndv_hdr_size;
-		attr->tm_caps.max_num_tags = resp->tm_caps.max_num_tags;
-		attr->tm_caps.flags = resp->tm_caps.flags;
-		attr->tm_caps.max_ops = resp->tm_caps.max_ops;
-		attr->tm_caps.max_sge = resp->tm_caps.max_sge;
-	}
-
-	if (CAN_COPY(cq_mod_caps, cq_moderation_caps)) {
-		attr->cq_mod_caps.max_cq_count =
-			resp->cq_moderation_caps.max_cq_moderation_count;
-		attr->cq_mod_caps.max_cq_period =
-			resp->cq_moderation_caps.max_cq_moderation_period;
-	}
-
-	if (CAN_COPY(max_dm_size, max_dm_size))
-		attr->max_dm_size = resp->max_dm_size;
-
-	if (CAN_COPY(xrc_odp_caps, xrc_odp_caps))
-		attr->xrc_odp_caps = resp->xrc_odp_caps;
-#undef CAN_COPY
-
-	return 0;
-}
-
 int ibv_cmd_alloc_pd(struct ibv_context *context, struct ibv_pd *pd,
 		     struct ibv_alloc_pd *cmd, size_t cmd_size,
 		     struct ib_uverbs_alloc_pd_resp *resp, size_t resp_size)
diff --git a/libibverbs/driver.h b/libibverbs/driver.h
index e54db0ea6413e8..33998e227c98ec 100644
--- a/libibverbs/driver.h
+++ b/libibverbs/driver.h
@@ -465,14 +465,6 @@  int ibv_cmd_query_device_any(struct ibv_context *context,
 			     struct ibv_device_attr_ex *attr, size_t attr_size,
 			     struct ib_uverbs_ex_query_device_resp *resp,
 			     size_t *resp_size);
-int ibv_cmd_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,
-			    uint64_t *raw_fw_ver,
-			    struct ibv_query_device_ex *cmd,
-			    size_t cmd_size,
-			    struct ib_uverbs_ex_query_device_resp *resp,
-			    size_t resp_size);
 int ibv_cmd_query_port(struct ibv_context *context, uint8_t port_num,
 		       struct ibv_port_attr *port_attr,
 		       struct ibv_query_port *cmd, size_t cmd_size);
diff --git a/libibverbs/libibverbs.map.in b/libibverbs/libibverbs.map.in
index c1f7e09b240ab0..672717a6fa551e 100644
--- a/libibverbs/libibverbs.map.in
+++ b/libibverbs/libibverbs.map.in
@@ -205,7 +205,6 @@  IBVERBS_PRIVATE_@IBVERBS_PABI_VERSION@ {
 		ibv_cmd_query_context;
 		ibv_cmd_query_device;
 		ibv_cmd_query_device_any;
-		ibv_cmd_query_device_ex;
 		ibv_cmd_query_mr;
 		ibv_cmd_query_port;
 		ibv_cmd_query_qp;