Re: [rdma-next v1 23/33] RDMA/netlink: Add nldev port dumpit implementation

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3607 bytes --]

On 8/9/2017 12:03 PM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> This patch implements the query interface to get all
> ports data for the specific device.
> 
> Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
> ---
>  drivers/infiniband/core/nldev.c | 61 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> index 666940f2b49a..4413eda8c435 100644
> --- a/drivers/infiniband/core/nldev.c
> +++ b/drivers/infiniband/core/nldev.c
> @@ -53,6 +53,18 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
>  	return 0;
>  }
>  
> +static int fill_port_info(struct sk_buff *msg,
> +			  struct ib_device *device, u32 port)
> +{
> +	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_DEV_INDEX, device->index))
> +		return -EMSGSIZE;
> +	if (nla_put_string(msg, RDMA_NLDEV_ATTR_DEV_NAME, device->name))
> +		return -EMSGSIZE;
> +	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_PORT_INDEX, port))
> +		return -EMSGSIZE;
> +	return 0;
> +}
> +
>  static int nldev_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
>  			  struct netlink_ext_ack *extack)
>  {
> @@ -129,11 +141,60 @@ static int nldev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
>  	return ib_enum_all_devs(_nldev_get_dumpit, skb, cb);
>  }
>  
> +static int nldev_port_get_dumpit(struct sk_buff *skb,
> +				 struct netlink_callback *cb)
> +{
> +	struct nlattr *tb[RDMA_NLDEV_ATTR_MAX];
> +	struct ib_device *device;
> +	int start = cb->args[0];
> +	struct nlmsghdr *nlh;
> +	u32 idx = 0;
> +	u32 ifindex;
> +	int err;
> +	u32 p;
> +
> +	err = nlmsg_parse(cb->nlh, 0, tb, RDMA_NLDEV_ATTR_MAX - 1,
> +			  nldev_policy, NULL);
> +	if (err || !tb[RDMA_NLDEV_ATTR_DEV_INDEX])
> +		return -EINVAL;
> +
> +	ifindex = nla_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
> +	device = __ib_device_get_by_index(ifindex);
> +	if (!device)
> +		return -EINVAL;
> +
> +	for (p = rdma_start_port(device); p <= rdma_end_port(device); ++p) {
> +		if (idx < start) {
> +			idx++;
> +			continue;
> +		}

So, to someone who has been studying netlink internals (aka, you), the
relationship between cb->args[0] to start to idx to p may make sense.
To those of us that aren't totally versed in what appears to be a
recursive callback, but which isn't clear on how cb->args[0] ever gets
initialized the first time, or how the idx in ib_enum_all_devs()
translates to the idx here, a few comments in the code might be helpful.

> +
> +		nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid,
> +				cb->nlh->nlmsg_seq,
> +				RDMA_NL_GET_TYPE(RDMA_NL_NLDEV,
> +						 RDMA_NLDEV_CMD_PORT_GET),
> +				0, NLM_F_MULTI);
> +
> +		if (fill_port_info(skb, device, p)) {
> +			nlmsg_cancel(skb, nlh);
> +			goto out;
> +		}
> +		idx++;
> +		nlmsg_end(skb, nlh);
> +	}
> +
> +out:	cb->args[0] = idx;
> +	return skb->len;
> +}
> +
>  static const struct rdma_nl_cbs nldev_cb_table[] = {
>  	[RDMA_NLDEV_CMD_GET] = {
>  		.doit = nldev_get_doit,
>  		.dump = nldev_get_dumpit,
>  	},
> +	[RDMA_NLDEV_CMD_PORT_GET] = {
> +		.dump = nldev_port_get_dumpit,
> +	},
>  };
>  
>  void __init nldev_init(void)
> 


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]