Re: [PATCH rdma-next v5 1/2] RDMA: Add indication for in kernel API support to IB device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 10, 2019 at 04:23:35PM +0200, Gal Pressman wrote:
> Drivers that do not provide kernel verbs support should not be used by
> ib kernel clients and fail.
> In case a device does not implement all mandatory verbs for kverbs usage
> mark it as a non kverbs provider and prevent its usage for all clients
> except for uverbs.
> 
> The device is marked as a non kverbs provider using the
> 'kverbs_provider' flag which should only be set by the core code.
> The clients can choose whether kverbs are requested for its usage using
> the 'no_kverbs_req' flag which is currently set for uverbs only.
> 
> This patch allows drivers to remove mandatory verbs stubs and simply set
> the callback to NULL. The IB device will be registered as a non-kverbs
> provider.
> 
> Signed-off-by: Gal Pressman <galpress@xxxxxxxxxx>
>  drivers/infiniband/core/device.c      | 35 ++++++++++++++++++++++-------------
>  drivers/infiniband/core/uverbs_main.c |  1 +
>  include/rdma/ib_verbs.h               |  5 +++++
>  3 files changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 8872453e26c0..3f32d05df55d 100644
> +++ b/drivers/infiniband/core/device.c
> @@ -121,13 +121,12 @@ static int ib_device_check_mandatory(struct ib_device *device)
>  	};
>  	int i;
>  
> +	device->kverbs_provider = true;
>  	for (i = 0; i < ARRAY_SIZE(mandatory_table); ++i) {
>  		if (!*(void **) ((void *) &device->ops +
>  				 mandatory_table[i].offset)) {
> -			dev_warn(&device->dev,
> -				 "Device is missing mandatory function %s\n",
> -				 mandatory_table[i].name);
> -			return -EINVAL;
> +			device->kverbs_provider = false;
> +			break;
>  		}
>  	}
>  
> @@ -330,6 +329,9 @@ static int add_client_context(struct ib_device *device, struct ib_client *client
>  {
>  	struct ib_client_data *context;
>  
> +	if (!device->kverbs_provider && !client->no_kverbs_req)
> +		return -EOPNOTSUPP;

Return 0, it is not a failure, the client is just not supported on
this device. Not that it matters right now..

> @@ -374,10 +376,12 @@ static int read_port_immutable(struct ib_device *device)
>  		return -ENOMEM;
>  
>  	for (port = start_port; port <= end_port; ++port) {
> -		ret = device->ops.get_port_immutable(
> -			device, port, &device->port_immutable[port]);
> -		if (ret)
> -			return ret;
> +		if (device->ops.get_port_immutable) {
> +			ret = device->ops.get_port_immutable(
> +				device, port, &device->port_immutable[port]);
> +			if (ret)
> +				return ret;
> +		}
>  
>  		if (verify_immutable(device, port))
>  			return -EINVAL;

Can we still call verify_immutable?

> @@ -537,11 +541,13 @@ static int setup_device(struct ib_device *device)
>  	}
>  
>  	memset(&device->attrs, 0, sizeof(device->attrs));
> -	ret = device->ops.query_device(device, &device->attrs, &uhw);
> -	if (ret) {
> -		dev_warn(&device->dev,
> -			 "Couldn't query the device attributes\n");
> -		goto port_cleanup;
> +	if (device->ops.query_device) {
> +		ret = device->ops.query_device(device, &device->attrs, &uhw);
> +		if (ret) {
> +			dev_warn(&device->dev,
> +				 "Couldn't query the device attributes\n");
> +			goto port_cleanup;
> +		}

Even uverbs_cmd.c needs attrs to be valid, drop this hunk. If a driver
providers a NULL query_device then it will rightly crash on
registration.

> @@ -920,6 +926,9 @@ int ib_query_port(struct ib_device *device,
>  	union ib_gid gid;
>  	int err;
>  
> +	if (!device->ops.query_port)
> +		return -EOPNOTSUPP;

Again, if query_port is not supported then the related sysfs file
should not even be created.

> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index c073a4720d28..fb6074d2e394 100644
> +++ b/include/rdma/ib_verbs.h
> @@ -2566,6 +2566,8 @@ struct ib_device {
>  	__be64			     node_guid;
>  	u32			     local_dma_lkey;
>  	u16                          is_switch:1;
> +	/* Indicates kernel verbs support, should not be used in drivers */
> +	u8                           kverbs_provider:1;

u16 surely?

Jason



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux