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

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>
---
 drivers/net/ring/rte_eth_ring.c |   10 ++++++++++
 1 file changed, 10 insertions(+)
  

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
  

> 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.
	 */
	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, 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?
  

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