Re: [PATCH rdma-next v1 1/6] IB/uverbs: Allow CQ moderation with modify CQ

Jason Gunthorpe
2017-10-31T15:40:46Z

On Tue, Oct 31, 2017 at 01:31:28PM +0200, Yishai Hadas wrote:
> On 10/31/2017 7:08 AM, Leon Romanovsky wrote:
> >On Mon, Oct 30, 2017 at 05:07:53PM -0600, Jason Gunthorpe wrote:
> >>On Mon, Oct 30, 2017 at 09:09:52PM +0200, Leon Romanovsky wrote:
> >>
> >>>However, I don't think that kernel to libibverbs API should follow the
> >>>same path. The centralized entry points to the kernel provides better
> >>>enforcement and minimizes system call bloat.
> >>
> >>How so? I didn't think selinux intersected with the CQs at all..
> >>I've never worried about the # of verbs entries, we have so many
> >>already.
> >
> >Not SELinux enforcement, but various copy_{to|from}_user checks, unified
> >return values, easy folding in case of errors, e.t.c.
> >
> >It is a matter of personal view and less technical thing. You prefer bazillion
> >entry points to the kernel and I think that such design good for user-space only,
> >and mostly not applicable for kernel<->user interfaces.
>
> Even for the user area having a dedicated 'setter' per field might bring
> some redundant overhead and limitation.

As a general rule, more simpler apis are saner than single complex
apis. It is much easier to understand and document, the error codes
work more sanely for debugging and generally needs less 'discovery'
support.

> This will prevent the option from application to modify few fields
> in one API and resulted in a system call per field.

And this is always the argument someone brings up - and as I said, I
don't see how it really applies to the CQ where we don't expect to
create them at a high rate.

> Also libibverbs and user drivers may need to maintain some wrapping
> over the kernel command which in turn might lead to some
> extra/duplication of code. I would vote to follow modify QP/SRQ/WQ
> by having one API with an 'attr_mask' field as this series has
> introduced in both kernel and user.

Yes, I'm not super excited about having them be different..

Jason