Re: [PATCH v2 rdma-next 3/5] RDMA/nldev: add provider-specific device/port tracking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 29, 2018 at 09:09:51AM -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@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  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 99df8d4..902517f 100644
> --- a/drivers/infiniband/core/nldev.c
> +++ b/drivers/infiniband/core/nldev.c
> @@ -115,6 +115,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))
> @@ -125,7 +141,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];
>
> @@ -156,10 +173,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;
> @@ -195,6 +216,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;
>  }
>
> @@ -554,7 +577,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);

I look on this line and think that providing "struct netlink_callback *cb"
to drivers is too broad approach. The right thing will be to pass
"u64 caps_falgs" variable which will have bits on/off per uapi/linux/capability.h.

However, you don't use this functionality for now, so it will make sense
do not implement it completely and simply leave comment in the code how
it should be implemented in the future.

Thanks

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux