From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <thomas@monjalon.net> Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by dpdk.org (Postfix) with ESMTP id 96D5D1B12A for <dev@dpdk.org>; Fri, 12 Apr 2019 15:29:24 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id EA1BD22165; Fri, 12 Apr 2019 09:29:23 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Fri, 12 Apr 2019 09:29:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=2eFU8eM1d56/LFyUTdfnYE0/aGyhcUM+oxNb4ai8rW4=; b=BeAdg+GojAUw XIUb/SL7hahSXdeOK5vFH2TtUEK1oIZKfiGMdhbc3q+g1d7SZDF2zkuTN/0wgOVp nkVpN4/xSMEwNl/Ze/sITbtLjrLe5d6ZSqJ1DNitKzEln5+/Sv4C4HYunkK15bNF nYzq0Z3CTmI+I9UcpUBzr1V1Zg4aboQ= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=2eFU8eM1d56/LFyUTdfnYE0/aGyhcUM+oxNb4ai8r W4=; b=lrqee5IF5K3XEQcDXaxmRS7tXq1Zz0Z8TrwY4bU6llQzH7gihcxsk+NWf NpNJAh/DZdWXZq3PC/Lp8atWj/yzWnjnNa00pirfeWlu4av1MVVxinIR+w4q3U8P fC5L6BAosLvIiHJRKACDaoGI/tMPxz7OCB8zdz3b/GuMuUZuMvMQn8c7zkOUiYZk wVou/A5l+ND06T06ObnyIjrksyk5/mDNWwvzQChDTnkU7KHhEuX99oAeOQr9PcFt HkVNlkULHPdECoKYRKJOzEh78y61QB+K39hlgAXTb+s3bI/sc85gEzVS5tdpJO3p +uijYMrdWi75rkOADaMTkC+jBoK7A== X-ME-Sender: <xms:spKwXKBzNhfa0kXaJN3VGWR-1HGt4moA-d3dnqjefZ-1u6pS_6BOgQ> X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddrvddugdegvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucfkph epjeejrddufeegrddvtdefrddukeegnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhho mhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsthgvrhfuihiivgeptd X-ME-Proxy: <xmx:spKwXIRiX0roV7mvXUlxUDIA01rbH7U1RxHxU3cQ7WnFSU9sowG1Fw> <xmx:spKwXNV2PoEMkZ4m3Ge2E4vx0dXBGKamIzIDnb0hB8jM1wrKpCfZRg> <xmx:spKwXIENwwrcRowSIa8R6prbpxaWEgw26Gqhx5JMBTmSVB8AV4g4zQ> <xmx:s5KwXBK9oR7n4EZkEEVwsNjE5152V7K0YRdLPJDbfgSjHavngvgr_A> Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id A684E10319; Fri, 12 Apr 2019 09:29:21 -0400 (EDT) From: Thomas Monjalon <thomas@monjalon.net> To: David Marchand <david.marchand@redhat.com> Cc: dev@dpdk.org, Ferruh Yigit <ferruh.yigit@intel.com>, Andrew Rybchenko <arybchenko@solarflare.com>, "Stokes, Ian" <ian.stokes@intel.com>, Stephen Hemminger <stephen@networkplumber.org> Date: Fri, 12 Apr 2019 15:29:20 +0200 Message-ID: <1745620.RIKK5OcHtz@xps> In-Reply-To: <CAJFAV8xmVb1W9Gyv2Mzq4s8x65zO_i8B96cvWanqsmR5Hs2RBw@mail.gmail.com> References: <CAJFAV8wAM-k7mvzwo6fe1P5x=fM5s2-qyHvayJhZ_y2E=Hy6Jg@mail.gmail.com> <f36e778c-0d19-df29-4cff-1a4be8daf356@intel.com> <CAJFAV8xmVb1W9Gyv2Mzq4s8x65zO_i8B96cvWanqsmR5Hs2RBw@mail.gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> X-List-Received-Date: Fri, 12 Apr 2019 13:29:24 -0000 26/03/2019 10:29, David Marchand: > On Tue, Mar 19, 2019 at 6:18 PM Ferruh Yigit <ferruh.yigit@intel.com> 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. > > 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. > 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. > The drivers manipulate queues in rx/tx_queue_setup dev_ops for example, > having rxq/txq_stats_get dev_ops is more consistent to me. +1 > > 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. > But so far, I don't feel the need to break the rte_eth_stats_get API, if we > want to go to this, we can define an entirely new api to expose > standardized bits and still use the rxq/txq_stats_get internal dev_ops I > propose. Good From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <dev-bounces@dpdk.org> Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 9CB95A0096 for <public@inbox.dpdk.org>; Fri, 12 Apr 2019 15:29:27 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 602BB1B134; Fri, 12 Apr 2019 15:29:26 +0200 (CEST) Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by dpdk.org (Postfix) with ESMTP id 96D5D1B12A for <dev@dpdk.org>; Fri, 12 Apr 2019 15:29:24 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id EA1BD22165; Fri, 12 Apr 2019 09:29:23 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Fri, 12 Apr 2019 09:29:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=2eFU8eM1d56/LFyUTdfnYE0/aGyhcUM+oxNb4ai8rW4=; b=BeAdg+GojAUw XIUb/SL7hahSXdeOK5vFH2TtUEK1oIZKfiGMdhbc3q+g1d7SZDF2zkuTN/0wgOVp nkVpN4/xSMEwNl/Ze/sITbtLjrLe5d6ZSqJ1DNitKzEln5+/Sv4C4HYunkK15bNF nYzq0Z3CTmI+I9UcpUBzr1V1Zg4aboQ= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=2eFU8eM1d56/LFyUTdfnYE0/aGyhcUM+oxNb4ai8r W4=; b=lrqee5IF5K3XEQcDXaxmRS7tXq1Zz0Z8TrwY4bU6llQzH7gihcxsk+NWf NpNJAh/DZdWXZq3PC/Lp8atWj/yzWnjnNa00pirfeWlu4av1MVVxinIR+w4q3U8P fC5L6BAosLvIiHJRKACDaoGI/tMPxz7OCB8zdz3b/GuMuUZuMvMQn8c7zkOUiYZk wVou/A5l+ND06T06ObnyIjrksyk5/mDNWwvzQChDTnkU7KHhEuX99oAeOQr9PcFt HkVNlkULHPdECoKYRKJOzEh78y61QB+K39hlgAXTb+s3bI/sc85gEzVS5tdpJO3p +uijYMrdWi75rkOADaMTkC+jBoK7A== X-ME-Sender: <xms:spKwXKBzNhfa0kXaJN3VGWR-1HGt4moA-d3dnqjefZ-1u6pS_6BOgQ> X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddrvddugdegvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucfkph epjeejrddufeegrddvtdefrddukeegnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhho mhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsthgvrhfuihiivgeptd X-ME-Proxy: <xmx:spKwXIRiX0roV7mvXUlxUDIA01rbH7U1RxHxU3cQ7WnFSU9sowG1Fw> <xmx:spKwXNV2PoEMkZ4m3Ge2E4vx0dXBGKamIzIDnb0hB8jM1wrKpCfZRg> <xmx:spKwXIENwwrcRowSIa8R6prbpxaWEgw26Gqhx5JMBTmSVB8AV4g4zQ> <xmx:s5KwXBK9oR7n4EZkEEVwsNjE5152V7K0YRdLPJDbfgSjHavngvgr_A> Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id A684E10319; Fri, 12 Apr 2019 09:29:21 -0400 (EDT) From: Thomas Monjalon <thomas@monjalon.net> To: David Marchand <david.marchand@redhat.com> Cc: dev@dpdk.org, Ferruh Yigit <ferruh.yigit@intel.com>, Andrew Rybchenko <arybchenko@solarflare.com>, "Stokes, Ian" <ian.stokes@intel.com>, Stephen Hemminger <stephen@networkplumber.org> Date: Fri, 12 Apr 2019 15:29:20 +0200 Message-ID: <1745620.RIKK5OcHtz@xps> In-Reply-To: <CAJFAV8xmVb1W9Gyv2Mzq4s8x65zO_i8B96cvWanqsmR5Hs2RBw@mail.gmail.com> References: <CAJFAV8wAM-k7mvzwo6fe1P5x=fM5s2-qyHvayJhZ_y2E=Hy6Jg@mail.gmail.com> <f36e778c-0d19-df29-4cff-1a4be8daf356@intel.com> <CAJFAV8xmVb1W9Gyv2Mzq4s8x65zO_i8B96cvWanqsmR5Hs2RBw@mail.gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="UTF-8" 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 <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> Message-ID: <20190412132920.4Uj2ed9Oyb08bHmIw9Zw1BJM2Fvqkidswz1G0aJF4OI@z> 26/03/2019 10:29, David Marchand: > On Tue, Mar 19, 2019 at 6:18 PM Ferruh Yigit <ferruh.yigit@intel.com> 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. > > 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. > 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. > The drivers manipulate queues in rx/tx_queue_setup dev_ops for example, > having rxq/txq_stats_get dev_ops is more consistent to me. +1 > > 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. > But so far, I don't feel the need to break the rte_eth_stats_get API, if we > want to go to this, we can define an entirely new api to expose > standardized bits and still use the rxq/txq_stats_get internal dev_ops I > propose. Good