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 EA404A0096 for ; Fri, 12 Apr 2019 16:32:16 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id ED9E11B146; Fri, 12 Apr 2019 16:32:14 +0200 (CEST) Received: from mail-vs1-f65.google.com (mail-vs1-f65.google.com [209.85.217.65]) by dpdk.org (Postfix) with ESMTP id 9A1CF1B144 for ; Fri, 12 Apr 2019 16:32:13 +0200 (CEST) Received: by mail-vs1-f65.google.com with SMTP id n4so5658220vsm.3 for ; Fri, 12 Apr 2019 07:32:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=K8qiz4sMuOi8zbRokDma6NDNxP68U8McyYDMVqplU/Q=; b=Jk0qsRGZWXny3BFyW7iIeMzfwHbfYejBH6jCrcjHAigD3B5YUo2T0yeQeho4mMszom bHQbqSKbIAftX1SMXUEOdb0oezg5UUUN1UQnq1PFHZxRd6CXNHpOChRFs+q6gPrEn/hq Sok9uyK9HftOUbM5QKPd9cjpSYDv2ZzEn7BljCHVa9hA6E45PRLPkWsBDoaSmlV1D86u Uh5Kecrqbkus1kTE+o7oZvR1feqqz9cdxK1xjNoj45GIfsaGKuyifk8GG5Mtr7KnEEbB tsb6lS27o9V9tUyVqub1NurZJGD7qSHmDoorkCJaIZT/4qLxP/hmveuLW7mbm4qPIJy9 QNCw== X-Gm-Message-State: APjAAAXldPVwAWCCxHPQ4LJXHdjdpxqh6ZN5LdPClzgJQVv+yf2pg/fi APbWeWDuCAo31swaBjG9B98992JXs0C8HtZm1YYHYg== X-Google-Smtp-Source: APXvYqxLyF01n9S4J+YHIlhqsn0KfjNr8kb7thyp7wyqpnb5PTa9vgzJ2Bbug7lwrtlX2teVNtGVcTpizAFzrZsagO8= X-Received: by 2002:a67:7ac9:: with SMTP id v192mr29403611vsc.100.1555079532815; Fri, 12 Apr 2019 07:32:12 -0700 (PDT) MIME-Version: 1.0 References: <1745620.RIKK5OcHtz@xps> In-Reply-To: <1745620.RIKK5OcHtz@xps> From: David Marchand Date: Fri, 12 Apr 2019 16:32:01 +0200 Message-ID: To: Thomas Monjalon Cc: dev , Ferruh Yigit , Andrew Rybchenko , "Stokes, Ian" , Stephen Hemminger Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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: <20190412143201.kN0vBfQ9_z7u0m3L9ihjhsysV1dn_KzT6T0HKkM9c74@z> 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. -- David Marchand