[ewg] Re: [PATCH] rdmaoe/libibverbs: handle binary compatibility

[Eli Cohen]

On Thu, Dec 10, 2009 at 02:49:45PM -0700, Jason Gunthorpe wrote:
> 
> - Change the library API for ibv_port_attr to include a link_layer
> - Change the kernel API to retrieve link_layer
> - Add ibv_cmd_get_mac and other stuff to support RDMAoE
> - Update verbs examples to support RDMAoE
> - Update man pages (you missed these)
> 

Will be addressed in the next patch set.
> >--- a/include/infiniband/kern-abi.h
> >+++ b/include/infiniband/kern-abi.h
> >@@ -46,7 +46,7 @@
> >  * The minimum and maximum kernel ABI that we can handle.
> >  */
> >  #define IB_USER_VERBS_MIN_ABI_VERSION  1
> > -#define IB_USER_VERBS_MAX_ABI_VERSION  6
> > +#define IB_USER_VERBS_MAX_ABI_VERSION  7
>  
> Whats this about? That seems like it needs a much bigger review,
> changing the kernel ABI version instantly breaks every existing
> libibverbs, shouldn't be done without alot of discussion!!
I think we can do without chagning the ABI version so I am going to
ommit it in the next patch set.

> 
> Extra include?

Yes, thanks.


> 
> >@@ -86,6 +86,7 @@ default_symver(__ibv_query_device, ibv_query_device);
> > int __ibv_query_port(struct ibv_context *context, uint8_t port_num,
> >                     struct ibv_port_attr *port_attr)
> > {
> >+       port_attr->link_layer = IBV_LINK_LAYER_UNSPECIFIED;
> >        return context->ops.query_port(context, port_num, port_attr);
> > }
> 
> Seems like this should be just
>  return ___ibv_query_port(context,port_num,port_attr);
> 

A leftover, thanks.

> > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> > index 012aadf..d592bd2 100644
> > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > @@ -452,7 +452,8 @@ ssize_t ib_uverbs_query_port(struct ib_uverbs_file *file,
> >  	resp.active_width    = attr.active_width;
> >  	resp.active_speed    = attr.active_speed;
> >  	resp.phys_state      = attr.phys_state;
> > -	resp.transport	     = attr.transport;
> > +	resp.transport	     = attr.transport == RDMA_TRANSPORT_RDMAOE ?
> > +		IB_LINK_LAYER_ETHERNET : IB_LINK_LAYER_INFINIBAND;
> 
> Are you going to change the kernel patches to use the new link_layer
> name?
> 

Yes, in the next patch set.