[ewg] libibumad.so version bump in OFED 1.5.2

[Todd Rimmer]

Is there a way to accomplish this feature without an ABI change?  Or should ABI altering features be only included in Major releases (such as OFED 1.6?)

Keep in mind, that ABI changes mean that 3rd parties must rebuild and re-release their applications.  While this isn't so bad for OFED itself and IB vendors, it's a major issue for other 3rd party apps.  Introducing a ABI change in a maintenance release just makes things that much more confusing.

One alternative would be for OFED 1.5.2 to include both the .2 and .3 versions of the library.

Another alternative would be to design the structures so there is room for expansion or pointers to opaque datatypes for fields only used by the library.

Todd Rimmer
Chief Architect 
QLogic Network Systems Group
Voice: 610-233-4852     Fax: 610-233-4777
Todd.Rimmer at QLogic.com  www.QLogic.com
 

> -----Original Message-----
> From: Sasha Khapyorsky [mailto:sashakvolt at gmail.com] On Behalf Of Sasha
> Khapyorsky
> Sent: Wednesday, July 07, 2010 9:34 AM
> To: Todd Rimmer
> Cc: ewg at openfabrics.org
> Subject: Re: libibumad.so version bump in OFED 1.5.2
> 
> Hi Todd,
> 
> On 06:48 Tue 06 Jul     , Todd Rimmer wrote:
> > I recently discovered that 3rd party applications built for OFED 1.5
> will not run on OFED 1.5.2rc2.
> >
> > The issue I found was that libibumad had changed from version 2 to
> version 3.  Can you provide insight as to why this change was made?
> >
> > In reviewing the source code and change logs there do not seem to be
> any changes which would impact the application API nor application ABI.
> >
> >
> >
> > I diffed the code and the main change seems to be:
> > diff -r --exclude CVS ./src/src/umad.c-orig ./src/src/umad.c
> > 161a162,166
> > >   if (sys_read_string(port_dir, SYS_PORT_LINK_LAYER,
> > >       port->link_layer, UMAD_CA_NAME_LEN) < 0)
> > >       /* assume IB by default */
> > >       sprintf(port->link_layer, "IB");
> > >
> > The above change might imply an update to an rpm dependency on ib-
> kernel versions, but should not result in a new library version.  If
> the library version remained at 2, then applications built for OFED 1.5
> would also run on OFED 1.5.2rc2 without recompile.
> 
> This is not an only change. The whole commit is:
> 
> 
> commit 99d0901f3702e22c178352524af18ec567781d51
> Author: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
> Date:   Tue Mar 9 12:39:40 2010 +0200
> 
>     libibumad: added link_layer for RoCEE support and updated umad
> version
> 
>     Added link_layer field to umad_port_t.
>     The field is implemented as char[].
>     If the relevant file doesn't exist in sysfs, the link layer
>     is "IB". Otherwise, the content of link_layer file is used.
> 
>     The libibumad version is promoted.
> 
>     Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
>     Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> 
> diff --git a/libibumad/include/infiniband/umad.h
> b/libibumad/include/infiniband/umad.h
> index 1f82183..f9da204 100644
> --- a/libibumad/include/infiniband/umad.h
> +++ b/libibumad/include/infiniband/umad.h
> @@ -116,6 +116,7 @@ typedef struct ib_user_mad {
>  #define SYS_PORT_RATE		"rate"
>  #define SYS_PORT_GUID		"port_guid"
>  #define SYS_PORT_GID		"gids/0"
> +#define SYS_PORT_LINK_LAYER	"link_layer"
> 
>  typedef struct umad_port {
>  	char ca_name[UMAD_CA_NAME_LEN];
> @@ -132,6 +133,7 @@ typedef struct umad_port {
>  	uint64_t port_guid;
>  	unsigned pkeys_size;
>  	uint16_t *pkeys;
> +	char link_layer[UMAD_CA_NAME_LEN];
>  } umad_port_t;
> 
>  typedef struct umad_ca {
> diff --git a/libibumad/libibumad.ver b/libibumad/libibumad.ver
> index 57cddbd..d36fbbc 100644
> --- a/libibumad/libibumad.ver
> +++ b/libibumad/libibumad.ver
> @@ -6,4 +6,4 @@
>  # API_REV - advance on any added API
>  # RUNNING_REV - advance any change to the vendor files
>  # AGE - number of backward versions the API still supports
> -LIBVERSION=2:1:0
> +LIBVERSION=2:2:0
> diff --git a/libibumad/src/umad.c b/libibumad/src/umad.c
> index 277ae6b..d16e750 100644
> --- a/libibumad/src/umad.c
> +++ b/libibumad/src/umad.c
> @@ -159,6 +159,11 @@ static int get_port(char *ca_name, char *dir, int
> portnum, umad_port_t * port)
>  	if (sys_read_uint(port_dir, SYS_PORT_CAPMASK, &port->capmask) <
> 0)
>  		goto clean;
> 
> +	if (sys_read_string(port_dir, SYS_PORT_LINK_LAYER,
> +	    port->link_layer, UMAD_CA_NAME_LEN) < 0)
> +		/* assume IB by default */
> +		sprintf(port->link_layer, "IB");
> +
>  	port->capmask = htonl(port->capmask);
> 
>  	if (sys_read_gid(port_dir, SYS_PORT_GID, gid) < 0)
> 
> 
> Altering 'struct umad_port' effectively breaks ABI.
> 
> Sasha