diff mbox

[v1,rdma-next,3/4] RDMA/nldev: add provider-specific device/port tracking

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

Commit Message

Steve Wise March 23, 2018, 4:49 p.m. UTC
Add fill_dev_info and fill_port_info functions to rdma_restrack_root.
This allows providers to have provider-specific fill functions for device
and port restrack operations.

Signed-off-by: Steve Wise <swise@opengridcomputing.com>
---
 drivers/infiniband/core/nldev.c | 35 +++++++++++++++++++++++++++++------
 include/rdma/restrack.h         | 15 +++++++++++++++
 2 files changed, 44 insertions(+), 6 deletions(-)

Comments

Leon Romanovsky March 24, 2018, 9:51 a.m. UTC | #1
On Fri, Mar 23, 2018 at 09:49:45AM -0700, Steve Wise wrote:
> Add fill_dev_info and fill_port_info functions to rdma_restrack_root.
> This allows providers to have provider-specific fill functions for device
> and port restrack operations.
>
> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> ---
>  drivers/infiniband/core/nldev.c | 35 +++++++++++++++++++++++++++++------
>  include/rdma/restrack.h         | 15 +++++++++++++++
>  2 files changed, 44 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> index 8346ede..16c5eba 100644
> --- a/drivers/infiniband/core/nldev.c
> +++ b/drivers/infiniband/core/nldev.c
> @@ -116,6 +116,22 @@ static int provider_fill_res_entry(struct rdma_restrack_root *resroot,
>  		resroot->fill_res_entry(msg, cb, res) : 0;
>  }
>
> +static int provider_fill_dev_info(struct sk_buff *msg,
> +				  struct netlink_callback *cb,
> +				  struct ib_device *device)
> +{
> +	return device->res.fill_dev_info ?
> +		device->res.fill_dev_info(msg, cb, device) : 0;
> +}
> +
> +static int provider_fill_port_info(struct sk_buff *msg,
> +				   struct netlink_callback *cb,
> +				   struct ib_device *device, u32 port)
> +{
> +	return device->res.fill_port_info ?
> +		device->res.fill_port_info(msg, cb, device, port) : 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))
> @@ -126,7 +142,8 @@ static int fill_nldev_handle(struct sk_buff *msg, struct ib_device *device)
>  	return 0;
>  }
>
> -static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
> +static int fill_dev_info(struct sk_buff *msg, struct netlink_callback *cb,
> +			 struct ib_device *device)
>  {
>  	char fw[IB_FW_VERSION_NAME_MAX];
>
> @@ -157,10 +174,14 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
>  		return -EMSGSIZE;
>  	if (nla_put_u8(msg, RDMA_NLDEV_ATTR_DEV_NODE_TYPE, device->node_type))
>  		return -EMSGSIZE;
> +
> +	if (provider_fill_dev_info(msg, cb, device))
> +		return -EMSGSIZE;
> +
>  	return 0;
>  }
>
> -static int fill_port_info(struct sk_buff *msg,
> +static int fill_port_info(struct sk_buff *msg, struct netlink_callback *cb,
>  			  struct ib_device *device, u32 port)
>  {
>  	struct ib_port_attr attr;
> @@ -196,6 +217,8 @@ static int fill_port_info(struct sk_buff *msg,
>  		return -EMSGSIZE;
>  	if (nla_put_u8(msg, RDMA_NLDEV_ATTR_PORT_PHYS_STATE, attr.phys_state))
>  		return -EMSGSIZE;
> +	if (provider_fill_port_info(msg, cb, device, port))
> +		return -EMSGSIZE;
>  	return 0;
>  }
>
> @@ -555,7 +578,7 @@ static int nldev_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
>  			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_GET),
>  			0, 0);
>
> -	err = fill_dev_info(msg, device);
> +	err = fill_dev_info(msg, NULL, device);
>  	if (err)
>  		goto err_free;
>
> @@ -586,7 +609,7 @@ static int _nldev_get_dumpit(struct ib_device *device,
>  			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_GET),
>  			0, NLM_F_MULTI);
>
> -	if (fill_dev_info(skb, device)) {
> +	if (fill_dev_info(skb, cb, device)) {
>  		nlmsg_cancel(skb, nlh);
>  		goto out;
>  	}
> @@ -646,7 +669,7 @@ static int nldev_port_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
>  			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_GET),
>  			0, 0);
>
> -	err = fill_port_info(msg, device, port);
> +	err = fill_port_info(msg, NULL, device, port);
>  	if (err)
>  		goto err_free;
>
> @@ -706,7 +729,7 @@ static int nldev_port_get_dumpit(struct sk_buff *skb,
>  						 RDMA_NLDEV_CMD_PORT_GET),
>  				0, NLM_F_MULTI);
>
> -		if (fill_port_info(skb, device, p)) {
> +		if (fill_port_info(skb, cb, device, p)) {
>  			nlmsg_cancel(skb, nlh);
>  			goto out;
>  		}
> diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
> index bd3cd9a..936b56f 100644
> --- a/include/rdma/restrack.h
> +++ b/include/rdma/restrack.h
> @@ -45,6 +45,7 @@ enum rdma_restrack_type {
>
>  #define RDMA_RESTRACK_HASH_BITS	8
>  struct rdma_restrack_entry;
> +struct ib_device;
>
>  /**
>   * struct rdma_restrack_root - main resource tracking management
> @@ -67,6 +68,20 @@ struct rdma_restrack_root {
>  	int (*fill_res_entry)(struct sk_buff *msg,
>  			      struct netlink_callback *cb,
>  			      struct rdma_restrack_entry *entry);
> +	/**
> +	 * @fill_dev_info: provider-specific fill function
> +	 *
> +	 * Allows rdma providers to add their own device attributes.
> +	 */
> +	int (*fill_dev_info)(struct sk_buff *msg, struct netlink_callback *cb,
> +			     struct ib_device *device);
> +	/**
> +	 * @fill_port_info: provider-specific fill function
> +	 *
> +	 * Allows rdma providers to add their own port attributes.
> +	 */
> +	int (*fill_port_info)(struct sk_buff *msg, struct netlink_callback *cb,
> +			      struct ib_device *device, u32 port);

Why do you pass "struct netlink_callback *cb" to the drivers? IMHO they
need to receive minimum information, pointer to message (they don't need
to allocate it), device, port, restrack entry.

Thanks

>  };
>
>  /**
> --
> 1.8.3.1
>
Steve Wise March 24, 2018, 7:46 p.m. UTC | #2
> 
> On Fri, Mar 23, 2018 at 09:49:45AM -0700, Steve Wise wrote:
> > Add fill_dev_info and fill_port_info functions to rdma_restrack_root.
> > This allows providers to have provider-specific fill functions for
device
> > and port restrack operations.
> >
> > Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> > ---
> >  drivers/infiniband/core/nldev.c | 35 +++++++++++++++++++++++++++++---
> ---
> >  include/rdma/restrack.h         | 15 +++++++++++++++
> >  2 files changed, 44 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/nldev.c
> b/drivers/infiniband/core/nldev.c
> > index 8346ede..16c5eba 100644
> > --- a/drivers/infiniband/core/nldev.c
> > +++ b/drivers/infiniband/core/nldev.c
> > @@ -116,6 +116,22 @@ static int provider_fill_res_entry(struct
> rdma_restrack_root *resroot,
> >  		resroot->fill_res_entry(msg, cb, res) : 0;
> >  }
> >
> > +static int provider_fill_dev_info(struct sk_buff *msg,
> > +				  struct netlink_callback *cb,
> > +				  struct ib_device *device)
> > +{
> > +	return device->res.fill_dev_info ?
> > +		device->res.fill_dev_info(msg, cb, device) : 0;
> > +}
> > +
> > +static int provider_fill_port_info(struct sk_buff *msg,
> > +				   struct netlink_callback *cb,
> > +				   struct ib_device *device, u32 port)
> > +{
> > +	return device->res.fill_port_info ?
> > +		device->res.fill_port_info(msg, cb, device, port) : 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))
> > @@ -126,7 +142,8 @@ static int fill_nldev_handle(struct sk_buff *msg,
> struct ib_device *device)
> >  	return 0;
> >  }
> >
> > -static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
> > +static int fill_dev_info(struct sk_buff *msg, struct netlink_callback
*cb,
> > +			 struct ib_device *device)
> >  {
> >  	char fw[IB_FW_VERSION_NAME_MAX];
> >
> > @@ -157,10 +174,14 @@ static int fill_dev_info(struct sk_buff *msg,
> struct ib_device *device)
> >  		return -EMSGSIZE;
> >  	if (nla_put_u8(msg, RDMA_NLDEV_ATTR_DEV_NODE_TYPE, device-
> >node_type))
> >  		return -EMSGSIZE;
> > +
> > +	if (provider_fill_dev_info(msg, cb, device))
> > +		return -EMSGSIZE;
> > +
> >  	return 0;
> >  }
> >
> > -static int fill_port_info(struct sk_buff *msg,
> > +static int fill_port_info(struct sk_buff *msg, struct netlink_callback
*cb,
> >  			  struct ib_device *device, u32 port)
> >  {
> >  	struct ib_port_attr attr;
> > @@ -196,6 +217,8 @@ static int fill_port_info(struct sk_buff *msg,
> >  		return -EMSGSIZE;
> >  	if (nla_put_u8(msg, RDMA_NLDEV_ATTR_PORT_PHYS_STATE,
> attr.phys_state))
> >  		return -EMSGSIZE;
> > +	if (provider_fill_port_info(msg, cb, device, port))
> > +		return -EMSGSIZE;
> >  	return 0;
> >  }
> >
> > @@ -555,7 +578,7 @@ static int nldev_get_doit(struct sk_buff *skb,
> struct nlmsghdr *nlh,
> >  			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV,
> RDMA_NLDEV_CMD_GET),
> >  			0, 0);
> >
> > -	err = fill_dev_info(msg, device);
> > +	err = fill_dev_info(msg, NULL, device);
> >  	if (err)
> >  		goto err_free;
> >
> > @@ -586,7 +609,7 @@ static int _nldev_get_dumpit(struct ib_device
> *device,
> >  			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV,
> RDMA_NLDEV_CMD_GET),
> >  			0, NLM_F_MULTI);
> >
> > -	if (fill_dev_info(skb, device)) {
> > +	if (fill_dev_info(skb, cb, device)) {
> >  		nlmsg_cancel(skb, nlh);
> >  		goto out;
> >  	}
> > @@ -646,7 +669,7 @@ static int nldev_port_get_doit(struct sk_buff *skb,
> struct nlmsghdr *nlh,
> >  			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV,
> RDMA_NLDEV_CMD_GET),
> >  			0, 0);
> >
> > -	err = fill_port_info(msg, device, port);
> > +	err = fill_port_info(msg, NULL, device, port);
> >  	if (err)
> >  		goto err_free;
> >
> > @@ -706,7 +729,7 @@ static int nldev_port_get_dumpit(struct sk_buff
> *skb,
> >
> RDMA_NLDEV_CMD_PORT_GET),
> >  				0, NLM_F_MULTI);
> >
> > -		if (fill_port_info(skb, device, p)) {
> > +		if (fill_port_info(skb, cb, device, p)) {
> >  			nlmsg_cancel(skb, nlh);
> >  			goto out;
> >  		}
> > diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
> > index bd3cd9a..936b56f 100644
> > --- a/include/rdma/restrack.h
> > +++ b/include/rdma/restrack.h
> > @@ -45,6 +45,7 @@ enum rdma_restrack_type {
> >
> >  #define RDMA_RESTRACK_HASH_BITS	8
> >  struct rdma_restrack_entry;
> > +struct ib_device;
> >
> >  /**
> >   * struct rdma_restrack_root - main resource tracking management
> > @@ -67,6 +68,20 @@ struct rdma_restrack_root {
> >  	int (*fill_res_entry)(struct sk_buff *msg,
> >  			      struct netlink_callback *cb,
> >  			      struct rdma_restrack_entry *entry);
> > +	/**
> > +	 * @fill_dev_info: provider-specific fill function
> > +	 *
> > +	 * Allows rdma providers to add their own device attributes.
> > +	 */
> > +	int (*fill_dev_info)(struct sk_buff *msg, struct netlink_callback
*cb,
> > +			     struct ib_device *device);
> > +	/**
> > +	 * @fill_port_info: provider-specific fill function
> > +	 *
> > +	 * Allows rdma providers to add their own port attributes.
> > +	 */
> > +	int (*fill_port_info)(struct sk_buff *msg, struct netlink_callback
*cb,
> > +			      struct ib_device *device, u32 port);
> 
> Why do you pass "struct netlink_callback *cb" to the drivers? IMHO they
> need to receive minimum information, pointer to message (they don't need
> to allocate it), device, port, restrack entry.

The reason is to allow the handler to enforce capabilities of the caller's
socket.  Namely, netlink_capable().  It cannot be called on msg because that
skb doesn't not have the callback info in it that cb->skb does.  Rather than
just passing if the socket sending the NL request is CAP_NET_ADMIN capable,
I chose to just pass the cb in case other info/state is needed by the
handler. 

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
diff mbox

Patch

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 8346ede..16c5eba 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -116,6 +116,22 @@  static int provider_fill_res_entry(struct rdma_restrack_root *resroot,
 		resroot->fill_res_entry(msg, cb, res) : 0;
 }
 
+static int provider_fill_dev_info(struct sk_buff *msg,
+				  struct netlink_callback *cb,
+				  struct ib_device *device)
+{
+	return device->res.fill_dev_info ?
+		device->res.fill_dev_info(msg, cb, device) : 0;
+}
+
+static int provider_fill_port_info(struct sk_buff *msg,
+				   struct netlink_callback *cb,
+				   struct ib_device *device, u32 port)
+{
+	return device->res.fill_port_info ?
+		device->res.fill_port_info(msg, cb, device, port) : 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))
@@ -126,7 +142,8 @@  static int fill_nldev_handle(struct sk_buff *msg, struct ib_device *device)
 	return 0;
 }
 
-static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
+static int fill_dev_info(struct sk_buff *msg, struct netlink_callback *cb,
+			 struct ib_device *device)
 {
 	char fw[IB_FW_VERSION_NAME_MAX];
 
@@ -157,10 +174,14 @@  static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
 		return -EMSGSIZE;
 	if (nla_put_u8(msg, RDMA_NLDEV_ATTR_DEV_NODE_TYPE, device->node_type))
 		return -EMSGSIZE;
+
+	if (provider_fill_dev_info(msg, cb, device))
+		return -EMSGSIZE;
+
 	return 0;
 }
 
-static int fill_port_info(struct sk_buff *msg,
+static int fill_port_info(struct sk_buff *msg, struct netlink_callback *cb,
 			  struct ib_device *device, u32 port)
 {
 	struct ib_port_attr attr;
@@ -196,6 +217,8 @@  static int fill_port_info(struct sk_buff *msg,
 		return -EMSGSIZE;
 	if (nla_put_u8(msg, RDMA_NLDEV_ATTR_PORT_PHYS_STATE, attr.phys_state))
 		return -EMSGSIZE;
+	if (provider_fill_port_info(msg, cb, device, port))
+		return -EMSGSIZE;
 	return 0;
 }
 
@@ -555,7 +578,7 @@  static int nldev_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_GET),
 			0, 0);
 
-	err = fill_dev_info(msg, device);
+	err = fill_dev_info(msg, NULL, device);
 	if (err)
 		goto err_free;
 
@@ -586,7 +609,7 @@  static int _nldev_get_dumpit(struct ib_device *device,
 			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_GET),
 			0, NLM_F_MULTI);
 
-	if (fill_dev_info(skb, device)) {
+	if (fill_dev_info(skb, cb, device)) {
 		nlmsg_cancel(skb, nlh);
 		goto out;
 	}
@@ -646,7 +669,7 @@  static int nldev_port_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_GET),
 			0, 0);
 
-	err = fill_port_info(msg, device, port);
+	err = fill_port_info(msg, NULL, device, port);
 	if (err)
 		goto err_free;
 
@@ -706,7 +729,7 @@  static int nldev_port_get_dumpit(struct sk_buff *skb,
 						 RDMA_NLDEV_CMD_PORT_GET),
 				0, NLM_F_MULTI);
 
-		if (fill_port_info(skb, device, p)) {
+		if (fill_port_info(skb, cb, device, p)) {
 			nlmsg_cancel(skb, nlh);
 			goto out;
 		}
diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
index bd3cd9a..936b56f 100644
--- a/include/rdma/restrack.h
+++ b/include/rdma/restrack.h
@@ -45,6 +45,7 @@  enum rdma_restrack_type {
 
 #define RDMA_RESTRACK_HASH_BITS	8
 struct rdma_restrack_entry;
+struct ib_device;
 
 /**
  * struct rdma_restrack_root - main resource tracking management
@@ -67,6 +68,20 @@  struct rdma_restrack_root {
 	int (*fill_res_entry)(struct sk_buff *msg,
 			      struct netlink_callback *cb,
 			      struct rdma_restrack_entry *entry);
+	/**
+	 * @fill_dev_info: provider-specific fill function
+	 *
+	 * Allows rdma providers to add their own device attributes.
+	 */
+	int (*fill_dev_info)(struct sk_buff *msg, struct netlink_callback *cb,
+			     struct ib_device *device);
+	/**
+	 * @fill_port_info: provider-specific fill function
+	 *
+	 * Allows rdma providers to add their own port attributes.
+	 */
+	int (*fill_port_info)(struct sk_buff *msg, struct netlink_callback *cb,
+			      struct ib_device *device, u32 port);
 };
 
 /**