dpdkstat: rework

1) Declaring int i = 0 in for loop
2) Removed -fPIC flag
3) Fixed indentation
4) Changed true and false to yes and no
5) Fixed empty var issue when --with-libdpdk=yes
6) Changed DPDK_LIB to DPDK_EXTRA_LIB (typo)
7) Removed dead code & FOUND_DPDK after AC_MSG_ERROR
8) Changed AC_CHECK_FILE to AC_CHECK_HEADER
9) Restructured DPDK part of configure.ac
10) Add comment explaining no-as-needed flag.
11) Added new if_* types to types.db
12) Updated type & type inst. selection for xstats

Change-Id: I7fb48852d8d0b94b4c67f204f1b1b430681e4509
Signed-off-by: Kim Jones <kim-marie.jones@intel.com>

diff --git a/configure.ac b/configure.ac
index 82ea4bb265a118e77c5b9dce8f34da859e74097f..42e558839ce831ca89e093134758683f64594a9c 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -2416,56 +2416,70 @@ AC_ARG_WITH(libdpdk, [AS_HELP_STRING([--with-libdpdk@<:@=PREFIX@:>@], [Path to t
 [
        if test "x$withval" != "xno" && test "x$withval" != "xyes"
        then
-               with_dpdk_path="$withval"
+               RTE_BUILD="$withval"
                with_libdpdk="yes"
        else
-               if test "x$withval" = "xno"
-               then
-                       with_libdpdk="no (disabled)"
-               fi
+               RTE_BUILD="/usr"
+               with_libdpdk="$withval"
        fi
+       DPDK_INCLUDE="$RTE_BUILD/include"
+       DPDK_LIB_DIR="$RTE_BUILD/lib"
+       FOUND_DPDK=yes
 ], [with_libdpdk="no"])
 
 if test "x$with_libdpdk" = "xyes"
 then
-       RTE_BUILD="$with_dpdk_path"
-       DPDK_INCLUDE="$RTE_BUILD/include"
-       AC_CHECK_FILE([$DPDK_INCLUDE/rte_config.h], [LOCAL_DPDK_INSTALL=true],
-               [AC_CHECK_FILE([$DPDK_INCLUDE/dpdk/rte_config.h],
-               [DPDK_INCLUDE=$DPDK_INCLUDE/dpdk], [])])
-       DPDK_LIB_DIR="$RTE_BUILD/lib"
-       DPDK_EXTRA_LIB=""
+       LOCAL_DPDK_INSTALL="no"
+       AC_CHECK_HEADER([$DPDK_INCLUDE/rte_config.h], [LOCAL_DPDK_INSTALL=yes],
+               [AC_CHECK_HEADER([$DPDK_INCLUDE/dpdk/rte_config.h],
+               [],
+               [FOUND_DPDK=no], [])], [])
 
-       SAVE_CFLAGS="$CFLAGS"
+       if test "x$LOCAL_DPDK_INSTALL" = "xno"
+       then
+               DPDK_INCLUDE=$DPDK_INCLUDE/dpdk
+       fi
+
+       if test "x$FOUND_DPDK" = "xno"
+       then
+               AC_MSG_ERROR([libdpdk error: rte_config.h not found])
+       fi
+fi
+
+if test "x$with_libdpdk" = "xyes"
+then
        SAVE_LDFLAGS="$LDFLAGS"
-       CFLAGS="$CFLAGS -I$DPDK_INCLUDE"
-       if test "x$LOCAL_DPDK_INSTALL" != "xtrue"
-        then
-            LDFLAGS="$LDFLAGS -L$DPDK_LIB_DIR"
+
+       if test "x$LOCAL_DPDK_INSTALL" != "xyes"
+       then
+               LDFLAGS="$LDFLAGS -L$DPDK_LIB_DIR"
         fi
-       FOUND_DPDK=false
-       SAVE_LIBS="$LIBS"
-       LIBS="$DPDK_LIB $LIBS"
+
        AC_CHECK_LIB(dpdk, rte_eal_init,
-                     [FOUND_DPDK=true],
-                     [AC_MSG_ERROR([did not find dpdk libs ])])
+                     [BUILD_WITH_DPDK_LIBS="-Wl,-ldpdk"],
+                     [FOUND_DPDK=no])
 
-       CFLAGS="$SAVE_CFLAGS"
        LDFLAGS="$SAVE_LDFLAGS"
-       LIBS="$SAVE_LIBS"
-       if test "x$FOUND_DPDK" != "xtrue"
-        then
-           AC_MSG_ERROR([cannot link with dpdk in $DPDK_LIB_DIR])
+       if test "x$FOUND_DPDK" = "xno"
+       then
+               AC_MSG_ERROR([libdpdk error: cannot link with dpdk in $DPDK_LIB_DIR])
        fi
+fi
 
-       BUILD_WITH_DPDK_CFLAGS+="-fPIC -I$DPDK_INCLUDE"
-       if test "x$LOCAL_DPDK_INSTALL" != "xtrue"
-        then
+#
+# Note: An issue on Ubuntu 14.04 necessitates the use of -Wl,--no-as-needed:
+# If you try compile with the older linker, the dpdk symbols will be undefined.
+# This workaround should be removed when no longer necessary.
+#
+if test "x$with_libdpdk" = "xyes"
+then
+       BUILD_WITH_DPDK_CFLAGS+="-I$DPDK_INCLUDE"
+       if test "x$LOCAL_DPDK_INSTALL" != "xyes"
+       then
                BUILD_WITH_DPDK_LDFLAGS="-Wl,--no-as-needed"
        else
                BUILD_WITH_DPDK_LDFLAGS="-L$DPDK_LIB_DIR -Wl,--no-as-needed"
         fi
-       BUILD_WITH_DPDK_LIBS="-Wl,-ldpdk"
        AC_SUBST(BUILD_WITH_DPDK_CFLAGS)
        AC_SUBST(BUILD_WITH_DPDK_LDFLAGS)
        AC_SUBST(BUILD_WITH_DPDK_LIBS)

diff --git a/src/dpdkstat.c b/src/dpdkstat.c
index 733bb0bbec584c389acdaf73508e15e3ceb140eb..549463584fd269210e3a35ee00aab21a71610801 100644 (file)
--- a/src/dpdkstat.c
+++ b/src/dpdkstat.c
@@ -122,8 +122,6 @@ static int dpdk_shm_init(size_t size);
 /* Write the default configuration to the g_configuration instances */
 static void dpdk_config_init_default(void)
 {
-    int i;
-
     g_configuration->interval = plugin_get_interval();
     WARNING("dpdkstat: No time interval was configured, default value %lu ms is set\n",
              CDTIME_T_TO_MS(g_configuration->interval));
@@ -137,13 +135,13 @@ static void dpdk_config_init_default(void)
     ssnprintf(g_configuration->file_prefix, DATA_MAX_NAME_LEN, "%s",
              "/var/run/.rte_config");
 
-    for (i = 0; i < RTE_MAX_ETHPORTS; i++)
+    for (int i = 0; i < RTE_MAX_ETHPORTS; i++)
       g_configuration->port_name[i][0] = 0;
 }
 
 static int dpdk_config(oconfig_item_t *ci)
 {
-  int i = 0, port_counter = 0;
+  int port_counter = 0;
 
   /* Initialize a POSIX SHared Memory (SHM) object. */
   int err = dpdk_shm_init(sizeof(dpdk_config_t));
@@ -155,7 +153,7 @@ static int dpdk_config(oconfig_item_t *ci)
   /* Set defaults for config, overwritten by loop if config item exists */
   dpdk_config_init_default();
 
-  for (i = 0; i < ci->children_num; i++) {
+  for (int i = 0; i < ci->children_num; i++) {
     oconfig_item_t *child = ci->children + i;
 
     if (strcasecmp("Interval", child->key) == 0) {
@@ -203,7 +201,7 @@ static int dpdk_config(oconfig_item_t *ci)
       WARNING ("dpdkstat: The config option \"%s\" is unknown.",
                child->key);
     }
-  } /* End for (i = 0; i < ci->children_num; i++)*/
+  } /* End for (int i = 0; i < ci->children_num; i++)*/
   g_configured = 1; /* Bypass configuration in dpdk_shm_init(). */
 
   return 0;
@@ -325,8 +323,7 @@ static int dpdk_helper_exit(int reset)
     g_configuration->num_ports = 0;
     memset(&g_configuration->xstats, 0, g_configuration->num_xstats* sizeof(struct rte_eth_xstats));
     g_configuration->num_xstats = 0;
-    int i = 0;
-    for (; i < RTE_MAX_ETHPORTS; i++)
+    for (int i = 0; i < RTE_MAX_ETHPORTS; i++)
       g_configuration->num_stats_in_port[i] = 0;
   }
   close(g_configuration->helper_pipes[1]);
@@ -431,7 +428,7 @@ static int dpdk_helper_init_eal(void)
     g_configuration->eal_initialized = 0;
     printf("dpdkstat: ERROR initializing EAL ret = %d\n", ret);
     printf("dpdkstat: EAL arguments: ");
-    for (i=0; i< g_configuration->eal_argc; i++) {
+    for (i = 0; i < g_configuration->eal_argc; i++) {
       printf("%s ", argp[i]);
     }
     printf("\n");
@@ -497,8 +494,8 @@ static int dpdk_helper_run (void)
     if (nb_ports > RTE_MAX_ETHPORTS)
       nb_ports = RTE_MAX_ETHPORTS;
 
-    int len = 0, enabled_port_count = 0, num_xstats = 0, i = 0;
-    for (; i < nb_ports; i++) {
+    int len = 0, enabled_port_count = 0, num_xstats = 0;
+    for (int i = 0; i < nb_ports; i++) {
       if (g_configuration->enabled_port_mask & (1 << i)) {
         if(g_configuration->helper_action == DPDK_HELPER_ACTION_COUNT_STATS) {
           len = rte_eth_xstats_get(i, NULL, 0);
@@ -623,9 +620,9 @@ static int dpdk_read (user_data_t *ud)
   }
 
   /* Dispatch the stats.*/
-  int count = 0, i = 0, port_num = 0;
+  int count = 0, port_num = 0;
 
-  for (; i < g_configuration->num_ports; i++) {
+  for (int i = 0; i < g_configuration->num_ports; i++) {
     cdtime_t time = g_configuration->port_read_time[i];
     char dev_name[64];
     int len = g_configuration->num_stats_in_port[i];
@@ -639,12 +636,12 @@ static int dpdk_read (user_data_t *ud)
       ssnprintf(dev_name, sizeof(dev_name), "port.%d", port_num);
     struct rte_eth_xstats *xstats = (&g_configuration->xstats);
     xstats += count; /* pointer arithmetic to jump to each stats struct */
-    int j = 0;
-    for (; j < len; j++) {
+    for (int j = 0; j < len; j++) {
       value_t dpdkstat_values[1];
       value_list_t dpdkstat_vl = VALUE_LIST_INIT;
+      char *type_end;
 
-      dpdkstat_values[0].counter = xstats[j].value;
+      dpdkstat_values[0].derive = (int64_t) xstats[j].value;
       dpdkstat_vl.values = dpdkstat_values;
       dpdkstat_vl.values_len = 1; /* Submit stats one at a time */
       dpdkstat_vl.time = time;
@@ -652,8 +649,58 @@ static int dpdk_read (user_data_t *ud)
       sstrncpy (dpdkstat_vl.plugin, "dpdkstat", sizeof (dpdkstat_vl.plugin));
       sstrncpy (dpdkstat_vl.plugin_instance, dev_name,
                 sizeof (dpdkstat_vl.plugin_instance));
-      sstrncpy (dpdkstat_vl.type, "counter",
-                sizeof (dpdkstat_vl.type));
+
+      type_end = strrchr(xstats[j].name, '_');
+
+      if ((type_end != NULL) &&
+                (strncmp(xstats[j].name, "rx_", sizeof("rx_") - 1) == 0)) {
+
+        if (strncmp(type_end, "_errors", sizeof("_errors") - 1) == 0) {
+          sstrncpy (dpdkstat_vl.type, "if_rx_errors",
+                  sizeof(dpdkstat_vl.type));
+        } else if (strncmp(type_end, "_dropped", sizeof("_dropped") - 1) == 0) {
+          sstrncpy (dpdkstat_vl.type, "if_rx_dropped",
+                  sizeof(dpdkstat_vl.type));
+        } else if (strncmp(type_end, "_bytes", sizeof("_bytes") - 1) == 0) {
+          sstrncpy (dpdkstat_vl.type, "if_rx_octets",
+                  sizeof(dpdkstat_vl.type));
+        } else if (strncmp(type_end, "_packets", sizeof("_packets") - 1) == 0) {
+          sstrncpy (dpdkstat_vl.type, "if_rx_packets",
+                  sizeof(dpdkstat_vl.type));
+        } else {
+          /* Does not fit obvious type: use a more generic one */
+          sstrncpy (dpdkstat_vl.type, "derive",
+                  sizeof(dpdkstat_vl.type));
+        }
+
+      } else if ((type_end != NULL) &&
+                (strncmp(xstats[j].name, "tx_", sizeof("tx_") - 1)) == 0) {
+
+        if (strncmp(type_end, "_errors", sizeof("_errors") - 1) == 0) {
+          sstrncpy (dpdkstat_vl.type, "if_tx_errors",
+                  sizeof(dpdkstat_vl.type));
+        } else if (strncmp(type_end, "_dropped", sizeof("_dropped") - 1) == 0) {
+          sstrncpy (dpdkstat_vl.type, "if_tx_dropped",
+                  sizeof(dpdkstat_vl.type));
+        } else if (strncmp(type_end, "_bytes", sizeof("_bytes") - 1) == 0) {
+          sstrncpy (dpdkstat_vl.type, "if_tx_octets",
+                  sizeof(dpdkstat_vl.type));
+        } else if (strncmp(type_end, "_packets", sizeof("_packets") - 1) == 0) {
+          sstrncpy (dpdkstat_vl.type, "if_tx_packets",
+                  sizeof(dpdkstat_vl.type));
+        } else {
+          /* Does not fit obvious type: use a more generic one */
+          sstrncpy (dpdkstat_vl.type, "derive",
+                  sizeof(dpdkstat_vl.type));
+        }
+
+      } else {
+        /* Does not fit obvious type, or strrchr error:
+         *   use a more generic type */
+        sstrncpy (dpdkstat_vl.type, "derive",
+                sizeof(dpdkstat_vl.type));
+      }
+
       sstrncpy (dpdkstat_vl.type_instance, xstats[j].name,
                 sizeof (dpdkstat_vl.type_instance));
       plugin_dispatch_values (&dpdkstat_vl);

diff --git a/src/types.db b/src/types.db
index 8c6a99560a4f51cb8a3972463f5975b0439deb17..6eade8ed7ee155815bfa7dea98b39d04890954f1 100644 (file)
--- a/src/types.db
+++ b/src/types.db
@@ -98,10 +98,14 @@ if_errors               rx:DERIVE:0:U, tx:DERIVE:0:U
 if_multicast            value:DERIVE:0:U
 if_octets               rx:DERIVE:0:U, tx:DERIVE:0:U
 if_packets              rx:DERIVE:0:U, tx:DERIVE:0:U
+if_rx_dropped           value:DERIVE:0:U
 if_rx_errors            value:DERIVE:0:U
 if_rx_octets            value:DERIVE:0:U
+if_rx_packets           value:DERIVE:0:U
+if_tx_dropped           value:DERIVE:0:U
 if_tx_errors            value:DERIVE:0:U
 if_tx_octets            value:DERIVE:0:U
+if_tx_packets           value:DERIVE:0:U
 invocations             value:DERIVE:0:U
 io_octets               rx:DERIVE:0:U, tx:DERIVE:0:U
 io_packets              rx:DERIVE:0:U, tx:DERIVE:0:U