From 98df65fb8d8f5dd67f596f7f3a411c56fab80458 Mon Sep 17 00:00:00 2001 From: Przemyslaw Szczerbik Date: Tue, 14 Feb 2017 13:02:43 +0000 Subject: [PATCH] dpdkstat: fix retrieval of statistics This change forces dpdkstat plugin to check if memory needs to be resized on every DPDK_CMD_GET_STATS command, rather than resizing it only once at initialization. Additionally it fixes incorrect metric retrieval logic, which allows plugin to dispatch metrics only if number of retrieved extended statistics is equal to allocated stats array size for port. Based on DPDK API documentation rte_eth_xstats_get (and rte_eth_xstats_get_names) function can return positive value lower than array size, which isn't considered a failure. If primary DPDK process is restarted with a greater number of ports bound to DPDK driver while collectd dpdkstat plugin is running, then memory isn't resized. Due to insufficient memory allocation dpdkstat plugin is unable to gather metrics from new ports. Change-Id: I25c8995105a322474653bf7065c2228047f886b1 Signed-off-by: Przemyslaw Szczerbik --- src/dpdkstat.c | 116 +++++++++++++++++++++---------------------------------- src/utils_dpdk.c | 20 ++++++++++ src/utils_dpdk.h | 1 + 3 files changed, 65 insertions(+), 72 deletions(-) diff --git a/src/dpdkstat.c b/src/dpdkstat.c index 4c73eab9..6b057f21 100644 --- a/src/dpdkstat.c +++ b/src/dpdkstat.c @@ -118,7 +118,7 @@ static int dpdk_stats_preinit(void) { if (ret != 0) { char errbuf[ERR_BUF_SIZE]; ERROR("%s: failed to initialize %s helper(error: %s)", DPDK_STATS_PLUGIN, - g_shm_name, sstrerror(errno, errbuf, sizeof(errbuf))); + g_shm_name, sstrerror(errno, errbuf, sizeof(errbuf))); return ret; } @@ -143,11 +143,9 @@ static int dpdk_stats_config(oconfig_item_t *ci) { ctx->config.enabled_port_mask = child->values[0].value.number; DEBUG("%s: Enabled Port Mask 0x%X", DPDK_STATS_PLUGIN, ctx->config.enabled_port_mask); - } else if (strcasecmp("SharedMemObj", child->key) == 0) { - cf_util_get_string_buffer(child, g_shm_name, - sizeof(g_shm_name)); - DEBUG("%s: Shared memory object %s", DPDK_STATS_PLUGIN, - g_shm_name); + } else if (strcasecmp("SharedMemObj", child->key) == 0) { + cf_util_get_string_buffer(child, g_shm_name, sizeof(g_shm_name)); + DEBUG("%s: Shared memory object %s", DPDK_STATS_PLUGIN, g_shm_name); dpdk_stats_reinit_helper(); } else if (strcasecmp("EAL", child->key) == 0) { ret = dpdk_helper_eal_config_parse(g_hc, child); @@ -180,73 +178,56 @@ static int dpdk_stats_config(oconfig_item_t *ci) { } static int dpdk_helper_stats_get(dpdk_helper_ctx_t *phc) { - dpdk_stats_ctx_t *ctx = DPDK_STATS_CTX_GET(phc); - - /* get stats from DPDK */ - - uint8_t ports_count = rte_eth_dev_count(); - if (ports_count == 0) { - DPDK_CHILD_LOG("%s: No DPDK ports available. " - "Check bound devices to DPDK driver.\n", - DPDK_STATS_PLUGIN); - return -ENODEV; - } - - if (ports_count > RTE_MAX_ETHPORTS) - ports_count = RTE_MAX_ETHPORTS; - - ctx->ports_count = ports_count; - int len = 0; int ret = 0; int stats = 0; + dpdk_stats_ctx_t *ctx = DPDK_STATS_CTX_GET(phc); - for (uint8_t i = 0; i < ports_count; i++) { + /* get stats from DPDK */ + for (uint8_t i = 0; i < ctx->ports_count; i++) { if (!(ctx->config.enabled_port_mask & (1 << i))) continue; + ctx->port_read_time[i] = cdtime(); + /* Store available stats array length for port */ len = ctx->port_stats_count[i]; + ret = rte_eth_xstats_get(i, &ctx->xstats[stats], len); - if (ret < 0 || ret != len) { - DPDK_CHILD_LOG("%s: Error reading stats (port=%d; len=%d)\n", - DPDK_STATS_PLUGIN, i, len); + if (ret < 0 || ret > len) { + DPDK_CHILD_LOG(DPDK_STATS_PLUGIN + ": Error reading stats (port=%d; len=%d, ret=%d)\n", + i, len, ret); + ctx->port_stats_count[i] = 0; return -1; } #if RTE_VERSION >= RTE_VERSION_16_07 ret = rte_eth_xstats_get_names(i, &ctx->xnames[stats], len); - if (ret < 0 || ret != len) { - DPDK_CHILD_LOG("%s: Error reading stat names (port=%d; len=%d)\n", - DPDK_STATS_PLUGIN, i, len); + if (ret < 0 || ret > len) { + DPDK_CHILD_LOG(DPDK_STATS_PLUGIN + ": Error reading stat names (port=%d; len=%d ret=%d)\n", + i, len, ret); + ctx->port_stats_count[i] = 0; return -1; } #endif - stats += len; + ctx->port_stats_count[i] = ret; + stats += ctx->port_stats_count[i]; } - assert(stats == ctx->stats_count); - + assert(stats <= ctx->stats_count); return 0; } static int dpdk_helper_stats_count_get(dpdk_helper_ctx_t *phc) { - dpdk_stats_ctx_t *ctx = DPDK_STATS_CTX_GET(phc); - - uint8_t ports = rte_eth_dev_count(); - if (ports == 0) { - DPDK_CHILD_LOG("%s: No DPDK ports available. " - "Check bound devices to DPDK driver.\n", - DPDK_STATS_PLUGIN); + uint8_t ports = dpdk_helper_eth_dev_count(); + if (ports == 0) return -ENODEV; - } - - if (ports > RTE_MAX_ETHPORTS) - ports = RTE_MAX_ETHPORTS; + dpdk_stats_ctx_t *ctx = DPDK_STATS_CTX_GET(phc); ctx->ports_count = ports; int len = 0; int stats_count = 0; - for (int i = 0; i < ports; i++) { if (!(ctx->config.enabled_port_mask & (1 << i))) continue; @@ -269,9 +250,12 @@ static int dpdk_helper_stats_count_get(dpdk_helper_ctx_t *phc) { return stats_count; } +static int dpdk_stats_get_size(dpdk_helper_ctx_t *phc) { + return (dpdk_helper_data_size_get(phc) - sizeof(dpdk_stats_ctx_t)); +} + int dpdk_helper_command_handler(dpdk_helper_ctx_t *phc, enum DPDK_CMD cmd) { /* this function is called from helper context */ - int ret = 0; if (phc == NULL) { DPDK_CHILD_LOG("%s: Invalid argument(phc)\n", DPDK_STATS_PLUGIN); @@ -283,34 +267,23 @@ int dpdk_helper_command_handler(dpdk_helper_ctx_t *phc, enum DPDK_CMD cmd) { return -EINVAL; } - dpdk_stats_ctx_t *ctx = DPDK_STATS_CTX_GET(phc); - - if (ctx->stats_count == 0) { - - int stats_count = dpdk_helper_stats_count_get(phc); + int stats_count = dpdk_helper_stats_count_get(phc); + if (stats_count < 0) { + return stats_count; + } - if (stats_count < 0) { - return stats_count; - } + DPDK_STATS_CTX_GET(phc)->stats_count = stats_count; + int stats_size = stats_count * DPDK_STATS_CTX_GET_XSTAT_SIZE; - int stats_size = stats_count * DPDK_STATS_CTX_GET_XSTAT_SIZE; - ctx->stats_count = stats_count; - - if ((dpdk_helper_data_size_get(phc) - sizeof(dpdk_stats_ctx_t)) < - stats_size) { - DPDK_CHILD_LOG( - "%s:%s:%d not enough space for stats (available=%d, " - "needed=%d)\n", - DPDK_STATS_PLUGIN, __FUNCTION__, __LINE__, - (int)(dpdk_helper_data_size_get(phc) - sizeof(dpdk_stats_ctx_t)), - stats_size); - return -ENOBUFS; - } + if (dpdk_stats_get_size(phc) < stats_size) { + DPDK_CHILD_LOG( + DPDK_STATS_PLUGIN + ":%s:%d not enough space for stats (available=%d, needed=%d)\n", + __FUNCTION__, __LINE__, (int)dpdk_stats_get_size(phc), stats_size); + return -ENOBUFS; } - ret = dpdk_helper_stats_get(phc); - - return ret; + return dpdk_helper_stats_get(phc); } static void dpdk_stats_resolve_cnt_type(char *cnt_type, size_t cnt_type_len, @@ -515,8 +488,7 @@ static int dpdk_stats_shutdown(void) { ret = dpdk_helper_shutdown(g_hc); g_hc = NULL; if (ret != 0) { - ERROR("%s: failed to cleanup %s helper", DPDK_STATS_PLUGIN, - g_shm_name); + ERROR("%s: failed to cleanup %s helper", DPDK_STATS_PLUGIN, g_shm_name); return ret; } diff --git a/src/utils_dpdk.c b/src/utils_dpdk.c index 640f08bc..e3c73793 100644 --- a/src/utils_dpdk.c +++ b/src/utils_dpdk.c @@ -38,6 +38,7 @@ #include #include +#include #include "common.h" #include "utils_dpdk.h" @@ -850,3 +851,22 @@ uint128_t str_to_uint128(const char *str, int len) { } return lcore_mask; } + +uint8_t dpdk_helper_eth_dev_count() { + uint8_t ports = rte_eth_dev_count(); + if (ports == 0) { + ERROR( + "%s:%d: No DPDK ports available. Check bound devices to DPDK driver.\n", + __FUNCTION__, __LINE__); + return ports; + } + + if (ports > RTE_MAX_ETHPORTS) { + ERROR("%s:%d: Number of DPDK ports (%u) is greater than " + "RTE_MAX_ETHPORTS=%d. Ignoring extra ports\n", + __FUNCTION__, __LINE__, ports, RTE_MAX_ETHPORTS); + ports = RTE_MAX_ETHPORTS; + } + + return ports; +} diff --git a/src/utils_dpdk.h b/src/utils_dpdk.h index c9bb14b0..f97a6b51 100644 --- a/src/utils_dpdk.h +++ b/src/utils_dpdk.h @@ -73,6 +73,7 @@ int dpdk_helper_command(dpdk_helper_ctx_t *phc, enum DPDK_CMD cmd, int *result, cdtime_t cmd_wait_time); void *dpdk_helper_priv_get(dpdk_helper_ctx_t *phc); int dpdk_helper_data_size_get(dpdk_helper_ctx_t *phc); +uint8_t dpdk_helper_eth_dev_count(); /* forward declaration of handler function that is called by helper from * child process. not implemented in helper. must be provided by client. */ -- 2.11.0