> -----Original Message-----
> From: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> Sent: Tuesday, March 27, 2018 9:50 AM
> To: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
> Cc: 'Leon Romanovsky' <leon@xxxxxxxxxx>; dledford@xxxxxxxxxx; linux-
> rdma@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v1 rdma-next 2/4] RDMA/nldev: add provider-specific
> resource tracking
>
> On Sat, Mar 24, 2018 at 02:42:15PM -0500, Steve Wise wrote:
> >
> > >
> > > On Fri, Mar 23, 2018 at 09:49:39AM -0700, Steve Wise wrote:
> > > > Each provider can register a "fill entry" function with the restrack
> > core.
> > > > This function will be called when filling out a resource, allowing the
> > > > provider to add provider-specific details. The details consist of a
> > > > table of nested attributes, that are in the form of "key, value" tuple.
> > > > The key nlattr must be strings, and the value nlattr can be one of
> > > > provider attributes that are generic, but typed, allowing the
> nlmessage
> > > > to ve validated. Currently the types include string, s32, u32, x32,
> > s64,
> > > > u64, and x64. The inclusion of x, s, and u variants for numeric
> > attributes
> > > > allows the user tool to print the number in the desired format.
> > > >
> > > > More attrs can be defined as they become needed by providers.
> > > >
> > > > Signed-off-by: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
> > > > drivers/infiniband/core/nldev.c | 39
> > > +++++++++++++++++++++++++++++++++++++++
> > > > include/rdma/restrack.h | 10 ++++++++++
> > > > include/uapi/rdma/rdma_netlink.h | 16 ++++++++++++++++
> > > > 3 files changed, 65 insertions(+)
> > > >
> > > > diff --git a/drivers/infiniband/core/nldev.c
> > > b/drivers/infiniband/core/nldev.c
> > > > index 884843e..8346ede 100644
> > > > +++ b/drivers/infiniband/core/nldev.c
> > > > @@ -95,8 +95,27 @@
> > > > [RDMA_NLDEV_ATTR_RES_PD_ENTRY] = { .type =
> > > NLA_NESTED },
> > > > [RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY] = { .type = NLA_U32 },
> > > > [RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY] = { .type =
> > > NLA_U32 },
> > > > + [RDMA_NLDEV_ATTR_PROVIDER] = { .type =
> > > NLA_NESTED },
> > > > + [RDMA_NLDEV_ATTR_PROVIDER_ENTRY] = { .type =
> > > NLA_NESTED },
> > > > + [RDMA_NLDEV_ATTR_PROVIDER_STRING] = { .type =
> > > NLA_NUL_STRING,
> > > > + .len =
> > > RDMA_NLDEV_ATTR_PROVIDER_STRLEN },
> > > > + [RDMA_NLDEV_ATTR_PROVIDER_D32] = { .type = NLA_S32 },
> > > > + [RDMA_NLDEV_ATTR_PROVIDER_U32] = { .type = NLA_U32 },
> > > > + [RDMA_NLDEV_ATTR_PROVIDER_X32] = { .type = NLA_U32 },
> > > > + [RDMA_NLDEV_ATTR_PROVIDER_D64] = { .type = NLA_S64 },
> > > > + [RDMA_NLDEV_ATTR_PROVIDER_U64] = { .type = NLA_U64 },
> > > > + [RDMA_NLDEV_ATTR_PROVIDER_X64] = { .type = NLA_U64 },
> > > > };
> > > >
> > > > +static int provider_fill_res_entry(struct rdma_restrack_root *resroot,
> > > > + struct sk_buff *msg,
> > > > + struct netlink_callback *cb,
> > > > + struct rdma_restrack_entry *res)
> > > > +{
> > > > + return resroot->fill_res_entry ?
> > > > + resroot->fill_res_entry(msg, cb, res) : 0;
> > > > +}
> > >
> > > Please add "fill_res_entry = NULL" line into rdma_restrack_init()
> > > despite kzalloc usage in ib_alloc_device().
> >
> >
> > Will do.
> >
> > >
> > > And I afraid that we didn't settle the PROVIDER_*64 thing.
> > >
> >
> > I didn't agree that your proposal was simpler, or even avoided the issues
> > you said were problems with the self-describing-print-formt attributes. I
> > asked if anyone else had an opinion. Nobody replied. So I chose to keep
> > the attributes as they are. Are you NAKing this?
> >
> > Jason and Doug, do you have an opinion either way? Our discussion of
> this
> > can be found here:
> >
> > https://www.spinics.net/lists/linux-rdma/msg62198.html
>
> I'm still not sure what you are two are arguing about..
>
> Is it the inclusion of 'X'? That does seem weird.
>
> Jason
Discussing, not arguing. 😊
Steve's proposed attributes like BLAH_U32, BLAH_X32, and BLAH_D32 are efficient because they convey, directly, how the user side should display them.
Leon prefers a separate string attribute that is provided along with the value to convey the display format, and the default would be unsigned so the display format attribute could be excluded and the user side knows to use "%u".
Do you have a preference, and why?
Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
