On 12/22/2016 2:47 PM, Thomas Monjalon wrote:
> 2016-12-22 14:36, Ferruh Yigit:
>> On 12/22/2016 11:07 AM, Thomas Monjalon wrote:
>>> I think it is OK to add a new dev_ops and a new API function for firmware
>>> query. Generally speaking, it is a good thing to avoid putting all
>>> informations in the same structure (e.g. rte_eth_dev_info).
>>
>> OK.
>>
>>> However, there
>>> is a balance to find. Could we plan to add more info to this new query?
>>> Instead of
>>> rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int fw_length)
> [...]
>>> could it fill a struct?
>>> rte_eth_dev_fw_info_get(uint8_t port_id, struct rte_eth_dev_fw_info
>>> *fw_info)
>>
>> I believe this is better. But the problem we are having with this usage
>> is: ABI breakage.
>>
>> Since this struct will be a public structure, in the future if we want
>> to add a new field to the struct, it will break the ABI, and just this
>> change will cause a new version for whole ethdev library!
>>
>> When all required fields received via arguments, one by one, instead of
>> struct, at least ABI versioning can be done on the API when new field
>> added, and can be possible to escape from ABI breakage. But this will be
>> ugly when number of arguments increased.
>>
>> Or any other opinion on how to define API to reduce ABI breakage?
>
> You're right.
> But I don't think we should have a function per data. Just because it would
> be ugly :)
I am no suggesting function per data, instead something like:
rte_eth_dev_fw_info_get(uint8_t port_id, uint32_t maj, uint32_t min);
And in the future if we need etrack_id too, we can have both in
versioned manner:
rte_eth_dev_fw_info_get(uint8_t port_id, uint32_t maj, uint32_t min);
rte_eth_dev_fw_info_get(uint8_t port_id, uint32_t maj, uint32_t min,
uint32_t etrack_id);
So my concern was if the number of the arguments becomes too many by time.
> I hope the ABI could become stable with time.
>
