From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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=US-ASCII 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: , X-List-Received-Date: Fri, 12 Apr 2019 16:05:51 -0000 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. 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.