From 13a93e2a066545b4bff7b4a50c5b21248db81f69 Mon Sep 17 00:00:00 2001 From: Krzysztof Matczak Date: Fri, 24 Feb 2017 09:15:07 +0000 Subject: [PATCH] dpdkevents: PR comments addressed RTE version check changed to compile time. Adressing PR review comments. Minor format changes Change-Id: Ie345a6009a291490323f614183362dfce8e6a5e6 --- Makefile.am | 1 - configure.ac | 77 ++++++++++++++++++--------------------------------- src/collectd.conf.pod | 14 +++++----- src/dpdkevents.c | 54 +++++++++++++++--------------------- 4 files changed, 57 insertions(+), 89 deletions(-) diff --git a/Makefile.am b/Makefile.am index 0825fd7b..efc16f7c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -770,7 +770,6 @@ if BUILD_PLUGIN_DPDKEVENTS pkglib_LTLIBRARIES += dpdkevents.la dpdkevents_la_SOURCES = src/dpdkevents.c src/utils_dpdk.c src/utils_dpdk.h dpdkevents_la_CPPFLAGS = $(AM_CPPFLAGS) $(LIBDPDK_CPPFLAGS) -#dpdkevents_la_CFLAGS = $(AM_CFLAGS) $(BUILD_WITH_DPDK_CFLAGS) dpdkevents_la_LDFLAGS = $(PLUGIN_LDFLAGS) $(LIBDPDK_LDFLAGS) dpdkevents_la_LIBADD = -ldpdk endif diff --git a/configure.ac b/configure.ac index 2d4e9dc7..0c9368d5 100644 --- a/configure.ac +++ b/configure.ac @@ -2700,62 +2700,46 @@ AC_ARG_VAR([LIBDPDK_LDFLAGS], [Linker flags for libdpdk]) AC_ARG_WITH([libdpdk], [AS_HELP_STRING([--without-libdpdk], [Disable libdpdk.])]) -rte_version_major="" -rte_version_minor="" -if test "x$with_libdpdk" != "xno" -then - if test "x$LIBDPDK_CPPFLAGS" = "x" - then - LIBDPDK_CPPFLAGS="-I/usr/include/dpdk" - fi - SAVE_CPPFLAGS="$CPPFLAGS" - CPPFLAGS="$LIBDPDK_CPPFLAGS $CPPFLAGS" - AC_CHECK_HEADERS([rte_config.h], - [ +if test "x$with_libdpdk" != "xno"; then + if test "x$LIBDPDK_CPPFLAGS" = "x"; then + LIBDPDK_CPPFLAGS="-I/usr/include/dpdk" + fi + SAVE_CPPFLAGS="$CPPFLAGS" + CPPFLAGS="$LIBDPDK_CPPFLAGS $CPPFLAGS" + AC_CHECK_HEADERS([rte_config.h], + [ with_libdpdk="yes" - AC_RUN_IFELSE( - [ - AC_LANG_PROGRAM( - [[ - #include - ]], - [[ - return (RTE_VER_YEAR & 0xFF) - ]] - ) - ], - [rte_version_major=0], - [rte_version_major=$?] - ) - AC_RUN_IFELSE( + AC_COMPILE_IFELSE( [ AC_LANG_PROGRAM( [[ #include + #if RTE_VERSION < RTE_VERSION_NUM(16,7,0,0) + #error "required DPDK >= 16.07" + #endif ]], [[ - return (RTE_VER_MONTH & 0xFF) + return 0; ]] ) ], - [rte_version_minor=0], - [rte_version_minor=$?] + [dpdk_keepalive="yes"], + [dpdk_keepalive="no (DPDK version < 16.07)"] ) ], - [with_libdpdk="no (rte_config.h not found)"] - ) - CPPFLAGS="$SAVE_CPPFLAGS" + [with_libdpdk="no (rte_config.h not found)"] + ) + CPPFLAGS="$SAVE_CPPFLAGS" fi -if test "x$with_libdpdk" = "xyes" -then - SAVE_LDFLAGS="$LDFLAGS" - LDFLAGS="$LIBDPDK_LDFLAGS $LDFLAGS" - AC_CHECK_LIB([dpdk], [rte_eal_init], - [with_libdpdk="yes"], - [with_libdpdk="no (symbol 'rte_eal_init' not found)"] - ) - LDFLAGS="$SAVE_LDFLAGS" +if test "x$with_libdpdk" = "xyes"; then + SAVE_LDFLAGS="$LDFLAGS" + LDFLAGS="$LIBDPDK_LDFLAGS $LDFLAGS" + AC_CHECK_LIB([dpdk], [rte_eal_init], + [with_libdpdk="yes"], + [with_libdpdk="no (symbol 'rte_eal_init' not found)"] + ) + LDFLAGS="$SAVE_LDFLAGS" fi # }}} @@ -6264,14 +6248,7 @@ fi if test "x$with_libdpdk" = "xyes" then - plugin_dpdkevents="no (DPDK version < 16.07)" - if test $rte_version_major -eq 16 - then - if test $rte_version_minor -ge 7 - then - plugin_dpdkevents="yes" - fi - fi + plugin_dpdkevents="$dpdk_keepalive" plugin_dpdkstat="yes" fi diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod index e7c00c30..a222da70 100644 --- a/src/collectd.conf.pod +++ b/src/collectd.conf.pod @@ -2458,9 +2458,9 @@ value is true. =item B I A hexidecimal bit mask of the DPDK ports which should be enabled. A mask -of 0x0 means that all ports will be disabled. A bitmask of all Fs means -that all ports will be enabled. This is an optional argument - default -is all ports enabled. +of 0x0 means that all ports will be disabled. A bitmask of all F's means +that all ports will be enabled. This is an optional argument - by default +all ports are enabled. =item B I @@ -2472,8 +2472,8 @@ convention will be used for the additional ports. =item B I -If set to true, link status notifications will be sent, instead of link -status being collected as a statistic. This is an optional argument - default +If set to true, link status notifications are sent, instead of link status +being collected as a statistic. This is an optional argument - default value is false. =back @@ -2499,8 +2499,8 @@ the keep alive cores state. =item B I -If set to true, keep alive notifications will be sent, instead of keep -alive information being collected as a statistic. This is an optional +If set to true, keep alive notifications are sent, instead of keep alive +information being collected as a statistic. This is an optional argument - default value is false. =back diff --git a/src/dpdkevents.c b/src/dpdkevents.c index 62e28b9c..6be6bc04 100644 --- a/src/dpdkevents.c +++ b/src/dpdkevents.c @@ -30,13 +30,15 @@ * Krzysztof Matczak */ +#include "collectd.h" + #include "common.h" #include "plugin.h" + #include "semaphore.h" #include "sys/mman.h" #include "utils_dpdk.h" #include "utils_time.h" -#include "collectd.h" #include #include @@ -51,8 +53,6 @@ #define KEEPALIVE_PLUGIN_INSTANCE "keepalive" #define RTE_KEEPALIVE_SHM_NAME "/dpdk_keepalive_shm_name" -#define RTE_VERSION_16_07 RTE_VERSION_NUM(16, 7, 0, 16) - typedef struct dpdk_keepalive_shm_s { sem_t core_died; enum rte_keepalive_state core_state[RTE_KEEPALIVE_MAXCORES]; @@ -304,6 +304,12 @@ static int dpdk_events_config(oconfig_item_t *ci) { } } + if (!ec->config.keep_alive.enabled && !ec->config.link_status.enabled) { + ERROR(DPDK_EVENTS_PLUGIN ": At least one type of events should be " + "configured for collecting. Plugin misconfigured"); + return -1; + } + return ret; } @@ -350,28 +356,27 @@ int dpdk_helper_command_handler(dpdk_helper_ctx_t *phc, enum DPDK_CMD cmd) { } dpdk_events_ctx_t *ec = DPDK_EVENTS_CTX_GET(phc); - + int ret = 0; if (ec->config.link_status.enabled) - dpdk_helper_link_status_get(phc); + ret = dpdk_helper_link_status_get(phc); - return 0; + return ret; } static void dpdk_events_notification_dispatch(int severity, - char *plugin_instance, - cdtime_t time, char *msg) { - notification_t n = {0}; - n.severity = severity; - n.time = time; + const char *plugin_instance, + cdtime_t time, const char *msg) { + notification_t n = { + .severity = severity, .time = time, .plugin = DPDK_EVENTS_PLUGIN}; sstrncpy(n.host, hostname_g, sizeof(n.host)); - sstrncpy(n.plugin, DPDK_EVENTS_PLUGIN, sizeof(n.plugin)); sstrncpy(n.plugin_instance, plugin_instance, sizeof(n.plugin_instance)); sstrncpy(n.message, msg, sizeof(n.message)); plugin_dispatch_notification(&n); } -static void dpdk_events_gauge_submit(char *plugin_instance, char *type, - gauge_t value, cdtime_t time) { +static void dpdk_events_gauge_submit(const char *plugin_instance, + const char *type_instance, gauge_t value, + cdtime_t time) { value_list_t vl = {.values = &(value_t){.gauge = value}, .values_len = 1, .time = time, @@ -380,7 +385,7 @@ static void dpdk_events_gauge_submit(char *plugin_instance, char *type, .meta = NULL}; sstrncpy(vl.host, hostname_g, sizeof(vl.host)); sstrncpy(vl.plugin_instance, plugin_instance, sizeof(vl.plugin_instance)); - sstrncpy(vl.type_instance, type, sizeof(vl.type_instance)); + sstrncpy(vl.type_instance, type_instance, sizeof(vl.type_instance)); plugin_dispatch_values(&vl); } @@ -426,7 +431,7 @@ static int dpdk_events_link_status_dispatch(dpdk_helper_ctx_t *phc) { return 0; } -static int dpdk_events_keep_alive_dispatch(dpdk_helper_ctx_t *phc) { +static void dpdk_events_keep_alive_dispatch(dpdk_helper_ctx_t *phc) { dpdk_events_ctx_t *ec = DPDK_EVENTS_CTX_GET(phc); /* dispatch Keep Alive values to collectd */ @@ -454,7 +459,6 @@ static int dpdk_events_keep_alive_dispatch(dpdk_helper_ctx_t *phc) { ec->core_info[i].lcore_state = ec->config.keep_alive.shm->core_state[i]; ec->core_info[i].read_time = cdtime(); -#if RTE_VERSION >= RTE_VERSION_16_07 if (ec->config.keep_alive.notify) { char msg[DATA_MAX_NAME_LEN]; int sev; @@ -500,15 +504,8 @@ static int dpdk_events_keep_alive_dispatch(dpdk_helper_ctx_t *phc) { ec->config.keep_alive.shm->core_state[i], ec->core_info[i].read_time); } -#else - dpdk_events_gauge_submit(KEEPALIVE_PLUGIN_INSTANCE, core_name, - ec->config.keep_alive.shm->core_state[i], - ec->core_info[i].read_time); -#endif /* #if RTE_VERSION >= RTE_VERSION_16_07 */ } } - - return 0; } static int dpdk_events_read(user_data_t *ud) { @@ -516,16 +513,11 @@ static int dpdk_events_read(user_data_t *ud) { if (g_hc == NULL) { ERROR(DPDK_EVENTS_PLUGIN ": plugin not initialized."); - return -EINVAL; + return -1; } dpdk_events_ctx_t *ec = DPDK_EVENTS_CTX_GET(g_hc); - if (!ec->config.keep_alive.enabled && !ec->config.link_status.enabled) { - /* nothing to do */ - return 0; - } - if (ec->config.link_status.enabled) { int cmd_res = 0; int ret = dpdk_helper_command(g_hc, DPDK_CMD_GET_EVENTS, &cmd_res, @@ -564,7 +556,7 @@ static int dpdk_events_init(void) { static int dpdk_events_shutdown(void) { DPDK_EVENTS_TRACE(); - int ret = 0; + int ret; dpdk_events_ctx_t *ec = DPDK_EVENTS_CTX_GET(g_hc); if (ec->config.keep_alive.enabled) { -- 2.11.0