From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id EF8041DA78; Thu, 14 Jun 2018 21:33:48 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Jun 2018 12:33:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,224,1526367600"; d="scan'208";a="237524507" Received: from rhorton-mobl1.ger.corp.intel.com (HELO [10.252.12.15]) ([10.252.12.15]) by fmsmga006.fm.intel.com with ESMTP; 14 Jun 2018 12:33:45 -0700 To: Ferruh Yigit , David Marchand , dev@dpdk.org References: <1528359323-22885-1-git-send-email-david.marchand@6wind.com> <94ff9382-dbcd-1afd-b149-349653e1222e@intel.com> <1c72a061-db06-8fac-b1fa-3d0695686de9@intel.com> Cc: wenzhuo.lu@intel.com, jingjing.wu@intel.com, bernard.iremonger@intel.com, stable@dpdk.org, Harry Van Haaren From: Remy Horton Organization: Intel Shannon Limited Message-ID: <9e34833d-cb17-170a-f6ba-15210db0c7eb@intel.com> Date: Thu, 14 Jun 2018 20:33:44 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <1c72a061-db06-8fac-b1fa-3d0695686de9@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: add sanity checks when retrieving xstats 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, 14 Jun 2018 19:33:49 -0000 On 14/06/2018 11:55, Ferruh Yigit wrote: > On 6/14/2018 7:39 AM, Remy Horton wrote: >> >> On 13/06/2018 16:39, Ferruh Yigit wrote: >>> On 6/7/2018 9:15 AM, David Marchand wrote: >>>> Testpmd should not expect the xstats names and values arrays to be >>>> aligned: neither the arrays sizes, nor the order in which the values are. [..] > APIs exist for getting subset of values (.._by_id) but they both assume > requested ids are array index. > As you said this works fine because of xstats[idx].id==idx Changing that coincidence into an assumption looks like a bug to me. > struct rte_eth_xstat_name { char name[]; } > struct rte_eth_xstat { uint64_t id; uint64_t value; } > > These two structs are for basic key-value match. > But one has the "id" field, but other doesn't. If we use "id" as match, this > will be the index of xstat_name[]. This is extra complexity, and xstats is > already unnecessarily complex. > > I am for documenting that "xstat_name" and "xstat" are aligned, both in size and > order, and array indexes are ids, clearly in API doc and continue with existing > implementation. What do you think? As long as none of the PMD vendors intend to take advantage of xstats[idx].id!=idx (e.g. to allow omitted values) then I'm OK with it. ..Remy