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
