[dpdk-dev] [RFC PATCH v1] rte: add bit-rate metrics to xstats

2016-08-29 Thread Remy Horton

On 29/08/2016 11:01, Pattan, Reshma wrote:
[..]
>> @@ -1506,6 +1508,19 @@ rte_eth_stats_reset(uint8_t port_id)
[..]
>> +/* Clear device running stat counts */
>> +dev_stats = >data->stats;
>> +memset(dev_stats->list_ibuckets, 0,
>> +sizeof(uint64_t) * dev_stats->cnt_buckets);
>> +memset(dev_stats->list_obuckets, 0,
>> +sizeof(uint64_t) * dev_stats->cnt_buckets);
>> +dev_stats->last_ibytes = 0;
>> +dev_stats->last_obytes = 0;
>> +dev_stats->peak_ibytes = 0;
>> +dev_stats->peak_obytes = 0;
>> +dev_stats->total_ibytes = 0;
>> +dev_stats->total_obytes = 0;

> Should the resetting has to be done inside rte_eth_xstats_reset() instead of 
> rte_eth_stats_reset()?

The bit-rate metrics are calculated from rte_eth_stats values rather 
than xstats values, which is why I put the reset here rather than in 
rte_eth_xstats_reset().

However this ought to be a moot point. I think it is a bug that 
rte_eth_xstats_reset() only calls rte_eth_stats_reset() if the driver 
doesn't implement xstats_reset, as the xstats output includes all the 
rte_eth_stats values unconditonally.



[dpdk-dev] [RFC PATCH v1] rte: add bit-rate metrics to xstats

2016-08-29 Thread Pattan, Reshma


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton
> Sent: Wednesday, August 24, 2016 3:58 PM
> To: thomas.monjalon at 6wind.com
> Cc: dev at dpdk.org
> Subject: [dpdk-dev] [RFC PATCH v1] rte: add bit-rate metrics to xstats
> 
> This patch adds peak and average data-rate metrics to the extended statistics.
> The intervals used to generate the statistics are controlled by any 
> application
> wishing to make use of these metrics.
> 
> Signed-off-by: Remy Horton 
> ---
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c 
> index
> f62a9ec..71549b4 100644
>  static const struct rte_eth_xstats_name_off rte_rxq_stats_strings[] = {
>   {"packets", offsetof(struct rte_eth_stats, q_ipackets)}, @@ -1499,6
> +1500,7 @@ void  rte_eth_stats_reset(uint8_t port_id)  {
>   struct rte_eth_dev *dev;
> + struct rte_eth_dev_stats *dev_stats;
> 
>   RTE_ETH_VALID_PORTID_OR_RET(port_id);
>   dev = _eth_devices[port_id];
> @@ -1506,6 +1508,19 @@ rte_eth_stats_reset(uint8_t port_id)
>   RTE_FUNC_PTR_OR_RET(*dev->dev_ops->stats_reset);
>   (*dev->dev_ops->stats_reset)(dev);
>   dev->data->rx_mbuf_alloc_failed = 0;
> +
> + /* Clear device running stat counts */
> + dev_stats = >data->stats;
> + memset(dev_stats->list_ibuckets, 0,
> + sizeof(uint64_t) * dev_stats->cnt_buckets);
> + memset(dev_stats->list_obuckets, 0,
> + sizeof(uint64_t) * dev_stats->cnt_buckets);
> + dev_stats->last_ibytes = 0;
> + dev_stats->last_obytes = 0;
> + dev_stats->peak_ibytes = 0;
> + dev_stats->peak_obytes = 0;
> + dev_stats->total_ibytes = 0;
> + dev_stats->total_obytes = 0;
>  }
> 

Should the resetting has to be done inside rte_eth_xstats_reset() instead of 
rte_eth_stats_reset()?

Thanks,
Reshma


[dpdk-dev] [RFC PATCH v1] rte: add bit-rate metrics to xstats

2016-08-26 Thread Pattan, Reshma
Hi,

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton
> Sent: Wednesday, August 24, 2016 3:58 PM
> To: thomas.monjalon at 6wind.com
> Cc: dev at dpdk.org
> Subject: [dpdk-dev] [RFC PATCH v1] rte: add bit-rate metrics to xstats
> 
> This patch adds peak and average data-rate metrics to the extended statistics.
> The intervals used to generate the statistics are controlled by any 
> application
> wishing to make use of these metrics.
> 
> Signed-off-by: Remy Horton 
> ---
> +int
> +rte_eth_dev_stats_init(uint8_t port_id, uint32_t cnt_buckets) {
> + struct rte_eth_dev *dev;
> + struct rte_eth_dev_stats *stats;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
> + dev = _eth_devices[port_id];
> + stats = >data->stats;
> +
> + memset(stats, 0, sizeof(struct rte_eth_dev_stats));
> + stats->list_ibuckets = rte_zmalloc(
> + NULL, sizeof(uint64_t) * cnt_buckets, 0);

We can have the sizeof(uint64_t) * cnt_buckets, calculated on top and use that 
in both the rte_zmallocs, Instead of performing * operation twice.

> + stats->list_obuckets = rte_zmalloc(
> + NULL, sizeof(uint64_t) * cnt_buckets, 0);
> + if (stats->list_ibuckets == NULL || stats->list_obuckets == NULL)
> + return -ENOMEM;

If  either of them has valid pointer we should free that before returning.

> + stats->cnt_buckets = cnt_buckets;
> + return 0;
> +}
> +

Thanks,
Reshma


[dpdk-dev] [RFC PATCH v1] rte: add bit-rate metrics to xstats

2016-08-24 Thread Remy Horton
This patch adds peak and average data-rate metrics to the extended
statistics. The intervals used to generate the statistics are
controlled by any application wishing to make use of these metrics.

Signed-off-by: Remy Horton 
---
 doc/guides/rel_notes/release_16_11.rst |   5 ++
 lib/librte_ether/rte_ethdev.c  | 107 -
 lib/librte_ether/rte_ethdev.h  |  41 +
 lib/librte_ether/rte_ether_version.map |   6 ++
 4 files changed, 157 insertions(+), 2 deletions(-)

diff --git a/doc/guides/rel_notes/release_16_11.rst 
b/doc/guides/rel_notes/release_16_11.rst
index 0b9022d..b319292 100644
--- a/doc/guides/rel_notes/release_16_11.rst
+++ b/doc/guides/rel_notes/release_16_11.rst
@@ -36,6 +36,11 @@ New Features

  This section is a comment. Make sure to start the actual text at the 
margin.

+   * **Added data-rate metrics to the extended statistics.**
+
+ Adds peak and average incoming and outgoing data-rate metrics to the
+ extended statistics, the calculation of which is controlled by
+ applications that wish for these to be derived.

 Resolved Issues
 ---
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index f62a9ec..71549b4 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -101,6 +101,7 @@ static const struct rte_eth_xstats_name_off 
rte_stats_strings[] = {
 };

 #define RTE_NB_STATS (sizeof(rte_stats_strings) / sizeof(rte_stats_strings[0]))
+#define RTE_NB_DEV_STATS 4

 static const struct rte_eth_xstats_name_off rte_rxq_stats_strings[] = {
{"packets", offsetof(struct rte_eth_stats, q_ipackets)},
@@ -1499,6 +1500,7 @@ void
 rte_eth_stats_reset(uint8_t port_id)
 {
struct rte_eth_dev *dev;
+   struct rte_eth_dev_stats *dev_stats;

RTE_ETH_VALID_PORTID_OR_RET(port_id);
dev = _eth_devices[port_id];
@@ -1506,6 +1508,19 @@ rte_eth_stats_reset(uint8_t port_id)
RTE_FUNC_PTR_OR_RET(*dev->dev_ops->stats_reset);
(*dev->dev_ops->stats_reset)(dev);
dev->data->rx_mbuf_alloc_failed = 0;
+
+   /* Clear device running stat counts */
+   dev_stats = >data->stats;
+   memset(dev_stats->list_ibuckets, 0,
+   sizeof(uint64_t) * dev_stats->cnt_buckets);
+   memset(dev_stats->list_obuckets, 0,
+   sizeof(uint64_t) * dev_stats->cnt_buckets);
+   dev_stats->last_ibytes = 0;
+   dev_stats->last_obytes = 0;
+   dev_stats->peak_ibytes = 0;
+   dev_stats->peak_obytes = 0;
+   dev_stats->total_ibytes = 0;
+   dev_stats->total_obytes = 0;
 }

 static int
@@ -1522,7 +1537,7 @@ get_xstats_count(uint8_t port_id)
return count;
} else
count = 0;
-   count += RTE_NB_STATS;
+   count += RTE_NB_STATS + RTE_NB_DEV_STATS;
count += dev->data->nb_rx_queues * RTE_NB_RXQ_STATS;
count += dev->data->nb_tx_queues * RTE_NB_TXQ_STATS;
return count;
@@ -1574,6 +1589,19 @@ rte_eth_xstats_get_names(uint8_t port_id,
}
}

+   snprintf(xstats_names[cnt_used_entries++].name,
+   sizeof(xstats_names[0].name),
+   "tx_peak_bytes");
+   snprintf(xstats_names[cnt_used_entries++].name,
+   sizeof(xstats_names[0].name),
+   "tx_mean_bytes");
+   snprintf(xstats_names[cnt_used_entries++].name,
+   sizeof(xstats_names[0].name),
+   "rx_peak_bytes");
+   snprintf(xstats_names[cnt_used_entries++].name,
+   sizeof(xstats_names[0].name),
+   "rx_mean_bytes");
+
if (dev->dev_ops->xstats_get_names != NULL) {
/* If there are any driver-specific xstats, append them
 * to end of list.
@@ -1600,14 +1628,16 @@ rte_eth_xstats_get(uint8_t port_id, struct 
rte_eth_xstat *xstats,
unsigned count = 0, i, q;
signed xcount = 0;
uint64_t val, *stats_ptr;
+   struct rte_eth_dev_stats *dev_stats;

RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);

dev = _eth_devices[port_id];
+   dev_stats = >data->stats;

/* Return generic statistics */
count = RTE_NB_STATS + (dev->data->nb_rx_queues * RTE_NB_RXQ_STATS) +
-   (dev->data->nb_tx_queues * RTE_NB_TXQ_STATS);
+   (dev->data->nb_tx_queues * RTE_NB_TXQ_STATS) + RTE_NB_DEV_STATS;

/* implemented by the driver */
if (dev->dev_ops->xstats_get != NULL) {
@@ -1659,12 +1689,85 @@ rte_eth_xstats_get(uint8_t port_id, struct 
rte_eth_xstat *xstats,
}
}

+   xstats[count++].value = dev_stats->peak_obytes;
+   xstats[count++].value =
+   dev_stats->total_obytes / dev_stats->cnt_buckets;
+   xstats[count++].value = dev_stats->peak_ibytes;
+   xstats[count++].value =
+   dev_stats->total_ibytes / dev_stats->cnt_buckets;
+
for (i = 0; i < count + xcount; i++)