[ewg] [PATCH 1/2 v2]libibvers: add create_qp_expanded

[Jack Morgenstein]

On Tuesday 12 August 2008 22:37, Roland Dreier wrote:
> Sorry for jumping in so late in the process, but a few big concerns:
> 
>  >         struct ibv_qp *ibv_create_qp_expanded(struct ibv_pd *pd,
>  >                                struct ibv_qp_init_attr *qp_init_attr,
>  >                                uint32_t create_flags);
> 
> I don't like the name "_expanded" when all we are doing is adding a
> flags parameter.  
We'll need to implement any changes here very quickly if this is to get
into the beta release of ofed 1.4 (we do not want to put an API into the
beta, and then change it immediately).

I suggested "expanded" because I was afraid of "extended" (possible
confusion with "extended" RC QPs)
> The next time we need to tweak this API, then we end 
> up with _extra_super_expanded or something like that.
The names can still be changed -- we've not yet put out the API for this feature
in userspace.

> or keep
> the name ibv_create_qp_expanded but instead of create_flags, have the
> new parameter be ext_mask, have one bit in ext_mask indicate create
> flags, and add create_flags to struct ibv_qp_init_attr -- then we can
> add more extra stuff by using more bits in ext_mask.

This may be doable.  We can define the ABI for the new verb using a new
struct ibv_qp_init_attr_expanded, with lots of reserved space up front 
(for future extensions -- note that the ABI unpacks the fields in 
struct ibv_create_qp_attr, passing values individually. Since this is not
data path, who cares if we have, say, 12 reserved DWORDS in the structure)?
If userspace sets an extension flag that the kernel does not support, the create_qp
verb in kernel space should return error.

We need the new new structure so that we can maintain backwards compatibility for the old ABI.
(new libraries running over an old kernel).

> 
> Also, I wonder if it's worth a new verb in the kernel ABI for this.
> Maybe we should add a new command in the ABI where libibverbs can pass
> in a bitmask of supported extensions, and the kernel can respond with
> which extensions it supports.  And then we can just continue to use the
> reserved field in the existing create_qp command if both kernel and
> userspace agree that they support create flags there.

I think there is a problem here, because the reserved field only has 8 bits --
we may run out of create flags quickly.  Then, we will have the problem of
still using the reserved field for the old create flags, but needing a new
structure for new create flags.

I think it is better to take the "ext_mask" approach.

- Jack