Re: [PATCH libibverbs V3 3/3] Add ibv_query_port_ex support

Roland Dreier
2014-05-09T14:01:36Z

On Thu, May 8, 2014 at 12:09 PM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
>> struct verbs_context {
>> /* "grows up" - new fields go here */
>> + int (*drv_query_port_ex)(struct ibv_context *context, uint8_t port_num,
>> + struct ibv_port_attr_ex *port_attr);
>> + int (*lib_query_port_ex)(struct ibv_context *context, uint8_t port_num,
>> + struct ibv_port_attr_ex *port_attr);
>> struct ibv_ah * (*drv_ibv_create_ah_ex)(struct ibv_pd *pd,
>> struct ibv_ah_attr_ex *attr);
>> int (*drv_ibv_destroy_flow) (struct ibv_flow *flow);
>
> I'm not sure what Roland decided to merge, but I am surprised to see
> drv_ elements here. That was nix'd in the round of review of the first
> patch set. eg create_qp_ex/etc don't have them.
>
> The flow is such that the verbs code has a chance to capture and
> override the function pointer after the driver sets it, there is no
> purpose to the drv_ values.
>
> I wouldn't like to see more added, and I think you should make a patch
> to ensure they are not necessary before it propogates too far.

Sorry, I was not aware of the feeling on drv_XXX members here and I
don't think I saw any review comments about them. (Maybe they got
lost in the flood of "merge it merge it merge it" emails)

Anyway I'm wondering if there's a way to recover without breaking ABI
here (or by breaking ABI in a manageable way). The only library using
this stuff (that I know of) is libmlx4, maybe we can spin quick
releases of both and pretend libibverbs 1.1.8 never happened?