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

[Sean Hefty]

Michael S. Tsirkin wrote:
>>+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 *.

The getsockopt / setsockopt calls both use char *optval in their interfaces. 
Internally, they do get_user(), put_user(), copy_to_user(), etc.

It's my understanding, which could be way off, that both getsockopt and 
setsockopt are also callable from kernel modules.  I have not had a chance to 
try these calls with kernel memory to see what copy_to_user() would do.

>>+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?

The call needs to be handled in the cma, as opposed to the ucma for a couple of 
reasons.  For the get_option, I need to protect against device removal while 
accessing the list of available path records.  For the set_option routine, the 
call changes the state of the rdma_cm_id.

- Sean