[ofw] Re: [ofa-general] [PATCH 1/3 v2] libibmad. 2nd revision for WinOF portablity.

Sasha Khapyorsky sashak at voltaire.com
Tue Jan 20 09:43:13 PST 2009


Hi Arlin,

On 11:50 Sat 17 Jan     , Arlin Davis wrote:
> 
> 
> Ok, here is revision 2 of the libibmad WinOF portability patches. I eliminated all #ifdef _WIN32
> crap and tried to limit the changes by adding os dependent file mad_osd.h. With these changes we
> could share the same code base for both OFED and WinOF. Please review and consider accepting this
> patch set. 

The patches (I tried to check only 1 and 2) are malformed - whitespaces
are mangled (in 2), also long lines are broken. Please check that it is
appliable with 'git am'.

> 
> [PATCH 1/3] libibmad: add os dependent definitions.
> [PATCH 2/3] field.c remove c99 definitions, better portability with WinOF.

Is it possible to preserve c99 stuff? It improves code maintainability
a lot and as far as I remember it is what was considered in previous
discussions (about infiniband-diags porting to WinOF).

> [PATCH 3/3] Minor changes to allow portability to WinOF
> 
> infiniband/mad_osd.h added to provide support for os specific defintions
> for portability. With these changes, WinOF can pull directly from OFED
> git tree and share a common code base with minimal changes to mad.h and 
> source tree.
> 
> mad.h modifications include MAD_EXPORT for export declarations where
> appropriate. Datatype llu changed to ULL for 64bit constants.
> 
> makefile.am modified to include new linux version of mad_osd.h
> 
> Signed-off-by: Arlin Davis <ardavis at ichips.intel.com>
> ---
>  libibmad/Makefile.am                  |    7 +-
>  libibmad/include/infiniband/mad.h     |  120 ++++++++++++++++-----------------
>  libibmad/include/infiniband/mad_osd.h |   48 +++++++++++++
>  3 files changed, 109 insertions(+), 66 deletions(-)
>  create mode 100644 libibmad/include/infiniband/mad_osd.h
> 
> diff --git a/libibmad/Makefile.am b/libibmad/Makefile.am
> index beae1a4..8dea157 100644
> --- a/libibmad/Makefile.am
> +++ b/libibmad/Makefile.am
> @@ -1,7 +1,7 @@
>  
>  SUBDIRS = .
>  
> -INCLUDES = -I$(srcdir)/include/infiniband -I$(includedir)
> +INCLUDES = -I$(srcdir)/include -I$(srcdir)/include/infiniband -I$(includedir)

Something not related to porting...

I would rather replace all '#include <mad.h>' occurrences to
'#include <infiniband/mad.h>' and then use '-I$(srcdir)/include' in
INCLUDES definition.

>  
>  lib_LTLIBRARIES = libibmad.la
>  
> @@ -23,9 +23,10 @@ libibmad_la_DEPENDENCIES = $(srcdir)/src/libibmad.map
>  
>  libibmadincludedir = $(includedir)/infiniband
>  
> -libibmadinclude_HEADERS = $(srcdir)/include/infiniband/mad.h
> +libibmadinclude_HEADERS = $(srcdir)/include/infiniband/mad.h $(srcdir)/include/infiniband/mad_osd.h
>  
> -EXTRA_DIST = $(srcdir)/include/infiniband/mad.h libibmad.spec.in libibmad.spec \
> +EXTRA_DIST = $(srcdir)/include/infiniband/mad.h $(srcdir)/include/infiniband/mad_osd.h \
> +	libibmad.spec.in libibmad.spec \
>  	$(srcdir)/src/libibmad.map libibmad.ver autogen.sh

And if file is listed in library_HEADERS it will be distributed, so no
need to list it in EXTRA_DIST (of course mad.h should be remove too).

>  
>  dist-hook:
> diff --git a/libibmad/include/infiniband/mad.h b/libibmad/include/infiniband/mad.h
> index 0a962c0..fe607a7 100644
> --- a/libibmad/include/infiniband/mad.h
> +++ b/libibmad/include/infiniband/mad.h
> @@ -33,13 +33,7 @@
>  #ifndef _MAD_H_
>  #define _MAD_H_
>  
> -#include <stdint.h>
> -#include <string.h>
> -#include <stdlib.h>
> -#include <stdio.h>
> -#include <sys/types.h>
> -#include <unistd.h>
> -#include <byteswap.h>
> +#include <infiniband/mad_osd.h>

Why should we remove all header files here? Some of them (such as
stdio.h) are not really system dependent.

Sasha

>  
>  #ifdef __cplusplus
>  #  define BEGIN_C_DECLS extern "C" {
> @@ -52,7 +46,7 @@
>  BEGIN_C_DECLS
>  
>  #define IB_SUBNET_PATH_HOPS_MAX	64
> -#define IB_DEFAULT_SUBN_PREFIX	0xfe80000000000000llu
> +#define IB_DEFAULT_SUBN_PREFIX	0xfe80000000000000ULL
>  #define IB_DEFAULT_QP1_QKEY	0x80010000
>  
>  #define IB_MAD_SIZE		256
> @@ -627,10 +621,10 @@ enum {
>  /******************************************************************************/
>  
>  /* portid.c */
> -char *	portid2str(ib_portid_t *portid);
> -int	portid2portnum(ib_portid_t *portid);
> -int	str2drpath(ib_dr_path_t *path, char *routepath, int drslid, int drdlid);
> -char *  drpath2str(ib_dr_path_t *path, char *dstr, size_t dstr_size);
> +MAD_EXPORT char * portid2str(ib_portid_t *portid);
> +MAD_EXPORT int	portid2portnum(ib_portid_t *portid);
> +MAD_EXPORT int	str2drpath(ib_dr_path_t *path, char *routepath, int drslid, int drdlid);
> +MAD_EXPORT char * drpath2str(ib_dr_path_t *path, char *dstr, size_t dstr_size);
>  
>  static inline int
>  ib_portid_set(ib_portid_t *portid, int lid, int qp, int qkey)
> @@ -644,44 +638,44 @@ ib_portid_set(ib_portid_t *portid, int lid, int qp, int qkey)
>  }
>  
>  /* fields.c */
> -uint32_t mad_get_field(void *buf, int base_offs, int field);
> -void mad_set_field(void *buf, int base_offs, int field, uint32_t val);
> +MAD_EXPORT uint32_t mad_get_field(void *buf, int base_offs, int field);
> +MAD_EXPORT void mad_set_field(void *buf, int base_offs, int field, uint32_t val);
>  /* field must be byte aligned */
> -uint64_t mad_get_field64(void *buf, int base_offs, int field);
> -void mad_set_field64(void *buf, int base_offs, int field, uint64_t val);
> -void mad_set_array(void *buf, int base_offs, int field, void *val);
> -void mad_get_array(void *buf, int base_offs, int field, void *val);
> -void mad_decode_field(uint8_t *buf, int field, void *val);
> -void mad_encode_field(uint8_t *buf, int field, void *val);
> -int mad_print_field(int field, const char *name, void *val);
> -char *mad_dump_field(int field, char *buf, int bufsz, void *val);
> -char *mad_dump_val(int field, char *buf, int bufsz, void *val);
> +MAD_EXPORT uint64_t mad_get_field64(void *buf, int base_offs, int field);
> +MAD_EXPORT void mad_set_field64(void *buf, int base_offs, int field, uint64_t val);
> +MAD_EXPORT void mad_set_array(void *buf, int base_offs, int field, void *val);
> +MAD_EXPORT void mad_get_array(void *buf, int base_offs, int field, void *val);
> +MAD_EXPORT void mad_decode_field(uint8_t *buf, int field, void *val);
> +MAD_EXPORT void mad_encode_field(uint8_t *buf, int field, void *val);
> +MAD_EXPORT int mad_print_field(int field, const char *name, void *val);
> +MAD_EXPORT char *mad_dump_field(int field, char *buf, int bufsz, void *val);
> +MAD_EXPORT char *mad_dump_val(int field, char *buf, int bufsz, void *val);
>  
>  /* mad.c */
> -void *mad_encode(void *buf, ib_rpc_t *rpc, ib_dr_path_t *drpath, void *data);
> -uint64_t mad_trid(void);
> -int mad_build_pkt(void *umad, ib_rpc_t *rpc, ib_portid_t *dport, ib_rmpp_hdr_t *rmpp, void *data);
> +MAD_EXPORT void *mad_encode(void *buf, ib_rpc_t *rpc, ib_dr_path_t *drpath, void *data);
> +MAD_EXPORT uint64_t mad_trid(void);
> +MAD_EXPORT int mad_build_pkt(void *umad, ib_rpc_t *rpc, ib_portid_t *dport, ib_rmpp_hdr_t *rmpp,
> void *data);
>  
>  /* register.c */
> -int	mad_register_port_client(int port_id, int mgmt, uint8_t rmpp_version);
> -int	mad_register_client(int mgmt, uint8_t rmpp_version);
> -int	mad_register_server(int mgmt, uint8_t rmpp_version,
> +MAD_EXPORT int	mad_register_port_client(int port_id, int mgmt, uint8_t rmpp_version);
> +MAD_EXPORT int	mad_register_client(int mgmt, uint8_t rmpp_version);
> +MAD_EXPORT int	mad_register_server(int mgmt, uint8_t rmpp_version,
>  			    long method_mask[16/sizeof(long)],
>  			    uint32_t class_oui);
> -int	mad_class_agent(int mgmt);
> -int	mad_agent_class(int agent);
> +MAD_EXPORT int	mad_class_agent(int mgmt);
> +MAD_EXPORT int	mad_agent_class(int agent);
>  
>  /* serv.c */
> -int	mad_send(ib_rpc_t *rpc, ib_portid_t *dport, ib_rmpp_hdr_t *rmpp,
> +MAD_EXPORT int	mad_send(ib_rpc_t *rpc, ib_portid_t *dport, ib_rmpp_hdr_t *rmpp,
>  		 void *data);
> -void *	mad_receive(void *umad, int timeout);
> -int	mad_respond(void *umad, ib_portid_t *portid, uint32_t rstatus);
> -void *	mad_alloc(void);
> -void	mad_free(void *umad);
> +MAD_EXPORT void * mad_receive(void *umad, int timeout);
> +MAD_EXPORT int	mad_respond(void *umad, ib_portid_t *portid, uint32_t rstatus);
> +MAD_EXPORT void * mad_alloc(void);
> +MAD_EXPORT void	mad_free(void *umad);
>  
>  /* vendor.c */
> -uint8_t *ib_vendor_call(void *data, ib_portid_t *portid,
> -			ib_vendor_call_t *call);
> +MAD_EXPORT uint8_t *ib_vendor_call(void *data, ib_portid_t *portid,
> +				   ib_vendor_call_t *call);
>  
>  static inline int
>  mad_is_vendor_range1(int mgmt)
> @@ -696,29 +690,29 @@ mad_is_vendor_range2(int mgmt)
>  }
>  
>  /* rpc.c */
> -int	madrpc_portid(void);
> -int	madrpc_set_retries(int retries);
> -int	madrpc_set_timeout(int timeout);
> -void *	madrpc(ib_rpc_t *rpc, ib_portid_t *dport, void *payload, void *rcvdata);
> -void *  madrpc_rmpp(ib_rpc_t *rpc, ib_portid_t *dport, ib_rmpp_hdr_t *rmpp,
> +MAD_EXPORT int	madrpc_portid(void);
> +MAD_EXPORT int	madrpc_set_retries(int retries);
> +MAD_EXPORT int	madrpc_set_timeout(int timeout);
> +void * madrpc(ib_rpc_t *rpc, ib_portid_t *dport, void *payload, void *rcvdata);
> +void * madrpc_rmpp(ib_rpc_t *rpc, ib_portid_t *dport, ib_rmpp_hdr_t *rmpp,
>  		    void *data);
> -void	madrpc_init(char *dev_name, int dev_port, int *mgmt_classes,
> +MAD_EXPORT void	madrpc_init(char *dev_name, int dev_port, int *mgmt_classes,
>  		    int num_classes);
>  void	madrpc_save_mad(void *madbuf, int len);
> -void	madrpc_show_errors(int set);
> +MAD_EXPORT void	madrpc_show_errors(int set);
>  
> -void *	mad_rpc_open_port(char *dev_name, int dev_port, int *mgmt_classes,
> +void * mad_rpc_open_port(char *dev_name, int dev_port, int *mgmt_classes,
>  			  int num_classes);
>  void	mad_rpc_close_port(void *ibmad_port);
> -void *	mad_rpc(const void *ibmad_port, ib_rpc_t *rpc, ib_portid_t *dport,
> +void * mad_rpc(const void *ibmad_port, ib_rpc_t *rpc, ib_portid_t *dport,
>  		void *payload, void *rcvdata);
> -void *  mad_rpc_rmpp(const void *ibmad_port, ib_rpc_t *rpc, ib_portid_t *dport,
> +void * mad_rpc_rmpp(const void *ibmad_port, ib_rpc_t *rpc, ib_portid_t *dport,
>  		     ib_rmpp_hdr_t *rmpp, void *data);
>  
>  /* smp.c */
> -uint8_t * smp_query(void *buf, ib_portid_t *id, unsigned attrid, unsigned mod,
> +MAD_EXPORT uint8_t * smp_query(void *buf, ib_portid_t *id, unsigned attrid, unsigned mod,
>  		    unsigned timeout);
> -uint8_t * smp_set(void *buf, ib_portid_t *id, unsigned attrid, unsigned mod,
> +MAD_EXPORT uint8_t * smp_set(void *buf, ib_portid_t *id, unsigned attrid, unsigned mod,
>  		  unsigned timeout);
>  uint8_t * smp_query_via(void *buf, ib_portid_t *id, unsigned attrid,
>  			unsigned mod, unsigned timeout, const void *srcport);
> @@ -730,18 +724,18 @@ uint8_t * sa_call(void *rcvbuf, ib_portid_t *portid, ib_sa_call_t *sa,
>  		  unsigned timeout);
>  uint8_t * sa_rpc_call(const void *ibmad_port, void *rcvbuf, ib_portid_t *portid,
>                        ib_sa_call_t *sa, unsigned timeout);
> -int	ib_path_query(ibmad_gid_t srcgid, ibmad_gid_t destgid, ib_portid_t *sm_id,
> +MAD_EXPORT int	ib_path_query(ibmad_gid_t srcgid, ibmad_gid_t destgid, ib_portid_t *sm_id,
>  		      void *buf);	/* returns lid */
>  int	ib_path_query_via(const void *srcport, ibmad_gid_t srcgid,
>  			  ibmad_gid_t destgid, ib_portid_t *sm_id, void *buf);
>  
>  /* resolve.c */
> -int	ib_resolve_smlid(ib_portid_t *sm_id, int timeout);
> -int	ib_resolve_guid(ib_portid_t *portid, uint64_t *guid,
> +MAD_EXPORT int	ib_resolve_smlid(ib_portid_t *sm_id, int timeout);
> +MAD_EXPORT int	ib_resolve_guid(ib_portid_t *portid, uint64_t *guid,
>  			ib_portid_t *sm_id, int timeout);
> -int	ib_resolve_portid_str(ib_portid_t *portid, char *addr_str,
> +MAD_EXPORT int	ib_resolve_portid_str(ib_portid_t *portid, char *addr_str,
>  			      int dest_type, ib_portid_t *sm_id);
> -int	ib_resolve_self(ib_portid_t *portid, int *portnum, ibmad_gid_t *gid);
> +MAD_EXPORT int	ib_resolve_self(ib_portid_t *portid, int *portnum, ibmad_gid_t *gid);
>  
>  int	ib_resolve_smlid_via(ib_portid_t *sm_id, int timeout,
>  			     const void *srcport);
> @@ -755,19 +749,19 @@ int	ib_resolve_self_via(ib_portid_t *portid, int *portnum, ibmad_gid_t
> *gid,
>  			    const void *srcport);
>  
>  /* gs.c */
> -uint8_t *perf_classportinfo_query(void *rcvbuf, ib_portid_t *dest, int port,
> +MAD_EXPORT uint8_t *perf_classportinfo_query(void *rcvbuf, ib_portid_t *dest, int port,
>  				  unsigned timeout);
> -uint8_t *port_performance_query(void *rcvbuf, ib_portid_t *dest, int port,
> +MAD_EXPORT uint8_t *port_performance_query(void *rcvbuf, ib_portid_t *dest, int port,
>  				unsigned timeout);
> -uint8_t *port_performance_reset(void *rcvbuf, ib_portid_t *dest, int port,
> +MAD_EXPORT uint8_t *port_performance_reset(void *rcvbuf, ib_portid_t *dest, int port,
>  				unsigned mask, unsigned timeout);
> -uint8_t *port_performance_ext_query(void *rcvbuf, ib_portid_t *dest, int port,
> +MAD_EXPORT uint8_t *port_performance_ext_query(void *rcvbuf, ib_portid_t *dest, int port,
>  				    unsigned timeout);
> -uint8_t *port_performance_ext_reset(void *rcvbuf, ib_portid_t *dest, int port,
> +MAD_EXPORT uint8_t *port_performance_ext_reset(void *rcvbuf, ib_portid_t *dest, int port,
>  				    unsigned mask, unsigned timeout);
> -uint8_t *port_samples_control_query(void *rcvbuf, ib_portid_t *dest, int port,
> +MAD_EXPORT uint8_t *port_samples_control_query(void *rcvbuf, ib_portid_t *dest, int port,
>  				    unsigned timeout);
> -uint8_t *port_samples_result_query(void *rcvbuf, ib_portid_t *dest, int port,
> +MAD_EXPORT uint8_t *port_samples_result_query(void *rcvbuf, ib_portid_t *dest, int port,
>  				   unsigned timeout);
>  
>  uint8_t *perf_classportinfo_query_via(void *rcvbuf, ib_portid_t *dest, int port,
> @@ -785,7 +779,7 @@ uint8_t *port_samples_control_query_via(void *rcvbuf, ib_portid_t *dest, int por
>  uint8_t *port_samples_result_query_via(void *rcvbuf, ib_portid_t *dest, int port,
>  				   unsigned timeout, const void *srcport);
>  /* dump.c */
> -ib_mad_dump_fn
> +MAD_EXPORT ib_mad_dump_fn
>  	mad_dump_int, mad_dump_uint, mad_dump_hex, mad_dump_rhex,
>  	mad_dump_bitfield, mad_dump_array, mad_dump_string,
>  	mad_dump_linkwidth, mad_dump_linkwidthsup, mad_dump_linkwidthen,
> diff --git a/libibmad/include/infiniband/mad_osd.h b/libibmad/include/infiniband/mad_osd.h
> new file mode 100644
> index 0000000..45741c5
> --- /dev/null
> +++ b/libibmad/include/infiniband/mad_osd.h
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright (c) 2004-2007 Voltaire Inc.  All rights reserved.
> + * Copyright (c) 2009 Intel Corporation   All rights reserved.
> + *
> + * This software is available to you under a choice of one of two
> + * licenses.  You may choose to be licensed under the terms of the GNU
> + * General Public License (GPL) Version 2, available from the file
> + * COPYING in the main directory of this source tree, or the
> + * OpenIB.org BSD license below:
> + *
> + *     Redistribution and use in source and binary forms, with or
> + *     without modification, are permitted provided that the following
> + *     conditions are met:
> + *
> + *      - Redistributions of source code must retain the above
> + *        copyright notice, this list of conditions and the following
> + *        disclaimer.
> + *
> + *      - Redistributions in binary form must reproduce the above
> + *        copyright notice, this list of conditions and the following
> + *        disclaimer in the documentation and/or other materials
> + *        provided with the distribution.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + */
> +#ifndef _MAD_OSD_H_
> +#define _MAD_OSD_H_
> +
> +#include <stdint.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <byteswap.h>
> +#include <inttypes.h>
> +#include <arpa/inet.h>
> +
> +#define MAD_EXPORT 
> +
> +#endif /* _MAD_OSD_H_ */
> -- 
> 1.5.2.5
> 
> 
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> 
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general



More information about the ofw mailing list