From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 2830410D9C for ; Thu, 22 Dec 2016 16:05:07 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga104.jf.intel.com with ESMTP; 22 Dec 2016 07:05:06 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,388,1477983600"; d="scan'208";a="915203884" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.29]) ([10.237.220.29]) by orsmga003.jf.intel.com with ESMTP; 22 Dec 2016 07:05:05 -0800 To: Thomas Monjalon References: <1479375779-46629-2-git-send-email-qiming.yang@intel.com> <1578263.GeZ0IiYehl@xps13> <6590239.9s5rXc1lKr@xps13> Cc: Qiming Yang , dev@dpdk.org, Remy Horton From: Ferruh Yigit Message-ID: <095d7632-fbb7-a72b-8a48-163ed2b32cb4@intel.com> Date: Thu, 22 Dec 2016 15:05:04 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <6590239.9s5rXc1lKr@xps13> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2 0/5] example/ethtool: add bus info and fw version get X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Dec 2016 15:05:08 -0000 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. >