ovs_stats plugin: Minor code cleanup.
[collectd.git] / src / ovs_stats.c
index bbdb600..e7859da 100644 (file)
@@ -3,14 +3,17 @@
  *
  * Copyright(c) 2016 Intel Corporation. All rights reserved.
  *
- * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of
  * this software and associated documentation files (the "Software"), to deal in
  * the Software without restriction, including without limitation the rights to
  * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
- * of the Software, and to permit persons to whom the Software is furnished to do
+ * of the Software, and to permit persons to whom the Software is furnished to
+ * do
  * so, subject to the following conditions:
  *
- * The above copyright notice and this permission notice shall be included in all
+ * The above copyright notice and this permission notice shall be included in
+ * all
  * copies or substantial portions of the Software.
  *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
@@ -33,7 +36,7 @@
 static const char plugin_name[] = "ovs_stats";
 
 typedef enum iface_counter {
-  not_supportred = -1,
+  not_supported = -1,
   collisions,
   rx_bytes,
   rx_crc_err,
@@ -128,19 +131,19 @@ static const char *const iface_counter_table[IFACE_COUNTER_COUNT] = {
 };
 
 /* Entry into the list of network bridges */
-static bridge_list_t *g_bridge_list_head = NULL;
+static bridge_list_t *g_bridge_list_head;
 
 /* Entry into the list of monitored network bridges */
-static bridge_list_t *g_monitored_bridge_list_head = NULL;
+static bridge_list_t *g_monitored_bridge_list_head;
 
 /* entry into the list of network bridges */
-static port_list_t *g_port_list_head = NULL;
+static port_list_t *g_port_list_head;
 
 /* lock for statistics cache */
 static pthread_mutex_t g_stats_lock;
 
 /* OvS DB socket */
-static ovs_db_t *ovs_db = NULL;
+static ovs_db_t *g_ovs_db;
 
 /* OVS stats configuration data */
 struct ovs_stats_config_s {
@@ -155,11 +158,11 @@ static ovs_stats_config_t ovs_stats_cfg = {
     .ovs_db_serv = "6640",      /* use default OVS DB service */
 };
 
-static const iface_counter ovs_stats_counter_name_to_type(const char *counter) {
-  iface_counter index = not_supportred;
+static iface_counter ovs_stats_counter_name_to_type(const char *counter) {
+  iface_counter index = not_supported;
 
   if (counter == NULL)
-    return not_supportred;
+    return not_supported;
 
   for (int i = 0; i < IFACE_COUNTER_COUNT; i++) {
     if (strncmp(iface_counter_table[i], counter,
@@ -308,7 +311,7 @@ static int ovs_stats_del_bridge(yajl_val bridge) {
     }
   } else
     WARNING("%s: Incorrect data for deleting bridge", plugin_name);
-  return (0);
+  return 0;
 }
 
 /* Update Bridge. Create bridge ports*/
@@ -327,10 +330,11 @@ static int ovs_stats_update_bridge(yajl_val bridge) {
         br = ovs_stats_get_bridge(g_bridge_list_head, YAJL_GET_STRING(br_name));
         pthread_mutex_lock(&g_stats_lock);
         if (br == NULL) {
-          br = (bridge_list_t *)calloc(1, sizeof(bridge_list_t));
+          br = calloc(1, sizeof(*br));
           if (!br) {
-            ERROR("%s: Error allocating memory for bridge", plugin_name);
-            return (-1);
+            pthread_mutex_unlock(&g_stats_lock);
+            ERROR("%s: calloc(%zu) failed.", plugin_name, sizeof(*br));
+            return -1;
           }
           char *tmp = YAJL_GET_STRING(br_name);
 
@@ -339,7 +343,8 @@ static int ovs_stats_update_bridge(yajl_val bridge) {
           if (br->name == NULL) {
             sfree(br);
             pthread_mutex_unlock(&g_stats_lock);
-            return (-1);
+            ERROR("%s: strdup failed.", plugin_name);
+            return -1;
           }
           br->next = g_bridge_list_head;
           g_bridge_list_head = br;
@@ -349,22 +354,24 @@ static int ovs_stats_update_bridge(yajl_val bridge) {
       if (br_ports && YAJL_IS_ARRAY(br_ports)) {
         char *tmp = YAJL_GET_STRING(br_ports->u.array.values[0]);
         if (tmp != NULL && strcmp("set", tmp) == 0) {
-          yajl_val *ports_arr =
-              YAJL_GET_ARRAY(br_ports->u.array.values[1])->values;
-          size_t ports_num = YAJL_GET_ARRAY(br_ports->u.array.values[1])->len;
-
-          for (int i = 0; i < ports_num; i++)
-            ovs_stats_new_port(
-                br, YAJL_GET_STRING(ports_arr[i]->u.array.values[1]));
+          yajl_val *array = YAJL_GET_ARRAY(br_ports)->values;
+          size_t array_len = YAJL_GET_ARRAY(br_ports)->len;
+          if (array != NULL && array_len > 0 && YAJL_IS_ARRAY(array[1])) {
+            yajl_val *ports_arr = YAJL_GET_ARRAY(array[1])->values;
+            size_t ports_num = YAJL_GET_ARRAY(array[1])->len;
+            for (size_t i = 0; i < ports_num && ports_arr != NULL; i++)
+              ovs_stats_new_port(
+                  br, YAJL_GET_STRING(ports_arr[i]->u.array.values[1]));
+          }
         } else
           ovs_stats_new_port(br, YAJL_GET_STRING(br_ports->u.array.values[1]));
       }
     }
   } else {
     ERROR("Incorrect JSON Bridge data");
-    return (-1);
+    return -1;
   }
-  return (0);
+  return 0;
 }
 
 /* Handle JSON with Bridge Table change event */
@@ -398,7 +405,7 @@ static void ovs_stats_bridge_table_change_cb(yajl_val jupdates) {
   yajl_val bridges = yajl_tree_get(jupdates, path, yajl_t_object);
 
   if (bridges && YAJL_IS_OBJECT(bridges)) {
-    for (int i = 0; i < YAJL_GET_OBJECT(bridges)->len; i++) {
+    for (size_t i = 0; i < YAJL_GET_OBJECT(bridges)->len; i++) {
       yajl_val bridge = YAJL_GET_OBJECT(bridges)->values[i];
       ovs_stats_update_bridge(bridge);
     }
@@ -412,7 +419,7 @@ static void ovs_stats_bridge_table_delete_cb(yajl_val jupdates) {
   yajl_val bridge;
   if (bridges && YAJL_IS_OBJECT(bridges)) {
     pthread_mutex_lock(&g_stats_lock);
-    for (int i = 0; i < YAJL_GET_OBJECT(bridges)->len; i++) {
+    for (size_t i = 0; i < YAJL_GET_OBJECT(bridges)->len; i++) {
       bridge = YAJL_GET_OBJECT(bridges)->values[i];
       ovs_stats_del_bridge(bridge);
     }
@@ -455,9 +462,9 @@ static int ovs_stats_update_port(const char *uuid, yajl_val port) {
     }
   } else {
     ERROR("Incorrect JSON Port data");
-    return (-1);
+    return -1;
   }
-  return (0);
+  return 0;
 }
 
 /* Delete port from global port list */
@@ -474,7 +481,7 @@ static int ovs_stats_del_port(const char *uuid) {
       break;
     }
   }
-  return (0);
+  return 0;
 }
 
 /* Handle JSON with Port Table change event */
@@ -498,7 +505,7 @@ static void ovs_stats_port_table_change_cb(yajl_val jupdates) {
   yajl_val ports = yajl_tree_get(jupdates, path, yajl_t_object);
   yajl_val port;
   if (ports && YAJL_IS_OBJECT(ports)) {
-    for (int i = 0; i < YAJL_GET_OBJECT(ports)->len; i++) {
+    for (size_t i = 0; i < YAJL_GET_OBJECT(ports)->len; i++) {
       port = YAJL_GET_OBJECT(ports)->values[i];
       ovs_stats_update_port(YAJL_GET_OBJECT(ports)->keys[i], port);
     }
@@ -521,7 +528,7 @@ static void ovs_stats_port_table_delete_cb(yajl_val jupdates) {
   yajl_val ports = yajl_tree_get(jupdates, path, yajl_t_object);
   pthread_mutex_lock(&g_stats_lock);
   if (ports && YAJL_IS_OBJECT(ports))
-    for (int i = 0; i < YAJL_GET_OBJECT(ports)->len; i++) {
+    for (size_t i = 0; i < YAJL_GET_OBJECT(ports)->len; i++) {
       ovs_stats_del_port(YAJL_GET_OBJECT(ports)->keys[i]);
     }
   pthread_mutex_unlock(&g_stats_lock);
@@ -535,17 +542,19 @@ static int ovs_stats_update_iface_stats(port_list_t *port, yajl_val stats) {
   char *counter_name = NULL;
   int64_t counter_value = 0;
   if (stats && YAJL_IS_ARRAY(stats))
-    for (int i = 0; i < YAJL_GET_ARRAY(stats)->len; i++) {
+    for (size_t i = 0; i < YAJL_GET_ARRAY(stats)->len; i++) {
       stat = YAJL_GET_ARRAY(stats)->values[i];
+      if (!YAJL_IS_ARRAY(stat))
+        return -1;
       counter_name = YAJL_GET_STRING(YAJL_GET_ARRAY(stat)->values[0]);
       counter_index = ovs_stats_counter_name_to_type(counter_name);
       counter_value = YAJL_GET_INTEGER(YAJL_GET_ARRAY(stat)->values[1]);
-      if (counter_index == not_supportred)
+      if (counter_index == not_supported)
         continue;
       port->stats[counter_index] = counter_value;
     }
 
-  return (0);
+  return 0;
 }
 
 /* Update interface external_ids */
@@ -555,8 +564,10 @@ static int ovs_stats_update_iface_ext_ids(port_list_t *port, yajl_val ext_ids) {
   char *value;
 
   if (ext_ids && YAJL_IS_ARRAY(ext_ids))
-    for (int i = 0; i < YAJL_GET_ARRAY(ext_ids)->len; i++) {
+    for (size_t i = 0; i < YAJL_GET_ARRAY(ext_ids)->len; i++) {
       ext_id = YAJL_GET_ARRAY(ext_ids)->values[i];
+      if (!YAJL_IS_ARRAY(ext_id))
+        return -1;
       key = YAJL_GET_STRING(YAJL_GET_ARRAY(ext_id)->values[0]);
       value = YAJL_GET_STRING(YAJL_GET_ARRAY(ext_id)->values[1]);
       if (key && value) {
@@ -567,7 +578,7 @@ static int ovs_stats_update_iface_ext_ids(port_list_t *port, yajl_val ext_ids) {
       }
     }
 
-  return (0);
+  return 0;
 }
 
 /* Get interface statistic and external_ids */
@@ -584,7 +595,7 @@ static int ovs_stats_update_iface(yajl_val iface) {
       if (iface_name && YAJL_IS_STRING(iface_name)) {
         port = ovs_stats_get_port_by_name(YAJL_GET_STRING(iface_name));
         if (port == NULL)
-          return (0);
+          return 0;
       }
       /*
        * {
@@ -620,9 +631,9 @@ static int ovs_stats_update_iface(yajl_val iface) {
     }
   } else {
     ERROR("Incorrect JSON Port data");
-    return (-1);
+    return -1;
   }
-  return (0);
+  return 0;
 }
 
 /* Handle JSON with Interface Table change event */
@@ -681,7 +692,7 @@ static void ovs_stats_interface_table_change_cb(yajl_val jupdates) {
   yajl_val ports = yajl_tree_get(jupdates, path, yajl_t_object);
   pthread_mutex_lock(&g_stats_lock);
   if (ports && YAJL_IS_OBJECT(ports))
-    for (int i = 0; i < YAJL_GET_OBJECT(ports)->len; i++)
+    for (size_t i = 0; i < YAJL_GET_OBJECT(ports)->len; i++)
       ovs_stats_update_iface(YAJL_GET_OBJECT(ports)->values[i]);
   pthread_mutex_unlock(&g_stats_lock);
   return;
@@ -705,51 +716,50 @@ static void ovs_stats_initialize(ovs_db_t *pdb) {
                                      "external_ids", NULL};
 
   /* subscribe to a tables */
-  ovs_db_table_cb_register(pdb, "Bridge", bridge_columns,
-                           ovs_stats_bridge_table_change_cb,
+  ovs_db_table_cb_register(
+      pdb, "Bridge", bridge_columns, ovs_stats_bridge_table_change_cb,
       ovs_stats_bridge_table_result_cb,
-                           OVS_DB_TABLE_CB_FLAG_INITIAL |
-                           OVS_DB_TABLE_CB_FLAG_INSERT |
+      OVS_DB_TABLE_CB_FLAG_INITIAL | OVS_DB_TABLE_CB_FLAG_INSERT |
           OVS_DB_TABLE_CB_FLAG_MODIFY);
 
   ovs_db_table_cb_register(pdb, "Bridge", bridge_columns,
                            ovs_stats_bridge_table_delete_cb, NULL,
                            OVS_DB_TABLE_CB_FLAG_DELETE);
 
-  ovs_db_table_cb_register(pdb, "Port", port_columns,
-                           ovs_stats_port_table_change_cb,
+  ovs_db_table_cb_register(
+      pdb, "Port", port_columns, ovs_stats_port_table_change_cb,
       ovs_stats_port_table_result_cb,
-                           OVS_DB_TABLE_CB_FLAG_INITIAL |
-                           OVS_DB_TABLE_CB_FLAG_INSERT |
+      OVS_DB_TABLE_CB_FLAG_INITIAL | OVS_DB_TABLE_CB_FLAG_INSERT |
           OVS_DB_TABLE_CB_FLAG_MODIFY);
 
   ovs_db_table_cb_register(pdb, "Port", port_columns,
                            ovs_stats_port_table_delete_cb, NULL,
                            OVS_DB_TABLE_CB_FLAG_DELETE);
 
-  ovs_db_table_cb_register(pdb, "Interface", interface_columns,
-                           ovs_stats_interface_table_change_cb,
+  ovs_db_table_cb_register(
+      pdb, "Interface", interface_columns, ovs_stats_interface_table_change_cb,
       ovs_stats_interface_table_result_cb,
-                           OVS_DB_TABLE_CB_FLAG_INITIAL |
-                           OVS_DB_TABLE_CB_FLAG_INSERT |
+      OVS_DB_TABLE_CB_FLAG_INITIAL | OVS_DB_TABLE_CB_FLAG_INSERT |
           OVS_DB_TABLE_CB_FLAG_MODIFY);
 }
 
 /* Check if bridge is configured to be monitored in config file */
 static int ovs_stats_is_monitored_bridge(const char *br_name) {
-  int rc = 0;
   /* if no bridges are configured, return true */
-  if (!(rc = (g_monitored_bridge_list_head == NULL)))
-    rc = (ovs_stats_get_bridge(g_monitored_bridge_list_head, br_name) != NULL);
-  return rc;
+  if (g_monitored_bridge_list_head == NULL)
+    return 1;
+
+  /* check if given bridge exists */
+  if (ovs_stats_get_bridge(g_monitored_bridge_list_head, br_name) != NULL)
+    return 1;
+
+  return 0;
 }
 
 /* Delete all ports from port list */
 static void ovs_stats_free_port_list(port_list_t *head) {
-  port_list_t *i, *del;
-
-  for (i = head; i != NULL;) {
-    del = i;
+  for (port_list_t *i = head; i != NULL;) {
+    port_list_t *del = i;
     i = i->next;
     sfree(del);
   }
@@ -757,10 +767,8 @@ static void ovs_stats_free_port_list(port_list_t *head) {
 
 /* Delete all bridges from bridge list */
 static void ovs_stats_free_bridge_list(bridge_list_t *head) {
-  bridge_list_t *i, *del;
-
-  for (i = head; i != NULL;) {
-    del = i;
+  for (bridge_list_t *i = head; i != NULL;) {
+    bridge_list_t *del = i;
     i = i->next;
     sfree(del->name);
     sfree(del);
@@ -769,7 +777,6 @@ static void ovs_stats_free_bridge_list(bridge_list_t *head) {
 
 /* Handle OVSDB lost connection callback */
 static void ovs_stats_conn_terminate() {
-
   WARNING("Lost connection to OVSDB server");
   pthread_mutex_lock(&g_stats_lock);
   ovs_stats_free_bridge_list(g_bridge_list_head);
@@ -784,7 +791,6 @@ static void ovs_stats_conn_terminate() {
  */
 static int ovs_stats_plugin_config(oconfig_item_t *ci) {
   bridge_list_t *bridge;
-  char *br_name;
 
   for (int i = 0; i < ci->children_num; i++) {
     oconfig_item_t *child = ci->children + i;
@@ -792,19 +798,19 @@ static int ovs_stats_plugin_config(oconfig_item_t *ci) {
       if (cf_util_get_string_buffer(child, ovs_stats_cfg.ovs_db_node,
                                     OVS_DB_ADDR_NODE_SIZE) != 0) {
         ERROR("%s: parse '%s' option failed", plugin_name, child->key);
-        return (-1);
+        return -1;
       }
     } else if (strcasecmp("Port", child->key) == 0) {
       if (cf_util_get_string_buffer(child, ovs_stats_cfg.ovs_db_serv,
                                     OVS_DB_ADDR_SERVICE_SIZE) != 0) {
         ERROR("%s: parse '%s' option failed", plugin_name, child->key);
-        return (-1);
+        return -1;
       }
     } else if (strcasecmp("Socket", child->key) == 0) {
       if (cf_util_get_string_buffer(child, ovs_stats_cfg.ovs_db_unix,
                                     OVS_DB_ADDR_UNIX_SIZE) != 0) {
         ERROR("%s: parse '%s' option failed", plugin_name, child->key);
-        return (-1);
+        return -1;
       }
     } else if (strcasecmp("Bridges", child->key) == 0) {
       for (int j = 0; j < child->values_num; j++) {
@@ -816,19 +822,22 @@ static int ovs_stats_plugin_config(oconfig_item_t *ci) {
           goto cleanup_fail;
         }
         /* get value */
-        if ((br_name = strdup(child->values[j].value.string)) == NULL) {
-          ERROR("%s: strdup() copy bridge name fail", plugin_name);
-          goto cleanup_fail;
-        }
+        char const *br_name = child->values[j].value.string;
         if ((bridge = ovs_stats_get_bridge(g_monitored_bridge_list_head,
                                            br_name)) == NULL) {
           if ((bridge = calloc(1, sizeof(bridge_list_t))) == NULL) {
             ERROR("%s: Error allocating memory for bridge", plugin_name);
             goto cleanup_fail;
           } else {
+            char *br_name_dup = strdup(br_name);
+            if (br_name_dup == NULL) {
+              ERROR("%s: strdup() copy bridge name fail", plugin_name);
+              goto cleanup_fail;
+            }
+
             pthread_mutex_lock(&g_stats_lock);
             /* store bridge name */
-            bridge->name = br_name;
+            bridge->name = br_name_dup;
             bridge->next = g_monitored_bridge_list_head;
             g_monitored_bridge_list_head = bridge;
             pthread_mutex_unlock(&g_stats_lock);
@@ -841,11 +850,11 @@ static int ovs_stats_plugin_config(oconfig_item_t *ci) {
       goto cleanup_fail;
     }
   }
-  return (0);
+  return 0;
 
 cleanup_fail:
   ovs_stats_free_bridge_list(g_monitored_bridge_list_head);
-  return (-1);
+  return -1;
 }
 
 /* Initialize OvS Stats plugin*/
@@ -857,26 +866,26 @@ static int ovs_stats_plugin_init(void) {
        plugin_name, ovs_stats_cfg.ovs_db_node, ovs_stats_cfg.ovs_db_serv,
        ovs_stats_cfg.ovs_db_unix);
   /* connect to OvS DB */
-  if ((ovs_db = ovs_db_init (ovs_stats_cfg.ovs_db_node,
-                             ovs_stats_cfg.ovs_db_serv,
+  if ((g_ovs_db =
+           ovs_db_init(ovs_stats_cfg.ovs_db_node, ovs_stats_cfg.ovs_db_serv,
                        ovs_stats_cfg.ovs_db_unix, &cb)) == NULL) {
     ERROR("%s: plugin: failed to connect to OvS DB server", plugin_name);
-    return (-1);
+    return -1;
   }
   int err = pthread_mutex_init(&g_stats_lock, NULL);
   if (err < 0) {
     ERROR("%s: plugin: failed to initialize cache lock", plugin_name);
-    ovs_db_destroy(ovs_db);
-    return (-1);
+    ovs_db_destroy(g_ovs_db);
+    return -1;
   }
-  return (0);
+  return 0;
 }
 
 /* OvS stats read callback. Read bridge/port information and submit it*/
 static int ovs_stats_plugin_read(__attribute__((unused)) user_data_t *ud) {
   bridge_list_t *bridge;
   port_list_t *port;
-  char devname[PORT_NAME_SIZE_MAX];
+  char devname[PORT_NAME_SIZE_MAX * 2];
 
   pthread_mutex_lock(&g_stats_lock);
   for (bridge = g_bridge_list_head; bridge != NULL; bridge = bridge->next) {
@@ -927,6 +936,9 @@ static int ovs_stats_plugin_read(__attribute__((unused)) user_data_t *ud) {
           ovs_stats_submit_two(devname, "if_packets", "128_to_255_packets",
                                port->stats[rx_128_to_255_packets],
                                port->stats[tx_128_to_255_packets], meta);
+          ovs_stats_submit_two(devname, "if_packets", "256_to_511_packets",
+                               port->stats[rx_256_to_511_packets],
+                               port->stats[tx_256_to_511_packets], meta);
           ovs_stats_submit_two(devname, "if_packets", "512_to_1023_packets",
                                port->stats[rx_512_to_1023_packets],
                                port->stats[tx_512_to_1023_packets], meta);
@@ -956,20 +968,20 @@ static int ovs_stats_plugin_read(__attribute__((unused)) user_data_t *ud) {
       continue;
   }
   pthread_mutex_unlock(&g_stats_lock);
-  return (0);
+  return 0;
 }
 
 /* Shutdown OvS Stats plugin */
 static int ovs_stats_plugin_shutdown(void) {
   pthread_mutex_lock(&g_stats_lock);
   DEBUG("OvS Statistics plugin shutting down");
-  ovs_db_destroy(ovs_db);
+  ovs_db_destroy(g_ovs_db);
   ovs_stats_free_bridge_list(g_bridge_list_head);
   ovs_stats_free_bridge_list(g_monitored_bridge_list_head);
   ovs_stats_free_port_list(g_port_list_head);
   pthread_mutex_unlock(&g_stats_lock);
   pthread_mutex_destroy(&g_stats_lock);
-  return (0);
+  return 0;
 }
 
 /* Register OvS Stats plugin callbacks */