Browse Source

Add tests for circuitstats.c

These tests primarily test the relaxed and measured behavior of
circuitstats.c, to make sure we did not break it with #23100 or #23114.
Mike Perry 6 years ago
parent
commit
050bb67974
7 changed files with 209 additions and 4 deletions
  1. 2 2
      src/or/circuitbuild.c
  2. 4 2
      src/or/circuituse.c
  3. 1 0
      src/test/Makefile.nmake
  4. 1 0
      src/test/include.am
  5. 1 0
      src/test/test.c
  6. 1 0
      src/test/test.h
  7. 199 0
      src/test/test_circuitstats.c

+ 2 - 2
src/or/circuitbuild.c

@@ -75,7 +75,7 @@ static int onion_pick_cpath_exit(origin_circuit_t *circ, extend_info_t *exit,
                                  int is_hs_v3_rp_circuit);
 static crypt_path_t *onion_next_hop_in_cpath(crypt_path_t *cpath);
 static int onion_extend_cpath(origin_circuit_t *circ);
-static int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice);
+STATIC int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice);
 static int circuit_send_first_onion_skin(origin_circuit_t *circ);
 static int circuit_build_no_more_hops(origin_circuit_t *circ);
 static int circuit_send_intermediate_onion_skin(origin_circuit_t *circ,
@@ -2586,7 +2586,7 @@ onion_extend_cpath(origin_circuit_t *circ)
 /** Create a new hop, annotate it with information about its
  * corresponding router <b>choice</b>, and append it to the
  * end of the cpath <b>head_ptr</b>. */
-static int
+STATIC int
 onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice)
 {
   crypt_path_t *hop = tor_malloc_zero(sizeof(crypt_path_t));

+ 4 - 2
src/or/circuituse.c

@@ -585,7 +585,8 @@ circuit_expire_building(void)
                    TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len :
                    -1,
                  circuit_state_to_string(victim->state),
-                 channel_state_to_string(victim->n_chan->state));
+                 victim->n_chan ?
+                    channel_state_to_string(victim->n_chan->state) : "none");
 
           /* We count the timeout here for CBT, because technically this
            * was a timeout, and the timeout value needs to reset if we
@@ -610,7 +611,8 @@ circuit_expire_building(void)
                    TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len :
                    -1,
                  circuit_state_to_string(victim->state),
-                 channel_state_to_string(victim->n_chan->state),
+                 victim->n_chan ?
+                    channel_state_to_string(victim->n_chan->state) : "none",
                  (long)build_close_ms);
       }
     }

+ 1 - 0
src/test/Makefile.nmake

@@ -18,6 +18,7 @@ TEST_OBJECTS = test.obj test_addr.obj test_channel.obj test_channeltls.obj \
         test_config.obj test_connection.obj \
 	test_cell_formats.obj test_relay.obj test_replay.obj \
 	test_channelpadding.obj \
+	test_circuitstats.obj \
 	test_scheduler.obj test_introduce.obj test_hs.obj tinytest.obj
 
 tinytest.obj: ..\ext\tinytest.c

+ 1 - 0
src/test/include.am

@@ -98,6 +98,7 @@ src_test_test_SOURCES = \
 	src/test/test_circuitmux.c \
 	src/test/test_circuitbuild.c \
 	src/test/test_circuituse.c \
+	src/test/test_circuitstats.c \
 	src/test/test_compat_libevent.c \
 	src/test/test_config.c \
 	src/test/test_connection.c \

+ 1 - 0
src/test/test.c

@@ -1178,6 +1178,7 @@ struct testgroup_t testgroups[] = {
   { "circuitlist/", circuitlist_tests },
   { "circuitmux/", circuitmux_tests },
   { "circuituse/", circuituse_tests },
+  { "circuitstats/", circuitstats_tests },
   { "compat/libevent/", compat_libevent_tests },
   { "config/", config_tests },
   { "connection/", connection_tests },

+ 1 - 0
src/test/test.h

@@ -188,6 +188,7 @@ extern struct testcase_t circuitbuild_tests[];
 extern struct testcase_t circuitlist_tests[];
 extern struct testcase_t circuitmux_tests[];
 extern struct testcase_t circuituse_tests[];
+extern struct testcase_t circuitstats_tests[];
 extern struct testcase_t compat_libevent_tests[];
 extern struct testcase_t config_tests[];
 extern struct testcase_t connection_tests[];

+ 199 - 0
src/test/test_circuitstats.c

@@ -0,0 +1,199 @@
+/* Copyright (c) 2017, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#define CIRCUITBUILD_PRIVATE
+#define CIRCUITSTATS_PRIVATE
+#define CHANNEL_PRIVATE_
+
+#include "or.h"
+#include "test.h"
+#include "test_helpers.h"
+#include "log_test_helpers.h"
+#include "config.h"
+#include "circuitlist.h"
+#include "circuitbuild.h"
+#include "circuitstats.h"
+#include "circuituse.h"
+#include "channel.h"
+
+void test_circuitstats_timeout(void *arg);
+void test_circuitstats_hoplen(void *arg);
+origin_circuit_t *subtest_fourhop_circuit(struct timeval, int);
+origin_circuit_t *add_opened_threehop(void);
+origin_circuit_t *build_unopened_fourhop(struct timeval);
+
+void circuit_free(circuit_t *circ);
+int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice);
+
+static int marked_for_close;
+/* Mock function because we are not trying to test the close circuit that does
+ * an awful lot of checks on the circuit object. */
+static void
+mock_circuit_mark_for_close(circuit_t *circ, int reason, int line,
+                            const char *file)
+{
+  (void) circ;
+  (void) reason;
+  (void) line;
+  (void) file;
+  marked_for_close = 1;
+  return;
+}
+
+origin_circuit_t *
+add_opened_threehop(void)
+{
+  origin_circuit_t *or_circ = origin_circuit_new();
+  extend_info_t fakehop;
+  memset(&fakehop, 0, sizeof(fakehop));
+
+  TO_CIRCUIT(or_circ)->purpose = CIRCUIT_PURPOSE_C_GENERAL;
+
+  or_circ->build_state = tor_malloc_zero(sizeof(cpath_build_state_t));
+  or_circ->build_state->desired_path_len = DEFAULT_ROUTE_LEN;
+
+  onion_append_hop(&or_circ->cpath, &fakehop);
+  onion_append_hop(&or_circ->cpath, &fakehop);
+  onion_append_hop(&or_circ->cpath, &fakehop);
+
+  or_circ->has_opened = 1;
+  TO_CIRCUIT(or_circ)->state = CIRCUIT_STATE_OPEN;
+  TO_CIRCUIT(or_circ)->purpose = CIRCUIT_PURPOSE_C_GENERAL;
+
+  return or_circ;
+}
+
+origin_circuit_t *
+build_unopened_fourhop(struct timeval circ_start_time)
+{
+  origin_circuit_t *or_circ = origin_circuit_new();
+  extend_info_t *fakehop = tor_malloc_zero(sizeof(extend_info_t));
+  memset(fakehop, 0, sizeof(extend_info_t));
+
+  TO_CIRCUIT(or_circ)->purpose = CIRCUIT_PURPOSE_C_GENERAL;
+  TO_CIRCUIT(or_circ)->timestamp_began = circ_start_time;
+  TO_CIRCUIT(or_circ)->timestamp_created = circ_start_time;
+
+  or_circ->build_state = tor_malloc_zero(sizeof(cpath_build_state_t));
+  or_circ->build_state->desired_path_len = 4;
+
+  onion_append_hop(&or_circ->cpath, fakehop);
+  onion_append_hop(&or_circ->cpath, fakehop);
+  onion_append_hop(&or_circ->cpath, fakehop);
+  onion_append_hop(&or_circ->cpath, fakehop);
+
+  return or_circ;
+}
+
+origin_circuit_t *
+subtest_fourhop_circuit(struct timeval circ_start_time, int should_timeout)
+{
+  origin_circuit_t *or_circ = build_unopened_fourhop(circ_start_time);
+
+  // Now make them open one at a time and call
+  // circuit_build_times_handle_completed_hop();
+  or_circ->cpath->state = CPATH_STATE_OPEN;
+  circuit_build_times_handle_completed_hop(or_circ);
+  tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 0);
+
+  or_circ->cpath->next->state = CPATH_STATE_OPEN;
+  circuit_build_times_handle_completed_hop(or_circ);
+  tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 0);
+
+  // Third hop: We should count it now.
+  or_circ->cpath->next->next->state = CPATH_STATE_OPEN;
+  circuit_build_times_handle_completed_hop(or_circ);
+  tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ,
+            !should_timeout); // 1 if counted, 0 otherwise
+
+  // Fourth hop: Don't double count
+  or_circ->cpath->next->next->next->state = CPATH_STATE_OPEN;
+  circuit_build_times_handle_completed_hop(or_circ);
+  tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ,
+            !should_timeout);
+
+ done:
+  return or_circ;
+}
+
+void
+test_circuitstats_hoplen(void *arg)
+{
+  /* Plan:
+   *   0. Test no other opened circs (relaxed timeout)
+   *   1. Check >3 hop circ building w/o timeout
+   *   2. Check >3 hop circs w/ timeouts..
+   */
+  struct timeval circ_start_time;
+  origin_circuit_t *threehop = NULL;
+  origin_circuit_t *fourhop = NULL;
+  (void)arg;
+  MOCK(circuit_mark_for_close_, mock_circuit_mark_for_close);
+
+  circuit_build_times_init(get_circuit_build_times_mutable());
+
+  // Let's set a close_ms to 2X the initial timeout, so we can
+  // test relaxed functionality (which uses the close_ms timeout)
+  get_circuit_build_times_mutable()->close_ms *= 2;
+
+  tor_gettimeofday(&circ_start_time);
+  circ_start_time.tv_sec -= 119; // make us hit "relaxed" cutoff
+
+  // Test 1: Build a fourhop circuit that should get marked
+  // as relaxed and eventually counted by circuit_expire_building
+  // (but not before)
+  fourhop = subtest_fourhop_circuit(circ_start_time, 0);
+  tt_int_op(fourhop->relaxed_timeout, OP_EQ, 0);
+  tt_int_op(marked_for_close, OP_EQ, 0);
+  circuit_expire_building();
+  tt_int_op(marked_for_close, OP_EQ, 0);
+  tt_int_op(fourhop->relaxed_timeout, OP_EQ, 1);
+  TO_CIRCUIT(fourhop)->timestamp_began.tv_sec -= 119;
+  circuit_expire_building();
+  tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 1);
+  tt_int_op(marked_for_close, OP_EQ, 1);
+
+  circuit_free(TO_CIRCUIT(fourhop));
+  circuit_build_times_reset(get_circuit_build_times_mutable());
+
+  // Test 2: Add a threehop circuit for non-relaxed timeouts
+  threehop = add_opened_threehop();
+
+  /* This circuit should not timeout */
+  tor_gettimeofday(&circ_start_time);
+  circ_start_time.tv_sec -= 59;
+  fourhop = subtest_fourhop_circuit(circ_start_time, 0);
+  circuit_expire_building();
+  tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 1);
+  tt_int_op(TO_CIRCUIT(fourhop)->purpose, OP_NE,
+            CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT);
+
+  circuit_free((circuit_t *)fourhop);
+  circuit_build_times_reset(get_circuit_build_times_mutable());
+
+  /* Test 3: This circuit should now time out and get marked as a
+   * measurement circuit, but still get counted (and counted only once)
+   */
+  circ_start_time.tv_sec -= 2;
+  fourhop = subtest_fourhop_circuit(circ_start_time, 0);
+  tt_int_op(TO_CIRCUIT(fourhop)->purpose, OP_EQ,
+            CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT);
+  tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 1);
+  circuit_expire_building();
+  tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 1);
+
+ done:
+  UNMOCK(circuit_mark_for_close_);
+  circuit_free(TO_CIRCUIT(threehop));
+  circuit_free(TO_CIRCUIT(fourhop));
+  circuit_build_times_free_timeouts(get_circuit_build_times_mutable());
+}
+
+#define TEST_CIRCUITSTATS(name, flags) \
+    { #name, test_##name, (flags), NULL, NULL }
+
+struct testcase_t circuitstats_tests[] = {
+  TEST_CIRCUITSTATS(circuitstats_hoplen, TT_FORK),
+  END_OF_TESTCASES
+};
+