From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 87132A0096 for ; Fri, 12 Apr 2019 18:05:54 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 02D861B21A; Fri, 12 Apr 2019 18:05:52 +0200 (CEST) Received: from mail-pl1-f195.google.com (mail-pl1-f195.google.com [209.85.214.195]) by dpdk.org (Postfix) with ESMTP id 6E7961B217 for ; Fri, 12 Apr 2019 18:05:51 +0200 (CEST) Received: by mail-pl1-f195.google.com with SMTP id n8so5331910plp.10 for ; Fri, 12 Apr 2019 09:05:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=LHyrjyEnevJNF+OVnmud7b2MN4RK1db6YbB+Pse3Nys=; b=EaY6bAp1Tlu0isu7pewoQXwwMCHPWHHRhg6cbqGA0/FcrEkYXBUWWolkk+Udcuj450 rIk38Pykb5YpZhUDkF4r5kakVmBui/E4iIe9tE/7sPvn7n3C/Khi5IzNmSyDqk2p7n1j 0QJeG5XIVizaMhvezHkAJMYFWsXa9gtdb/H71/b5JfnRZpI9LEu6ln+6LClGFtefKimD OCZBQelkEYOZ8FwheIw49X6XJ/uKGw6nKD4IHGQy3zYGgsi9MXYWQF3ZMWqfdl+XTyH5 zU5JzpZqtsgY0LpG0PadRs/jeE8PN/EpMnigH/j+Iuh3iKv0F93b5v8eGk8WJLxvr2a3 SnkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=LHyrjyEnevJNF+OVnmud7b2MN4RK1db6YbB+Pse3Nys=; b=IDRMhAIwwJfaf6V3dQJZ13QSJGNw0e6E6t9AnE5mO0EV8km+bWcl0XLncKk1TuKwqh tPqeL6pTzghN6xrpkSjp3wXeDBTBChyrndfytIt4D5GS021JhN2agUh7sM9N2QMQGlqY 8n0aVmcFtl4SVKT3Za9m0rSd4qTwDzwVuohlQRzWEtsMs5Tedv3bLfF9cOhHQg5l477e Clnrri9VneyCGXeTIpMPLIs/1zsYT7HevbACUWPrMiiZBewREc8I9d4hfSENNBOsYAS+ V7F1jZN3CjcxZZ10wCtotw9OoqW7lnfoClqFZAwjYVlNViu687nj+dNsRoE1oPu9EyyE vkaQ== X-Gm-Message-State: APjAAAUnTaQVrxj53xJ27lpBdJgWyYe0i8UHUYOu+qTrCVVlwbk06Bd4 zazHZT7J3chEvPMsGVkLyQO35Q== X-Google-Smtp-Source: APXvYqzLNTEb7k9R5jlZuEyMKPjbCjq5TTVHrhOmaOgtRs+AfyCeYKulInmIp2BiMLYr3zeny4nEvg== X-Received: by 2002:a17:902:a607:: with SMTP id u7mr58940313plq.66.1555085150546; Fri, 12 Apr 2019 09:05:50 -0700 (PDT) Received: from shemminger-XPS-13-9360 (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id b15sm58337731pgg.90.2019.04.12.09.05.50 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 12 Apr 2019 09:05:50 -0700 (PDT) Date: Fri, 12 Apr 2019 09:05:47 -0700 From: Stephen Hemminger To: David Marchand Cc: Thomas Monjalon , dev , Ferruh Yigit , Andrew Rybchenko , "Stokes, Ian" Message-ID: <20190412090547.72769262@shemminger-XPS-13-9360> In-Reply-To: References: <1745620.RIKK5OcHtz@xps> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC PATCH 1/2] ethdev: introduce internal rxq/txq stats API 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190412160547.dqHcpQGAq6KJxlTKEnc5jVDc_mHxnUbwiaRGVA16I70@z> On Fri, 12 Apr 2019 16:32:01 +0200 David Marchand wrote: > On Fri, Apr 12, 2019 at 3:29 PM Thomas Monjalon wrote: > > > 26/03/2019 10:29, David Marchand: > > > On Tue, Mar 19, 2019 at 6:18 PM Ferruh Yigit > > wrote: > > > > > > > On 3/14/2019 3:13 PM, David Marchand wrote: > > > > > Introduce a new api to retrieve per queue statistics from the > > drivers. > > > > > The api objectives: > > > > > - easily add some common per queue statistics and have it exposed > > > > > through the user xstats api while the user stats api is left > > untouched > > > > > - remove the limitations on the per queue statistics count (inherited > > > > > from ixgbe) and avoid recurrent bugs on stats array overflow > > > > First comment, I think it would be easier to read if renaming the legacy > > basic stats interface was in a separate patch. > > > > It will be quite artificial, but I can do this yes. > > > > > > The patch is adding two new dev_ops 'rxq_stats_get' & 'txq_stats_get', > > my > > > > concern is if it is overkill to have three dev_ops to get stats > > > > and I am feeling that is making xstat code more complex. > > > > > > Having these new (meant to be) internal dev_ops has the avantage of > > > separating the statistics reported from the drivers from the exported > > api. > > > This is also why I did not prefix the structure names with rte_. > > > > Yes, and to make it clear, please do not talk about API, > > as it is only a driver interface. > > > > Ok, so I will describe this as a "driver interface" update. > > > > > > The "complex" part is in a single place, ethdev and this is when > > > translating from an internal representation to the exposed bits in the > > > public apis. > > > > > > Would it be simpler to add 'q_ierrors' & 'q_oerrors' to 'struct > > > > rte_eth_stats'? > > > > > > > > > > It does not solve the problem of drivers that are buggy because of the > > > limit on RTE_ETHDEV_QUEUE_STAT_CNTRS. > > > All drivers need to be aware of this limitation of the rte_eth_stats > > > structure. > > > > Yes, this limitation should be dropped. > > I would like to see the functions rte_eth_dev_set_?x_queue_stats_mapping() > > deprecated as they were a bad abstraction of ixgbe limitation. > > > > That's a different topic from my pov, but yes, this mapping stuff should go > away, later. > > > > > And perhaps we can do the 'fix rxq q_errors' patchset [1] after this > > > > change, so > > > > fix can be done with less changes, although it will push the fix into > > next > > > > release because of the ABI break. > > > > > > I am fine with merging this together, we don't want to backport this > > > anyway, right? > > > > No, it would make some behaviours changing in stable releases, > > so better to not backport it and keep the buggy behaviour in old branches. > > > > Since the time I had posted this RFC, I have worked on a RFC v2, I will > post this next week, with the drivers I found time to convert. > We will have to take a decision on what goes to -rc2 between this and the > q_errors[] patchset. > > It looks like this all about maintaining source compatiablity with older or out of tree drivers. This is not something DPDK has to worry about. Why not just do a mondo patch that fixes all the drivers to use the new stats API. You do need to keep the same ethdev interface for applications, but driver API can change.