On Fri, Jul 13, 2018 at 10:18:10AM -0600, Jason Gunthorpe wrote:
> On Fri, Jul 13, 2018 at 01:26:29PM +0300, Kamal Heib wrote:
> > Make sure the providers implement the verbs callbacks before calling
> > them, otherwise return -EOPNOTSUPP.
> >
> > Signed-off-by: Kamal Heib <kamalheib1@xxxxxxxxx>
> > drivers/infiniband/core/mad.c | 10 ++++++----
> > drivers/infiniband/core/uverbs_cmd.c | 3 ++-
> > drivers/infiniband/core/verbs.c | 6 ++++++
> > 3 files changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
> > index 34e9b2768324..e528387605c8 100644
> > +++ b/drivers/infiniband/core/mad.c
> > @@ -882,10 +882,12 @@ static int handle_outgoing_dr_smp(struct ib_mad_agent_private *mad_agent_priv,
> > }
> >
> > /* No GRH for DR SMP */
> > - ret = device->process_mad(device, 0, port_num, &mad_wc, NULL,
> > - (const struct ib_mad_hdr *)smp, mad_size,
> > - (struct ib_mad_hdr *)mad_priv->mad,
> > - &mad_size, &out_mad_pkey_index);
> > + ret = device->process_mad ?
> > + device->process_mad(device, 0, port_num, &mad_wc, NULL,
> > + (const struct ib_mad_hdr *)smp, mad_size,
> > + (struct ib_mad_hdr *)mad_priv->mad,
> > + &mad_size, &out_mad_pkey_index) :
> > + -EOPNOTSUPP;
> > switch (ret)
> > {
> > case IB_MAD_RESULT_SUCCESS | IB_MAD_RESULT_REPLY:
> > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> > index bd6eefaecbd6..6ab00f322b62 100644
> > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > @@ -2507,7 +2507,8 @@ ssize_t ib_uverbs_post_srq_recv(struct ib_uverbs_file *file,
> > goto out;
> >
> > resp.bad_wr = 0;
> > - ret = srq->device->post_srq_recv(srq, wr, &bad_wr);
> > + ret = srq->device->post_srq_recv ?
> > + srq->device->post_srq_recv(srq, wr, &bad_wr) : -EOPNOTSUPP;
> > uobj_put_obj_read(srq);
> >
> > diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> > index b6ceb6fd6a67..03b8c5448303 100644
> > +++ b/drivers/infiniband/core/verbs.c
> > @@ -479,6 +479,9 @@ static struct ib_ah *_rdma_create_ah(struct ib_pd *pd,
> > {
> > struct ib_ah *ah;
> >
> > + if (!pd->device->create_ah)
> > + return ERR_PTR(-EOPNOTSUPP);
>
> > +
> > ah = pd->device->create_ah(pd, ah_attr, udata);
> >
> > if (!IS_ERR(ah)) {
> > @@ -911,6 +914,9 @@ int rdma_destroy_ah(struct ib_ah *ah)
> > struct ib_pd *pd;
> > int ret;
> >
> > + if (!ah->device->destroy_ah)
> > + return -EOPNOTSUPP;
>
> Instead of this, put a WARN_ON in ib_device_register like
>
> WARN_ON(ibdev->create_ah && !ibdev->destroy_ah)
>
> And don't test here.. We don't need to cope with broken drivers.
>
> Jason
I agree, the test shouldn't be in rdma_destroy_ah(), I'll remove it in V2.
After doing more research I found that within ib_device_register() there
is call for ib_device_check_mandatory() which will fail if {create,
destroy}_ah() aren't implemented by the providers. I'll add another
patch to this patch set that will remove this check and avoid
ib_device_register() fail.
With regards to adding the WARN_ON, I suggest to create a separate patch
that will handle all the {create, destroy}_*() verbs.
Thanks,
Kamal
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
