Re: [PATCH rdma-next v1 2/4] RDMA/nldev: Allow different fill function per resource

[Jason Gunthorpe <jgg@mellanox.com>]

On Fri, Aug 30, 2019 at 11:16:10AM +0300, Leon Romanovsky wrote:
> From: Erez Alfasi <ereza@mellanox.com>
> 
> So far res_get_common_{dumpit, doit} was using the default
> resource fill function which was defined as part of the
> nldev_fill_res_entry fill_entries.
> 
> Add a fill function pointer as an argument allows us to use
> different fill function in case we want to dump different
> values then 'rdma resource' flow do, but still use the same
> existing general resources dumping flow.
> 
> If a NULL value is passed, it will be using the default
> fill function that was defined in 'fill_entries' for a
> given resource type.
> 
> Signed-off-by: Erez Alfasi <ereza@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>  drivers/infiniband/core/nldev.c | 42 +++++++++++++++++++++++----------
>  1 file changed, 29 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> index cc08218f1ef7..47f7fe5432db 100644
> +++ b/drivers/infiniband/core/nldev.c
> @@ -1181,7 +1181,10 @@ static const struct nldev_fill_res_entry fill_entries[RDMA_RESTRACK_MAX] = {
>  
>  static int res_get_common_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
>  			       struct netlink_ext_ack *extack,
> -			       enum rdma_restrack_type res_type)
> +			       enum rdma_restrack_type res_type,
> +			       int (*fill_func)(struct sk_buff*, bool,
> +						struct rdma_restrack_entry*,
> +						uint32_t))

Use a typedef?

>  {
>  	const struct nldev_fill_res_entry *fe = &fill_entries[res_type];
>  	struct nlattr *tb[RDMA_NLDEV_ATTR_MAX];
> @@ -1244,7 +1247,12 @@ static int res_get_common_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	}
>  
>  	has_cap_net_admin = netlink_capable(skb, CAP_NET_ADMIN);
> -	ret = fe->fill_res_func(msg, has_cap_net_admin, res, port);
> +
> +	if (fill_func)
> +		ret = fill_func(msg, has_cap_net_admin, res, port);
> +	else
> +		ret = fe->fill_res_func(msg, has_cap_net_admin, res, port);

Weird to now be choosing between two function pointers

> -#define RES_GET_FUNCS(name, type)                                              \
> -	static int nldev_res_get_##name##_dumpit(struct sk_buff *skb,          \
> +#define RES_GET_FUNCS(name, type)					       \
> +	static int nldev_res_get_##name##_dumpit(struct sk_buff *skb,	       \
>  						 struct netlink_callback *cb)  \
> -	{                                                                      \
> -		return res_get_common_dumpit(skb, cb, type);                   \
> -	}                                                                      \
> -	static int nldev_res_get_##name##_doit(struct sk_buff *skb,            \
> -					       struct nlmsghdr *nlh,           \
> +	{								       \
> +		return res_get_common_dumpit(skb, cb, type, NULL);	       \
> +	}								       \
> +	static int nldev_res_get_##name##_doit(struct sk_buff *skb,	       \
> +					       struct nlmsghdr *nlh,	       \
>  					       struct netlink_ext_ack *extack) \
> -	{                                                                      \
> -		return res_get_common_doit(skb, nlh, extack, type);            \
> +	{								       \
> +		return res_get_common_doit(skb, nlh, extack, type, NULL);      \
>  	}

ie the NULL should be fill_entries[type]->fill_res_func?

Jason