From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCH v2] ethdev: check number of queues less than
	RTE_ETHDEV_QUEUE_STAT_CNTRS
Date: Mon, 28 Nov 2016 12:13:08 +0100
Message-ID: <7874818.0HvcekTQN4@xps13>
References: <1479722378-23959-1-git-send-email-alejandro.lucero@netronome.com>
 <1480006746.31853.14.camel@6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: Alejandro Lucero <alejandro.lucero@netronome.com>,
 Bert van Leeuwen <bert.vanleeuwen@netronome.com>
To: Olivier Matz <olivier.matz@6wind.com>, dev@dpdk.org
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f53.google.com (mail-wm0-f53.google.com [74.125.82.53])
 by dpdk.org (Postfix) with ESMTP id 4B581B6D
 for <dev@dpdk.org>; Mon, 28 Nov 2016 12:13:09 +0100 (CET)
Received: by mail-wm0-f53.google.com with SMTP id g23so202006185wme.1
 for <dev@dpdk.org>; Mon, 28 Nov 2016 03:13:09 -0800 (PST)
In-Reply-To: <1480006746.31853.14.camel@6wind.com>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

2016-11-24 17:59, Olivier Matz:
> Hi,
> 
> On Mon, 2016-11-21 at 09:59 +0000, Alejandro Lucero wrote:
> > From: Bert van Leeuwen <bert.vanleeuwen@netronome.com>
> > 
> > Arrays inside rte_eth_stats have size=RTE_ETHDEV_QUEUE_STAT_CNTRS.
> > Some devices report more queues than that and this code blindly uses
> > the reported number of queues by the device to fill those arrays up.
> > This patch fixes the problem using MIN between the reported number of
> > queues and RTE_ETHDEV_QUEUE_STAT_CNTRS.
> > 
> > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> > 
> 
> Reviewed-by: Olivier Matz <olivier.matz@6wind.com>
> 
> 
> As a next step, I'm wondering if it would be possible to remove
> this limitation. We could replace the tables in struct rte_eth_stats
> by a pointer to an array allocated dynamically at pmd setup.

Yes that's definitely the right way to handle these statistics.

> It would break the API, so it should be announced first. I'm thinking
> of something like:
> 
> struct rte_eth_generic_stats {
>         uint64_t ipackets;
>         uint64_t opackets;
>         uint64_t ibytes;
>         uint64_t obytes;
>         uint64_t imissed;
>         uint64_t ierrors;
>         uint64_t oerrors;
>         uint64_t rx_nombuf
> };
> 
> struct rte_eth_stats {
> 	struct rte_eth_generic_stats port_stats;
> 	struct rte_eth_generic_stats *queue_stats;
> };
> 
> The queue_stats array would always be indexed by queue_id.
> The xstats would continue to report the generic stats per-port and
> per-queue.
> 
> About the mapping API, either we keep it as-is, or it could
> become a driver-specific API.

Yes I agree to remove the queue statistics mapping which is very specific.
I will send a patch with a deprecation notice to move the mapping API
to a driver-specific API.

Any objection?