From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 4470F2B9C for ; Thu, 5 Jan 2017 04:05:04 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP; 04 Jan 2017 19:05:03 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,319,1477983600"; d="scan'208";a="1079333674" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga001.jf.intel.com with ESMTP; 04 Jan 2017 19:04:59 -0800 Received: from fmsmsx158.amr.corp.intel.com (10.18.116.75) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 4 Jan 2017 19:04:34 -0800 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by fmsmsx158.amr.corp.intel.com (10.18.116.75) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 4 Jan 2017 19:04:35 -0800 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.20]) by shsmsx102.ccr.corp.intel.com ([169.254.2.88]) with mapi id 14.03.0248.002; Thu, 5 Jan 2017 11:04:32 +0800 From: "Zhang, Helin" To: Thomas Monjalon , "Yigit, Ferruh" CC: "Yang, Qiming" , "dev@dpdk.org" , "Horton, Remy" Thread-Topic: [dpdk-dev] [PATCH v2 0/5] example/ethtool: add bus info and fw version get Thread-Index: AQHSXGh9ChdlnSlbGEa5PibjZ1HCHqEpRd0Q Date: Thu, 5 Jan 2017 03:04:31 +0000 Message-ID: References: <1479375779-46629-2-git-send-email-qiming.yang@intel.com> <6590239.9s5rXc1lKr@xps13> <095d7632-fbb7-a72b-8a48-163ed2b32cb4@intel.com> <4171707.MgFeI6QLbH@xps13> In-Reply-To: <4171707.MgFeI6QLbH@xps13> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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, 05 Jan 2017 03:05:05 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@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 >=20 > 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_in= fo). > > >> > > >> OK. > > >> > > >>> However, there > > >>> is a balance to find. Could we plan to add more info to this new qu= ery? > > >>> 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); >=20 > Oh I see. So it can be versioned with compat macros. >=20 > > So my concern was if the number of the arguments becomes too many by > time. >=20 > 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 F= reeBSD) does, which should be well discussed. In addition, for future extention, and avoid breaking any ABI in a strcutur= e, we can just pre-define a lot of bytes as reserved, e.g. 64 bytes. Inside DP= DK, there are several strucutres defined like this, e.g. mbuf. Thanks, Helin