[dpdk-dev] [PATCH v3 5/8] ring: queue stats mapping set dummy implementation

[Thomas Monjalon]

2015-07-09 09:55, Kulasek, TomaszX:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > Sent: Thursday, July 9, 2015 03:58
> > To: Kulasek, TomaszX
> > Cc: dev at 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 at 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