>
> On Tue, Mar 13, 2018 at 10:30:32AM -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 | 40
> ++++++++++++++++++++++++++++++++++++++++
> > include/rdma/restrack.h | 10 ++++++++++
> > include/uapi/rdma/rdma_netlink.h | 17 +++++++++++++++++
> > 3 files changed, 67 insertions(+)
> >
> > diff --git a/drivers/infiniband/core/nldev.c
b/drivers/infiniband/core/nldev.c
> > index 192084c..933df61 100644
> > --- a/drivers/infiniband/core/nldev.c
> > +++ b/drivers/infiniband/core/nldev.c
> > @@ -95,8 +95,28 @@
> > [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 },
>
> Two comments here and I would like to hear other opinion too, before we
> are rushing to implement it.
>
> 1. I don't think that we need to distinguish between 32 and 64 and
> better to provide one U64 type only.
Why? Seems wasteful to pass 8, 16, and 32b quantities via 64b. I
originally had U16 and U8, but you advised against that. So lemme know why
supporting only 64 is a good idea. I don't think "simplicity" is a good
answer, by the way. Have u8-u64 doesn't make it much more complex...
> 2. The type of attribute is better to be different NLA.
> Something like:
>
> + [RDMA_NLDEV_ATTR_PROVIDER_64] = { .type = NLA_U64 },
> + [RDMA_NLDEV_ATTR_PROVIDER_PRINT_TYPE] = { .type =
> NLA_NUL_STRING },
>
> and RDMA_NLDEV_ATTR_PROVIDER_PRINT_TYPE can be x64 or u64 if it is not
> set.
So then each attribute would be a 3-tuple: string print name, string print
type, and value. Seems wasteful of skb space. I'm concerned because there
is a lot of data to pass up for cxgb4 data structures, and I'm afraid we'll
run out of room. And the current design doesn't handle a single table
spanning many skbs...
Why do you dislike the attributes having the print type described in the
attribute containing the value? It seems clean to me.
Steve.
>
> 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
