diff mbox

[v2,rdma-next,2/5] RDMA/nldev: add provider-specific resource tracking

Message ID b335a7f0e0f54357e37253cb4f788b4f1845a3d7.1522341686.git.swise@opengridcomputing.com (mailing list archive)
State Superseded
Headers show

Commit Message

Steve Wise March 29, 2018, 4:09 p.m. UTC
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
nltable of nested attributes, that are in the form of <key, [print-type],
value> tuples.  Both key and value attributes are mandatory.  The key
nlattr must be a string, and the value nlattr can be one of the provider
attributes that are generic, but typed, allowing the attributes to be
validated.  Currently the provider nlattr types include string, s32,
u32, s64, and u64.  The print-type nlattr allows a provider to specify
an alternative display format for user tools displaying the attribute.
For example, a u32 attribute will default to "%u", but a print-type
attribute can be included for it to be displayed in hex.  This allows
the user tool to print the number in the format desired by the provider
driver.

More attrs can be defined as they become needed by providers.

Signed-off-by: Steve Wise <swise@opengridcomputing.com>
---
 drivers/infiniband/core/nldev.c  | 38 ++++++++++++++++++++++++++++++++++++++
 include/rdma/restrack.h          | 10 ++++++++++
 include/uapi/rdma/rdma_netlink.h | 21 +++++++++++++++++++++
 3 files changed, 69 insertions(+)

Comments

Leon Romanovsky March 30, 2018, 10:19 a.m. UTC | #1
On Thu, Mar 29, 2018 at 09:09:47AM -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
> nltable of nested attributes, that are in the form of <key, [print-type],
> value> tuples.  Both key and value attributes are mandatory.  The key
> nlattr must be a string, and the value nlattr can be one of the provider
> attributes that are generic, but typed, allowing the attributes to be
> validated.  Currently the provider nlattr types include string, s32,
> u32, s64, and u64.  The print-type nlattr allows a provider to specify
> an alternative display format for user tools displaying the attribute.
> For example, a u32 attribute will default to "%u", but a print-type
> attribute can be included for it to be displayed in hex.  This allows
> the user tool to print the number in the format desired by the provider
> driver.
>
> More attrs can be defined as they become needed by providers.
>
> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> ---
>  drivers/infiniband/core/nldev.c  | 38 ++++++++++++++++++++++++++++++++++++++
>  include/rdma/restrack.h          | 10 ++++++++++
>  include/uapi/rdma/rdma_netlink.h | 21 +++++++++++++++++++++
>  3 files changed, 69 insertions(+)
>

Steve,

You forgot to nullify those new functions in restrack_init(), not a big
deal because we are calling to restrack_init() after kzalloc(), but
better to be explicit about it.

Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
Steve Wise March 30, 2018, 1:06 p.m. UTC | #2
> 
> On Thu, Mar 29, 2018 at 09:09:47AM -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
> > nltable of nested attributes, that are in the form of <key,
[print-type],
> > value> tuples.  Both key and value attributes are mandatory.  The key
> > nlattr must be a string, and the value nlattr can be one of the provider
> > attributes that are generic, but typed, allowing the attributes to be
> > validated.  Currently the provider nlattr types include string, s32,
> > u32, s64, and u64.  The print-type nlattr allows a provider to specify
> > an alternative display format for user tools displaying the attribute.
> > For example, a u32 attribute will default to "%u", but a print-type
> > attribute can be included for it to be displayed in hex.  This allows
> > the user tool to print the number in the format desired by the provider
> > driver.
> >
> > More attrs can be defined as they become needed by providers.
> >
> > Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> > ---
> >  drivers/infiniband/core/nldev.c  | 38
> ++++++++++++++++++++++++++++++++++++++
> >  include/rdma/restrack.h          | 10 ++++++++++
> >  include/uapi/rdma/rdma_netlink.h | 21 +++++++++++++++++++++
> >  3 files changed, 69 insertions(+)
> >
> 
> Steve,
> 
> You forgot to nullify those new functions in restrack_init(), not a big
> deal because we are calling to restrack_init() after kzalloc(), but
> better to be explicit about it.
> 
> Thanks,
> Reviewed-by: Leon Romanovsky <leonro@mellanox.com>

I apologize.  My mistake; I intended to add them.  

Thanks for the review!

Steve.

--
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
Leon Romanovsky March 30, 2018, 2:47 p.m. UTC | #3
On Fri, Mar 30, 2018 at 08:06:16AM -0500, Steve Wise wrote:
> >
> > On Thu, Mar 29, 2018 at 09:09:47AM -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
> > > nltable of nested attributes, that are in the form of <key,
> [print-type],
> > > value> tuples.  Both key and value attributes are mandatory.  The key
> > > nlattr must be a string, and the value nlattr can be one of the provider
> > > attributes that are generic, but typed, allowing the attributes to be
> > > validated.  Currently the provider nlattr types include string, s32,
> > > u32, s64, and u64.  The print-type nlattr allows a provider to specify
> > > an alternative display format for user tools displaying the attribute.
> > > For example, a u32 attribute will default to "%u", but a print-type
> > > attribute can be included for it to be displayed in hex.  This allows
> > > the user tool to print the number in the format desired by the provider
> > > driver.
> > >
> > > More attrs can be defined as they become needed by providers.
> > >
> > > Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> > > ---
> > >  drivers/infiniband/core/nldev.c  | 38
> > ++++++++++++++++++++++++++++++++++++++
> > >  include/rdma/restrack.h          | 10 ++++++++++
> > >  include/uapi/rdma/rdma_netlink.h | 21 +++++++++++++++++++++
> > >  3 files changed, 69 insertions(+)
> > >
> >
> > Steve,
> >
> > You forgot to nullify those new functions in restrack_init(), not a big
> > deal because we are calling to restrack_init() after kzalloc(), but
> > better to be explicit about it.
> >
> > Thanks,
> > Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
>
> I apologize.  My mistake; I intended to add them.

And if you intend to respin, please drop _BIN print type, you don't need
it yet, so no need to expose.

>
> Thanks for the review!

Thanks for doing this.

>
> Steve.
>
diff mbox

Patch

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 884843e..99df8d4 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -95,8 +95,26 @@ 
 	[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_ENTRY_STRLEN },
+	[RDMA_NLDEV_ATTR_PROVIDER_PRINT_TYPE]	= { .type = NLA_U8 },
+	[RDMA_NLDEV_ATTR_PROVIDER_S32]		= { .type = NLA_S32 },
+	[RDMA_NLDEV_ATTR_PROVIDER_U32]		= { .type = NLA_U32 },
+	[RDMA_NLDEV_ATTR_PROVIDER_S64]		= { .type = NLA_S64 },
+	[RDMA_NLDEV_ATTR_PROVIDER_U64]		= { .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;
+}
+
 static int fill_nldev_handle(struct sk_buff *msg, struct ib_device *device)
 {
 	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_DEV_INDEX, device->index))
@@ -264,6 +282,7 @@  static int fill_res_qp_entry(struct sk_buff *msg, struct netlink_callback *cb,
 			     struct rdma_restrack_entry *res, uint32_t port)
 {
 	struct ib_qp *qp = container_of(res, struct ib_qp, res);
+	struct rdma_restrack_root *resroot = &qp->device->res;
 	struct ib_qp_init_attr qp_init_attr;
 	struct nlattr *entry_attr;
 	struct ib_qp_attr qp_attr;
@@ -313,6 +332,9 @@  static int fill_res_qp_entry(struct sk_buff *msg, struct netlink_callback *cb,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
+	if (provider_fill_res_entry(resroot, msg, cb, res))
+		goto err;
+
 	nla_nest_end(msg, entry_attr);
 	return 0;
 
@@ -328,6 +350,7 @@  static int fill_res_cm_id_entry(struct sk_buff *msg,
 {
 	struct rdma_id_private *id_priv =
 				container_of(res, struct rdma_id_private, res);
+	struct rdma_restrack_root *resroot = &id_priv->id.device->res;
 	struct rdma_cm_id *cm_id = &id_priv->id;
 	struct nlattr *entry_attr;
 
@@ -369,6 +392,9 @@  static int fill_res_cm_id_entry(struct sk_buff *msg,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
+	if (provider_fill_res_entry(resroot, msg, cb, res))
+		goto err;
+
 	nla_nest_end(msg, entry_attr);
 	return 0;
 
@@ -382,6 +408,7 @@  static int fill_res_cq_entry(struct sk_buff *msg, struct netlink_callback *cb,
 			     struct rdma_restrack_entry *res, uint32_t port)
 {
 	struct ib_cq *cq = container_of(res, struct ib_cq, res);
+	struct rdma_restrack_root *resroot = &cq->device->res;
 	struct nlattr *entry_attr;
 
 	entry_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_RES_CQ_ENTRY);
@@ -402,6 +429,9 @@  static int fill_res_cq_entry(struct sk_buff *msg, struct netlink_callback *cb,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
+	if (provider_fill_res_entry(resroot, msg, cb, res))
+		goto err;
+
 	nla_nest_end(msg, entry_attr);
 	return 0;
 
@@ -415,6 +445,7 @@  static int fill_res_mr_entry(struct sk_buff *msg, struct netlink_callback *cb,
 			     struct rdma_restrack_entry *res, uint32_t port)
 {
 	struct ib_mr *mr = container_of(res, struct ib_mr, res);
+	struct rdma_restrack_root *resroot = &mr->pd->device->res;
 	struct nlattr *entry_attr;
 
 	entry_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_RES_MR_ENTRY);
@@ -438,6 +469,9 @@  static int fill_res_mr_entry(struct sk_buff *msg, struct netlink_callback *cb,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
+	if (provider_fill_res_entry(resroot, msg, cb, res))
+		goto err;
+
 	nla_nest_end(msg, entry_attr);
 	return 0;
 
@@ -451,6 +485,7 @@  static int fill_res_pd_entry(struct sk_buff *msg, struct netlink_callback *cb,
 			     struct rdma_restrack_entry *res, uint32_t port)
 {
 	struct ib_pd *pd = container_of(res, struct ib_pd, res);
+	struct rdma_restrack_root *resroot = &pd->device->res;
 	struct nlattr *entry_attr;
 
 	entry_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_RES_PD_ENTRY);
@@ -477,6 +512,9 @@  static int fill_res_pd_entry(struct sk_buff *msg, struct netlink_callback *cb,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
+	if (provider_fill_res_entry(resroot, msg, cb, res))
+		goto err;
+
 	nla_nest_end(msg, entry_attr);
 	return 0;
 
diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
index f3b3e35..bd3cd9a 100644
--- a/include/rdma/restrack.h
+++ b/include/rdma/restrack.h
@@ -44,6 +44,8 @@  enum rdma_restrack_type {
 };
 
 #define RDMA_RESTRACK_HASH_BITS	8
+struct rdma_restrack_entry;
+
 /**
  * struct rdma_restrack_root - main resource tracking management
  * entity, per-device
@@ -57,6 +59,14 @@  struct rdma_restrack_root {
 	 * @hash: global database for all resources per-device
 	 */
 	DECLARE_HASHTABLE(hash, RDMA_RESTRACK_HASH_BITS);
+	/**
+	 * @fill_res_entry: provider-specific fill function
+	 *
+	 * Allows rdma providers to add their own restrack attributes.
+	 */
+	int (*fill_res_entry)(struct sk_buff *msg,
+			      struct netlink_callback *cb,
+			      struct rdma_restrack_entry *entry);
 };
 
 /**
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index 84b3f63..b953e9d 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -249,6 +249,16 @@  enum rdma_nldev_command {
 	RDMA_NLDEV_NUM_OPS
 };
 
+enum {
+	RDMA_NLDEV_ATTR_ENTRY_STRLEN = 16,
+};
+
+enum rdma_nldev_print_type {
+	RDMA_NLDEV_PRINT_TYPE_UNSPEC,
+	RDMA_NLDEV_PRINT_TYPE_HEX,
+	RDMA_NLDEV_PRINT_TYPE_BIN,
+};
+
 enum rdma_nldev_attr {
 	/* don't change the order or add anything between, this is ABI! */
 	RDMA_NLDEV_ATTR_UNSPEC,
@@ -390,6 +400,17 @@  enum rdma_nldev_attr {
 	RDMA_NLDEV_ATTR_RES_PD_ENTRY,		/* nested table */
 	RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY,	/* u32 */
 	RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY,	/* u32 */
+	/*
+	 * provider-specific attributes.
+	 */
+	RDMA_NLDEV_ATTR_PROVIDER,		/* nested table */
+	RDMA_NLDEV_ATTR_PROVIDER_ENTRY,		/* nested table */
+	RDMA_NLDEV_ATTR_PROVIDER_STRING,	/* string */
+	RDMA_NLDEV_ATTR_PROVIDER_PRINT_TYPE,	/* u8 */
+	RDMA_NLDEV_ATTR_PROVIDER_S32,		/* s32 */
+	RDMA_NLDEV_ATTR_PROVIDER_U32,		/* u32 */
+	RDMA_NLDEV_ATTR_PROVIDER_S64,		/* s64 */
+	RDMA_NLDEV_ATTR_PROVIDER_U64,		/* u64 */
 
 	RDMA_NLDEV_ATTR_MAX
 };