Re: [PATCH rdma-core 03/13] verbs: Introduce ibv_import_device() verb

[Jason Gunthorpe <jgg@mellanox.com>]

On Wed, Jun 17, 2020 at 10:45:46AM +0300, Yishai Hadas wrote:
> +static struct ibv_context *verbs_import_device(int cmd_fd)
> +{
> +	struct verbs_device *verbs_device = NULL;
> +	struct verbs_context *context_ex;
> +	struct ibv_device **dev_list;
> +	struct ibv_context *ctx = NULL;
> +	struct stat st;
> +	int i;
> +
> +	if (fstat(cmd_fd, &st) || !S_ISCHR(st.st_mode)) {
> +		errno = EINVAL;
> +		return NULL;
> +	}
> +
> +	dev_list = ibv_get_device_list(NULL);
> +	if (!dev_list) {
> +		errno = ENODEV;
> +		return NULL;
> +	}
> +
> +	for (i = 0; dev_list[i]; ++i) {
> +		if (verbs_get_device(dev_list[i])->sysfs->sysfs_cdev ==
> +					st.st_rdev) {
> +			verbs_device = verbs_get_device(dev_list[i]);
> +			break;
> +		}
> +	}

Unfortunately it looks like there is a small race here, the struct
ib_uverbs_file can exist beyond the lifetime of the cdev number - the
uverbs_ida is freed in ib_uverbs_remove_one() and files can still be
open past that point.

I guess we should add a special ioctl to return the driver_id and
match that way?

> +	if (!verbs_device) {
> +		errno = ENODEV;
> +		goto out;
> +	}
> +
> +	if (!verbs_device->ops->import_context) {
> +		errno = EOPNOTSUPP;
> +		goto out;
> +	}
> +
> +	context_ex = verbs_device->ops->import_context(&verbs_device->device, cmd_fd);
> +	if (!context_ex)
> +		goto out;
> +
> +	set_lib_ops(context_ex);
> +
> +	ctx = &context_ex->context;
> +out:
> +	ibv_free_device_list(dev_list);
> +	return ctx;
> +}
> +
> +struct ibv_context *ibv_import_device(int cmd_fd)
> +{
> +	return verbs_import_device(cmd_fd);
> +}

Why two functions? No reason for verbs_import_device()..

Jason