Re: [PATCH rdma-core 4/5] verbs: Add an option to provide vendor private data when creating a CQ

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Wed, Mar 08, 2017 at 12:10:18AM +0200, Yishai Hadas wrote:

> >>1) From developer point of view it's fully preferred to use single API (e.g.
> >>ibv_create_cq_ex) otherwise developer should choose, do I use
> >
> >Why? That isn't right, from a developer point of view it is better to
> >have strong type safety so the API is obvious how to use correctly.
> 
> Having 2 APIs for similar purpose usually doesn't make sense and might bring
> confusion and code duplication. The mlx5dv_create_cq() from application
> point of view can be used to create a regular CQ when no vendor data will be
> supplied, the idea is to have a single API for clarity.

It is very common to have 'basic' and 'complex' API entry points. I
don't see a problem here.

> Once the correct structure is used there should be *no* safety issue, please
> note that the vendor data defined to be as some extended structure which can
> only be grown based on comp_mask so that compatibility will be saved in any
> case.

No, you still have to make sure that your ib_device is somehow a mlx5.

mlx5dv_create_cq can directly verify this on entry, but ibv_create_cq
will assume the vendor data is for the device that is bound - this is
why it is inherently unsafe and unfriendly to discard the type
information via the void *.

> >This makes it very clear to the user that they are touching a
> >non-portable API and they need to plan accordingly.
> 
> This is correct for using DV APIs which are fully driver specific but its
> not a must to add an API which is quite similar to the ibv_xxx one.

When I suggested this approach at Plumbers, I explained that all
create methods should be the vendor specific call, and that they should
return a generic object that could be used with the other standard API
entry points.

The idea you want to pollute many of the existing common APIs with a
vendor_data argument is ugly as sin, the entire *point* of this
approach is to keep all that uglyness in *your* dv driver and make it
*very clear* to users that they are outside the standard and cannot
export portability when they touch this stuff.

I don't find your arguments against this idea very compelling..

> >We have caused ourselves alot of pain by trying to be too clever with
> >function pointers in the past. That totally defeats the usual symver
> >mechanism for compatability and should *NOT* be done by this dv stuff.
> 
> That's why we want to prevent from new APIs when really not required.

Symbol versions are well understood, we should use them sanely when
possible.

Jason
--
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