[ofw] ib_portinfo_record_t padding history?
Smith, Stan
stan.smith at intel.com
Thu May 20 10:37:03 PDT 2010
Hal Rosenstock wrote:
> Hi Fab,
>
> On Wed, May 19, 2010 at 1:23 AM, Fab Tillier <ftillier at microsoft.com>
> wrote:
>> Hi Hal,
>>
>> Hal Rosenstock wrote on Tue, 18 May 2010 at 19:22:37
>>
>>>> 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) ?
>>
>> So that the returned offset is not rounded down incorrect. Say the
>> attribute size is not a multiple of 8, the >> 3 would return a value
>> 1 less than required.
>>
>> Not sure how the Linux code works properly, unless the structure
>> sizes somehow get rounded to a multiple of 8.
>
> The record structs which are not modulo 8 bytes are explicitly padded
> out (e.g NodeRecord, LinkRecord, SMInfoRecord, InformInfoRecord). If
> this is followed in the struct definitions, then the if clause is
> never used.
>
> -- Hal
>
>> -Fab
Turning the 'if( attr_size & 0x07 )' into 'CL_ASSERT((attr_size & 0x07) == 0)' along with converting the numerous osmtest instances of 'cl_hton16( (uint16_t)(xxx >> 3) )' into 'ib_get_attr_offset(xxx)' illuminated 'ib_switch_info_record_t' contained unnecessary padding.
With padding removed from ib_switch_info_record_t and ib_portinfo_record_t, such that the structs match their Linux counterparts, Windows osmtest correctly generates an inventory file which passes all osmtests; A first!
osmtest -f [vasf] -s[1234]
Thank you all for the insights and history.
stan.
More information about the ofw
mailing list