Re: [PATCH rdma-next v2 08/11] RDMA: Add support to dump resource tracker in RAW format

[Jason Gunthorpe <jgg@nvidia.com>]

On Sun, Jun 21, 2020 at 10:55:32AM +0300, Leon Romanovsky wrote:
> On Thu, Jun 18, 2020 at 08:20:09PM -0300, Jason Gunthorpe wrote:
> > On Tue, Jun 16, 2020 at 01:40:03PM +0300, Leon Romanovsky wrote:
> >
> > > +static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
> > > +			     struct rdma_restrack_entry *res, uint32_t port,
> > > +			     bool raw)
> > > +{
> > > +	struct ib_qp *qp = container_of(res, struct ib_qp, res);
> > > +	struct ib_device *dev = qp->device;
> > > +	int ret;
> > > +
> > > +	if (port && port != qp->port)
> > > +		return -EAGAIN;
> > > +
> > > +	/* In create_qp() port is not set yet */
> > > +	if (qp->port && nla_put_u32(msg, RDMA_NLDEV_ATTR_PORT_INDEX, qp->port))
> > > +		return -EINVAL;
> > > +
> > > +	ret = nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_LQPN, qp->qp_num);
> > > +	if (ret)
> > > +		goto err;
> > > +
> > >  	if (!rdma_is_kernel_res(res) &&
> > >  	    nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_PDN, qp->pd->res.id))
> > >  		goto err;
> > >
> > > -	if (fill_res_name_pid(msg, res))
> > > +	ret = fill_res_name_pid(msg, res);
> > > +	if (ret)
> > >  		goto err;
> > >
> > > -	if (dev->ops.fill_res_qp_entry)
> > > -		return dev->ops.fill_res_qp_entry(msg, qp);
> > > -	return 0;
> > > +	if (!raw)
> > > +		return fill_res_qp_entry_query(msg, res, dev, qp);
> >
> > Are you sure the RAW query should duplicate all the stuff the normal
> > query does? Shouldn't the raw query return only the raw blob?
> >
> > The rest seems fine, but this is rather odd?
> 
> RAW duplicates only fields that are not known to FW, like PID, name
> for process identification and port_index, device_index, LQPN for entry
> identification. The only one in question is PDN, but it helps to
> understand relation between PD and QP, so I would like to keep it.

It makes more sense to do the normal query then follow it with a RAW
query, that is more netlink-like, IMHO.

De-normalizing things by returning the same data in multiple places is
usually not good design.

Jason