Re: [PATCH libibverbs] Fix create/destroy flow API

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

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

On 09/04/2015 05:51 PM, Doug Ledford wrote:
> On 09/04/2015 05:32 PM, Jason Gunthorpe wrote:
>> On Fri, Sep 04, 2015 at 05:17:38PM -0400, Doug Ledford wrote:
>>> +		/*
>>> +		 * In order to maintain backward/forward binary compatibility
>>> +		 * with libmlx4-1.0.6, which has the original version of the
>>> +		 * flow steering patches, we need to set the two
>>> +		 * ABI_compat_placeholder entries to match the driver
>>> +		 * set flow entries.  This is because, in the specific instance
>>> +		 * of using libmlx4-1.0.6 with the fixed version of
>>> +		 * libibvberbs, the ibv_create_flow inline function already
>>> +		 * compiled into libmlx4-1.0.6 will be loooking in the
>>> +		 * ABI_placeholder spots for the function pointer to the
>>> +		 * create and destroy flow verbs.
>>>  		 */
>>
>> This isn't quite the right comment, it has very little to do with mlx,
>> ibv_create_flow is the user entry point, the above applies to
>> everything linked to ibverbs.
> 
> You're right.  I'll correct that.

The new cokment:

                /*
                 * In order to maintain backward/forward binary
compatibility
                 * with apps compiled against libibverbs-1.1.8 that use the
                 * flow steering addition, we need to set the two
                 * ABI_placeholder entries to match the driver set flow
                 * entries.  This is because apps compiled against
                 * libibverbs-1.1.8 use an inline ibv_create_flow and
                 * ibv_destroy_flow function that looks in the placeholder
                 * spots for the proper entry points.  For apps compiled
                 * against libibverbs-1.1.9 and later, the inline functions
                 * will be looking in the right place.
                 */


>> My suggestion was to not change the ibverbs->user ABI at all and just
>> mangle the driver side, ie move the ABI_placeholder to what was drv_
>> instead of lib_.
> 
> As you pointed out, this doesn't make the entire matrix of old/new
> driver/libibverbs/app work.  I can think of two things that would be
> ugly about doing it this way if we wanted to make that matrix fully
> operational.  With the way I did it, things are clean in the new driver
> and mostly clean in the new libibverbs, and the full matrix works.
> 
>> Can't see anything wrong with it this way, off hand, other than the
>> comment.
> 
> Thanks for looking at it ;-)
> 


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]