| Message ID | 03e401d3bbc7$f552d9d0$dff88d70$@opengridcomputing.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, Mar 14, 2018 at 02:09:21PM -0500, Steve Wise wrote: > > On Wed, Mar 14, 2018 at 01:00:27PM -0500, Steve Wise wrote: > > > > On Wed, Mar 14, 2018 at 07:33:53PM +0200, Leon Romanovsky wrote: > > > > > 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@opengridcomputing.com> > > > > > > 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 > > > > > > +++ 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 }, > > > > > > + [RDMA_NLDEV_ATTR_PAD] = { }, > > > > > > > > > > RDMA_NLDEV_ATTR_PAD ???? > > > > > > > > This was supposed to have been done earlier.. > > > > > > > > > > It wasn't clear whether we needed it. Leon sez 0 is fine. > > > > > > > Seperate patch please, and change every caller of nla_put_u64_64bit to > > > > use the new constant. > > > > > > > > > > Yea ok. > > > > > > > And I think it has to be 0 for ABI compat, doesn't it? > > > > > > > > > > I guess so, since 0 was used originally. > > > > Yes, we are using 0, which is equal to RDMA_NLDEV_ATTR_UNSPEC. > > > > All zeros can be replaced for clarity, but from ABI perspective we are > > fine. > > > > > Will this work? > > > [root@stevo1 linux]# git diff > diff --git a/include/uapi/rdma/rdma_netlink.h > b/include/uapi/rdma/rdma_netlink.h > index 351139c..84b3f63 100644 > +++ b/include/uapi/rdma/rdma_netlink.h > @@ -253,6 +253,9 @@ enum rdma_nldev_attr { > /* don't change the order or add anything between, this is ABI! */ > RDMA_NLDEV_ATTR_UNSPEC, > > + /* Pad attribute for 64b alignment */ > + RDMA_NLDEV_ATTR_PAD = RDMA_NLDEV_ATTR_UNSPEC, > + > /* Identifier for ib_device */ > RDMA_NLDEV_ATTR_DEV_INDEX, /* u32 */ Yes, I think so. Best to check that RDMA_NLDEV_ATTR_DEV_INDEX = 1 though Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Will this work? > > > > > > [root@stevo1 linux]# git diff > > diff --git a/include/uapi/rdma/rdma_netlink.h > > b/include/uapi/rdma/rdma_netlink.h > > index 351139c..84b3f63 100644 > > +++ b/include/uapi/rdma/rdma_netlink.h > > @@ -253,6 +253,9 @@ enum rdma_nldev_attr { > > /* don't change the order or add anything between, this is ABI! */ > > RDMA_NLDEV_ATTR_UNSPEC, > > > > + /* Pad attribute for 64b alignment */ > > + RDMA_NLDEV_ATTR_PAD = RDMA_NLDEV_ATTR_UNSPEC, > > + > > /* Identifier for ib_device */ > > RDMA_NLDEV_ATTR_DEV_INDEX, /* u32 */ > > Yes, I think so. Best to check that RDMA_NLDEV_ATTR_DEV_INDEX = 1 > though > > Jason I verified that it is still 1. :) -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 14, 2018 at 02:09:21PM -0500, Steve Wise wrote: > > On Wed, Mar 14, 2018 at 01:00:27PM -0500, Steve Wise wrote: > > > > On Wed, Mar 14, 2018 at 07:33:53PM +0200, Leon Romanovsky wrote: > > > > > 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@opengridcomputing.com> > > > > > > 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 > > > > > > +++ 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 }, > > > > > > + [RDMA_NLDEV_ATTR_PAD] = { }, > > > > > > > > > > RDMA_NLDEV_ATTR_PAD ???? > > > > > > > > This was supposed to have been done earlier.. > > > > > > > > > > It wasn't clear whether we needed it. Leon sez 0 is fine. > > > > > > > Seperate patch please, and change every caller of nla_put_u64_64bit to > > > > use the new constant. > > > > > > > > > > Yea ok. > > > > > > > And I think it has to be 0 for ABI compat, doesn't it? > > > > > > > > > > I guess so, since 0 was used originally. > > > > Yes, we are using 0, which is equal to RDMA_NLDEV_ATTR_UNSPEC. > > > > All zeros can be replaced for clarity, but from ABI perspective we are > > fine. > > > > > Will this work? Why don't we write RDMA_NLDEV_ATTR_UNSPEC instead of 0 and forget about it? I don't understand this desire to overcomplicate things. > > > [root@stevo1 linux]# git diff > diff --git a/include/uapi/rdma/rdma_netlink.h > b/include/uapi/rdma/rdma_netlink.h > index 351139c..84b3f63 100644 > --- a/include/uapi/rdma/rdma_netlink.h > +++ b/include/uapi/rdma/rdma_netlink.h > @@ -253,6 +253,9 @@ enum rdma_nldev_attr { > /* don't change the order or add anything between, this is ABI! */ > RDMA_NLDEV_ATTR_UNSPEC, > > + /* Pad attribute for 64b alignment */ > + RDMA_NLDEV_ATTR_PAD = RDMA_NLDEV_ATTR_UNSPEC, > + > /* Identifier for ib_device */ > RDMA_NLDEV_ATTR_DEV_INDEX, /* u32 */ > >
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h index 351139c..84b3f63 100644 --- a/include/uapi/rdma/rdma_netlink.h +++ b/include/uapi/rdma/rdma_netlink.h @@ -253,6 +253,9 @@ enum rdma_nldev_attr { /* don't change the order or add anything between, this is ABI! */ RDMA_NLDEV_ATTR_UNSPEC, + /* Pad attribute for 64b alignment */ + RDMA_NLDEV_ATTR_PAD = RDMA_NLDEV_ATTR_UNSPEC, + /* Identifier for ib_device */ RDMA_NLDEV_ATTR_DEV_INDEX, /* u32 */