[ofa-general] [RFC] 3/5: IB ACM: libibacm

[Ira Weiny]

On Wed, 16 Sep 2009 23:45:05 -0700
"Sean Hefty" <sean.hefty at intel.com> wrote:

> Add an end-user library with simple interfaces for communicating
> with the ib_acm service.
> 
> The linux and windows specific files for the library are simple and not
> shown for this review
> 
> Signed-off-by: Sean Hefty <sean.hefty at intel.com>
> ---
> 
> ib_acm.h: defines library interfaces.
> These are the end-user application interfaces to the ib acm.
> 

[snip]

> 
> #define IB_PATH_RECORD_REVERSIBLE 0x80
> 
> struct ib_path_record
> {
> 	uint64_t        service_id;
> 	union ibv_gid   dgid;
> 	union ibv_gid   sgid;
> 	uint16_t        dlid;
> 	uint16_t        slid;
> 	uint32_t        flowlabel_hoplimit; /* resv-31:28 flow label-27:8 hop limit-7:0*/
> 	uint8_t         tclass;
> 	uint8_t         reversible_numpath; /* reversible-7:7 num path-6:0 */
> 	uint16_t        pkey;
> 	uint16_t        qosclass_sl;        /* qos class-15:4 sl-3:0 */
> 	uint8_t         mtu;                /* mtu selector-7:6 mtu-5:0 */
> 	uint8_t         rate;               /* rate selector-7:6 rate-5:0 */
> 	uint8_t         packetlifetime;     /* lifetime selector-7:6 lifetime-5:0 */
> 	uint8_t         preference;
> 	uint8_t         reserved[6];
> };

I would prefer to use the structures already defined in ib_types.h...  I understand your not wanting to make ACM dependant on the OpenSM packages so is it time to move ib_types.h out of the OpenSM tree and somewhere more generic?  Perhaps libibumad?  This also applies to ib_sa_mad in your 5th patch.

OTOH, ib_types.h is a 10K line file with multiple long (>10 lines) inlined functions.  Perhaps it deserves it's own library?

Ira

[snip]



-- 
Ira Weiny
Math Programmer/Computer Scientist
Lawrence Livermore National Lab
925-423-8008
weiny2 at llnl.gov