> -----Original Message-----
> From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma-
> owner@xxxxxxxxxxxxxxx> On Behalf Of Leon Romanovsky
> Sent: Wednesday, January 2, 2019 7:24 PM
> To: Gal Pressman <galpress@xxxxxxxxxx>
> Cc: Jason Gunthorpe <jgg@xxxxxxxx>; Doug Ledford <dledford@xxxxxxxxxx>;
> linux-rdma@xxxxxxxxxxxxxxx; Alexander Matushevsky
> <matua@xxxxxxxxxx>; Yossi Leybovich <sleybo@xxxxxxxxxx>; Dave
> Goodell <goodell@xxxxxxxxxx>; Brian Barrett <bbarrett@xxxxxxxxxx>;
> Leah Shalev <shalevl@xxxxxxxxxx>; Sean Hefty <sean.hefty@xxxxxxxxx>
> Subject: Re: [PATCH RFC 1/2] RDMA: Add indication for in kernel API support
> to IB device
>
> 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.
Not to miss out the need of an additional link layer type.
