[ofw] ib_portinfo_record_t padding history?

[Smith, Stan]

Fab Tillier wrote:
> Smith, Stan wrote on Tue, 18 May 2010 at 14:18:09
>
>> 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.
>
> Is the array being addressed as an array, or each entry being
> accessed using ib_get_query_result (or ib_get_query_portinfo_rec?)

>From ulp\opensm\user\include\osm_vendor_sa_api.h

(ib_portinfo_record_t *) osmv_get_query_result(p_result_madw, result_index);


>
> Which direction are you off by 2?  Is the next entry 2 bytes too
> soon, or two bytes too late?

Actual data in the result_madw is correct; calculation of result_index 'osmv_get_query_result()'is off by +2.
I did not verify the 3rd portinfo_record start.
Once I removed the ib_portinfo_record_t padding the problem was resolved.
The gold standard inventory file was created using a Linux osmtest & Linux opensm.
Sans the differences in where openSM is running, inventory files should be very similar.

>
>> 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) ) ); }
>
> Unpadded, sizeof(ib_portinfo_record_t) gives 58, and
> ib_get_attr_offset( 58 ) returns 8.  Padded it gives 64 and
> ib_get_attr_offset( 64 ) also returns 8.
>
>> 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) ) );
>> }
>
> Unpadded, sizeof(ib_portinfo_record_t) gives 58, and
> ib_get_attr_offset( 58 ) returns 7.  Padded it gives 64 and
> ib_get_attr_offset( 64 ) also returns 8.
>
>>
>> 'attr_size' is passed in osmtest.c as 'sizeof(ib_portinfo_record_t)'
>> when building an SA query.
>
> You're running osmtest on Windows against OpenSM on Linux?  Or both
> on Windows?  Both on Linux?

gold std osmtest.dat (inventory file) was generated completely on Linux.
Windows osmtest using a Linux openSM with Linux generated inventory file works correctly 'osmtest -f v'; the goal.

Switch to a windows generated inventory file and same osmtest fails.
Further examination of the windows inventory file showed the off-by-2 problem as the DEFINE_PORT entries were bad.

>
>> 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 attr_size is only valid in RMPP DATA packets.  Is the query using
> RMPP?  If not, the spec says that the field is ignored.
>
>> The question becomes 'which is the correct version of
>> ib_get_attr_offset()?'.
>
> The Windows version seems correct, especially if you don't have
> padding in the structure.  The Linux version will tell you the next
> attribute will start 56 bytes after the first, giving you an
> off-by-two error.  So using unpadded in conjunction with the Linux
> ib_get_attr_offset will give you the wrong offset.
>
> Can you check that offset is 8, not 7, in the RMPP response to the
> query?

offset == 8.


>
> -Fab