[ofw] ib_portinfo_record_t padding history?

[Smith, Stan]

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) ) );
}

'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()?'.