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

[Jack Morgenstein]

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