Rewrite time-handling in circuitmux_ewma to use monotime_coarse

This part of the code was the only part that used "cached
getttimeofday" feature, which wasn't monotonic, which we updated at
slight expense, and which I'd rather not maintain.

diff --git a/changes/ticket25927.1 b/changes/ticket25927.1
new file mode 100644 (file)
index 0000000..84ac86e
--- /dev/null
+++ b/changes/ticket25927.1
@@ -0,0 +1,6 @@
+  o Minor features (timekeeping, circuit scheduling):
+    - When keeping track of how busy each circuit have been recently on
+      a given connection, use coarse-grained monotonic timers rather than
+      gettimeofday(). This change should marginally increase accuracy
+      and performance.  Implements part of ticket 25927.
+

diff --git a/src/or/circuitmux_ewma.c b/src/or/circuitmux_ewma.c
index 4b80124a7776350a08181dbfcebe74dc4b30a918..a8a645ca57db7c4cd93c897d2a515d2130488854 100644 (file)
--- a/src/or/circuitmux_ewma.c
+++ b/src/or/circuitmux_ewma.c
@@ -28,7 +28,7 @@
  *
  **/
 
-#define TOR_CIRCUITMUX_EWMA_C_
+#define CIRCUITMUX_EWMA_PRIVATE
 
 #include "orconfig.h"
 
@@ -169,8 +169,6 @@ TO_EWMA_POL_CIRC_DATA(circuitmux_policy_circ_data_t *pol)
 
 static void add_cell_ewma(ewma_policy_data_t *pol, cell_ewma_t *ewma);
 static int compare_cell_ewma_counts(const void *p1, const void *p2);
-static unsigned cell_ewma_tick_from_timeval(const struct timeval *now,
-                                            double *remainder_out);
 static circuit_t * cell_ewma_to_circuit(cell_ewma_t *ewma);
 static inline double get_scale_factor(unsigned from_tick, unsigned to_tick);
 static cell_ewma_t * pop_first_cell_ewma(ewma_policy_data_t *pol);
@@ -239,6 +237,13 @@ circuitmux_policy_t ewma_policy = {
   /*.cmp_cmux =*/ ewma_cmp_cmux
 };
 
+/** Have we initialized the ewma tick-counting logic? */
+static int ewma_ticks_initialized = 0;
+/** At what monotime_coarse_t did the current tick begin? */
+static monotime_coarse_t start_of_current_tick;
+/** What is the number of the current tick? */
+static unsigned current_tick_num;
+
 /*** EWMA method implementations using the below EWMA helper functions ***/
 
 /** Compute and return the current cell_ewma tick. */
@@ -421,8 +426,6 @@ ewma_notify_xmit_cells(circuitmux_t *cmux,
   ewma_policy_circ_data_t *cdata = NULL;
   unsigned int tick;
   double fractional_tick, ewma_increment;
-  /* The current (hi-res) time */
-  struct timeval now_hires;
   cell_ewma_t *cell_ewma, *tmp;
 
   tor_assert(cmux);
@@ -435,8 +438,7 @@ ewma_notify_xmit_cells(circuitmux_t *cmux,
   cdata = TO_EWMA_POL_CIRC_DATA(pol_circ_data);
 
   /* Rescale the EWMAs if needed */
-  tor_gettimeofday_cached(&now_hires);
-  tick = cell_ewma_tick_from_timeval(&now_hires, &fractional_tick);
+  tick = cell_ewma_get_current_tick_and_fraction(&fractional_tick);
 
   if (tick != pol->active_circuit_pqueue_last_recalibrated) {
     scale_active_circuits(pol, tick);
@@ -597,24 +599,48 @@ cell_ewma_to_circuit(cell_ewma_t *ewma)
    rescale.
  */
 
-/** Given a timeval <b>now</b>, compute the cell_ewma tick in which it occurs
- * and the fraction of the tick that has elapsed between the start of the tick
- * and <b>now</b>.  Return the former and store the latter in
- * *<b>remainder_out</b>.
+/**
+ * Initialize the system that tells which ewma tick we are in.
+ */
+STATIC void
+cell_ewma_initialize_ticks(void)
+{
+  if (ewma_ticks_initialized)
+    return;
+  monotime_coarse_get(&start_of_current_tick);
+  crypto_rand((char*)&current_tick_num, sizeof(current_tick_num));
+  ewma_ticks_initialized = 1;
+}
+
+/** Compute the current cell_ewma tick and the fraction of the tick that has
+ * elapsed between the start of the tick and the current time.  Return the
+ * former and store the latter in *<b>remainder_out</b>.
  *
  * These tick values are not meant to be shared between Tor instances, or used
  * for other purposes. */
-
-static unsigned
-cell_ewma_tick_from_timeval(const struct timeval *now,
-                            double *remainder_out)
+STATIC unsigned
+cell_ewma_get_current_tick_and_fraction(double *remainder_out)
 {
-  unsigned res = (unsigned) (now->tv_sec / EWMA_TICK_LEN);
-  /* rem */
-  double rem = (now->tv_sec % EWMA_TICK_LEN) +
-    ((double)(now->tv_usec)) / 1.0e6;
-  *remainder_out = rem / EWMA_TICK_LEN;
-  return res;
+  if (BUG(!ewma_ticks_initialized)) {
+    cell_ewma_initialize_ticks(); // LCOV_EXCL_LINE
+  }
+  monotime_coarse_t now;
+  monotime_coarse_get(&now);
+  // XXXX this does a division operation that can be slow on 32-bit
+  // XXXX systems.
+  int32_t msec_diff =
+    (int32_t)monotime_coarse_diff_msec(&start_of_current_tick,
+                                       &now);
+  if (msec_diff > (1000*EWMA_TICK_LEN)) {
+    unsigned ticks_difference = msec_diff / (1000*EWMA_TICK_LEN);
+    monotime_coarse_add_msec(&start_of_current_tick,
+                             &start_of_current_tick,
+                             ticks_difference * 1000 * EWMA_TICK_LEN);
+    current_tick_num += ticks_difference;
+    msec_diff %= 1000*EWMA_TICK_LEN;
+  }
+  *remainder_out = ((double)msec_diff) / (1.0e3 * EWMA_TICK_LEN);
+  return current_tick_num;
 }
 
 /* Default value for the CircuitPriorityHalflifeMsec consensus parameter in
@@ -678,6 +704,8 @@ cmux_ewma_set_options(const or_options_t *options,
   double halflife;
   const char *source;
 
+  cell_ewma_initialize_ticks();
+
   /* Both options and consensus can be NULL. This assures us to either get a
    * valid configured value or the default one. */
   halflife = get_circuit_priority_halflife(options, consensus, &source);
@@ -788,3 +816,12 @@ pop_first_cell_ewma(ewma_policy_data_t *pol)
                               offsetof(cell_ewma_t, heap_index));
 }
 
+/**
+ * Drop all resources held by circuitmux_ewma.c, and deinitialize the
+ * module. */
+void
+circuitmux_ewma_free_all(void)
+{
+  ewma_ticks_initialized = 0;
+}
+

diff --git a/src/or/circuitmux_ewma.h b/src/or/circuitmux_ewma.h
index 2ef8c2586dd7e9c4c0ebcaad96f73f719d914702..f0c4c36095ad671ea0bb6f5eace6fb1eec8e1596 100644 (file)
--- a/src/or/circuitmux_ewma.h
+++ b/src/or/circuitmux_ewma.h
@@ -19,5 +19,12 @@ extern circuitmux_policy_t ewma_policy;
 void cmux_ewma_set_options(const or_options_t *options,
                            const networkstatus_t *consensus);
 
+void circuitmux_ewma_free_all(void);
+
+#ifdef CIRCUITMUX_EWMA_PRIVATE
+STATIC unsigned cell_ewma_get_current_tick_and_fraction(double *remainder_out);
+STATIC void cell_ewma_initialize_ticks(void);
+#endif
+
 #endif /* !defined(TOR_CIRCUITMUX_EWMA_H) */
 

diff --git a/src/or/main.c b/src/or/main.c
index ffe4073295776ce65b8e927c5c031e9e7482cd0e..ee6a2459a925e6bd606fc3d7e6281918b60016c5 100644 (file)
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -59,6 +59,7 @@
 #include "circuitbuild.h"
 #include "circuitlist.h"
 #include "circuituse.h"
+#include "circuitmux_ewma.h"
 #include "command.h"
 #include "compress.h"
 #include "config.h"
@@ -3488,6 +3489,7 @@ tor_free_all(int postfork)
   consdiffmgr_free_all();
   hs_free_all();
   dos_free_all();
+  circuitmux_ewma_free_all();
   if (!postfork) {
     config_free_all();
     or_state_free_all();

diff --git a/src/test/test_channel.c b/src/test/test_channel.c
index 812ec6c1ac92af37161809296f251da4582d3629..7d5018ef5b5af5a5d2f93f96729db807e2e6452f 100644 (file)
--- a/src/test/test_channel.c
+++ b/src/test/test_channel.c
@@ -544,6 +544,8 @@ test_channel_outbound_cell(void *arg)
 
   (void) arg;
 
+  cmux_ewma_set_options(NULL,NULL);
+
   /* The channel will be freed so we need to hijack this so the scheduler
    * doesn't get confused. */
   MOCK(scheduler_release_channel, scheduler_release_channel_mock);

diff --git a/src/test/test_circuitmux.c b/src/test/test_circuitmux.c
index 75b7a0ea47d8548d02a2f1930baa7c265f17c2b5..14c7598703936d04def348ac66f14d7c916906ef 100644 (file)
--- a/src/test/test_circuitmux.c
+++ b/src/test/test_circuitmux.c
@@ -3,6 +3,7 @@
 
 #define TOR_CHANNEL_INTERNAL_
 #define CIRCUITMUX_PRIVATE
+#define CIRCUITMUX_EWMA_PRIVATE
 #define RELAY_PRIVATE
 #include "or.h"
 #include "channel.h"
@@ -79,8 +80,47 @@ test_cmux_destroy_cell_queue(void *arg)
   tor_free(dc);
 }
 
+static void
+test_cmux_compute_ticks(void *arg)
+{
+  const int64_t NS_PER_S = 1000 * 1000 * 1000;
+  const int64_t START_NS = U64_LITERAL(1217709000)*NS_PER_S;
+  int64_t now;
+  double rem;
+  unsigned tick;
+  (void)arg;
+  circuitmux_ewma_free_all();
+  monotime_enable_test_mocking();
+
+  monotime_coarse_set_mock_time_nsec(START_NS);
+  cell_ewma_initialize_ticks();
+  const unsigned tick_zero = cell_ewma_get_current_tick_and_fraction(&rem);
+  tt_double_op(rem, OP_GT, -1e-9);
+  tt_double_op(rem, OP_LT, 1e-9);
+
+  /* 1.5 second later and we should still be in the same tick. */
+  now = START_NS + NS_PER_S + NS_PER_S/2;
+  monotime_coarse_set_mock_time_nsec(now);
+  tick = cell_ewma_get_current_tick_and_fraction(&rem);
+  tt_uint_op(tick, OP_EQ, tick_zero);
+  tt_double_op(rem, OP_GT, .149999999);
+  tt_double_op(rem, OP_LT, .150000001);
+
+  /* 25 second later and we should be in another tick. */
+  now = START_NS + NS_PER_S * 25;
+  monotime_coarse_set_mock_time_nsec(now);
+  tick = cell_ewma_get_current_tick_and_fraction(&rem);
+  tt_uint_op(tick, OP_EQ, tick_zero + 2);
+  tt_double_op(rem, OP_GT, .499999999);
+  tt_double_op(rem, OP_LT, .500000001);
+
+ done:
+  ;
+}
+
 struct testcase_t circuitmux_tests[] = {
   { "destroy_cell_queue", test_cmux_destroy_cell_queue, TT_FORK, NULL, NULL },
+  { "compute_ticks", test_cmux_compute_ticks, TT_FORK, NULL, NULL },
   END_OF_TESTCASES
 };