On Tue, Mar 27, 2018 at 04:18:39PM -0500, Steve Wise wrote:
> > On Tue, Mar 27, 2018 at 08:50:22AM -0600, Jason Gunthorpe wrote:
> > > 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.
> >
> > Yes, I'm proposing to have three attributes
> > (RDMA_NLDEV_ATTR_PROVIDER_32, RDMA_NLDEV_ATTR_PROVIDER_64 and
> > RDMA_NLDEV_ATTR_PROVIDER_PRINT_TYPE) instead of variants proposed
> > by Steve.
> >
>
> Hey Leon, with your PRINT_TYPE string attribute proposal, how would you
> handle PRIu64 and friends? IE what specific strings would be passed up to
> the user for 64b attributes?
I will try to summarize the users behavior and their prints:
PRINT_TYPE | PROVIDER_U64 | PROVIDER_S64 | PROVIDER_U32 | PROVIDER_S32 |
-------------------------------------------------------------------------
unspecified | %PRIu64 | %PRId64 | %u | %d |
hex | %PRIx64 | %PRIx64 | %x | %x |
-------------------------------------------------------------------------
The "unspecified", it is current behaviour of libnetlink.
So once, user will want to provide information for the kernel, he will
fill PROVIDER_U64 with data and will send it to the kernel without need
to fill PRINT_TYPE.
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
