[ofw] API breakage in trunk

[Smith, Stan]

Perhaps I'm missing an important point here but what's so terribly wrong with accepting the fact that the API change is good in the long term?
Instead of OFED 2.3 we move forward to the next 'major' version and produce OFED 3.0 ?

Yes - an application recompile is required but do we really understand how 'many' applications are impacted? (< 10, > 10) ??? Are the app developers willing to recompile? has anyone asked them?

Leonid makes a good point w.r.t. the lack of versioning and vendor specific data.
Perhaps the versioning, Vendor specific issues and array issues can all be 'corrected' in the same API change?

stan.

Leonid Keller wrote:
>> 1. Define a new structure and interface that provides this data.
>> 2. Define separate ib_port_attr_t for kernel and user space.
>
> I see a third way.
>
> But first about some old problems of this (ib_ca_attr_t) structure.
> It is not versioned so we can't expand it on need without breaking
> the backward compatability.
> There is no vendor data definition in the structure, so it's not that
> simple to understand how vstat handles Mellanox-specific info, which
> it takes out from ib_ca_attr_t and not simple to add other vendor
> info.
>
> Here is my idea.
> As well as the structure is not packed, we can use holes to add new
> fields.
> For example, there is a 4 byte hole between the following two fields:
>
>       uint32_t                                init_regions;
>       uint64_t                                init_region_size;
>
> We can add there a version field:
>
>       uint32_t                                init_regions;
>       uint32_t                                version;
>       uint64_t                                init_region_size;
>
> mlnx_query_ca() function, which performs QUERY_CA request, always
> cleans the structure before filling it:
>
>       ...
>       RtlZeroMemory(p_ca_attr, required_size);
>       ...
>
> so we have got version = 0 so far.
>
> We can now add new fields at the end of the structure for version =
> 1, like
>
>       // for LLE
>       enum rdma_transport_type[MAX_PORT_NUM]  transport;
>
>       // for vendor specific info
>       enum vendor_info_type           vendor_info;
>       union {
>               uplink_info_t           mlnx_vendor_info;                       // for Mellanox
>       };
>
> It doesn't break the backward compability as far as I see.
>
> What do you think ?
>
> Implementation note:
>  The structure ib_ca_attr_t contains a variable part, consisting of
>  two arrays (page_sizes and port_attrs). Their start is shown by
>  pointers p_page_size and p_port_attr. In version 1 these arrays will
> be placed *after* new fields and the pointers will be appropriately
> adjusted.
>
>
>
>> -----Original Message-----
>> From: ofw-bounces at lists.openfabrics.org
>> [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Sean Hefty
>> Sent: Friday, February 19, 2010 10:04 PM
>> To: 'James Yang'; Tzachi Dar; ofw_list
>> Subject: Re: [ofw] API breakage in trunk
>>
>>> It seems we have to stuck with old ib_port_attr_t.
>>>
>>> To get new port attribute, we can add these new definition at the
>>> end of ib_ca_attr_t, I'm assuming its size is OK to expand...
>>
>> I think it's okay adding it to the end of ib_ca_attr_t.  Some other
>> alternatives:
>>
>> 1. Define a new structure and interface that provides this data.
>> 2. Define separate ib_port_attr_t for kernel and user space.
>> Add this to the kernel, but not user space.  Select a
>> different way for exposing this data to user space
>> application, if it is needed.
>>
>>
>> _______________________________________________
>> ofw mailing list
>> ofw at lists.openfabrics.org
>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
>>
> _______________________________________________
> ofw mailing list
> ofw at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw