diff mbox series

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

Message ID 1729591916-29134-2-git-send-email-selvin.xavier@broadcom.com (mailing list archive)
State Superseded
Headers show
Series RDMA/bnxt_re: Debug enhancements for bnxt_re driver | expand

Commit Message

Selvin Xavier Oct. 22, 2024, 10:11 a.m. UTC 
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(+)

Comments

Leon Romanovsky Oct. 29, 2024, 2:03 p.m. UTC | #1 
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
Selvin Xavier Oct. 30, 2024, 8:29 a.m. UTC | #2 
On Tue, Oct 29, 2024 at 7:33 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> 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.
Got it. I will remove this in the follow up patch.
>
> > +             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?
device level max_sge that can be retrieved from devinfo.
This is a per QP value we are displaying here . It is important for
the latest GenP7 adapters as we support variable size
 Work Queue entries (and variable size SGEs) now.
This information is available in the query_qp response. But it can be
handy to dump this while debugging.

>
> Thanks
Leon Romanovsky Oct. 30, 2024, 1:43 p.m. UTC | #3 
On Wed, Oct 30, 2024 at 01:59:06PM +0530, Selvin Xavier wrote:
> On Tue, Oct 29, 2024 at 7:33 PM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > 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.
> Got it. I will remove this in the follow up patch.

Thanks
diff mbox series

Patch

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"))
+		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;
+
+	nla_nest_end(msg, table_attr);
+	return 0;
+
+err:
+	nla_nest_cancel(msg, table_attr);
+	return -EMSGSIZE;
+}
+
+static int bnxt_re_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ib_cq)
+{
+	struct bnxt_qplib_hwq *cq_hwq;
+	struct nlattr *table_attr;
+	struct bnxt_re_cq *cq;
+
+	cq = container_of(ib_cq, struct bnxt_re_cq, ib_cq);
+	cq_hwq = &cq->qplib_cq.hwq;
+
+	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
+	if (!table_attr)
+		return -EMSGSIZE;
+
+	if (rdma_nl_put_driver_u32(msg, "cq_depth", cq_hwq->depth))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "max_elements", cq_hwq->max_elements))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "element_size", cq_hwq->element_size))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "max_wqe", cq->qplib_cq.max_wqe))
+		goto err;
+
+	nla_nest_end(msg, table_attr);
+	return 0;
+
+err:
+	nla_nest_cancel(msg, table_attr);
+	return -EMSGSIZE;
+}
+
+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))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "sq_wqe_size", qplib_qp->sq.wqe_size))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "sq_swq_start", qplib_qp->sq.swq_start))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "sq_swq_last", qplib_qp->sq.swq_last))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "rq_max_wqe", qplib_qp->rq.max_wqe))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "rq_max_sge", qplib_qp->rq.max_sge))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "rq_wqe_size", qplib_qp->rq.wqe_size))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "rq_swq_start", qplib_qp->rq.swq_start))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "rq_swq_last", qplib_qp->rq.swq_last))
+		goto err;
+	if (rdma_nl_put_driver_u32(msg, "timeout", qplib_qp->timeout))
+		goto err;
+
+	nla_nest_end(msg, table_attr);
+	return 0;
+
+err:
+	nla_nest_cancel(msg, table_attr);
+	return -EMSGSIZE;
+}
+
+static int bnxt_re_fill_res_srq_entry(struct sk_buff *msg, struct ib_srq *ib_srq)
+{
+	struct nlattr *table_attr;
+	struct bnxt_re_srq *srq;
+
+	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
+	if (!table_attr)
+		return -EMSGSIZE;
+
+	srq = container_of(ib_srq, struct bnxt_re_srq, ib_srq);
+
+	if (rdma_nl_put_driver_u32_hex(msg, "wqe_size", srq->qplib_srq.wqe_size))
+		goto err;
+	if (rdma_nl_put_driver_u32_hex(msg, "max_wqe", srq->qplib_srq.max_wqe))
+		goto err;
+	if (rdma_nl_put_driver_u32_hex(msg, "max_sge", srq->qplib_srq.max_sge))
+		goto err;
+
+	nla_nest_end(msg, table_attr);
+	return 0;
+
+err:
+	nla_nest_cancel(msg, table_attr);
+	return -EMSGSIZE;
+}
+
 static const struct ib_device_ops bnxt_re_dev_ops = {
 	.owner = THIS_MODULE,
 	.driver_id = RDMA_DRIVER_BNXT_RE,
@@ -939,6 +1079,13 @@  static const struct ib_device_ops bnxt_re_dev_ops = {
 	INIT_RDMA_OBJ_SIZE(ib_ucontext, bnxt_re_ucontext, ib_uctx),
 };
 
+static const struct ib_device_ops restrack_ops = {
+	.fill_res_cq_entry = bnxt_re_fill_res_cq_entry,
+	.fill_res_qp_entry = bnxt_re_fill_res_qp_entry,
+	.fill_res_mr_entry = bnxt_re_fill_res_mr_entry,
+	.fill_res_srq_entry = bnxt_re_fill_res_srq_entry,
+};
+
 static int bnxt_re_register_ib(struct bnxt_re_dev *rdev)
 {
 	struct ib_device *ibdev = &rdev->ibdev;
@@ -960,6 +1107,7 @@  static int bnxt_re_register_ib(struct bnxt_re_dev *rdev)
 		ibdev->driver_def = bnxt_re_uapi_defs;
 
 	ib_set_device_ops(ibdev, &bnxt_re_dev_ops);
+	ib_set_device_ops(ibdev, &restrack_ops);
 	ret = ib_device_set_netdev(&rdev->ibdev, rdev->netdev, 1);
 	if (ret)
 		return ret;