[ofa-general] RE: PATCH[1/6] Windows port of libibmad - mad.h

[Sean Hefty]

>Port of libibmad to windows. Dependencies on libibumad port and complib (in
>lieu of libcommon). Removed dependency on libibcommon.
>
>Intent is to allow common mad code base for Windows and Linux to simplify
>maintainablity across OFED and WinOF. This patch set was built and tested on
>Windows and built on Linux (not tested yet).

Thanks for posting this.  Seeing the actual changes helps understand the impact
better.

Note that the libib_u_mad implementations are not shared.  Only the interface is
maintained to simplify porting.

Looking at the changes, do the management developers think that it makes sense
to share the libibmad implementation, or should separate implementations be
maintained, similar to libibumad?  If the implementations are not shared, can
the Linux side treat the API as an external interface, rather than a private
interface?

>+#if defined(_WIN32) || defined(_WIN64)
>+#define MAD_EXPORT     __declspec(dllexport)
>+#else
>+#define MAD_EXPORT     extern

I don't know that 'extern' is appropriate here.

>+#endif
>+
> #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
>@@ -620,10 +628,10 @@
>
>/******************************************************************************
>/
>
> /* 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)
>@@ -639,77 +647,49 @@
> /* fields.c */
> extern ib_field_t ib_mad_f[];
>
>-void   _set_field(void *buf, int base_offs, ib_field_t *f, uint32_t val);
>+void _set_field(void *buf, int base_offs, ib_field_t *f, uint32_t val);
> uint32_t _get_field(void *buf, int base_offs, ib_field_t *f);
>-void   _set_array(void *buf, int base_offs, ib_field_t *f, void *val);
>-void   _get_array(void *buf, int base_offs, ib_field_t *f, void *val);
>-void   _set_field64(void *buf, int base_offs, ib_field_t *f, uint64_t val);
>+void _set_array(void *buf, int base_offs, ib_field_t *f, void *val);
>+void _get_array(void *buf, int base_offs, ib_field_t *f, void *val);
>+void _set_field64(void *buf, int base_offs, ib_field_t *f, uint64_t val);
> uint64_t _get_field64(void *buf, int base_offs, ib_field_t *f);

Are these really the functions that should be exported from the library or in
the header file?  (I'm probably missing some history here.)

- Sean