[openib-general] [RFC] [PATCH 1/3] RDMA CM: add rdma_get/set_optioncalls to get/set path records

[Michael S. Tsirkin]

Quoting r. Sean Hefty <sean.hefty at intel.com>:
> @@ -251,5 +262,28 @@ int rdma_reject(struct rdma_cm_id *id, c
>   */
>  int rdma_disconnect(struct rdma_cm_id *id);
>  
> +/**
> + * rdma_get_option - Retrieve options for an rdma_cm_id.
> + * @id: Communication identifier to retrieve option for.
> + * @level: Protocol level of the option to retrieve.
> + * @optname: Name of the option to retrieve.
> + * @optval: Buffer to receive the returned options.
> + * @optlen: On input, the size of the %optval buffer.  On output, the
> + *   size of the returned data.
> + */
> +int rdma_get_option(struct rdma_cm_id *id, int level, int optname,
> +		    void *optval, size_t *optlen);
> +
> +/**
> + * rdma_set_option - Set options for an rdma_cm_id.
> + * @id: Communication identifier to set option for.
> + * @level: Protocol level of the option to set.
> + * @optname: Name of the option to set.
> + * @optval: Reference to the option data.
> + * @optlen: The size of the %optval buffer.
> + */
> +int rdma_set_option(struct rdma_cm_id *id, int level, int optname,
> +		    void *optval, size_t optlen);
> +

It seems optval is a user pointer. Should it be parked as such
void __user *.

>  #endif /* RDMA_CM_H */
>  
> Index: core/cma.c
> ===================================================================
> --- core/cma.c	(revision 6627)
> +++ core/cma.c	(working copy)

....

> +static int cma_set_ib_paths(struct rdma_id_private *id_priv,
> +			    void *optval, size_t optlen)
> +{
> +	struct rdma_route *route = &id_priv->id.route;
> +	struct ib_user_path_rec user_path;
> +	int ret, i;
> +
> +	if (!cma_comp_exch(id_priv, CMA_ADDR_RESOLVED, CMA_ROUTE_RESOLVED))
> +		return -EINVAL;
> +
> +	if (optlen == sizeof(user_path))
> +		route->num_paths = 1;
> +	else if (optlen == (sizeof(user_path) << 1))
> +		route->num_paths = 2;
> +	else {
> +		ret = -EINVAL;
> +		goto err1;
> +	}
> +
> +	route->path_rec = kmalloc(sizeof *route->path_rec * route->num_paths,
> +				  GFP_KERNEL);
> +	if (!route->path_rec) {
> +		ret = -ENOMEM;
> +		goto err2;
> +	}
> +
> +	for (i = 0; i < route->num_paths; i++, optval += sizeof(user_path)) {
> +		if (copy_from_user(&user_path, (void __user *) optval,
> +				   sizeof(user_path))) {

Apparently you assume userspace pointer here: so the interface is not intended
for kernel users? So why is it not in ucma?

>
> +			ret = -EFAULT;
> +			goto err3;
> +		}
> +		ib_copy_path_rec_from_user(&route->path_rec[i], &user_path);
> +	}
> +	return 0;
> +err3:
> +	kfree(route->path_rec);
> +err2:
> +	route->num_paths = 0;
> +err1:
> +	cma_comp_exch(id_priv, CMA_ROUTE_RESOLVED, CMA_ADDR_RESOLVED);
> +	return ret;
> +}

-- 
MST