Re: [PATCH rdma-next 06/13] RDMA/core: query the semantics a queue pair type guarantees

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] rdma: query_qp_semantics lacks MR attachment capability flag
--

commit 5f36c96aa3fcf03ebbebf03d834b66a1b7642e0a
Author: Eric Davis <eric.davis@broadcom.com>

RDMA/core: query the semantics a queue pair type guarantees

This commit adds a device method to report the guarantees of a queue pair
type, such as operations ordering, largest packet size, and immediate data
width.

>     An unconnected transport orders far less than a connected one, and how
>     much varies by device. Add a device method that reports, for a queue pair
>     type, which operations it orders against which, the largest packet it
>     puts on the wire, how wide an immediate it carries, and whether a memory
>     region may be attached to a QP so it's only reachable over that QP.

[Severity: Medium]
The commit message states that query_qp_semantics reports "whether a memory
region may be attached to a QP so it's only reachable over that QP".

Is this description accurate for this patch? I notice that a later commit
reports this capability via query_device instead. Could the commit message
be updated to reflect that the MR attachment capability is not included
in query_qp_semantics?

> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 68f7a271cf7a7..003b704244293 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
[ ... ]
> @@ -2496,6 +2496,18 @@ struct ib_counters {
>  	atomic_t	usecnt;
>  };
>  
> +/* what a queue pair of a given type guarantees (query_qp_semantics) */
> +struct ib_qp_semantics {
> +	u32	comp_mask;		/* enum ib_uverbs_qp_semantics_mask */
> +	u32	msg_order;		/* enum ib_uverbs_qp_msg_order */
> +	u32	max_rdma_raw_size;
> +	u32	max_rdma_war_size;
> +	u32	max_rdma_waw_size;
> +	u32	max_pdu;
> +	u32	usage_flags;		/* enum ib_uverbs_qp_usage_flags */
> +	u8	imm_data_size;		/* bits */
> +};

[Severity: Medium]
Does struct ib_qp_semantics omit the MR attachment capability claimed by the
commit message? There doesn't appear to be a field or flag here for it.

> diff --git a/include/uapi/rdma/ib_user_ioctl_verbs.h b/include/uapi/rdma/ib_user_ioctl_verbs.h
> index 210f3f945bd73..3d05a6c7ca8f6 100644
> --- a/include/uapi/rdma/ib_user_ioctl_verbs.h
> +++ b/include/uapi/rdma/ib_user_ioctl_verbs.h
[ ... ]
> +enum ib_uverbs_qp_usage_flags {
> +	/* immediate data consumes a posted receive */
> +	IB_UVERBS_QP_USAGE_IMM_DATA_RQ	= 1 << 0,
> +};

[Severity: Medium]
The usage flags enum only contains IB_UVERBS_QP_USAGE_IMM_DATA_RQ. Should
there be a flag here indicating whether an MR can be attached, or is this
omission expected since it's later reported via query_device?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-uet-ib-core-v1-0-a0e626117113@broadcom.com?part=6