> -----Original Message----- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas Monjalon > Sent: Thursday, December 22, 2016 11:31 PM > To: Yigit, Ferruh > Cc: Yang, Qiming; dev@dpdk.org; Horton, Remy > Subject: Re: [dpdk-dev] [PATCH v2 0/5] example/ethtool: add bus info and fw > version get > > 2016-12-22 15:05, Ferruh Yigit: > > 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); > > Oh I see. So it can be versioned with compat macros. > > > So my concern was if the number of the arguments becomes too many by > time. > > It looks to be a good proposal. We should not have a dozen of arguments.
I'd suggest to do that the similar way of kernel driver/ethtool (Linux or FreeBSD) does, which should be well discussed. In addition, for future extention, and avoid breaking any ABI in a strcuture, we can just pre-define a lot of bytes as reserved, e.g. 64 bytes. Inside DPDK, there are several strucutres defined like this, e.g. mbuf. Thanks, Helin