* [dpdk-dev] [PATCH] lib/metrics: add unregister api for metrics
@ 2019-02-22 6:16 =?gb18030?B?zfK/ob3c?=
2019-02-22 13:56 ` [dpdk-dev] [PATCH v2] " wanjunjie
0 siblings, 1 reply; 13+ messages in thread
From: =?gb18030?B?zfK/ob3c?= @ 2019-02-22 6:16 UTC (permalink / raw)
To: =?gb18030?B?ZGV2?=
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb18030", Size: 10102 bytes --]
Use the bitmap lib to help maintain the metrics. we can dynamically
add and remove metrics data. After uninit latency, it can
remove itself from the metrics to make the result from rte_metrics_get_names more simple.
Signed-off-by: junka <wan.junjie@foxmail.com>
---
lib/librte_latencystats/rte_latencystats.c | 4 +-
lib/librte_metrics/rte_metrics.c | 189 +++++++++++++++++++++--------
lib/librte_metrics/rte_metrics.h | 21 ++++
3 files changed, 161 insertions(+), 53 deletions(-)
diff --git a/lib/librte_latencystats/rte_latencystats.c b/lib/librte_latencystats/rte_latencystats.c
index 5715549..b18c3ea 100644
--- a/lib/librte_latencystats/rte_latencystats.c
+++ b/lib/librte_latencystats/rte_latencystats.c
@@ -291,7 +291,9 @@ struct latency_stats_nameoff {
"qid=%d\n", pid, qid);
}
}
-
+
+rte_metrics_unreg_values(latency_stats_index, NUM_LATENCY_STATS);
+
/* free up the memzone */
mz = rte_memzone_lookup(MZ_RTE_LATENCY_STATS);
if (mz)
diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
index 99a96b6..704e5ae 100644
--- a/lib/librte_metrics/rte_metrics.c
+++ b/lib/librte_metrics/rte_metrics.c
@@ -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,7 @@ struct rte_metrics_data_s {
{
struct rte_metrics_data_s *stats;
const struct rte_memzone *memzone;
+uint32_t bmp_size;
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return;
@@ -73,6 +70,24 @@ 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);
+void *bmpmem = rte_malloc("metrics_bits", bmp_size,
+RTE_CACHE_LINE_SIZE);
+if (bmpmem == NULL) {
+rte_exit(EXIT_FAILURE,
+"Failed to allocate vlan bitmap for device\n");
+}
+
+stats->bits = rte_bitmap_init(RTE_METRICS_MAX_METRICS,
+bmpmem, bmp_size);
+if (stats->bits == NULL) {
+rte_exit(EXIT_FAILURE,
+"Failed to init vlan bitmap for bonded device \n");
+rte_free(bmpmem);
+}
+
rte_spinlock_init(&stats->lock);
}
@@ -90,7 +105,7 @@ 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_name,idx;
uint16_t idx_base;
/* Some sanity checks */
@@ -110,19 +125,36 @@ 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);
-memset(entry->value, 0, sizeof(entry->value));
-entry->idx_next_stat = idx_name + stats->cnt_stats + 1;
+ //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;
+}
}
-entry->idx_next_stat = 0;
-entry->idx_next_set = 0;
+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->global_value = 0;
+ }
stats->cnt_stats += cnt_names;
rte_spinlock_unlock(&stats->lock);
@@ -142,7 +174,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 +194,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)
-break;
-cnt_setsize++;
-idx_metric++;
+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) {
@@ -199,12 +226,67 @@ struct rte_metrics_data_s {
}
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 +301,14 @@ 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 +323,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,22 +343,21 @@ 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);
diff --git a/lib/librte_metrics/rte_metrics.h b/lib/librte_metrics/rte_metrics.h
index 67a60fa..fb1859b 100644
--- a/lib/librte_metrics/rte_metrics.h
+++ b/lib/librte_metrics/rte_metrics.h
@@ -123,6 +123,27 @@ struct rte_metric_value {
int rte_metrics_reg_names(const char * const *names, uint16_t cnt_names);
/**
+ * Un-register set of metrics.
+ *
+ * Remove the metrics previously registerd
+ *
+ * @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
--
1.8.3.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH v2] lib/metrics: add unregister api for metrics
2019-02-22 6:16 [dpdk-dev] [PATCH] lib/metrics: add unregister api for metrics =?gb18030?B?zfK/ob3c?=
@ 2019-02-22 13:56 ` wanjunjie
2019-02-22 15:03 ` [dpdk-dev] [PATCH v3] " wanjunjie
0 siblings, 1 reply; 13+ messages in thread
From: wanjunjie @ 2019-02-22 13:56 UTC (permalink / raw)
To: dev; +Cc: junka
From: junka <wan.junjie@foxmail.com>
The bitmap will help maintain the metrics. We can dynamically
add and remove metrics data. After uninit latency, it can
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_latencystats/rte_latencystats.c | 7 +-
lib/librte_metrics/rte_metrics.c | 188 +++++++++++++++++++++--------
lib/librte_metrics/rte_metrics.h | 21 ++++
3 files changed, 163 insertions(+), 53 deletions(-)
mode change 100644 => 100755 lib/librte_latencystats/rte_latencystats.c
mode change 100644 => 100755 lib/librte_metrics/rte_metrics.c
diff --git a/lib/librte_latencystats/rte_latencystats.c b/lib/librte_latencystats/rte_latencystats.c
old mode 100644
new mode 100755
index 5715549..8f74b59
--- a/lib/librte_latencystats/rte_latencystats.c
+++ b/lib/librte_latencystats/rte_latencystats.c
@@ -31,7 +31,7 @@
#define RTE_LOGTYPE_LATENCY_STATS RTE_LOGTYPE_USER1
static const char *MZ_RTE_LATENCY_STATS = "rte_latencystats";
-static int latency_stats_index;
+static int latency_stats_index = -1;
static uint64_t samp_intvl;
static uint64_t timer_tsc;
static uint64_t prev_tsc;
@@ -291,7 +291,10 @@ struct latency_stats_nameoff {
"qid=%d\n", pid, qid);
}
}
-
+
+ rte_metrics_unreg_values(latency_stats_index, NUM_LATENCY_STATS);
+ latency_stats_index = -1;
+
/* free up the memzone */
mz = rte_memzone_lookup(MZ_RTE_LATENCY_STATS);
if (mz)
diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
old mode 100644
new mode 100755
index 99a96b6..2100b63
--- a/lib/librte_metrics/rte_metrics.c
+++ b/lib/librte_metrics/rte_metrics.c
@@ -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,7 @@ struct rte_metrics_data_s {
{
struct rte_metrics_data_s *stats;
const struct rte_memzone *memzone;
+ uint32_t bmp_size;
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return;
@@ -73,6 +70,24 @@ 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);
+ void *bmpmem = rte_malloc("metrics_bits", bmp_size,
+ RTE_CACHE_LINE_SIZE);
+ if (bmpmem == NULL) {
+ rte_exit(EXIT_FAILURE,
+ "Failed to allocate vlan bitmap for device\n");
+ }
+
+ stats->bits = rte_bitmap_init(RTE_METRICS_MAX_METRICS,
+ bmpmem, bmp_size);
+ if (stats->bits == NULL) {
+ rte_exit(EXIT_FAILURE,
+ "Failed to init vlan bitmap for bonded device \n");
+ rte_free(bmpmem);
+ }
+
rte_spinlock_init(&stats->lock);
}
@@ -90,7 +105,7 @@ 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_name,idx;
uint16_t idx_base;
/* Some sanity checks */
@@ -110,19 +125,36 @@ 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);
- memset(entry->value, 0, sizeof(entry->value));
- entry->idx_next_stat = idx_name + stats->cnt_stats + 1;
+ /* 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;
+ }
}
- entry->idx_next_stat = 0;
- entry->idx_next_set = 0;
+ 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->global_value = 0;
+ }
stats->cnt_stats += cnt_names;
rte_spinlock_unlock(&stats->lock);
@@ -142,7 +174,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 +194,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) {
@@ -199,12 +226,67 @@ struct rte_metrics_data_s {
}
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 +301,14 @@ 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 +323,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 +343,24 @@ 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;
}
diff --git a/lib/librte_metrics/rte_metrics.h b/lib/librte_metrics/rte_metrics.h
index 67a60fa..fb1859b 100644
--- a/lib/librte_metrics/rte_metrics.h
+++ b/lib/librte_metrics/rte_metrics.h
@@ -123,6 +123,27 @@ struct rte_metric_value {
int rte_metrics_reg_names(const char * const *names, uint16_t cnt_names);
/**
+ * Un-register set of metrics.
+ *
+ * Remove the metrics previously registerd
+ *
+ * @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
--
1.8.3.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH v3] lib/metrics: add unregister api for metrics
2019-02-22 13:56 ` [dpdk-dev] [PATCH v2] " wanjunjie
@ 2019-02-22 15:03 ` wanjunjie
2019-02-22 15:39 ` [dpdk-dev] [PATCH v4] " wanjunjie
0 siblings, 1 reply; 13+ messages in thread
From: wanjunjie @ 2019-02-22 15:03 UTC (permalink / raw)
To: dev; +Cc: junka
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 10494 bytes --]
From: junka <wan.junjie@foxmail.com>
The bitmap will help maintain the metrics. We can dynamically
add and remove metrics data. After uninit latency, it can
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_latencystats/rte_latencystats.c | 5 +-
lib/librte_metrics/rte_metrics.c | 180 +++++++++++++++++++++--------
lib/librte_metrics/rte_metrics.h | 21 ++++
3 files changed, 155 insertions(+), 51 deletions(-)
diff --git a/lib/librte_latencystats/rte_latencystats.c b/lib/librte_latencystats/rte_latencystats.c
index 5715549..2d3474c 100644
--- a/lib/librte_latencystats/rte_latencystats.c
+++ b/lib/librte_latencystats/rte_latencystats.c
@@ -31,7 +31,7 @@
#define RTE_LOGTYPE_LATENCY_STATS RTE_LOGTYPE_USER1
static const char *MZ_RTE_LATENCY_STATS = "rte_latencystats";
-static int latency_stats_index;
+static int latency_stats_index = -1;
static uint64_t samp_intvl;
static uint64_t timer_tsc;
static uint64_t prev_tsc;
@@ -292,6 +292,9 @@ struct latency_stats_nameoff {
}
}
+ rte_metrics_unreg_values(latency_stats_index, NUM_LATENCY_STATS);
+ latency_stats_index = -1;
+
/* free up the memzone */
mz = rte_memzone_lookup(MZ_RTE_LATENCY_STATS);
if (mz)
diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
index 99a96b6..8b6e863 100644
--- a/lib/librte_metrics/rte_metrics.c
+++ b/lib/librte_metrics/rte_metrics.c
@@ -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,22 @@ 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,7 +104,7 @@ 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_name, idx;
uint16_t idx_base;
/* Some sanity checks */
@@ -110,19 +124,30 @@ 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;
+ }
+ }
+ 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 +167,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 +187,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 +214,73 @@ 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;
}
diff --git a/lib/librte_metrics/rte_metrics.h b/lib/librte_metrics/rte_metrics.h
index 67a60fa..afde1c0 100644
--- a/lib/librte_metrics/rte_metrics.h
+++ b/lib/librte_metrics/rte_metrics.h
@@ -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
--
1.8.3.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH v4] lib/metrics: add unregister api for metrics
2019-02-22 15:03 ` [dpdk-dev] [PATCH v3] " wanjunjie
@ 2019-02-22 15:39 ` wanjunjie
2019-02-26 16:10 ` Remy Horton
2019-02-27 17:19 ` [dpdk-dev] [PATCH v5] " Junjie Wan
0 siblings, 2 replies; 13+ messages in thread
From: wanjunjie @ 2019-02-22 15:39 UTC (permalink / raw)
To: dev; +Cc: junka
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(-)
diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
index 99a96b6..767baf4 100644
--- a/lib/librte_metrics/rte_metrics.c
+++ b/lib/librte_metrics/rte_metrics.c
@@ -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;
}
diff --git a/lib/librte_metrics/rte_metrics.h b/lib/librte_metrics/rte_metrics.h
index 67a60fa..afde1c0 100644
--- a/lib/librte_metrics/rte_metrics.h
+++ b/lib/librte_metrics/rte_metrics.h
@@ -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
--
1.8.3.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v4] lib/metrics: add unregister api for metrics
2019-02-22 15:39 ` [dpdk-dev] [PATCH v4] " wanjunjie
@ 2019-02-26 16:10 ` Remy Horton
2019-02-27 17:19 ` [dpdk-dev] [PATCH v5] " Junjie Wan
1 sibling, 0 replies; 13+ messages in thread
From: Remy Horton @ 2019-02-26 16:10 UTC (permalink / raw)
To: wanjunjie, dev
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>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH v5] lib/metrics: add unregister api for metrics
2019-02-22 15:39 ` [dpdk-dev] [PATCH v4] " wanjunjie
2019-02-26 16:10 ` Remy Horton
@ 2019-02-27 17:19 ` Junjie Wan
2019-02-28 11:53 ` Remy Horton
2019-04-02 0:27 ` Thomas Monjalon
1 sibling, 2 replies; 13+ messages in thread
From: Junjie Wan @ 2019-02-27 17:19 UTC (permalink / raw)
To: remy.horton; +Cc: dev, junka
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: Junjie Wan <wan.junjie@foxmail.com>
---
v5:
* Fix test case and coding style issues
lib/librte_metrics/rte_metrics.c | 221 ++++++++++++++++++++++++++-------------
lib/librte_metrics/rte_metrics.h | 21 ++++
2 files changed, 170 insertions(+), 72 deletions(-)
diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
index 99a96b6..d3bbdd5 100644
--- a/lib/librte_metrics/rte_metrics.c
+++ b/lib/librte_metrics/rte_metrics.c
@@ -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,33 @@ 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;
+ /* 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_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);
+ 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 +169,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 +189,69 @@ struct rte_metrics_data_s {
rte_spinlock_lock(&stats->lock);
- if (key >= stats->cnt_stats) {
+ idx_metric = key;
+ cnt_setsize = 0;
+ 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 (cnt_setsize == 0 || count > cnt_setsize) {
rte_spinlock_unlock(&stats->lock);
return -EINVAL;
}
+
+ for (idx_value = 0; idx_value < count; idx_value++) {
+ idx_metric = key + idx_value;
+ if (port_id == RTE_METRICS_GLOBAL)
+ stats->metadata[idx_metric].global_value =
+ values[idx_value];
+ else
+ 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 < 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) {
@@ -182,29 +259,26 @@ struct rte_metrics_data_s {
return -ERANGE;
}
- if (port_id == RTE_METRICS_GLOBAL)
- for (idx_value = 0; idx_value < count; idx_value++) {
- idx_metric = key + idx_value;
- stats->metadata[idx_metric].global_value =
- values[idx_value];
- }
- else
- for (idx_value = 0; idx_value < count; idx_value++) {
- idx_metric = key + idx_value;
- stats->metadata[idx_metric].value[port_id] =
- values[idx_value];
- }
+ 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);
@@ -213,18 +287,23 @@ struct rte_metrics_data_s {
stats = memzone->addr;
rte_spinlock_lock(&stats->lock);
- if (names != NULL) {
- if (capacity < stats->cnt_stats) {
- return_value = stats->cnt_stats;
- rte_spinlock_unlock(&stats->lock);
- return return_value;
- }
- for (idx_name = 0; idx_name < stats->cnt_stats; idx_name++)
- strlcpy(names[idx_name].name,
+
+ return_value = stats->cnt_stats;
+ if (names == NULL || capacity < stats->cnt_stats) {
+ rte_spinlock_unlock(&stats->lock);
+ return return_value;
+ }
+
+ 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);
return return_value;
}
@@ -237,7 +316,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 &&
@@ -251,30 +330,28 @@ struct rte_metrics_data_s {
stats = memzone->addr;
rte_spinlock_lock(&stats->lock);
- if (values != NULL) {
- if (capacity < stats->cnt_stats) {
- return_value = stats->cnt_stats;
- rte_spinlock_unlock(&stats->lock);
- return return_value;
+ return_value = stats->cnt_stats;
+
+ if (values == NULL || capacity < stats->cnt_stats) {
+ rte_spinlock_unlock(&stats->lock);
+ return return_value;
+ }
+
+ 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].key = idx_name;
+ if (port_id == RTE_METRICS_GLOBAL)
+ values[idx].value = entry->global_value;
+ else
+ values[idx].value = entry->value[port_id];
+ idx++;
}
- 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++) {
- entry = &stats->metadata[idx_name];
- values[idx_name].key = idx_name;
- values[idx_name].value = entry->value[port_id];
- }
}
- return_value = stats->cnt_stats;
+
rte_spinlock_unlock(&stats->lock);
+
return return_value;
}
diff --git a/lib/librte_metrics/rte_metrics.h b/lib/librte_metrics/rte_metrics.h
index 67a60fa..afde1c0 100644
--- a/lib/librte_metrics/rte_metrics.h
+++ b/lib/librte_metrics/rte_metrics.h
@@ -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
--
1.8.3.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v5] lib/metrics: add unregister api for metrics
2019-02-27 17:19 ` [dpdk-dev] [PATCH v5] " Junjie Wan
@ 2019-02-28 11:53 ` Remy Horton
2019-02-28 13:28 ` Wan Junjie
2019-04-02 0:27 ` Thomas Monjalon
1 sibling, 1 reply; 13+ messages in thread
From: Remy Horton @ 2019-02-28 11:53 UTC (permalink / raw)
To: Junjie Wan; +Cc: dev
The tests for metrics themselves now all pass, but there is failure with
bitrate statistics (bitrate stats uses metrics underneath). Latency
statistics, which also uses metrics, seems ok though.
RTE>>bitratestats_autotest
+ ------------------------------------------------------- +
+ Test Suite : BitRate Stats Unit Test Suite
port in ring setup : 0
+ ------------------------------------------------------- +
+ TestCase [ 0] : test_stats_bitrate_create succeeded
+ TestCase [ 1] : test_stats_bitrate_reg failed
+ TestCase [ 2] : test_stats_bitrate_reg_invalidpointer succeeded
+ TestCase [ 3] : test_stats_bitrate_calc_invalid_bitrate_data succeeded
Invalid port_id=33
+ TestCase [ 4] : test_stats_bitrate_calc_invalid_portid_1 succeeded
Invalid port_id=65535
+ TestCase [ 5] : test_stats_bitrate_calc_invalid_portid_2 succeeded
Invalid port_id=31
+ TestCase [ 6] : test_stats_bitrate_calc_non_existing_portid succeeded
+ TestCase [ 7] : test_stats_bitrate_calc succeeded
+ ------------------------------------------------------- +
+ Test Suite Summary
+ Tests Total : 8
+ Tests Skipped : 0
+ Tests Executed : 8
+ Tests Unsupported: 0
+ Tests Passed : 7
+ Tests Failed : 1
+ ------------------------------------------------------- +
Test Failed
Personally if I was reimplementing metrics I would be inclined to
replace the single fixed-size metadata[] array that is carved up with
something more dynamic, perhaps a 2d linked-list. Doing so would
probably negate the need for a bitmap.
On 27/02/2019 17:19, Junjie Wan 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: Junjie Wan <wan.junjie@foxmail.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v5] lib/metrics: add unregister api for metrics
2019-02-28 11:53 ` Remy Horton
@ 2019-02-28 13:28 ` Wan Junjie
2019-02-28 13:55 ` Remy Horton
0 siblings, 1 reply; 13+ messages in thread
From: Wan Junjie @ 2019-02-28 13:28 UTC (permalink / raw)
To: Remy Horton; +Cc: dev
Bitratestats test fails even without my patch. The reason is that metrics lib has no de-init process. Harman Kalra has submited a patch for this.
Check https://patches.dpdk.org/patch/50451
As for metrics lib, the array block of metadata may be better on cache side. Since rte_metrics_update_values is called periodicly in bitrate lib.
---Original---
From: "Remy Horton"<remy.horton@intel.com>
Date: Thu, Feb 28, 2019 19:53 PM
To: "Junjie Wan"<wan.junjie@foxmail.com>;
Cc: "dev"<dev@dpdk.org>;
Subject: Re: [PATCH v5] lib/metrics: add unregister api for metrics
The tests for metrics themselves now all pass, but there is failure with
bitrate statistics (bitrate stats uses metrics underneath). Latency
statistics, which also uses metrics, seems ok though.
RTE>>bitratestats_autotest
+ ------------------------------------------------------- +
+ Test Suite : BitRate Stats Unit Test Suite
port in ring setup : 0
+ ------------------------------------------------------- +
+ TestCase [ 0] : test_stats_bitrate_create succeeded
+ TestCase [ 1] : test_stats_bitrate_reg failed
+ TestCase [ 2] : test_stats_bitrate_reg_invalidpointer succeeded
+ TestCase [ 3] : test_stats_bitrate_calc_invalid_bitrate_data succeeded
Invalid port_id=33
+ TestCase [ 4] : test_stats_bitrate_calc_invalid_portid_1 succeeded
Invalid port_id=65535
+ TestCase [ 5] : test_stats_bitrate_calc_invalid_portid_2 succeeded
Invalid port_id=31
+ TestCase [ 6] : test_stats_bitrate_calc_non_existing_portid succeeded
+ TestCase [ 7] : test_stats_bitrate_calc succeeded
+ ------------------------------------------------------- +
+ Test Suite Summary
+ Tests Total : 8
+ Tests Skipped : 0
+ Tests Executed : 8
+ Tests Unsupported: 0
+ Tests Passed : 7
+ Tests Failed : 1
+ ------------------------------------------------------- +
Test Failed
Personally if I was reimplementing metrics I would be inclined to
replace the single fixed-size metadata[] array that is carved up with
something more dynamic, perhaps a 2d linked-list. Doing so would
probably negate the need for a bitmap.
On 27/02/2019 17:19, Junjie Wan 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: Junjie Wan <wan.junjie@foxmail.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v5] lib/metrics: add unregister api for metrics
2019-02-28 13:28 ` Wan Junjie
@ 2019-02-28 13:55 ` Remy Horton
0 siblings, 0 replies; 13+ messages in thread
From: Remy Horton @ 2019-02-28 13:55 UTC (permalink / raw)
To: Wan Junjie; +Cc: dev
On 28/02/2019 13:28, Wan Junjie wrote:
> Bitratestats test fails even without my patch. The reason is that
> metrics lib has no de-init process. Harman Kalra has submited a patch
> for this.
> Check https://patches.dpdk.org/patch/50451
Ah forgot about that patch. :S
> As for metrics lib, the array block of metadata may be better on cache
> side. Since rte_metrics_update_values is called periodicly in bitrate lib.
Quite possibly, at least for smallish datasets like bitrate.
>> Signed-off-by: Junjie Wan <wan.junjie@foxmail.com>
Acked-by: Remy Horton <remy.horton@intel.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v5] lib/metrics: add unregister api for metrics
2019-02-27 17:19 ` [dpdk-dev] [PATCH v5] " Junjie Wan
2019-02-28 11:53 ` Remy Horton
@ 2019-04-02 0:27 ` Thomas Monjalon
2019-04-02 0:27 ` Thomas Monjalon
2019-04-02 0:30 ` Thomas Monjalon
1 sibling, 2 replies; 13+ messages in thread
From: Thomas Monjalon @ 2019-04-02 0:27 UTC (permalink / raw)
To: Junjie Wan; +Cc: dev, remy.horton
27/02/2019 18:19, Junjie Wan:
> 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: Junjie Wan <wan.junjie@foxmail.com>
> ---
> --- a/lib/librte_metrics/rte_metrics.h
> +++ b/lib/librte_metrics/rte_metrics.h
> /**
> + * 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);
The rule is to add new API as experimental for some time.
Please check how other new API functions are introduced.
You will need to add the function in rte_metrics_version.map
to make it work in a shared library.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v5] lib/metrics: add unregister api for metrics
2019-04-02 0:27 ` Thomas Monjalon
@ 2019-04-02 0:27 ` Thomas Monjalon
2019-04-02 0:30 ` Thomas Monjalon
1 sibling, 0 replies; 13+ messages in thread
From: Thomas Monjalon @ 2019-04-02 0:27 UTC (permalink / raw)
To: Junjie Wan; +Cc: dev, remy.horton
27/02/2019 18:19, Junjie Wan:
> 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: Junjie Wan <wan.junjie@foxmail.com>
> ---
> --- a/lib/librte_metrics/rte_metrics.h
> +++ b/lib/librte_metrics/rte_metrics.h
> /**
> + * 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);
The rule is to add new API as experimental for some time.
Please check how other new API functions are introduced.
You will need to add the function in rte_metrics_version.map
to make it work in a shared library.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v5] lib/metrics: add unregister api for metrics
2019-04-02 0:27 ` Thomas Monjalon
2019-04-02 0:27 ` Thomas Monjalon
@ 2019-04-02 0:30 ` Thomas Monjalon
2019-04-02 0:30 ` Thomas Monjalon
1 sibling, 1 reply; 13+ messages in thread
From: Thomas Monjalon @ 2019-04-02 0:30 UTC (permalink / raw)
To: Junjie Wan; +Cc: dev, remy.horton
02/04/2019 02:27, Thomas Monjalon:
> 27/02/2019 18:19, Junjie Wan:
> > 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: Junjie Wan <wan.junjie@foxmail.com>
> > ---
> > --- a/lib/librte_metrics/rte_metrics.h
> > +++ b/lib/librte_metrics/rte_metrics.h
> > /**
> > + * 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);
>
> The rule is to add new API as experimental for some time.
> Please check how other new API functions are introduced.
>
> You will need to add the function in rte_metrics_version.map
> to make it work in a shared library.
Few more requests:
Please would you like to add a test in app/test/test_metrics.c?
And maybe add some words in doc/guides/prog_guide/metrics_lib.rst?
Thanks
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v5] lib/metrics: add unregister api for metrics
2019-04-02 0:30 ` Thomas Monjalon
@ 2019-04-02 0:30 ` Thomas Monjalon
0 siblings, 0 replies; 13+ messages in thread
From: Thomas Monjalon @ 2019-04-02 0:30 UTC (permalink / raw)
To: Junjie Wan; +Cc: dev, remy.horton
02/04/2019 02:27, Thomas Monjalon:
> 27/02/2019 18:19, Junjie Wan:
> > 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: Junjie Wan <wan.junjie@foxmail.com>
> > ---
> > --- a/lib/librte_metrics/rte_metrics.h
> > +++ b/lib/librte_metrics/rte_metrics.h
> > /**
> > + * 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);
>
> The rule is to add new API as experimental for some time.
> Please check how other new API functions are introduced.
>
> You will need to add the function in rte_metrics_version.map
> to make it work in a shared library.
Few more requests:
Please would you like to add a test in app/test/test_metrics.c?
And maybe add some words in doc/guides/prog_guide/metrics_lib.rst?
Thanks
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-04-02 0:30 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-22 6:16 [dpdk-dev] [PATCH] lib/metrics: add unregister api for metrics =?gb18030?B?zfK/ob3c?=
2019-02-22 13:56 ` [dpdk-dev] [PATCH v2] " wanjunjie
2019-02-22 15:03 ` [dpdk-dev] [PATCH v3] " wanjunjie
2019-02-22 15:39 ` [dpdk-dev] [PATCH v4] " wanjunjie
2019-02-26 16:10 ` Remy Horton
2019-02-27 17:19 ` [dpdk-dev] [PATCH v5] " Junjie Wan
2019-02-28 11:53 ` Remy Horton
2019-02-28 13:28 ` Wan Junjie
2019-02-28 13:55 ` Remy Horton
2019-04-02 0:27 ` Thomas Monjalon
2019-04-02 0:27 ` Thomas Monjalon
2019-04-02 0:30 ` Thomas Monjalon
2019-04-02 0:30 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).