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 B1067558A for ; Fri, 29 Apr 2016 12:22:04 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP; 29 Apr 2016 03:21:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,551,1455004800"; d="scan'208";a="965292547" Received: from rhorton-mobl.ger.corp.intel.com (HELO [163.33.228.53]) ([163.33.228.53]) by orsmga002.jf.intel.com with ESMTP; 29 Apr 2016 03:21:47 -0700 Message-ID: <572335BA.7000202@intel.com> Date: Fri, 29 Apr 2016 11:21:46 +0100 From: Remy Horton Organization: Intel Shannon Limited User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: "David Harton (dharton)" , "Tahhan, Maryam" , "dev@dpdk.org" CC: "Mcnamara, John" , "Van Haaren, Harry" References: <1460731462-21229-1-git-send-email-remy.horton@intel.com> <1A27633A6DA49C4A92FCD5D4312DBF536B1814BF@IRSMSX106.ger.corp.intel.com> In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC PATCH v1 0/3] Remove string operations from xstats X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Apr 2016 10:22:05 -0000 Morning, On 28/04/2016 16:58, David Harton (dharton) wrote: [..] >>>> *) id-name & id-value pairs for both lookup and query Permits >>>> out-of-order and non-contigious returning of names/ids/values, even >>>> though expected implmentations would in practice return items in >>>> sorted order by id. Is this sufficent/desirable future proofing? >>>> Idea is to allow possibility of drivers returning partial statistics. >>> >>> I believe forcing drivers to match to a common id-space will become >>> burdensome. If the stats id-space isn't common then matching strings >>> is probably just as sufficient as long as drivers don't add/remove >>> stats ad hoc between the time the device is initialized and removed. >> >> I'm not aware of drivers adding/removing the stats ad hoc? The idea is to >> have a common-id space otherwise it will be a free for all and we won't >> have alignment across the drivers. I don't see it being any more >> burdensome than having a common register naming across the board which is >> what is there today. The advantage being that you don't have to pull the >> strings every time. Returning both stats (id,value) and names (id,string) as pairs would allow (amoung other things) common ids but actually having common ids is not an intended goal of mine. I think the whole idea of common ids was implicity vetoed when the idea of ENUMs was thrown out. I opted for both stats and lookup to be provided as pairs because when it comes to APIs, I have a slight preference for having that bit of extra generality. Not sure its really worth it, so might change stats to just use id-indexed integer arrays (ethtool-like basically) rather than a typedef that includes the numeric id. >>>> *) Bulk name-id mapping lookup only [..] >>> I'm not sure I see the value of looking up a single stat from a user >>> perspective. I can see where the drivers might say that some stats >>> are less disruptive/etc but the user doesn't have that knowledge and >>> wouldn't know how to take advantage. Usually all stats are grabbed >>> multiple times and the changes noted during debug sessions. >>> >> >> I believe Remy's change doesn't suggest/support individual lookup. It is >> just a statement that we don't want to burden drivers with individual >> stats lookups. Correct. >>>> *) Replacement or additional API [..] >>> However, if >>> we want to go forward with cleaning up in order to reduce the support >>> drivers provide I'm all for it. Whether we want to do such a cleanup is my open question. > Maybe I misread the patch series or missed one but I don't see where > stats can be obtained without copying strings? This is the real issue I > raised originally. See http://dpdk.org/dev/patchwork/patch/12096/ where xstats[].key is used to lookup string from ptr_names[].name - I didn't delete the name field from rte_eth_xstats because doing so would cause a compile error with the drivers I've not yet converted. > I did not add "get the count" because it wasn't provided in the current API > and instead followed the convention but I do believe overtly getting the > count it is the better approach. I didn't either for the same reason, but if the API is going to be broken, I think it should be added. ..Remy