[dpdk-dev] [PATCH v3 1/3] lib: add information metrics library

[Pattan, Reshma]

Hi,

Few comments below.

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton
> Sent: Friday, November 4, 2016 3:36 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v3 1/3] lib: add information metrics library
> 
> This patch adds a new information metric library that allows other modules
> to register named metrics and update their values. It is intended to be
> independent of ethdev, rather than mixing ethdev and non-ethdev
> information in xstats.
> 
> Signed-off-by: Remy Horton <remy.horton at intel.com>
> ---
> +int
> +
> +int
> +rte_metrics_get_values(int port_id,
> +	struct rte_stat_value *values,
> +	uint16_t capacity)
> +{
> +	struct rte_metrics_meta_s *entry;
> +	struct rte_metrics_data_s *stats;
> +	const struct rte_memzone *memzone;
> +	uint16_t idx_name;
> +	int return_value;
> +
> +	memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
> +	/* If not allocated, fail silently */
> +	if (memzone == NULL)
> +		return 0;
> +	stats = memzone->addr;
> +	rte_spinlock_lock(&stats->lock);
> +
> +	if (values != NULL) {
> +		if (capacity < stats->cnt_stats) {
> +			rte_spinlock_unlock(&stats->lock);
> +			return -ERANGE;
> +		}
> +		for (idx_name = 0; idx_name < stats->cnt_stats; idx_name++)
> {
> +			entry = &stats->metadata[idx_name];
> +			values[idx_name].key = idx_name;
> +			values[idx_name].value = entry->value[port_id];
> +		}

Here you also  need to include logic to return values for  port independent metrics.

> diff --git a/lib/librte_metrics/rte_metrics.h b/lib/librte_metrics/rte_metrics.h
> new file mode 100644
> index 0000000..6b75404
> --- /dev/null
> +++ b/lib/librte_metrics/rte_metrics.h
> @@ -0,0 +1,204 @@
> +/**
> + * Statistic name
> + */
> +struct rte_metric_name {
> +	/** String describing statistic */
> +	char name[RTE_METRICS_MAX_NAME_LEN];
> +};
> +
> +
> +/**
> + * Statistic name.
> + */

Need to correct the description to "stats values" or "metric values."

> +struct rte_stat_value {

Need to change the name to rte_metric_value.

> +	/** Numeric identifier of statistic */
> +	uint16_t key;
> +	/** Value for statistic */
> +	uint64_t value;
> +};
> +
> +
> +/**
> + * Initialises statistic module. This only has to be explicitly called

Typo < Initialises>

Thanks,
Reshma