***SPAM*** Re: [ofa-general] Re: [PATCH 3/3 v4] ib/ipoib: blocking multicast loopback ipoib packets

[Ron Livne]

Hi,

What do you think of the following approach?
Instead of adding creation flags to the qp_init_attr, I can add a new verb:
ibv_qp *create_qp_extended(struct ibv_pd *pd, struct ibv_qp_init_attr,
*init_attr, enum ibv_qp_create_flags create_flags)

I'm aware that adding a new verb isn't optimal, but at least we can
avoid incrementing the libibverbs version.

In your opinion, which way is better?

Ron

On Sun, Jul 6, 2008 at 1:00 PM, Jack Morgenstein
<jackm at dev.mellanox.co.il> wrote:
> The ABI for ibv_create_qp (see file libibverbs/include/infiniband/kern-abi.h)
> currently has 1 reserved byte.  If we can use this for create flags, we don't need
> to break the ABI, but clearly note that:
> a. we will have space for only 8 flag bits -- not very many, they seem to be going
>   like hotcakes.
> b. Kernel space create flags are 32 bits -- it may be weird to have userspace flags
>   be only 8 bits.  We've also tried to keep bit offsets between kernel space and
>   userspace the same (e.g., see device capabilities flags) -- and this would
>   clearly make it impossible.
>
> On the other hand, I don't like the idea of adding a new verb for each new tweak.
>
> In the case of xrc receive qp's, new verbs were needed because the parameter list
> for XRC receive qp's was very different (XRC domain handle and QP num, instead of
> qp handle).
>
> In the case multicast loopback, a creation flag is most appropriate, because this
> is really a qp-creation-only issue (the kernel core layer only needs this info at
> QP creation time).
>
> Probably, the least short-term pain (no ABI change) would be to use the last reserved
> create-qp ABI byte for flags, while defining the flags field in
> ibv_qp_init_attr as uint32_t (preparing for more than 8 future flags).
>
> We would not have the same flag bits in user as in kernel, and if we at some point
> needed more that 8 user-space qp create flags, we would increment the ABI at that point,
> keep the originally defined 8 flags as they were, and just add a _u32 with additional flags.
>
> Adding the flags field to the qp_init_attr structure does, however, require incrementing the
> libibverbs version to 1.2, necessitating an IBVERBS_1.1 compatibility layer.
>
> - Jack
>
> On Friday 04 July 2008 01:04, Or Gerlitz wrote:
>> On 7/4/08, Roland Dreier <rdreier at cisco.com> wrote:
>>
>> > Sorry, I don't have a magic solution for you.  To minimize pain about
>> >  ABI I think the only thing to do is minimize ABI changes.
>>
>> fair-enough, so lets just try to take ABI changes to minimum...
>>
>> > If a QP creation flag works for XRC then I would prefer not to add more
>> >  QP types I guess.  One of the ways that the QP creation flags were
>> >  originally sold to me was that XRC needed them anyway.  So I'm a bit
>> >  confused about where things have ended up.
>>
>> I can't take this, we think that creation flags can be a good way to
>> go wrt minimizing ABI changes, I am not enough into the XRC impl to
>> tell why creation flags are not used there.
>>
>>
>> > I don't want to add both an XRC-specific mechanism and a more general
>> >  mechanism that XRC could have used but doesn't.  So yes the "framework
>> >  Z" approach that works for both seems like the best way forward.
>>
>> OK, so do we agree that ABI wise one thing to handle is the new verb/s
>> plus their associated data structures (in our case just create_flags
>> field added to qp_init_attr) and second thing is placing the new verbs
>> into the verbs context structure?
>>
>> Lets see what Jack says, we are fine with anything (yes) he would suggest.
>>
>> Or.
>>
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
>