On Wed, Jan 02, 2019 at 03:43:13PM +0200, Gal Pressman wrote:
> 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.
introduce ib_register_client_nocheck() and use it for uverbs.
Anyway, it is better to close main thing raised in cover letter, before rushing
and doing extra version.
Thanks
Attachment:
signature.asc
Description: PGP signature
