Tweak CELL_STATS event based on comments by nickm.

- Move cell_command_to_string from control.c to command.c.
- Use accessor for global_circuitlist instead of extern.
- Add a struct for cell statistics by command instead of six arrays.
- Split up control_event_circuit_cell_stats by using two helper functions.
- Add TestingEnableCellStatsEvent option.
- Prepare functions for testing.
- Rename a few variables and document a few things better.

doc/tor.1.txt

@@ -2006,6 +2006,7 @@ The following options are used for running a testing Tor network.
        TestingAuthDirTimeToLearnReachability 0 minutes
        TestingEstimatedDescriptorPropagationTime 0 minutes
        TestingEnableConnBwEvent 1
+       TestingEnableCellStatsEvent 1
 
 **TestingV3AuthInitialVotingInterval** __N__ **minutes**|**hours**::
     Like V3AuthVotingInterval, but for initial voting interval before the first
@@ -2041,6 +2042,11 @@ The following options are used for running a testing Tor network.
     events.  Changing this requires that **TestingTorNetwork** is set.
     (Default: 0)
 
+**TestingEnableCellStatsEvent** **0**|**1**::
+    If this option is set, then Tor controllers may register for CELL_STATS
+    events.  Changing this requires that **TestingTorNetwork** is set.
+    (Default: 0)
+
 
 SIGNALS
 -------

src/or/command.c

@@ -52,6 +52,33 @@ static void command_process_created_cell(cell_t *cell, channel_t *chan);
 static void command_process_relay_cell(cell_t *cell, channel_t *chan);
 static void command_process_destroy_cell(cell_t *cell, channel_t *chan);
 
+/** Convert the cell <b>command</b> into a lower-case, human-readable
+ * string. */
+const char *
+cell_command_to_string(uint8_t command)
+{
+  switch (command) {
+    case CELL_PADDING: return "padding";
+    case CELL_CREATE: return "create";
+    case CELL_CREATED: return "created";
+    case CELL_RELAY: return "relay";
+    case CELL_DESTROY: return "destroy";
+    case CELL_CREATE_FAST: return "create_fast";
+    case CELL_CREATED_FAST: return "created_fast";
+    case CELL_VERSIONS: return "versions";
+    case CELL_NETINFO: return "netinfo";
+    case CELL_RELAY_EARLY: return "relay_early";
+    case CELL_CREATE2: return "create2";
+    case CELL_CREATED2: return "created2";
+    case CELL_VPADDING: return "vpadding";
+    case CELL_CERTS: return "certs";
+    case CELL_AUTH_CHALLENGE: return "auth_challenge";
+    case CELL_AUTHENTICATE: return "authenticate";
+    case CELL_AUTHORIZE: return "authorize";
+    default: return "unrecognized";
+  }
+}
+
 #ifdef KEEP_TIMING_STATS
 /** This is a wrapper function around the actual function that processes the
  * <b>cell</b> that just arrived on <b>conn</b>. Increment <b>*time</b>

src/or/command.h

@@ -19,6 +19,8 @@ void command_process_var_cell(channel_t *chan, var_cell_t *cell);
 void command_setup_channel(channel_t *chan);
 void command_setup_listener(channel_listener_t *chan_l);
 
+const char *cell_command_to_string(uint8_t command);
+
 extern uint64_t stats_n_padding_cells_processed;
 extern uint64_t stats_n_create_cells_processed;
 extern uint64_t stats_n_created_cells_processed;

src/or/config.c

@@ -219,6 +219,7 @@ static config_var_t option_vars_[] = {
   V(DNSListenAddress,            LINELIST, NULL),
   V(DownloadExtraInfo,           BOOL,     "0"),
   V(TestingEnableConnBwEvent,    BOOL,     "0"),
+  V(TestingEnableCellStatsEvent, BOOL,     "0"),
   V(EnforceDistinctSubnets,      BOOL,     "1"),
   V(EntryNodes,                  ROUTERSET,   NULL),
   V(EntryStatistics,             BOOL,     "0"),
@@ -463,6 +464,7 @@ static const config_var_t testing_tor_network_defaults[] = {
   V(TestingEstimatedDescriptorPropagationTime, INTERVAL, "0 minutes"),
   V(MinUptimeHidServDirectoryV2, INTERVAL, "0 minutes"),
   V(TestingEnableConnBwEvent,    BOOL,     "1"),
+  V(TestingEnableCellStatsEvent, BOOL,     "1"),
   VAR("___UsingTestNetworkDefaults", BOOL, UsingTestNetworkDefaults_, "1"),
 
   { NULL, CONFIG_TYPE_OBSOLETE, 0, NULL }
@@ -3244,6 +3246,12 @@ options_validate(or_options_t *old_options, or_options_t *options,
            "Tor networks!");
   }
 
+  if (options->TestingEnableCellStatsEvent &&
+      !options->TestingTorNetwork && !options->UsingTestNetworkDefaults_) {
+    REJECT("TestingEnableCellStatsEvent may only be changed in testing "
+           "Tor networks!");
+  }
+
   if (options->TestingTorNetwork) {
     log_warn(LD_CONFIG, "TestingTorNetwork is set. This will make your node "
                         "almost unusable in the public Tor network, and is "

src/or/control.c

@@ -19,6 +19,7 @@
 #include "circuitlist.h"
 #include "circuitstats.h"
 #include "circuituse.h"
+#include "command.h"
 #include "config.h"
 #include "confparse.h"
 #include "connection.h"
@@ -46,8 +47,6 @@
 #include <sys/resource.h>
 #endif
 
-extern circuit_t *global_circuitlist; /* from circuitlist.c */
-
 #include "procmon.h"
 
 /** Yield true iff <b>s</b> is the state of a control_connection_t that has
@@ -279,7 +278,7 @@ control_update_global_event_mask(void)
       (new_mask & EVENT_CIRC_BANDWIDTH_USED)) {
     circuit_t *circ;
     origin_circuit_t *ocirc;
-    for (circ = global_circuitlist; circ; circ = circ->next) {
+    for (circ = circuit_get_global_list_(); circ; circ = circ->next) {
       if (!CIRCUIT_IS_ORIGIN(circ))
         continue;
       ocirc = TO_ORIGIN_CIRCUIT(circ);
@@ -3944,7 +3943,7 @@ control_event_circ_bandwidth_used(void)
   if (!EVENT_IS_INTERESTING(EVENT_CIRC_BANDWIDTH_USED))
     return 0;
 
-  for (circ = global_circuitlist; circ; circ = circ->next) {
+  for (circ = circuit_get_global_list_(); circ; circ = circ->next) {
     if (!CIRCUIT_IS_ORIGIN(circ))
       continue;
     ocirc = TO_ORIGIN_CIRCUIT(circ);
@@ -4009,162 +4008,150 @@ control_event_conn_bandwidth_used(void)
   return 0;
 }
 
-/** Convert the cell <b>command</b> into a lower-case, human-readable
- * string. */
-static const char *
-cell_command_to_string(uint8_t command)
+/** Helper structure: temporarily stores cell statistics for a circuit. */
+typedef struct cell_stats_t {
+  /** Number of cells added in app-ward direction by command. */
+  uint64_t added_cells_appward[CELL_COMMAND_MAX_ + 1];
+  /** Number of cells added in exit-ward direction by command. */
+  uint64_t added_cells_exitward[CELL_COMMAND_MAX_ + 1];
+  /** Number of cells removed in app-ward direction by command. */
+  uint64_t removed_cells_appward[CELL_COMMAND_MAX_ + 1];
+  /** Number of cells removed in exit-ward direction by command. */
+  uint64_t removed_cells_exitward[CELL_COMMAND_MAX_ + 1];
+  /** Total waiting time of cells in app-ward direction by command. */
+  uint64_t total_time_appward[CELL_COMMAND_MAX_ + 1];
+  /** Total waiting time of cells in exit-ward direction by command. */
+  uint64_t total_time_exitward[CELL_COMMAND_MAX_ + 1];
+} cell_stats_t;
+
+/** Helper: iterate over cell statistics of <b>circ</b> and sum up added
+ * cells, removed cells, and waiting times by cell command and direction.
+ * Store results in <b>cell_stats</b>.  Free cell statistics of the
+ * circuit afterwards. */
+static void
+sum_up_cell_stats_by_command(circuit_t *circ, cell_stats_t *cell_stats)
 {
-  switch (command) {
-    case CELL_PADDING: return "padding";
-    case CELL_CREATE: return "create";
-    case CELL_CREATED: return "created";
-    case CELL_RELAY: return "relay";
-    case CELL_DESTROY: return "destroy";
-    case CELL_CREATE_FAST: return "create_fast";
-    case CELL_CREATED_FAST: return "created_fast";
-    case CELL_VERSIONS: return "versions";
-    case CELL_NETINFO: return "netinfo";
-    case CELL_RELAY_EARLY: return "relay_early";
-    case CELL_CREATE2: return "create2";
-    case CELL_CREATED2: return "created2";
-    case CELL_VPADDING: return "vpadding";
-    case CELL_CERTS: return "certs";
-    case CELL_AUTH_CHALLENGE: return "auth_challenge";
-    case CELL_AUTHENTICATE: return "authenticate";
-    case CELL_AUTHORIZE: return "authorize";
-    default: return "unrecognized";
-  }
+  memset(cell_stats, 0, sizeof(cell_stats_t));
+  SMARTLIST_FOREACH_BEGIN(circ->testing_cell_stats,
+                          testing_cell_stats_entry_t *, ent) {
+    tor_assert(ent->command <= CELL_COMMAND_MAX_);
+    if (!ent->removed && !ent->exitward) {
+      cell_stats->added_cells_appward[ent->command] += 1;
+    } else if (!ent->removed && ent->exitward) {
+      cell_stats->added_cells_exitward[ent->command] += 1;
+    } else if (!ent->exitward) {
+      cell_stats->removed_cells_appward[ent->command] += 1;
+      cell_stats->total_time_appward[ent->command] += ent->waiting_time * 10;
+    } else {
+      cell_stats->removed_cells_exitward[ent->command] += 1;
+      cell_stats->total_time_exitward[ent->command] += ent->waiting_time * 10;
+    }
+    tor_free(ent);
+  } SMARTLIST_FOREACH_END(ent);
+  smartlist_free(circ->testing_cell_stats);
+  circ->testing_cell_stats = NULL;
 }
 
 /** Helper: append a cell statistics string to <code>event_parts</code>,
  * prefixed with <code>key</code>=.  Statistics consist of comma-separated
  * key:value pairs with lower-case command strings as keys and cell
  * numbers or total waiting times as values.  A key:value pair is included
- * if the entry in <code>include_if_positive</code> is positive, but with
+ * if the entry in <code>include_if_non_zero</code> is not zero, but with
  * the (possibly zero) entry from <code>number_to_include</code>.  If no
- * entry in <code>include_if_positive</code> is positive, no string will
+ * entry in <code>include_if_non_zero</code> is positive, no string will
  * be added to <code>event_parts</code>. */
 static void
 append_cell_stats_by_command(smartlist_t *event_parts, const char *key,
-                             uint64_t *include_if_positive,
+                             uint64_t *include_if_non_zero,
                              uint64_t *number_to_include)
 {
   smartlist_t *key_value_strings = smartlist_new();
   int i;
-  for (i = 0; i <= CELL_MAX_; i++) {
-    if (include_if_positive[i] > 0) {
+  for (i = 0; i <= CELL_COMMAND_MAX_; i++) {
+    if (include_if_non_zero[i] > 0) {
       smartlist_add_asprintf(key_value_strings, "%s:"U64_FORMAT,
                              cell_command_to_string(i),
                              U64_PRINTF_ARG(number_to_include[i]));
     }
   }
-  if (key_value_strings->num_used > 0) {
+  if (smartlist_len(key_value_strings) > 0) {
     char *joined = smartlist_join_strings(key_value_strings, ",", 0, NULL);
-    char *result;
-    tor_asprintf(&result, "%s=%s", key, joined);
-    smartlist_add(event_parts, result);
+    smartlist_add_asprintf(event_parts, "%s=%s", key, joined);
     SMARTLIST_FOREACH(key_value_strings, char *, cp, tor_free(cp));
     tor_free(joined);
   }
   smartlist_free(key_value_strings);
 }
 
+/** Helper: format <b>cell_stats</b> for <b>circ</b> for inclusion in a
+ * CELL_STATS event and write result string to <b>event_string</b>. */
+static void
+format_cell_stats(char **event_string, circuit_t *circ,
+                  cell_stats_t *cell_stats)
+{
+  smartlist_t *event_parts = smartlist_new();
+  if (CIRCUIT_IS_ORIGIN(circ)) {
+    origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
+    smartlist_add_asprintf(event_parts, "ID=%lu",
+                 (unsigned long)ocirc->global_identifier);
+  } else {
+    or_circuit_t *or_circ = TO_OR_CIRCUIT(circ);
+    smartlist_add_asprintf(event_parts, "InboundQueue=%lu",
+                 (unsigned long)or_circ->p_circ_id);
+    smartlist_add_asprintf(event_parts, "InboundConn="U64_FORMAT,
+                 U64_PRINTF_ARG(or_circ->p_chan->global_identifier));
+    append_cell_stats_by_command(event_parts, "InboundAdded",
+                                 cell_stats->added_cells_appward,
+                                 cell_stats->added_cells_appward);
+    append_cell_stats_by_command(event_parts, "InboundRemoved",
+                                 cell_stats->removed_cells_appward,
+                                 cell_stats->removed_cells_appward);
+    append_cell_stats_by_command(event_parts, "InboundTime",
+                                 cell_stats->removed_cells_appward,
+                                 cell_stats->total_time_appward);
+  }
+  if (circ->n_chan) {
+    smartlist_add_asprintf(event_parts, "OutboundQueue=%lu",
+                     (unsigned long)circ->n_circ_id);
+    smartlist_add_asprintf(event_parts, "OutboundConn="U64_FORMAT,
+                 U64_PRINTF_ARG(circ->n_chan->global_identifier));
+    append_cell_stats_by_command(event_parts, "OutboundAdded",
+                                 cell_stats->added_cells_exitward,
+                                 cell_stats->added_cells_exitward);
+    append_cell_stats_by_command(event_parts, "OutboundRemoved",
+                                 cell_stats->removed_cells_exitward,
+                                 cell_stats->removed_cells_exitward);
+    append_cell_stats_by_command(event_parts, "OutboundTime",
+                                 cell_stats->removed_cells_exitward,
+                                 cell_stats->total_time_exitward);
+  }
+  *event_string = smartlist_join_strings(event_parts, " ", 0, NULL);
+  SMARTLIST_FOREACH(event_parts, char *, cp, tor_free(cp));
+  smartlist_free(event_parts);
+}
+
 /** A second or more has elapsed: tell any interested control connection
  * how many cells have been processed for a given circuit. */
 int
 control_event_circuit_cell_stats(void)
 {
-  /* These arrays temporarily consume slightly over 6 KiB on the stack
-   * every second, most of which are wasted for the non-existant commands
-   * between CELL_RELAY_EARLY (9) and CELL_VPADDING (128).  But nothing
-   * beats the stack when it comes to performance. */
-  uint64_t added_cells_appward[CELL_MAX_ + 1],
-           added_cells_exitward[CELL_MAX_ + 1],
-           removed_cells_appward[CELL_MAX_ + 1],
-           removed_cells_exitward[CELL_MAX_ + 1],
-           total_time_appward[CELL_MAX_ + 1],
-           total_time_exitward[CELL_MAX_ + 1];
   circuit_t *circ;
-  if (!get_options()->TestingTorNetwork ||
+  cell_stats_t *cell_stats;
+  char *event_string;
+  if (!get_options()->TestingEnableCellStatsEvent ||
       !EVENT_IS_INTERESTING(EVENT_CELL_STATS))
     return 0;
-  for (circ = global_circuitlist; circ; circ = circ->next) {
-    smartlist_t *event_parts;
-    char *event_string;
-
+  cell_stats = tor_malloc(sizeof(cell_stats_t));;
+  for (circ = circuit_get_global_list_(); circ; circ = circ->next) {
     if (!circ->testing_cell_stats)
       continue;
-
-    memset(added_cells_appward, 0, (CELL_MAX_ + 1) * sizeof(uint64_t));
-    memset(added_cells_exitward, 0, (CELL_MAX_ + 1) * sizeof(uint64_t));
-    memset(removed_cells_appward, 0, (CELL_MAX_ + 1) * sizeof(uint64_t));
-    memset(removed_cells_exitward, 0, (CELL_MAX_ + 1) * sizeof(uint64_t));
-    memset(total_time_appward, 0, (CELL_MAX_ + 1) * sizeof(uint64_t));
-    memset(total_time_exitward, 0, (CELL_MAX_ + 1) * sizeof(uint64_t));
-    SMARTLIST_FOREACH_BEGIN(circ->testing_cell_stats,
-                            testing_cell_stats_entry_t *, ent) {
-      tor_assert(ent->command <= CELL_MAX_);
-      if (!ent->removed && !ent->exit_ward) {
-        added_cells_appward[ent->command] += 1;
-      } else if (!ent->removed && ent->exit_ward) {
-        added_cells_exitward[ent->command] += 1;
-      } else if (!ent->exit_ward) {
-        removed_cells_appward[ent->command] += 1;
-        total_time_appward[ent->command] += ent->waiting_time * 10;
-      } else {
-        removed_cells_exitward[ent->command] += 1;
-        total_time_exitward[ent->command] += ent->waiting_time * 10;
-      }
-      tor_free(ent);
-    } SMARTLIST_FOREACH_END(ent);
-    smartlist_free(circ->testing_cell_stats);
-    circ->testing_cell_stats = NULL;
-
-    event_parts = smartlist_new();
-    if (CIRCUIT_IS_ORIGIN(circ)) {
-      origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
-      char *id_string;
-      tor_asprintf(&id_string, "ID=%lu",
-                   (unsigned long)ocirc->global_identifier);
-      smartlist_add(event_parts, id_string);
-    } else {
-      or_circuit_t *or_circ = TO_OR_CIRCUIT(circ);
-      char *queue_string, *conn_string;
-      tor_asprintf(&queue_string, "InboundQueue=%lu",
-                   (unsigned long)or_circ->p_circ_id);
-      tor_asprintf(&conn_string, "InboundConn="U64_FORMAT,
-                   U64_PRINTF_ARG(or_circ->p_chan->global_identifier));
-      smartlist_add(event_parts, queue_string);
-      smartlist_add(event_parts, conn_string);
-      append_cell_stats_by_command(event_parts, "InboundAdded",
-                  added_cells_appward, added_cells_appward);
-      append_cell_stats_by_command(event_parts, "InboundRemoved",
-                  removed_cells_appward, removed_cells_appward);
-      append_cell_stats_by_command(event_parts, "InboundTime",
-                  removed_cells_appward, total_time_appward);
-    }
-    if (circ->n_chan) {
-      char *queue_string, *conn_string;
-      tor_asprintf(&queue_string, "OutboundQueue=%lu",
-                       (unsigned long)circ->n_circ_id);
-      tor_asprintf(&conn_string, "OutboundConn="U64_FORMAT,
-                   U64_PRINTF_ARG(circ->n_chan->global_identifier));
-      smartlist_add(event_parts, queue_string);
-      smartlist_add(event_parts, conn_string);
-      append_cell_stats_by_command(event_parts, "OutboundAdded",
-                  added_cells_exitward, added_cells_exitward);
-      append_cell_stats_by_command(event_parts, "OutboundRemoved",
-                  removed_cells_exitward, removed_cells_exitward);
-      append_cell_stats_by_command(event_parts, "OutboundTime",
-                  removed_cells_exitward, total_time_exitward);
-    }
-    event_string = smartlist_join_strings(event_parts, " ", 0, NULL);
+    sum_up_cell_stats_by_command(circ, cell_stats);
+    format_cell_stats(&event_string, circ, cell_stats);
     send_control_event(EVENT_CELL_STATS, ALL_FORMATS,
                        "650 CELL_STATS %s\r\n", event_string);
-    SMARTLIST_FOREACH(event_parts, char *, cp, tor_free(cp));
-    smartlist_free(event_parts);
     tor_free(event_string);
   }
+  tor_free(cell_stats);
   return 0;
 }
 

src/or/or.h

@@ -839,7 +839,7 @@ typedef enum {
 #define CELL_AUTH_CHALLENGE 130
 #define CELL_AUTHENTICATE 131
 #define CELL_AUTHORIZE 132
-#define CELL_MAX_ 132
+#define CELL_COMMAND_MAX_ 132
 
 /** How long to test reachability before complaining to the user. */
 #define TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT (20*60)
@@ -1089,7 +1089,7 @@ typedef struct insertion_time_queue_t {
 } insertion_time_queue_t;
 
 /** Number of cells with the same command consecutively added to a circuit
- * queue; used for cell statistics only in TestingTorNetwork mode. */
+ * queue; used for cell statistics only if CELL_STATS events are enabled. */
 typedef struct insertion_command_elem_t {
   struct insertion_command_elem_t *next; /**< Next element in queue. */
   /** Which command did these consecutively added cells have? */
@@ -2756,8 +2756,8 @@ typedef struct {
 
 struct create_cell_t;
 
-/** Entry in the cell stats list of a circuit; used only when
- * TestingTorNetwork is set. */
+/** Entry in the cell stats list of a circuit; used only if CELL_STATS
+ * events are enabled. */
 typedef struct testing_cell_stats_entry_t {
   uint8_t command; /**< cell command number. */
   /** Waiting time in centiseconds if this event is for a removed cell,
@@ -2766,7 +2766,7 @@ typedef struct testing_cell_stats_entry_t {
    * delay would long have been closed. */
   unsigned int waiting_time:22;
   unsigned int removed:1; /**< 0 for added to, 1 for removed from queue. */
-  unsigned int exit_ward:1; /**< 0 for app-ward, 1 for exit-ward. */
+  unsigned int exitward:1; /**< 0 for app-ward, 1 for exit-ward. */
 } testing_cell_stats_entry_t;
 
 /**
@@ -2896,8 +2896,8 @@ typedef struct circuit_t {
   struct circuit_t *prev_active_on_n_chan;
 
   /** Various statistics about cells being added to or removed from this
-   * circuit's queues; used only when TestingTorNetwork is set and cleared
-   * after being sent to control port. */
+   * circuit's queues; used only if CELL_STATS events are enabled and
+   * cleared after being sent to control port. */
   smartlist_t *testing_cell_stats;
 } circuit_t;
 
@@ -3988,6 +3988,9 @@ typedef struct {
   /** Enable CONN_BW events.  Only altered on testing networks. */
   int TestingEnableConnBwEvent;
 
+  /** Enable CELL_STATS events.  Only altered on testing networks. */
+  int TestingEnableCellStatsEvent;
+
   /** If true, and we have GeoIP data, and we're a bridge, keep a per-country
    * count of how many client addresses have contacted us so that we can help
    * the bridge authority guess which countries have blocked access to us. */

src/or/relay.c

@@ -2046,7 +2046,7 @@ static mp_pool_t *cell_pool = NULL;
 static mp_pool_t *it_pool = NULL;
 
 /** Memory pool to allocate insertion_command_elem_t objects used for cell
- * statistics in TestingTorNetwork mode. */
+ * statistics if CELL_STATS events are enabled. */
 static mp_pool_t *ic_pool = NULL;
 
 /** Allocate structures to hold cells. */
@@ -2058,7 +2058,7 @@ init_cell_pool(void)
 }
 
 /** Free all storage used to hold cells (and insertion times/commands if we
- * measure cell statistics and/or are in TestingTorNetwork mode). */
+ * measure cell statistics and/or if CELL_STATS events are enabled). */
 void
 free_cell_pool(void)
 {
@@ -2153,16 +2153,68 @@ cell_queue_append(cell_queue_t *queue, packed_cell_t *cell)
   ++queue->n;
 }
 
+/** Append command of type <b>command</b> in direction to <b>queue</b> for
+ * CELL_STATS event. */
+static void
+cell_command_queue_append(cell_queue_t *queue, uint8_t command)
+{
+  insertion_command_queue_t *ic_queue = queue->insertion_commands;
+  if (!ic_pool)
+    ic_pool = mp_pool_new(sizeof(insertion_command_elem_t), 1024);
+  if (!ic_queue) {
+    ic_queue = tor_malloc_zero(sizeof(insertion_command_queue_t));
+    queue->insertion_commands = ic_queue;
+  }
+  if (ic_queue->last && ic_queue->last->command == command) {
+    ic_queue->last->counter++;
+  } else {
+    insertion_command_elem_t *elem = mp_pool_get(ic_pool);
+    elem->next = NULL;
+    elem->command = command;
+    elem->counter = 1;
+    if (ic_queue->last) {
+      ic_queue->last->next = elem;
+      ic_queue->last = elem;
+    } else {
+      ic_queue->first = ic_queue->last = elem;
+    }
+  }
+}
+
+/** Retrieve oldest command from <b>queue</b> and write it to
+ * <b>command</b> for CELL_STATS event.  Return 0 for success, -1
+ * otherwise. */
+static int
+cell_command_queue_pop(uint8_t *command, cell_queue_t *queue)
+{
+  int res = -1;
+  insertion_command_queue_t *ic_queue = queue->insertion_commands;
+  if (ic_queue && ic_queue->first) {
+    insertion_command_elem_t *ic_elem = ic_queue->first;
+    ic_elem->counter--;
+    if (ic_elem->counter < 1) {
+      ic_queue->first = ic_elem->next;
+      if (ic_elem == ic_queue->last)
+        ic_queue->last = NULL;
+      mp_pool_release(ic_elem);
+    }
+    *command = ic_elem->command;
+    res = 0;
+  }
+  return res;
+}
+
 /** Append a newly allocated copy of <b>cell</b> to the end of the
- * <b>exit_ward</b> (or app-ward) <b>queue</b> of <b>circ</b>. */
+ * <b>exitward</b> (or app-ward) <b>queue</b> of <b>circ</b>. */
 void
 cell_queue_append_packed_copy(circuit_t *circ, cell_queue_t *queue,
-                              int exit_ward, const cell_t *cell,
+                              int exitward, const cell_t *cell,
                               int wide_circ_ids)
 {
   packed_cell_t *copy = packed_cell_copy(cell, wide_circ_ids);
   /* Remember the time when this cell was put in the queue. */
-  if (get_options()->CellStatistics || get_options()->TestingTorNetwork) {
+  if (get_options()->CellStatistics ||
+      get_options()->TestingEnableCellStatsEvent) {
     struct timeval now;
     uint32_t added;
     insertion_time_queue_t *it_queue = queue->insertion_times;
@@ -2193,35 +2245,15 @@ cell_queue_append_packed_copy(circuit_t *circ, cell_queue_t *queue,
   }
   /* Remember that we added a cell to the queue, and remember the cell
    * command. */
-  if (get_options()->TestingTorNetwork) {
-    insertion_command_queue_t *ic_queue = queue->insertion_commands;
+  if (get_options()->TestingEnableCellStatsEvent) {
     testing_cell_stats_entry_t *ent =
                       tor_malloc_zero(sizeof(testing_cell_stats_entry_t));
     ent->command = cell->command;
-    ent->exit_ward = exit_ward;
+    ent->exitward = exitward;
     if (!circ->testing_cell_stats)
       circ->testing_cell_stats = smartlist_new();
     smartlist_add(circ->testing_cell_stats, ent);
-    if (!ic_pool)
-      ic_pool = mp_pool_new(sizeof(insertion_command_elem_t), 1024);
-    if (!ic_queue) {
-      ic_queue = tor_malloc_zero(sizeof(insertion_command_queue_t));
-      queue->insertion_commands = ic_queue;
-    }
-    if (ic_queue->last && ic_queue->last->command == cell->command) {
-      ic_queue->last->counter++;
-    } else {
-      insertion_command_elem_t *elem = mp_pool_get(ic_pool);
-      elem->next = NULL;
-      elem->command = cell->command;
-      elem->counter = 1;
-      if (ic_queue->last) {
-        ic_queue->last->next = elem;
-        ic_queue->last = elem;
-      } else {
-        ic_queue->first = ic_queue->last = elem;
-      }
-    }
+    cell_command_queue_append(queue, cell->command);
   }
   cell_queue_append(queue, copy);
 }
@@ -2429,7 +2461,7 @@ channel_flush_from_first_active_circuit(channel_t *chan, int max)
 
     /* Calculate the exact time that this cell has spent in the queue. */
     if (get_options()->CellStatistics ||
-        get_options()->TestingTorNetwork) {
+        get_options()->TestingEnableCellStatsEvent) {
       struct timeval tvnow;
       uint32_t flushed;
       uint32_t cell_waiting_time;
@@ -2460,28 +2492,20 @@ channel_flush_from_first_active_circuit(channel_t *chan, int max)
           or_circ->total_cell_waiting_time += cell_waiting_time;
           or_circ->processed_cells++;
         }
-        if (get_options()->TestingTorNetwork) {
-          insertion_command_queue_t *ic_queue = queue->insertion_commands;
-          if (!ic_queue || !ic_queue->first) {
-            log_info(LD_BUG, "Cannot determine command of cell, which "
-                             "is a bug, because TestingTorNetwork cannot "
-                             "be enabled while running.");
+        if (get_options()->TestingEnableCellStatsEvent) {
+          uint8_t command;
+          if (cell_command_queue_pop(&command, queue) < 0) {
+            log_info(LD_GENERAL, "Cannot determine command of cell. "
+                                 "Looks like the CELL_STATS event was "
+                                 "recently enabled.");
           } else {
             testing_cell_stats_entry_t *ent =
                         tor_malloc_zero(sizeof(testing_cell_stats_entry_t));
-            insertion_command_elem_t *ic_elem = ic_queue->first;
-            ent->command = ic_elem->command;
-            ic_elem->counter--;
-            if (ic_elem->counter < 1) {
-              ic_queue->first = ic_elem->next;
-              if (ic_elem == ic_queue->last)
-                ic_queue->last = NULL;
-              mp_pool_release(ic_elem);
-            }
+            ent->command = command;
             ent->waiting_time = (unsigned int)cell_waiting_time / 10;
             ent->removed = 1;
             if (circ->n_chan == chan)
-              ent->exit_ward = 1;
+              ent->exitward = 1;
             if (!circ->testing_cell_stats)
               circ->testing_cell_stats = smartlist_new();
             smartlist_add(circ->testing_cell_stats, ent);
@@ -2542,12 +2566,12 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
 {
   cell_queue_t *queue;
   int streams_blocked;
-  int exit_ward;
+  int exitward;
   if (circ->marked_for_close)
     return;
 
-  exit_ward = (direction == CELL_DIRECTION_OUT);
-  if (exit_ward) {
+  exitward = (direction == CELL_DIRECTION_OUT);
+  if (exitward) {
     queue = &circ->n_chan_cells;
     streams_blocked = circ->streams_blocked_on_n_chan;
   } else {
@@ -2556,7 +2580,7 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
     streams_blocked = circ->streams_blocked_on_p_chan;
   }
 
-  cell_queue_append_packed_copy(circ, queue, exit_ward, cell,
+  cell_queue_append_packed_copy(circ, queue, exitward, cell,
                                 chan->wide_circ_ids);
 
   /* If we have too many cells on the circuit, we should stop reading from

src/or/relay.h

@@ -53,7 +53,7 @@ void packed_cell_free(packed_cell_t *cell);
 void cell_queue_clear(cell_queue_t *queue);
 void cell_queue_append(cell_queue_t *queue, packed_cell_t *cell);
 void cell_queue_append_packed_copy(circuit_t *circ, cell_queue_t *queue,
-                                   int exit_ward, const cell_t *cell,
+                                   int exitward, const cell_t *cell,
                                    int wide_circ_ids);
 
 void append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,