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
>
> >>
> >>>
> >>>>
> >>>> No functional changes were introduced in this patch, all devices are
> >>>> marked as kernel providers. Downstream patch will adjust relevant
> >>>> drivers.
> >>
Attachment:
signature.asc
Description: PGP signature
