This patch has checkpatch errors that will need to be fixed:
ERROR:SPACING: space prohibited before that close parenthesis ')'
#179: FILE: lib/librte_metrics/rte_metrics.c:233:
+ if (count < 1 )
ERROR:TRAILING_WHITESPACE: trailing whitespace
#242: FILE: lib/librte_metrics/rte_metrics.c:298:
+^I^Ifor (idx_name = 0; idx < stats->cnt_stats && $
If the patch is applied, two of the unit-tests for metrics break:
# ./test/build/app/test
RTE>>metrics_autotest
+ ------------------------------------------------------- +
+ Test Suite : Metrics Unit Test Suite
+ ------------------------------------------------------- +
+ TestCase [ 0] : test_metrics_without_init succeeded
+ TestCase [ 1] : test_metrics_reg_name_with_validname succeeded
+ TestCase [ 2] : test_metrics_reg_names succeeded
+ TestCase [ 3] : test_metrics_update_value failed
+ TestCase [ 4] : test_metrics_update_values failed
+ TestCase [ 5] : test_metrics_get_names succeeded
+ TestCase [ 6] : test_metrics_get_values succeeded
+ ------------------------------------------------------- +
+ Test Suite Summary
+ Tests Total : 7
+ Tests Skipped : 0
+ Tests Executed : 7
+ Tests Unsupported: 0
+ Tests Passed : 5
+ Tests Failed : 2
+ ------------------------------------------------------- +
Both of these issues will need to be addressed.
On 22/02/2019 15:39, wanjunjie wrote:
> From: junka <wan.junjie@foxmail.com>
>
> The bitmap will help maintain the metrics. We can dynamically
> add and remove metrics data. For example, after uninit latency lib,
> it could remove itself from the metrics. This could make the result
> from rte_metrics_get_names much more simple to display the wanted
> metrics data only.
>
> Signed-off-by: junka <wan.junjie@foxmail.com>
> ---
> lib/librte_metrics/rte_metrics.c | 182 ++++++++++++++++++++++++++++-----------
> lib/librte_metrics/rte_metrics.h | 21 +++++
> 2 files changed, 152 insertions(+), 51 deletions(-)
<snip>
@@ -12,6 +12,7 @@
#include <rte_lcore.h>
#include <rte_memzone.h>
#include <rte_spinlock.h>
+#include <rte_bitmap.h>
#define RTE_METRICS_MAX_METRICS 256
#define RTE_METRICS_MEMZONE_NAME "RTE_METRICS"
@@ -28,10 +29,7 @@ struct rte_metrics_meta_s {
uint64_t value[RTE_MAX_ETHPORTS];
/** Used for global metrics */
uint64_t global_value;
- /** Index of next root element (zero for none) */
- uint16_t idx_next_set;
- /** Index of next metric in set (zero for none) */
- uint16_t idx_next_stat;
+
};
/**
@@ -43,14 +41,12 @@ struct rte_metrics_meta_s {
* processes is not guaranteed.
*/
struct rte_metrics_data_s {
- /** Index of last metadata entry with valid data.
- * This value is not valid if cnt_stats is zero.
- */
- uint16_t idx_last_set;
/** Number of metrics. */
uint16_t cnt_stats;
/** Metric data memory block. */
struct rte_metrics_meta_s metadata[RTE_METRICS_MAX_METRICS];
+ /** Metric data bitmap in use */
+ struct rte_bitmap *bits;
/** Metric data access lock */
rte_spinlock_t lock;
};
@@ -60,6 +56,8 @@ struct rte_metrics_data_s {
{
struct rte_metrics_data_s *stats;
const struct rte_memzone *memzone;
+ uint32_t bmp_size;
+ void *bmp_mem;
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return;
@@ -73,6 +71,21 @@ struct rte_metrics_data_s {
rte_exit(EXIT_FAILURE, "Unable to allocate stats memzone\n");
stats = memzone->addr;
memset(stats, 0, sizeof(struct rte_metrics_data_s));
+
+ bmp_size =
+ rte_bitmap_get_memory_footprint(RTE_METRICS_MAX_METRICS);
+ bmp_mem = rte_malloc("metrics_bits", bmp_size,
+ RTE_CACHE_LINE_SIZE);
+ if (bmp_mem == NULL)
+ rte_exit(EXIT_FAILURE, "Failed to allocate metrics bitmap\n");
+
+ stats->bits = rte_bitmap_init(RTE_METRICS_MAX_METRICS,
+ bmp_mem, bmp_size);
+ if (stats->bits == NULL) {
+ rte_exit(EXIT_FAILURE, "Failed to init metrics bitmap\n");
+ rte_free(bmp_mem);
+ }
+
rte_spinlock_init(&stats->lock);
}
@@ -90,8 +103,8 @@ struct rte_metrics_data_s {
struct rte_metrics_meta_s *entry = NULL;
struct rte_metrics_data_s *stats;
const struct rte_memzone *memzone;
- uint16_t idx_name;
- uint16_t idx_base;
+ uint16_t idx_name, idx;
+ uint16_t idx_base = RTE_METRICS_MAX_METRICS;
/* Some sanity checks */
if (cnt_names < 1 || names == NULL)
@@ -110,19 +123,32 @@ struct rte_metrics_data_s {
rte_spinlock_lock(&stats->lock);
- /* Overwritten later if this is actually first set.. */
- stats->metadata[stats->idx_last_set].idx_next_set = stats->cnt_stats;
-
- stats->idx_last_set = idx_base = stats->cnt_stats;
-
- for (idx_name = 0; idx_name < cnt_names; idx_name++) {
- entry = &stats->metadata[idx_name + stats->cnt_stats];
- strlcpy(entry->name, names[idx_name], RTE_METRICS_MAX_NAME_LEN);
+ /* search for a continuous array, fail if not enough*/
+ for (idx_name = 0; idx_name < RTE_METRICS_MAX_METRICS; idx_name++) {
+ if (!rte_bitmap_get(stats->bits, idx_name)) {
+ idx_base = idx_name;
+ if (idx_base + cnt_names > RTE_METRICS_MAX_METRICS)
+ return -ENOMEM;
+ for (idx = idx_base; idx < idx_base + cnt_names; idx++) {
+ if (rte_bitmap_get(stats->bits, idx))
+ break;
+ }
+ if (idx == idx_base + cnt_names) {
+ break;
+ }
+ idx_name = idx;
+ }
+ }
+ if (idx_base == RTE_METRICS_MAX_METRICS) {
+ return -ENOMEM;
+ }
+ for (idx = idx_base; idx < idx_base + cnt_names; idx++) {
+ rte_bitmap_set(stats->bits, idx);
+ entry = &stats->metadata[idx];
+ strlcpy(entry->name, names[idx-idx_base], RTE_METRICS_MAX_NAME_LEN);
memset(entry->value, 0, sizeof(entry->value));
- entry->idx_next_stat = idx_name + stats->cnt_stats + 1;
+ entry->global_value = 0;
}
- entry->idx_next_stat = 0;
- entry->idx_next_set = 0;
stats->cnt_stats += cnt_names;
rte_spinlock_unlock(&stats->lock);
@@ -142,7 +168,6 @@ struct rte_metrics_data_s {
const uint64_t *values,
uint32_t count)
{
- struct rte_metrics_meta_s *entry;
struct rte_metrics_data_s *stats;
const struct rte_memzone *memzone;
uint16_t idx_metric;
@@ -163,18 +188,14 @@ struct rte_metrics_data_s {
rte_spinlock_lock(&stats->lock);
- if (key >= stats->cnt_stats) {
- rte_spinlock_unlock(&stats->lock);
- return -EINVAL;
- }
idx_metric = key;
cnt_setsize = 1;
- while (idx_metric < stats->cnt_stats) {
- entry = &stats->metadata[idx_metric];
- if (entry->idx_next_stat == 0)
+ while (idx_metric < RTE_METRICS_MAX_METRICS) {
+ if (rte_bitmap_get(stats->bits, idx_metric)) {
+ cnt_setsize++;
+ idx_metric++;
+ } else
break;
- cnt_setsize++;
- idx_metric++;
}
/* Check update does not cross set border */
if (count > cnt_setsize) {
@@ -194,17 +215,72 @@ struct rte_metrics_data_s {
stats->metadata[idx_metric].value[port_id] =
values[idx_value];
}
+
rte_spinlock_unlock(&stats->lock);
return 0;
}
int
+rte_metrics_unreg_values(uint16_t key, uint16_t count)
+{
+ struct rte_metrics_data_s *stats;
+ const struct rte_memzone *memzone;
+ uint16_t idx_metric;
+ uint16_t idx_value;
+ uint16_t cnt_setsize;
+
+ /* Some sanity checks */
+ if (count < 1 )
+ return -EINVAL;
+
+ memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
+ if (memzone == NULL)
+ return -EIO;
+
+ stats = memzone->addr;
+ if (stats->cnt_stats < count)
+ return -EINVAL;
+
+ if (key >= RTE_METRICS_MAX_METRICS)
+ return -EINVAL;
+
+ rte_spinlock_lock(&stats->lock);
+
+ idx_metric = key;
+ cnt_setsize = 1;
+ while (idx_metric < RTE_METRICS_MAX_METRICS) {
+ if (rte_bitmap_get(stats->bits, idx_metric)) {
+ cnt_setsize++;
+ idx_metric++;
+ } else
+ break;
+ }
+ /* Check update does not cross set border */
+ if (count > cnt_setsize) {
+ rte_spinlock_unlock(&stats->lock);
+ return -ERANGE;
+ }
+
+ for (idx_value = 0; idx_value < count; idx_value++) {
+ idx_metric = key + idx_value;
+ memset(stats->metadata[idx_metric].name, 0,
+ RTE_METRICS_MAX_NAME_LEN);
+ rte_bitmap_clear(stats->bits, idx_metric);
+ }
+ stats->cnt_stats -= count;
+ rte_spinlock_unlock(&stats->lock);
+
+ return 0;
+}
+
+
+int
rte_metrics_get_names(struct rte_metric_name *names,
uint16_t capacity)
{
struct rte_metrics_data_s *stats;
const struct rte_memzone *memzone;
- uint16_t idx_name;
+ uint16_t idx_name, idx = 0;
int return_value;
memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
@@ -219,10 +295,15 @@ struct rte_metrics_data_s {
rte_spinlock_unlock(&stats->lock);
return return_value;
}
- for (idx_name = 0; idx_name < stats->cnt_stats; idx_name++)
- strlcpy(names[idx_name].name,
- stats->metadata[idx_name].name,
- RTE_METRICS_MAX_NAME_LEN);
+ for (idx_name = 0; idx < stats->cnt_stats &&
+ idx_name < RTE_METRICS_MAX_METRICS; idx_name++) {
+ if (rte_bitmap_get(stats->bits, idx_name)) {
+ strlcpy(names[idx].name,
+ stats->metadata[idx_name].name,
+ RTE_METRICS_MAX_NAME_LEN);
+ idx++;
+ }
+ }
}
return_value = stats->cnt_stats;
rte_spinlock_unlock(&stats->lock);
@@ -237,7 +318,7 @@ struct rte_metrics_data_s {
struct rte_metrics_meta_s *entry;
struct rte_metrics_data_s *stats;
const struct rte_memzone *memzone;
- uint16_t idx_name;
+ uint16_t idx_name, idx = 0;
int return_value;
if (port_id != RTE_METRICS_GLOBAL &&
@@ -257,24 +338,23 @@ struct rte_metrics_data_s {
rte_spinlock_unlock(&stats->lock);
return return_value;
}
- if (port_id == RTE_METRICS_GLOBAL)
- 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->global_value;
- }
- else
- for (idx_name = 0;
- idx_name < stats->cnt_stats;
- idx_name++) {
+
+ for (idx_name = 0; idx < stats->cnt_stats &&
+ idx_name < RTE_METRICS_MAX_METRICS;
+ idx_name++) {
+ if (rte_bitmap_get(stats->bits, idx_name)) {
entry = &stats->metadata[idx_name];
- values[idx_name].key = idx_name;
- values[idx_name].value = entry->value[port_id];
+ values[idx].key = idx_name;
+ if (port_id == RTE_METRICS_GLOBAL)
+ values[idx].value = entry->global_value;
+ else
+ values[idx].value = entry->value[port_id];
+ idx++;
}
+ }
}
return_value = stats->cnt_stats;
rte_spinlock_unlock(&stats->lock);
+
return return_value;
}
@@ -123,6 +123,27 @@ struct rte_metric_value {
int rte_metrics_reg_names(const char * const *names, uint16_t cnt_names);
/**
+ * Unregister set of metrics.
+ *
+ * Remove the metrics previously registered
+ *
+ * @param key
+ * Id of metrics to remove
+ *
+ * @param count
+ * Number of metrics
+ *
+ * @return
+ * - Zero: Success
+ * - -EIO: Error, unable to access metrics shared memory
+ * (rte_metrics_init() not called)
+ * - -EINVAL: Error, invalid parameters
+ * - -ERANGE: Error, oversized
+ */
+int
+rte_metrics_unreg_values(uint16_t key, uint16_t count);
+
+/**
* Get metric name-key lookup table.
*
* @param names