On Tue, Feb 05, 2019 at 03:14:27PM -0700, Jason Gunthorpe wrote:
> On Sat, Feb 02, 2019 at 01:42:52PM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> >
> > Give to the user space tools unique identifier for PD, MR, CQ and CM_ID
> > objects, so they can be able to query on them with .doit callbacks.
> >
> > QP .doit is not supported yet, till all drivers will be updated
> > to provide their LQPN to be equal to their restrack ID.
> >
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > drivers/infiniband/core/nldev.c | 48 ++++++++++++++++++++++++++++++++
> > include/uapi/rdma/rdma_netlink.h | 9 ++++++
> > 2 files changed, 57 insertions(+)
> >
> > diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> > index 1fd06ca18829..e460025fbddf 100644
> > +++ b/drivers/infiniband/core/nldev.c
> > @@ -107,6 +107,10 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
> > [RDMA_NLDEV_ATTR_DRIVER_U32] = { .type = NLA_U32 },
> > [RDMA_NLDEV_ATTR_DRIVER_S64] = { .type = NLA_S64 },
> > [RDMA_NLDEV_ATTR_DRIVER_U64] = { .type = NLA_U64 },
> > + [RDMA_NLDEV_ATTR_RES_PDN] = { .type = NLA_U32 },
> > + [RDMA_NLDEV_ATTR_RES_CQN] = { .type = NLA_U32 },
> > + [RDMA_NLDEV_ATTR_RES_MRN] = { .type = NLA_U32 },
> > + [RDMA_NLDEV_ATTR_RES_CM_IDN] = { .type = NLA_U32 },
> > };
> >
> > static int put_driver_name_print_type(struct sk_buff *msg, const char *name,
> > @@ -369,6 +373,34 @@ static bool fill_res_entry(struct ib_device *dev, struct sk_buff *msg,
> > return dev->ops.fill_res_entry(msg, res);
> > }
> >
> > +static bool fill_res_ids(struct sk_buff *msg, struct rdma_restrack_entry *res)
> > +{
> > + bool ret;
> > +
> > + switch (res->type) {
> > + case RDMA_RESTRACK_PD:
> > + ret = nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_PDN,
> > + rdma_res_to_id(res));
> > + break;
> > + case RDMA_RESTRACK_CQ:
> > + ret = nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_CQN,
> > + rdma_res_to_id(res));
> > + break;
> > + case RDMA_RESTRACK_MR:
> > + ret = nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_MRN,
> > + rdma_res_to_id(res));
> > + break;
> > + case RDMA_RESTRACK_CM_ID:
> > + ret = nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_CM_IDN,
> > + rdma_res_to_id(res));
> > + break;
> > + default:
> > + ret = true;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
> > struct rdma_restrack_entry *res, uint32_t port)
> > {
> > @@ -465,6 +497,9 @@ static int fill_res_cm_id_entry(struct sk_buff *msg, bool has_cap_net_admin,
> > &cm_id->route.addr.dst_addr))
> > goto err;
> >
> > + if (fill_res_ids(msg, res))
> > + goto err;
>
> Is there some reason this isn't just written as
>
> ret = nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_CM_IDN, rdma_res_to_id(res));
> if (ret)
> return ret;
>
> ?
Yes, in next patch, I'm filling parent ID in that function.
>
> I'm confused why we jump to a case statement when every caller in this
> patch seems to already know what type it is.
I did it to localize all changes of IDs in one place.
>
> Jason
Attachment:
signature.asc
Description: PGP signature
