From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCH v3 5/8] ring: queue stats mapping set dummy
	implementation
Date: Thu, 09 Jul 2015 12:13:56 +0200
Message-ID: <2174109.IecguKbUpa@xps13>
References: <1434723200-7528-1-git-send-email-tomaszx.kulasek@intel.com>
 <12941587.YRQEe5TYyV@xps13>
 <3042915272161B4EB253DA4D77EB373A019FD874@IRSMSX102.ger.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org
To: "Kulasek, TomaszX" <tomaszx.kulasek@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wi0-f177.google.com (mail-wi0-f177.google.com
 [209.85.212.177]) by dpdk.org (Postfix) with ESMTP id B3687E72
 for <dev@dpdk.org>; Thu,  9 Jul 2015 12:15:02 +0200 (CEST)
Received: by wibdq8 with SMTP id dq8so237464521wib.1
 for <dev@dpdk.org>; Thu, 09 Jul 2015 03:15:02 -0700 (PDT)
In-Reply-To: <3042915272161B4EB253DA4D77EB373A019FD874@IRSMSX102.ger.corp.intel.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>

2015-07-09 09:55, Kulasek, TomaszX:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Thursday, July 9, 2015 03:58
> > To: Kulasek, TomaszX
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v3 5/8] ring: queue stats mapping set dummy
> > implementation
> > 
> > 2015-06-29 16:50, Tomasz Kulasek:
> > > Per queue statistics are already implemented for ring device, but with
> > > static mapping (stat_idx == queue_id).
> > >
> > > This fix is required, if you want to use ring device in test
> > > application and is used only to point that per queue statistics are
> > > provided for this device.
> > >
> > > Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> > [...]
> > > +static int
> > > +eth_queue_stats_mapping_set(__rte_unused struct rte_eth_dev *dev,
> > > +		__rte_unused uint16_t queue_id, __rte_unused uint8_t stat_idx,
> > > +		__rte_unused uint8_t is_rx)
> > > +{
> > > +	/* Do nothing, just return ok */
> > > +	return 0;
> > > +}
> > 
> > I've just realized how this is broken.
> > Some Intel devices use a mapping to select hardware queues which will have
> > some stats. But we may have stats per queues without requiring such
> > mapping.
> > 
> > I may miss something but I suggest these 3 actions:
> > - remove this patch
> > - replace checks on stats_mapping by an ethdev flag
> > - remove device-specific stats_mapping from ethdev
> 
> 
> For Niantic NICs all queues stats for port are mapped to stats_idx=0 by default, and stats mapping is required to have per-queue statistics (even in testpmd). 
> 
> Anyway, this patch, for ring pmd, was intended more as a cleanup than feature and was inspired by implementation in virtio driver:
> 
> virtio_ethdev.c:
> 
> 	/*
> 	 * It enables testpmd to collect per queue stats.
> 	 */

It should be removed in virtio.

> 	static int
> 	virtio_dev_queue_stats_mapping_set(__rte_unused struct rte_eth_dev *eth_dev,
> 	__rte_unused uint16_t queue_id, __rte_unused uint8_t stat_idx,
> 	__rte_unused uint8_t is_rx)
> 	{
> 		return 0;
> 	}
> 
> 
> This patch can be safely removed, if you think such a cleanup is not required,

OK

> or lack of this implementation should be common behavior for this case.
> That will cause only few more warning messages, if you want to use ring pmd
> as a slave in example application.
> 
> Do you need v4?

No, thank you