Re: [PATCH for-next] RDMA/vmw_pvrdma: Add shared receive queue support

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1928 bytes --]

On Thu, Nov 02, 2017 at 11:23:25AM -0700, Bryan Tan wrote:
> On Thu, Nov 02, 2017 at 08:02:47PM +0200, Leon Romanovsky wrote:
> > > Here we assumed that the verbs layer will only call down to us to
> > > destroy a valid SRQ. If an SRQ has been created, then the srq_tbl
> > > should exist. I can add a check here if that's not behaviour we
> > > should rely on.
> >
> > By saying "verbs layer", are you referring to libibverbs or IB/core?
> >
>
> IB/core. Looking at core/verbs.c, it seems like the parameters are
> assumed to be good. I'm unclear on who is responsible for ensuring
> or checking for good inputs, kernel clients, IB/core, or the driver.

The first one who can recognize the wrong parameters - fail early.
So if IB/core provides to you clean input, there is no need to check it
again.

>
> > > > > +int pvrdma_modify_srq(struct ib_srq *ibsrq, struct ib_srq_attr *attr,
> > > > > +		      enum ib_srq_attr_mask attr_mask, struct ib_udata *udata)
> > > > > +{
> > > > > +	struct pvrdma_srq *vsrq = to_vsrq(ibsrq);
> > > > > +	union pvrdma_cmd_req req;
> > > > > +	struct pvrdma_cmd_modify_srq *cmd = &req.modify_srq;
> > > > > +	struct pvrdma_dev *dev = to_vdev(ibsrq->device);
> > > > > +	int ret;
> > > > > +
> > > > > +	/* No support for resizing SRQs. */
> > > >
> > > > Better to write that you support, so future additions to ib_srq_attr_mask
> > > > won't need to change your code.
> > > >
> > >
> > > I'm a little unclear on what you mean here. Do you mean that we
> > > should explicitly check for the only attribute we support
> > > (IB_SRQ_LIMIT) and fail otherwise?
> >
> > Yes, it will ensure that your driver doesn't get untested input.
> >
>
> Sure, I will do that then.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]