Re: [PATCH for-next 1/4] RDMA/bnxt_re: Support driver specific data collection using rdma tool

[Leon Romanovsky <leon@kernel.org>]

On Tue, Oct 22, 2024 at 03:11:53AM -0700, Selvin Xavier wrote:
> From: Kashyap Desai <kashyap.desai@broadcom.com>
> 
> Allow users to dump driver specific resource details when
> queried through rdma tool. This supports the driver data
> for QP, CQ, MR and SRQ.
> 
> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> ---
>  drivers/infiniband/hw/bnxt_re/main.c | 148 +++++++++++++++++++++++++++++++++++
>  1 file changed, 148 insertions(+)
> 
> diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
> index 6715c96..5bed9af 100644
> --- a/drivers/infiniband/hw/bnxt_re/main.c
> +++ b/drivers/infiniband/hw/bnxt_re/main.c
> @@ -882,6 +882,146 @@ static const struct attribute_group bnxt_re_dev_attr_group = {
>  	.attrs = bnxt_re_attributes,
>  };
>  
> +static int bnxt_re_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr)
> +{
> +	struct bnxt_qplib_hwq *mr_hwq;
> +	struct nlattr *table_attr;
> +	struct bnxt_re_mr *mr;
> +
> +	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
> +	if (!table_attr)
> +		return -EMSGSIZE;
> +
> +	mr = container_of(ib_mr, struct bnxt_re_mr, ib_mr);
> +	mr_hwq = &mr->qplib_mr.hwq;
> +
> +	if (rdma_nl_put_driver_string(msg, "owner",
> +				      mr_hwq->is_user ?  "user" : "kernel"))

Two comments:
1. There is already a helper function to decide if owner is user or kernel - rdma_is_kernel_res().
2. This print duplicates existing information. The difference between
user and kernel can be easily seen by looking on the PID output.

> +		goto err;
> +	if (rdma_nl_put_driver_u32(msg, "page_size",
> +				   mr_hwq->qe_ppg * mr_hwq->element_size))
> +		goto err;
> +	if (rdma_nl_put_driver_u32(msg, "max_elements", mr_hwq->max_elements))
> +		goto err;
> +	if (rdma_nl_put_driver_u32(msg, "element_size", mr_hwq->element_size))
> +		goto err;
> +	if (rdma_nl_put_driver_u64_hex(msg, "hwq", (unsigned long)mr_hwq))
> +		goto err;
> +	if (rdma_nl_put_driver_u64_hex(msg, "va", mr->qplib_mr.va))
> +		goto err;

<...>

> +static int bnxt_re_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ib_qp)
> +{
> +	struct bnxt_qplib_qp *qplib_qp;
> +	struct nlattr *table_attr;
> +	struct bnxt_re_qp *qp;
> +
> +	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
> +	if (!table_attr)
> +		return -EMSGSIZE;
> +
> +	qp = container_of(ib_qp, struct bnxt_re_qp, ib_qp);
> +	qplib_qp = &qp->qplib_qp;
> +
> +	if (rdma_nl_put_driver_string(msg, "owner",
> +				      ib_qp->uobject ?  "user" : "kernel"))
> +		goto err;
> +
> +	if (rdma_nl_put_driver_u32(msg, "sq_max_wqe", qplib_qp->sq.max_wqe))
> +		goto err;
> +	if (rdma_nl_put_driver_u32(msg, "sq_max_sge", qplib_qp->sq.max_sge))

Doesn't this information already exist in other places? devinfo?

Thanks