Re: [PATCH rdma-core 1/8] verbs: Annoate ibv_wc helpers with endian

Leon Romanovsky
2017-07-13T06:50:57Z

[-- Attachment #1: Type: text/plain, Size: 4026 bytes --]

On Wed, Jul 12, 2017 at 03:17:35PM -0600, Jason Gunthorpe wrote:
> This follows the scheme of used in the wc by introducing a
> ibv_wc_read_invalidated_rkey to access the host endian
> invalidated_rkey value with proper annotations.
>
> This is just an inline wrapper to allow sparse to work sensibly,
> not really a good reason to add another driver entry point.
>
> Fixes: 32186550 ("verbs: Add be annotations to public headers")
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
> libibverbs/man/ibv_create_cq_ex.3 | 5 ++++-
> libibverbs/verbs.h | 13 +++++++++++--
> 2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/libibverbs/man/ibv_create_cq_ex.3 b/libibverbs/man/ibv_create_cq_ex.3
> index 7dfbef28d2413b..e943e0e266c582 100644
> --- a/libibverbs/man/ibv_create_cq_ex.3
> +++ b/libibverbs/man/ibv_create_cq_ex.3
> @@ -104,9 +104,12 @@ Below members and functions are used in order to poll the current completion. Th
> .BI "uint32_t ibv_wc_read_byte_len(struct ibv_cq_ex " "*cq"); \c
> Get the vendor error from the current completion.
>
> -.BI "uint32_t ibv_wc_read_imm_data(struct ibv_cq_ex " "*cq"); \c
> +.BI "__be32 ibv_wc_read_imm_data(struct ibv_cq_ex " "*cq"); \c
> Get the immediate data field from the current completion.
>
> +.BI "uint32_t ibv_wc_read_invalidated_rkey(struct ibv_cq_ex " "*cq"); \c
> + Get the rkey invalided by the SEND_INVAL from the current completion.
> +
> .BI "uint32_t ibv_wc_read_qp_num(struct ibv_cq_ex " "*cq"); \c
> Get the QP number field from the current completion.
>
> diff --git a/libibverbs/verbs.h b/libibverbs/verbs.h
> index 4f0765e0476db8..997ef248b26b62 100644
> --- a/libibverbs/verbs.h
> +++ b/libibverbs/verbs.h
> @@ -1093,7 +1093,7 @@ struct ibv_cq_ex {
> enum ibv_wc_opcode (*read_opcode)(struct ibv_cq_ex *current);
> uint32_t (*read_vendor_err)(struct ibv_cq_ex *current);
> uint32_t (*read_byte_len)(struct ibv_cq_ex *current);
> - uint32_t (*read_imm_data)(struct ibv_cq_ex *current);
> + __be32 (*read_imm_data)(struct ibv_cq_ex *current);

The functions which use this function call are still uint32_t:
➜ rdma-core git:(master) ✗ grep mlx4_cq_read_wc_imm_data * -rI
providers/mlx4/cq.c:static uint32_t mlx4_cq_read_wc_imm_data(struct ibv_cq_ex *ibcq)
providers/mlx4/cq.c: cq->ibv_cq.read_imm_data = mlx4_cq_read_wc_imm_data;
➜ rdma-core git:(master) ✗ grep mlx5_cq_read_wc_imm_data * -rI
providers/mlx5/cq.c:static inline uint32_t mlx5_cq_read_wc_imm_data(struct ibv_cq_ex *ibcq)
providers/mlx5/cq.c: cq->ibv_cq.read_imm_data = mlx5_cq_read_wc_imm_data;

> 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);
> @@ -1141,11 +1141,20 @@ static inline uint32_t ibv_wc_read_byte_len(struct ibv_cq_ex *cq)
> return cq->read_byte_len(cq);
> }
>
> -static inline uint32_t ibv_wc_read_imm_data(struct ibv_cq_ex *cq)
> +static inline __be32 ibv_wc_read_imm_data(struct ibv_cq_ex *cq)

Doesn't this change break user applications?

> {
> return cq->read_imm_data(cq);
> }
>
> +static inline uint32_t ibv_wc_read_invalidated_rkey(struct ibv_cq_ex *cq)
> +{
> +#ifdef __CHECKER__
> + return (__attribute__((force)) uint32_t)cq->read_imm_data(cq);
> +#else
> + return cq->read_imm_data(cq);
> +#endif
> +}

I don't think that those __CHECKER__ ifdefs should be part of the code.
They are part of infrastructure to support development of library, but
are not required for the user of that library.

> +
> static inline uint32_t ibv_wc_read_qp_num(struct ibv_cq_ex *cq)
> {
> return cq->read_qp_num(cq);
> --
> 2.7.4
>
> --
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]