[PATCH rdma-core 0/8] Small verbs cleanups

[PATCH rdma-core 0/8] Small verbs cleanups

[Jason Gunthorpe]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Small verbs changes that should have no binary impact. These are just little
cleanups I've noticed lately.

This is a PR:

https://github.com/linux-rdma/rdma-core/pull/270

Jason Gunthorpe (8):
  verbs: Fix typo in copying IBV_FLOW_SPEC_UDP/TCP 'val'
  verbs: Remove duplicate NULL test from flow ops
  verbs: Get rid of verbs_context->has_comp_mask
  verbs: Bitwise flag values should be unsigned
  verbs: Remove bogus cq_fd
  verbs: Fill in the libibverbs.map file
  verbs: Remove dead _HAS_ defines for providers
  verbs: Get rid of verbs_set_ctx_op

 libibverbs/cmd.c                  |  2 +-
 libibverbs/device.c               |  3 ---
 libibverbs/driver.h               |  2 --
 libibverbs/libibverbs.map.in      | 56 ++++++++++++++++++++++++++++-----------
 libibverbs/man/ibv_query_device.3 |  2 +-
 libibverbs/verbs.h                | 36 ++++++++-----------------
 providers/mlx4/cq.c               |  2 +-
 providers/mlx4/mlx4.c             | 36 ++++++++++++-------------
 providers/mlx5/cq.c               |  2 +-
 providers/mlx5/mlx5.c             | 34 ++++++++++++------------
 providers/mlx5/mlx5.h             |  2 +-
 11 files changed, 90 insertions(+), 87 deletions(-)

-- 
2.15.1

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

[PATCH 1/8] verbs: Fix typo in copying IBV_FLOW_SPEC_UDP/TCP 'val'

[Jason Gunthorpe]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The 'val' is a struct ibv_flow_tcp_udp_filter, not a ipv4_filter.

ipv4_filter is 8 bytes while tcp_udp_filter is only 4 bytes. So this
does not result in any bug, it is just a mistake.

Fixes: 389de6a6ef4e ("Add receive flow steering support")
Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 libibverbs/cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libibverbs/cmd.c b/libibverbs/cmd.c
index 62fcd0e4e2f400..b957550ed6fbdb 100644
--- a/libibverbs/cmd.c
+++ b/libibverbs/cmd.c
@@ -1844,7 +1844,7 @@ static int ib_spec_to_kern_spec(struct ibv_flow_spec *ib_spec,
 	case IBV_FLOW_SPEC_UDP | IBV_FLOW_SPEC_INNER:
 		kern_spec->tcp_udp.size = sizeof(struct ibv_kern_spec_tcp_udp);
 		memcpy(&kern_spec->tcp_udp.val, &ib_spec->tcp_udp.val,
-		       sizeof(struct ibv_flow_ipv4_filter));
+		       sizeof(struct ibv_flow_tcp_udp_filter));
 		memcpy(&kern_spec->tcp_udp.mask, &ib_spec->tcp_udp.mask,
 		       sizeof(struct ibv_flow_tcp_udp_filter));
 		break;
-- 
2.15.1

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

[PATCH 2/8] verbs: Remove duplicate NULL test from flow ops

[Jason Gunthorpe]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The verbs_get_ctx_op macro already checks if the op is NULL.

Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 libibverbs/verbs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libibverbs/verbs.h b/libibverbs/verbs.h
index b39dc30a6d2a6e..b9db76bc628707 100644
--- a/libibverbs/verbs.h
+++ b/libibverbs/verbs.h
@@ -1832,7 +1832,7 @@ static inline struct ibv_flow *ibv_create_flow(struct ibv_qp *qp,
 {
 	struct verbs_context *vctx = verbs_get_ctx_op(qp->context,
 						      ibv_create_flow);
-	if (!vctx || !vctx->ibv_create_flow) {
+	if (!vctx) {
 		errno = ENOSYS;
 		return NULL;
 	}
@@ -1844,7 +1844,7 @@ static inline int ibv_destroy_flow(struct ibv_flow *flow_id)
 {
 	struct verbs_context *vctx = verbs_get_ctx_op(flow_id->context,
 						      ibv_destroy_flow);
-	if (!vctx || !vctx->ibv_destroy_flow)
+	if (!vctx)
 		return -ENOSYS;
 	return vctx->ibv_destroy_flow(flow_id);
 }
-- 
2.15.1

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

[PATCH 3/8] verbs: Get rid of verbs_context->has_comp_mask

[Jason Gunthorpe]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

This never made any sense, and was never used. Just get rid of the
vestigial remains.

Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 libibverbs/verbs.h    | 11 +----------
 providers/mlx4/mlx4.c |  2 --
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/libibverbs/verbs.h b/libibverbs/verbs.h
index b9db76bc628707..9a3d09e27dc7f8 100644
--- a/libibverbs/verbs.h
+++ b/libibverbs/verbs.h
@@ -1650,15 +1650,6 @@ struct ibv_values_ex {
 	struct timespec raw_clock;
 };
 
-enum verbs_context_mask {
-	VERBS_CONTEXT_XRCD	= 1 << 0,
-	VERBS_CONTEXT_SRQ	= 1 << 1,
-	VERBS_CONTEXT_QP	= 1 << 2,
-	VERBS_CONTEXT_CREATE_FLOW = 1 << 3,
-	VERBS_CONTEXT_DESTROY_FLOW = 1 << 4,
-	VERBS_CONTEXT_RESERVED	= 1 << 5
-};
-
 struct verbs_context {
 	/*  "grows up" - new fields go here */
 	int (*modify_cq)(struct ibv_cq *cq, struct ibv_modify_cq_attr *attr);
@@ -1696,7 +1687,7 @@ struct verbs_context {
 	struct ibv_xrcd *	(*open_xrcd)(struct ibv_context *context,
 					     struct ibv_xrcd_init_attr *xrcd_init_attr);
 	int			(*close_xrcd)(struct ibv_xrcd *xrcd);
-	uint64_t has_comp_mask;
+	uint64_t _ABI_placeholder3;
 	size_t   sz;			/* Must be immediately before struct ibv_context */
 	struct ibv_context context;	/* Must be last field in the struct */
 };
diff --git a/providers/mlx4/mlx4.c b/providers/mlx4/mlx4.c
index 0ed77d2c021513..1ba2b427071fcb 100644
--- a/providers/mlx4/mlx4.c
+++ b/providers/mlx4/mlx4.c
@@ -238,8 +238,6 @@ static int mlx4_init_context(struct verbs_device *v_device,
 			mlx4_map_internal_clock(dev, ibv_ctx);
 	}
 
-	verbs_ctx->has_comp_mask = VERBS_CONTEXT_XRCD | VERBS_CONTEXT_SRQ |
-					VERBS_CONTEXT_QP;
 	verbs_set_ctx_op(verbs_ctx, close_xrcd, mlx4_close_xrcd);
 	verbs_set_ctx_op(verbs_ctx, open_xrcd, mlx4_open_xrcd);
 	verbs_set_ctx_op(verbs_ctx, create_srq_ex, mlx4_create_srq_ex);
-- 
2.15.1

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

[PATCH 4/8] verbs: Bitwise flag values should be unsigned

[Jason Gunthorpe]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Casting rules in C call for sign extension in a number of
cases, so if a bitwise value has the high bit set, then
it can become subtly corrupted if the value is carelessly
casted to a larger type. eg:

  uint64_t val64 = (int32_t)(1ULL<<31);

Gives 0xffffffff80000000 instead of 0x80000000

A trivial fix is to make all the storage for bitwise
values unsigned.

Since switching 'int' to 'unsigned int' is an ABI compatible
change, just go ahead and switch all obvious cases.

Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 libibverbs/man/ibv_query_device.3 |  2 +-
 libibverbs/verbs.h                | 16 ++++++++--------
 providers/mlx4/cq.c               |  2 +-
 providers/mlx5/cq.c               |  2 +-
 providers/mlx5/mlx5.h             |  2 +-
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/libibverbs/man/ibv_query_device.3 b/libibverbs/man/ibv_query_device.3
index 0e9d2860d2b4f4..94dc8a50c06b87 100644
--- a/libibverbs/man/ibv_query_device.3
+++ b/libibverbs/man/ibv_query_device.3
@@ -32,7 +32,7 @@ uint32_t                vendor_part_id;         /* Vendor supplied part ID */
 uint32_t                hw_ver;                 /* Hardware version */
 int                     max_qp;                 /* Maximum number of supported QPs */
 int                     max_qp_wr;              /* Maximum number of outstanding WR on any work queue */
-int                     device_cap_flags;       /* HCA capabilities mask */
+unsigned int            device_cap_flags;       /* HCA capabilities mask */
 int                     max_sge;                /* Maximum number of s/g per WR for SQ & RQ of QP for non RDMA Read operations */
 int                     max_sge_rd;             /* Maximum number of s/g per WR for RDMA Read operations */
 int                     max_cq;                 /* Maximum number of supported CQs */
diff --git a/libibverbs/verbs.h b/libibverbs/verbs.h
index 9a3d09e27dc7f8..34995d9c4437fa 100644
--- a/libibverbs/verbs.h
+++ b/libibverbs/verbs.h
@@ -161,7 +161,7 @@ struct ibv_device_attr {
 	uint32_t		hw_ver;
 	int			max_qp;
 	int			max_qp_wr;
-	int			device_cap_flags;
+	unsigned int		device_cap_flags;
 	int			max_sge;
 	int			max_sge_rd;
 	int			max_cq;
@@ -527,7 +527,7 @@ struct ibv_wc {
 	};
 	uint32_t		qp_num;
 	uint32_t		src_qp;
-	int			wc_flags;
+	unsigned int		wc_flags;
 	uint16_t		pkey_index;
 	uint16_t		slid;
 	uint8_t			sl;
@@ -548,7 +548,7 @@ struct ibv_mw_bind_info {
 	struct ibv_mr	*mr;
 	uint64_t	 addr;
 	uint64_t	 length;
-	int		 mw_access_flags; /* use ibv_access_flags */
+	unsigned int	 mw_access_flags; /* use ibv_access_flags */
 };
 
 struct ibv_pd {
@@ -954,7 +954,7 @@ struct ibv_qp_attr {
 	uint32_t		rq_psn;
 	uint32_t		sq_psn;
 	uint32_t		dest_qp_num;
-	int			qp_access_flags;
+	unsigned int		qp_access_flags;
 	struct ibv_qp_cap	cap;
 	struct ibv_ah_attr	ah_attr;
 	struct ibv_ah_attr	alt_ah_attr;
@@ -1008,7 +1008,7 @@ struct ibv_send_wr {
 	struct ibv_sge	       *sg_list;
 	int			num_sge;
 	enum ibv_wr_opcode	opcode;
-	int			send_flags;
+	unsigned int		send_flags;
 	/* When opcode is *_WITH_IMM: Immediate data in network byte order.
 	 * When opcode is *_INV: Stores the rkey to invalidate
 	 */
@@ -1090,7 +1090,7 @@ struct ibv_ops_wr {
 
 struct ibv_mw_bind {
 	uint64_t		wr_id;
-	int			send_flags;
+	unsigned int		send_flags;
 	struct ibv_mw_bind_info bind_info;
 };
 
@@ -1204,7 +1204,7 @@ struct ibv_cq_ex {
 	__be32 (*read_imm_data)(struct ibv_cq_ex *current);
 	uint32_t (*read_qp_num)(struct ibv_cq_ex *current);
 	uint32_t (*read_src_qp)(struct ibv_cq_ex *current);
-	int (*read_wc_flags)(struct ibv_cq_ex *current);
+	unsigned int (*read_wc_flags)(struct ibv_cq_ex *current);
 	uint32_t (*read_slid)(struct ibv_cq_ex *current);
 	uint8_t (*read_sl)(struct ibv_cq_ex *current);
 	uint8_t (*read_dlid_path_bits)(struct ibv_cq_ex *current);
@@ -1290,7 +1290,7 @@ static inline uint32_t ibv_wc_read_src_qp(struct ibv_cq_ex *cq)
 	return cq->read_src_qp(cq);
 }
 
-static inline int ibv_wc_read_wc_flags(struct ibv_cq_ex *cq)
+static inline unsigned int ibv_wc_read_wc_flags(struct ibv_cq_ex *cq)
 {
 	return cq->read_wc_flags(cq);
 }
diff --git a/providers/mlx4/cq.c b/providers/mlx4/cq.c
index 8ced49e9592430..697525f43ecaef 100644
--- a/providers/mlx4/cq.c
+++ b/providers/mlx4/cq.c
@@ -501,7 +501,7 @@ static uint32_t mlx4_cq_read_wc_qp_num(struct ibv_cq_ex *ibcq)
 	return be32toh(cq->cqe->vlan_my_qpn) & MLX4_CQE_QPN_MASK;
 }
 
-static int mlx4_cq_read_wc_flags(struct ibv_cq_ex *ibcq)
+static unsigned int mlx4_cq_read_wc_flags(struct ibv_cq_ex *ibcq)
 {
 	struct mlx4_cq *cq = to_mcq(ibv_cq_ex_to_cq(ibcq));
 	int is_send  = cq->cqe->owner_sr_opcode & MLX4_CQE_IS_SEND_MASK;
diff --git a/providers/mlx5/cq.c b/providers/mlx5/cq.c
index 8a1b1671c77426..e96418f6c66ad5 100644
--- a/providers/mlx5/cq.c
+++ b/providers/mlx5/cq.c
@@ -1300,7 +1300,7 @@ static inline uint32_t mlx5_cq_read_wc_qp_num(struct ibv_cq_ex *ibcq)
 	return be32toh(cq->cqe64->sop_drop_qpn) & 0xffffff;
 }
 
-static inline int mlx5_cq_read_wc_flags(struct ibv_cq_ex *ibcq)
+static inline unsigned int mlx5_cq_read_wc_flags(struct ibv_cq_ex *ibcq)
 {
 	struct mlx5_cq *cq = to_mcq(ibv_cq_ex_to_cq(ibcq));
 	int wc_flags = 0;
diff --git a/providers/mlx5/mlx5.h b/providers/mlx5/mlx5.h
index b4782dd0e313c1..f5b493104cf197 100644
--- a/providers/mlx5/mlx5.h
+++ b/providers/mlx5/mlx5.h
@@ -278,7 +278,7 @@ struct mlx5_context {
 	struct list_head                hugetlb_list;
 	int				cqe_version;
 	uint8_t				cached_link_layer[MLX5_MAX_PORTS_NUM];
-	int				cached_device_cap_flags;
+	unsigned int			cached_device_cap_flags;
 	enum ibv_atomic_cap		atomic_cap;
 	struct {
 		uint64_t                offset;
-- 
2.15.1

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

[PATCH 5/8] verbs: Remove bogus cq_fd

[Jason Gunthorpe]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

No reason to call close on something we know is -1.

This was left by accident after ABI version < 2 support was removed.

Fixes: 6be16586e081 ("Infrastructure to support verbs extensions")
Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 libibverbs/device.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/libibverbs/device.c b/libibverbs/device.c
index 16a7d4caeac5a0..4fb759c6474ad7 100644
--- a/libibverbs/device.c
+++ b/libibverbs/device.c
@@ -275,7 +275,6 @@ LATEST_SYMVER_FUNC(ibv_close_device, 1_1, "IBVERBS_1.1",
 {
 	int async_fd = context->async_fd;
 	int cmd_fd   = context->cmd_fd;
-	int cq_fd    = -1;
 	struct verbs_context *context_ex;
 	struct verbs_device *verbs_device = verbs_get_device(context->device);
 	struct ibv_device *device = context->device;
@@ -291,8 +290,6 @@ LATEST_SYMVER_FUNC(ibv_close_device, 1_1, "IBVERBS_1.1",
 
 	close(async_fd);
 	close(cmd_fd);
-	if (abi_ver <= 2)
-		close(cq_fd);
 	ibverbs_device_put(device);
 
 	return 0;
-- 
2.15.1

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

[PATCH 6/8] verbs: Fill in the libibverbs.map file

[Jason Gunthorpe]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Several of the 'latest' verbs are missing from the map file.

The linker does the right thing anyhow since they are tagged with the
.symver assembler directive, but let us list them anyhow so that the
map file is a complete list of symbols.

This also sorts the list

Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 libibverbs/libibverbs.map.in | 56 +++++++++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 16 deletions(-)

diff --git a/libibverbs/libibverbs.map.in b/libibverbs/libibverbs.map.in
index d65f06c43de992..3f635a94b82d58 100644
--- a/libibverbs/libibverbs.map.in
+++ b/libibverbs/libibverbs.map.in
@@ -53,29 +53,53 @@ IBVERBS_1.0 {
 
 IBVERBS_1.1 {
 	global:
-		ibv_get_device_list;
-		ibv_free_device_list;
-		ibv_get_device_name;
-		ibv_get_device_guid;
-		ibv_open_device;
+		ibv_ack_async_event;
+		ibv_ack_cq_events;
+		ibv_alloc_pd;
+		ibv_attach_mcast;
 		ibv_close_device;
-
-		ibv_init_ah_from_wc;
+		ibv_create_ah;
 		ibv_create_ah_from_wc;
-		ibv_fork_init;
-		ibv_dontfork_range;
+		ibv_create_cq;
+		ibv_create_qp;
+		ibv_create_srq;
+		ibv_dealloc_pd;
+		ibv_dereg_mr;
+		ibv_destroy_ah;
+		ibv_destroy_cq;
+		ibv_destroy_qp;
+		ibv_destroy_srq;
+		ibv_detach_mcast;
 		ibv_dofork_range;
-		ibv_register_driver;
-
+		ibv_dontfork_range;
+		ibv_event_type_str;
+		ibv_fork_init;
+		ibv_free_device_list;
+		ibv_get_async_event;
+		ibv_get_cq_event;
+		ibv_get_device_guid;
+		ibv_get_device_list;
+		ibv_get_device_name;
+		ibv_init_ah_from_wc;
+		ibv_modify_qp;
+		ibv_modify_srq;
 		ibv_node_type_str;
+		ibv_open_device;
 		ibv_port_state_str;
-		ibv_event_type_str;
-		ibv_wc_status_str;
-
+		ibv_query_device;
+		ibv_query_gid;
+		ibv_query_pkey;
+		ibv_query_port;
+		ibv_query_qp;
+		ibv_query_srq;
 		ibv_rate_to_mbps;
-		mbps_to_ibv_rate;
-
+		ibv_reg_mr;
+		ibv_register_driver;
+		ibv_rereg_mr;
+		ibv_resize_cq;
 		ibv_resolve_eth_l2_from_gid;
+		ibv_wc_status_str;
+		mbps_to_ibv_rate;
 
 		/* These historical symbols are now private to libibverbs, but used by
 		   other rdma-core libraries. Do not change them. */
-- 
2.15.1

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

[PATCH 7/8] verbs: Remove dead _HAS_ defines for providers

[Jason Gunthorpe]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Nothing looks at these anymore.

Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 libibverbs/driver.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libibverbs/driver.h b/libibverbs/driver.h
index 04071a73583406..4f2a9486880e11 100644
--- a/libibverbs/driver.h
+++ b/libibverbs/driver.h
@@ -247,7 +247,6 @@ int ibv_cmd_open_xrcd(struct ibv_context *context, struct verbs_xrcd *xrcd,
 		      struct ibv_open_xrcd *cmd, size_t cmd_size,
 		      struct ibv_open_xrcd_resp *resp, size_t resp_size);
 int ibv_cmd_close_xrcd(struct verbs_xrcd *xrcd);
-#define IBV_CMD_REG_MR_HAS_RESP_PARAMS
 int ibv_cmd_reg_mr(struct ibv_pd *pd, void *addr, size_t length,
 		   uint64_t hca_va, int access,
 		   struct ibv_mr *mr, struct ibv_reg_mr *cmd,
@@ -281,7 +280,6 @@ int ibv_cmd_create_cq_ex(struct ibv_context *context,
 			 size_t resp_size);
 int ibv_cmd_poll_cq(struct ibv_cq *cq, int ne, struct ibv_wc *wc);
 int ibv_cmd_req_notify_cq(struct ibv_cq *cq, int solicited_only);
-#define IBV_CMD_RESIZE_CQ_HAS_RESP_PARAMS
 int ibv_cmd_resize_cq(struct ibv_cq *cq, int cqe,
 		      struct ibv_resize_cq *cmd, size_t cmd_size,
 		      struct ibv_resize_cq_resp *resp, size_t resp_size);
-- 
2.15.1

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

[PATCH 8/8] verbs: Get rid of verbs_set_ctx_op

[Jason Gunthorpe]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

No longer necessary now that the providers are private to verbs.

Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 libibverbs/verbs.h    |  5 -----
 providers/mlx4/mlx4.c | 34 +++++++++++++++++-----------------
 providers/mlx5/mlx5.c | 34 +++++++++++++++++-----------------
 3 files changed, 34 insertions(+), 39 deletions(-)

diff --git a/libibverbs/verbs.h b/libibverbs/verbs.h
index 34995d9c4437fa..58d8d8e3c31509 100644
--- a/libibverbs/verbs.h
+++ b/libibverbs/verbs.h
@@ -1703,11 +1703,6 @@ static inline struct verbs_context *verbs_get_ctx(struct ibv_context *ctx)
 	(!__vctx || (__vctx->sz < sizeof(*__vctx) - offsetof(struct verbs_context, op)) || \
 	 !__vctx->op) ? NULL : __vctx; })
 
-#define verbs_set_ctx_op(_vctx, op, ptr) ({ \
-	struct verbs_context *vctx = _vctx; \
-	if (vctx && (vctx->sz >= sizeof(*vctx) - offsetof(struct verbs_context, op))) \
-		vctx->op = ptr; })
-
 /**
  * ibv_get_device_list - Get list of IB devices currently available
  * @num_devices: optional.  if non-NULL, set to the number of devices
diff --git a/providers/mlx4/mlx4.c b/providers/mlx4/mlx4.c
index 1ba2b427071fcb..d2ccd328625c80 100644
--- a/providers/mlx4/mlx4.c
+++ b/providers/mlx4/mlx4.c
@@ -238,23 +238,23 @@ static int mlx4_init_context(struct verbs_device *v_device,
 			mlx4_map_internal_clock(dev, ibv_ctx);
 	}
 
-	verbs_set_ctx_op(verbs_ctx, close_xrcd, mlx4_close_xrcd);
-	verbs_set_ctx_op(verbs_ctx, open_xrcd, mlx4_open_xrcd);
-	verbs_set_ctx_op(verbs_ctx, create_srq_ex, mlx4_create_srq_ex);
-	verbs_set_ctx_op(verbs_ctx, get_srq_num, verbs_get_srq_num);
-	verbs_set_ctx_op(verbs_ctx, create_qp_ex, mlx4_create_qp_ex);
-	verbs_set_ctx_op(verbs_ctx, open_qp, mlx4_open_qp);
-	verbs_set_ctx_op(verbs_ctx, ibv_create_flow, mlx4_create_flow);
-	verbs_set_ctx_op(verbs_ctx, ibv_destroy_flow, mlx4_destroy_flow);
-	verbs_set_ctx_op(verbs_ctx, create_cq_ex, mlx4_create_cq_ex);
-	verbs_set_ctx_op(verbs_ctx, query_device_ex, mlx4_query_device_ex);
-	verbs_set_ctx_op(verbs_ctx, query_rt_values, mlx4_query_rt_values);
-	verbs_set_ctx_op(verbs_ctx, create_wq, mlx4_create_wq);
-	verbs_set_ctx_op(verbs_ctx, modify_wq, mlx4_modify_wq);
-	verbs_set_ctx_op(verbs_ctx, destroy_wq, mlx4_destroy_wq);
-	verbs_set_ctx_op(verbs_ctx, create_rwq_ind_table, mlx4_create_rwq_ind_table);
-	verbs_set_ctx_op(verbs_ctx, destroy_rwq_ind_table, mlx4_destroy_rwq_ind_table);
-	verbs_set_ctx_op(verbs_ctx, modify_cq, mlx4_modify_cq);
+	verbs_ctx->close_xrcd = mlx4_close_xrcd;
+	verbs_ctx->open_xrcd = mlx4_open_xrcd;
+	verbs_ctx->create_srq_ex = mlx4_create_srq_ex;
+	verbs_ctx->get_srq_num = verbs_get_srq_num;
+	verbs_ctx->create_qp_ex = mlx4_create_qp_ex;
+	verbs_ctx->open_qp = mlx4_open_qp;
+	verbs_ctx->ibv_create_flow = mlx4_create_flow;
+	verbs_ctx->ibv_destroy_flow = mlx4_destroy_flow;
+	verbs_ctx->create_cq_ex = mlx4_create_cq_ex;
+	verbs_ctx->query_device_ex = mlx4_query_device_ex;
+	verbs_ctx->query_rt_values = mlx4_query_rt_values;
+	verbs_ctx->create_wq = mlx4_create_wq;
+	verbs_ctx->modify_wq = mlx4_modify_wq;
+	verbs_ctx->destroy_wq = mlx4_destroy_wq;
+	verbs_ctx->create_rwq_ind_table = mlx4_create_rwq_ind_table;
+	verbs_ctx->destroy_rwq_ind_table = mlx4_destroy_rwq_ind_table;
+	verbs_ctx->modify_cq = mlx4_modify_cq;
 
 	return 0;
 
diff --git a/providers/mlx5/mlx5.c b/providers/mlx5/mlx5.c
index 70afbd46f57b05..f1379abac482e1 100644
--- a/providers/mlx5/mlx5.c
+++ b/providers/mlx5/mlx5.c
@@ -999,23 +999,23 @@ static int mlx5_init_context(struct verbs_device *vdev,
 
 	context->ibv_ctx.ops = mlx5_ctx_ops;
 
-	verbs_set_ctx_op(v_ctx, create_qp_ex, mlx5_create_qp_ex);
-	verbs_set_ctx_op(v_ctx, open_xrcd, mlx5_open_xrcd);
-	verbs_set_ctx_op(v_ctx, close_xrcd, mlx5_close_xrcd);
-	verbs_set_ctx_op(v_ctx, create_srq_ex, mlx5_create_srq_ex);
-	verbs_set_ctx_op(v_ctx, get_srq_num, mlx5_get_srq_num);
-	verbs_set_ctx_op(v_ctx, query_device_ex, mlx5_query_device_ex);
-	verbs_set_ctx_op(v_ctx, query_rt_values, mlx5_query_rt_values);
-	verbs_set_ctx_op(v_ctx, ibv_create_flow, mlx5_create_flow);
-	verbs_set_ctx_op(v_ctx, ibv_destroy_flow, mlx5_destroy_flow);
-	verbs_set_ctx_op(v_ctx, create_cq_ex, mlx5_create_cq_ex);
-	verbs_set_ctx_op(v_ctx, create_wq, mlx5_create_wq);
-	verbs_set_ctx_op(v_ctx, modify_wq, mlx5_modify_wq);
-	verbs_set_ctx_op(v_ctx, destroy_wq, mlx5_destroy_wq);
-	verbs_set_ctx_op(v_ctx, create_rwq_ind_table, mlx5_create_rwq_ind_table);
-	verbs_set_ctx_op(v_ctx, destroy_rwq_ind_table, mlx5_destroy_rwq_ind_table);
-	verbs_set_ctx_op(v_ctx, post_srq_ops, mlx5_post_srq_ops);
-	verbs_set_ctx_op(v_ctx, modify_cq, mlx5_modify_cq);
+	v_ctx->create_qp_ex = mlx5_create_qp_ex;
+	v_ctx->open_xrcd = mlx5_open_xrcd;
+	v_ctx->close_xrcd = mlx5_close_xrcd;
+	v_ctx->create_srq_ex = mlx5_create_srq_ex;
+	v_ctx->get_srq_num = mlx5_get_srq_num;
+	v_ctx->query_device_ex = mlx5_query_device_ex;
+	v_ctx->query_rt_values = mlx5_query_rt_values;
+	v_ctx->ibv_create_flow = mlx5_create_flow;
+	v_ctx->ibv_destroy_flow = mlx5_destroy_flow;
+	v_ctx->create_cq_ex = mlx5_create_cq_ex;
+	v_ctx->create_wq = mlx5_create_wq;
+	v_ctx->modify_wq = mlx5_modify_wq;
+	v_ctx->destroy_wq = mlx5_destroy_wq;
+	v_ctx->create_rwq_ind_table = mlx5_create_rwq_ind_table;
+	v_ctx->destroy_rwq_ind_table = mlx5_destroy_rwq_ind_table;
+	v_ctx->post_srq_ops = mlx5_post_srq_ops;
+	v_ctx->modify_cq = mlx5_modify_cq;
 
 	memset(&device_attr, 0, sizeof(device_attr));
 	if (!mlx5_query_device_ex(ctx, NULL, &device_attr,
-- 
2.15.1

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

Re: [PATCH 6/8] verbs: Fill in the libibverbs.map file

[Yishai Hadas]

On 12/6/2017 1:17 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Several of the 'latest' verbs are missing from the map file.

Which 'latest' verbs are you referring to ? quite hard to follow post 
the sort.

> The linker does the right thing anyhow since they are tagged with the
> .symver assembler directive, but let us list them anyhow so that the
> map file is a complete list of symbols.
> 

If the linker does the work why do we really need to maintain it now and 
in the future ?

> This also sorts the list
> 
> Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>   libibverbs/libibverbs.map.in | 56 +++++++++++++++++++++++++++++++-------------
>   1 file changed, 40 insertions(+), 16 deletions(-)
> 
> diff --git a/libibverbs/libibverbs.map.in b/libibverbs/libibverbs.map.in
> index d65f06c43de992..3f635a94b82d58 100644
> --- a/libibverbs/libibverbs.map.in
> +++ b/libibverbs/libibverbs.map.in
> @@ -53,29 +53,53 @@ IBVERBS_1.0 {
>   
>   IBVERBS_1.1 {
>   	global:
> -		ibv_get_device_list;
> -		ibv_free_device_list;
> -		ibv_get_device_name;
> -		ibv_get_device_guid;
> -		ibv_open_device;
> +		ibv_ack_async_event;
> +		ibv_ack_cq_events;
> +		ibv_alloc_pd;
> +		ibv_attach_mcast;
>   		ibv_close_device;
> -
> -		ibv_init_ah_from_wc;
> +		ibv_create_ah;
>   		ibv_create_ah_from_wc;
> -		ibv_fork_init;
> -		ibv_dontfork_range;
> +		ibv_create_cq;
> +		ibv_create_qp;
> +		ibv_create_srq;
> +		ibv_dealloc_pd;
> +		ibv_dereg_mr;
> +		ibv_destroy_ah;
> +		ibv_destroy_cq;
> +		ibv_destroy_qp;
> +		ibv_destroy_srq;
> +		ibv_detach_mcast;
>   		ibv_dofork_range;
> -		ibv_register_driver;
> -
> +		ibv_dontfork_range;
> +		ibv_event_type_str;
> +		ibv_fork_init;
> +		ibv_free_device_list;
> +		ibv_get_async_event;
> +		ibv_get_cq_event;
> +		ibv_get_device_guid;
> +		ibv_get_device_list;
> +		ibv_get_device_name;
> +		ibv_init_ah_from_wc;
> +		ibv_modify_qp;
> +		ibv_modify_srq;
>   		ibv_node_type_str;
> +		ibv_open_device;
>   		ibv_port_state_str;
> -		ibv_event_type_str;
> -		ibv_wc_status_str;
> -
> +		ibv_query_device;
> +		ibv_query_gid;
> +		ibv_query_pkey;
> +		ibv_query_port;
> +		ibv_query_qp;
> +		ibv_query_srq;
>   		ibv_rate_to_mbps;
> -		mbps_to_ibv_rate;
> -
> +		ibv_reg_mr;
> +		ibv_register_driver;
> +		ibv_rereg_mr;
> +		ibv_resize_cq;
>   		ibv_resolve_eth_l2_from_gid;
> +		ibv_wc_status_str;
> +		mbps_to_ibv_rate;
>   
>   		/* These historical symbols are now private to libibverbs, but used by
>   		   other rdma-core libraries. Do not change them. */
> 

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

Re: [PATCH 6/8] verbs: Fill in the libibverbs.map file

[Jason Gunthorpe]

On Sun, Dec 10, 2017 at 07:01:52PM +0200, Yishai Hadas wrote:
> On 12/6/2017 1:17 AM, Jason Gunthorpe wrote:
> >From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> >Several of the 'latest' verbs are missing from the map file.
> 
> Which 'latest' verbs are you referring to ? quite hard to follow post the
> sort.

Pretty much all the ones listed as '+' in - 'latest' means they are
using LATEST_SYMVER_FUNC

> >The linker does the right thing anyhow since they are tagged with the
> >.symver assembler directive, but let us list them anyhow so that the
> >map file is a complete list of symbols.
> 
> If the linker does the work why do we really need to maintain it now and in
> the future ?

Technicaly all symbols should be in the map file. I would view the
linker exposing symbols not in the map file as some weird behavior we
shouldn't rely on.

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

Re: [PATCH 6/8] verbs: Fill in the libibverbs.map file

[Yishai Hadas]

On 12/11/2017 5:48 PM, Jason Gunthorpe wrote:
> On Sun, Dec 10, 2017 at 07:01:52PM +0200, Yishai Hadas wrote:
>> On 12/6/2017 1:17 AM, Jason Gunthorpe wrote:
>>> From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>>
>>> Several of the 'latest' verbs are missing from the map file.
>>
>> Which 'latest' verbs are you referring to ? quite hard to follow post the
>> sort.
> 
> Pretty much all the ones listed as '+' in - 'latest' means they are
> using LATEST_SYMVER_FUNC
> 
>>> The linker does the right thing anyhow since they are tagged with the
>>> .symver assembler directive, but let us list them anyhow so that the
>>> map file is a complete list of symbols.
>>
>> If the linker does the work why do we really need to maintain it now and in
>> the future ?
> 
> Technicaly all symbols should be in the map file. I would view the
> linker exposing symbols not in the map file as some weird behavior we
> shouldn't rely on.

You have added the extra symbols under 'IBVERBS_1.1', but this section 
already includes 'IBVERBS_1.0' at part of its definition in the map 
file. For example 'ibv_destroy_srq' now appears twice in both 1.0 and 1.1.
Why current map file is not well-defined without this patch ?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/8] verbs: Fill in the libibverbs.map file

[Jason Gunthorpe]

On Mon, Dec 11, 2017 at 06:22:37PM +0200, Yishai Hadas wrote:
> On 12/11/2017 5:48 PM, Jason Gunthorpe wrote:
 
> You have added the extra symbols under 'IBVERBS_1.1', but this section
> already includes 'IBVERBS_1.0' at part of its definition in the map
> file.

Each section only defines the smybols for a single symbol version.

> For example 'ibv_destroy_srq' now appears twice in both 1.0 and 1.1.

Correct - ibv_destroy_srq has two symbols and must appear twice in the
map file:

debian/libibverbs1.symbols: ibv_destroy_srq@IBVERBS_1.0 1.1.6
debian/libibverbs1.symbols: ibv_destroy_srq@IBVERBS_1.1 1.1.6

libibverbs/compat-1_0.c:COMPAT_SYMVER_FUNC(ibv_destroy_srq, 1_0, "IBVERBS_1.0",
libibverbs/verbs.c:LATEST_SYMVER_FUNC(ibv_destroy_srq, 1_1, "IBVERBS_1.1",

> Why current map file is not well-defined without this patch ?

Every stanza should list every symobl defined for the varsion.

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

Re: [PATCH 6/8] verbs: Fill in the libibverbs.map file

[Yishai Hadas]

On 12/11/2017 6:27 PM, Jason Gunthorpe wrote:
> On Mon, Dec 11, 2017 at 06:22:37PM +0200, Yishai Hadas wrote:
>> On 12/11/2017 5:48 PM, Jason Gunthorpe wrote:
>   
>> You have added the extra symbols under 'IBVERBS_1.1', but this section
>> already includes 'IBVERBS_1.0' at part of its definition in the map
>> file.
> 
> Each section only defines the smybols for a single symbol version.
> 
>> For example 'ibv_destroy_srq' now appears twice in both 1.0 and 1.1.
> 
> Correct - ibv_destroy_srq has two symbols and must appear twice in the
> map file:
> 
> debian/libibverbs1.symbols: ibv_destroy_srq@IBVERBS_1.0 1.1.6
> debian/libibverbs1.symbols: ibv_destroy_srq@IBVERBS_1.1 1.1.6
> 
> libibverbs/compat-1_0.c:COMPAT_SYMVER_FUNC(ibv_destroy_srq, 1_0, "IBVERBS_1.0",
> libibverbs/verbs.c:LATEST_SYMVER_FUNC(ibv_destroy_srq, 1_1, "IBVERBS_1.1",
> 
>> Why current map file is not well-defined without this patch ?
> 
> Every stanza should list every symobl defined for the varsion.
> 

Agree, so this patch comes to improve/fix the original patch [1] from 
2007 which introduced 'IBVERBS_1.1' and relied on the linker with the 
symvers mechanism, correct ?

[1]
commit fd448acccecab2740b7f35f1fadc64fae6d6b9d3
Author: Roland Dreier <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
Date:   Mon Jan 29 09:22:18 2007 -0800

     Add ABI compatibility for apps linked against libibverbs 1.0

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

Re: [PATCH 6/8] verbs: Fill in the libibverbs.map file

[Jason Gunthorpe]

On Mon, Dec 11, 2017 at 07:59:43PM +0200, Yishai Hadas wrote:
> On 12/11/2017 6:27 PM, Jason Gunthorpe wrote:
> >On Mon, Dec 11, 2017 at 06:22:37PM +0200, Yishai Hadas wrote:
> >>On 12/11/2017 5:48 PM, Jason Gunthorpe wrote:
> >>You have added the extra symbols under 'IBVERBS_1.1', but this section
> >>already includes 'IBVERBS_1.0' at part of its definition in the map
> >>file.
> >
> >Each section only defines the smybols for a single symbol version.
> >
> >>For example 'ibv_destroy_srq' now appears twice in both 1.0 and 1.1.
> >
> >Correct - ibv_destroy_srq has two symbols and must appear twice in the
> >map file:
> >
> >debian/libibverbs1.symbols: ibv_destroy_srq@IBVERBS_1.0 1.1.6
> >debian/libibverbs1.symbols: ibv_destroy_srq@IBVERBS_1.1 1.1.6
> >
> >libibverbs/compat-1_0.c:COMPAT_SYMVER_FUNC(ibv_destroy_srq, 1_0, "IBVERBS_1.0",
> >libibverbs/verbs.c:LATEST_SYMVER_FUNC(ibv_destroy_srq, 1_1, "IBVERBS_1.1",
> >
> >>Why current map file is not well-defined without this patch ?
> >
> >Every stanza should list every symobl defined for the varsion.
> >
> 
> Agree, so this patch comes to improve/fix the original patch [1] from 2007
> which introduced 'IBVERBS_1.1' and relied on the linker with the symvers
> mechanism, correct ?
> commit fd448acccecab2740b7f35f1fadc64fae6d6b9d3

Yes.. that commit has a mixture of doing some right (eg
ibv_get_device_list) and doing some wrong (eg ibv_attach_mcast)

However this isn't a Fixes for that, since it fixes a whole range of
commits changing the export list..

Jason

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