On 02-Jan-19 14:39, Leon Romanovsky wrote:
> On Wed, Jan 02, 2019 at 01:51:29PM +0200, Gal Pressman wrote:
>> On 02-Jan-19 13:22, Leon Romanovsky wrote:
>>> On Wed, Jan 02, 2019 at 10:40:50AM +0200, Gal Pressman wrote:
>>>> On 01-Jan-19 21:17, Leon Romanovsky wrote:
>>>>> On Tue, Jan 01, 2019 at 11:30:24AM +0200, Gal Pressman wrote:
>>>>>> Drivers that do not provide kernel verbs support should communicate that
>>>>>> to the ULPs before they try to use the device and fail.
>>>>>> This patch allows drivers to indicate whether the device provides
>>>>>> support for kernel API usage with the 'kverbs_provider' flag and makes
>>>>>> ib clients not add unsupported devices.
>>>>>
>>>>> General comment, if we decide to proceed this path, the implementation
>>>>> shouldn't touch ULPs and drivers and needs to be implemented in IB/core
>>>>> only.
>>>>>
>>>>> Thanks
>>>>
>>>> Hi Leon,
>>>> How would that work?
>>>> Each ULP should decide whether it wants to add the device or not, shouldn't that
>>>> be coded in the ULP?
>>>
>>> Something like that,
>>>
>>> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
>>> index ac011836bb54..b2d307d3c193 100644
>>> --- a/drivers/infiniband/core/verbs.c
>>> +++ b/drivers/infiniband/core/verbs.c
>>> @@ -512,6 +512,13 @@ static struct ib_ah *_rdma_create_ah(struct ib_pd *pd,
>>> return ah;
>>> }
>>>
>>> +static bool is_kverbs_supported(struct ib_device *dev)
>>> +{
>>> + if (unlikely(dev->driver_id == RDMA_DRIVER_USNIC))
>>> + return false;
>>> + return true;
>>> +}
>>> +
>>> /**
>>> * rdma_create_ah - Creates an address handle for the
>>> * given address vector.
>>> @@ -530,6 +537,9 @@ struct ib_ah *rdma_create_ah(struct ib_pd *pd, struct rdma_ah_attr *ah_attr,
>>> struct ib_ah *ah;
>>> int ret;
>>>
>>> + if (!is_kverbs_supported(pd->device))
>>> + return ERR_PTR(-ENOSYS);
>>> +
>>> ret = rdma_fill_sgid_attr(pd->device, ah_attr, &old_sgid_attr);
>>> if (ret)
>>> return ERR_PTR(ret);
>>>
>>
>> Thanks,
>> This approach falls back to the "try and fail" way, why should the client even
>> try to use the device if it's not supported in the first place?
>> In the ULP's perspective create AH fails, does it really matter if the core
>> returns the error or the driver?
>
> It was an example, put is_kverbs_supported() into ib_register_client()
> and it will do the trick.
>
> Thanks
I can do that, it will require an additional check of
"if (client != uverbs)", that should be registered regardless of whether kverbs
are supported or not.
This feels off to me, but it's possible.
