On Tue, May 18, 2010 at 5:18 PM, Smith, Stan <[email protected]> wrote: > Fab Tillier wrote: >> Sean Hefty wrote on Tue, 18 May 2010 at 12:08:08 >> >>>> Thoughts? >>> >>> The padding looks wrong based on the IB spec. PortInfoRecord should >>> have PortInfo as its last field. The size of PortInfoRecord should >>> be 58 bytes. >> >> AttributeOffset in the MAD is defined as number of 8-byte words >> between attributes. I believe the extra pad is there so that you can >> treat the records in a GET_TABLE response as an array of >> PortInfoRecords. I don't know if any code actually uses it this way, >> but that's what I guess the intent is. >> >>>> #include <complib/cl_packon.h> >>>> typedef struct _ib_portinfo_record >>>> { >>>> ib_net16_t lid; >>>> uint8_t port_num; >>>> uint8_t resv; >>>> ib_port_info_t port_info; >>>> uint8_t pad[6]; <-- ?? >>>> } PACK_SUFFIX ib_portinfo_record_t; >>>> #include <complib/cl_packoff.h> >>> >>> My guess (and this is only a guess) is that the padding was added to >>> make PortInfoRecord align on a 64-bit boundary, probably in case it >>> were sent as part of an RMPP mad. The PortInfo structure contains >>> 64- bit values at the top of the structure. >> >> Nah, the attributes end up unaligned anyway thanks to the stellar >> layout of the SA MADs. I'm pretty sure it's there to allow indexing >> as an array. While the pad isn't formally documented, it will always >> be there. It should be harmless. >> >> -Fab > > Indeed a problem arises in osmtest when building a fabric inventory file > 'osmtest.dat' via 'osmtest -f c'. > The problem is exactly that of an SA response containing an array of > ib_portinfo_records. > With the padding the 2nd ib_portinfo_record[1] index computation is > incorrectly offset by 2 bytes. > Without the padding the index calculation is correct. This observation led me > to the comparison of the Linux vs. Windows ib_portinfo_record_t definitions > and subsequent removal of the padding. > I agree the padding should not have been an issue; all things coded correctly. > > Digging further into the issue, I find the following Windows vs. Linux > definition: > > AL_INLINE ib_net16_t AL_API > ib_get_attr_offset( > IN const uint32_t > attr_size ) > { > if( attr_size & 0x07 ) > return( cl_hton16( (uint16_t)(attr_size >> 3) + 1 ) ); > else > return( cl_hton16( (uint16_t)(attr_size >> 3) ) ); > } > > Linux > > static inline ib_net16_t AL_API > ib_get_attr_offset(const uint32_t attr_size ) > { > return( cl_hton16( (uint16_t)(attr_size >> 3) ) ); > }
Why does Windows have the if clause in ib_get_attr_off (and not match the "Linux" version) ? -- Hal > 'attr_size' is passed in osmtest.c as 'sizeof(ib_portinfo_record_t)' when > building an SA query. > I suspect this is the root of the problem, as osmtest.c code which builds the > SA query for all known portinfo records sets the madw attr_size as > '(attr_size >> 3)'. > The question becomes 'which is the correct version of ib_get_attr_offset()?'. > > > _______________________________________________ > ofw mailing list > [email protected] > http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw > _______________________________________________ ofw mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
