Browse Source

Merge branch 'ticket25268_034_01'

Nick Mathewson 6 years ago
parent
commit
a324cd9020

+ 7 - 0
changes/ticket25268

@@ -0,0 +1,7 @@
+  o Removed features:
+    - The old "round-robin" circuit multiplexer (circuitmux)
+      implementation has been removed, along with a fairly large set of
+      code that existed to support it.  It has not been the default
+      circuitmux since we introduced the "EWMA" circuitmux in 0.2.4.x,
+      but it still required an unreasonable amount of memory and CPU.
+      Closes ticket 25268.

+ 8 - 10
doc/tor.1.txt

@@ -780,17 +780,15 @@ GENERAL OPTIONS
     This is useful when running on flash memory or other media that support
     only a limited number of writes. (Default: 0)
 
-[[CircuitPriorityHalflife]] **CircuitPriorityHalflife** __NUM1__::
+[[CircuitPriorityHalflife]] **CircuitPriorityHalflife** __NUM__::
     If this value is set, we override the default algorithm for choosing which
-    circuit's cell to deliver or relay next. When the value is 0, we
-    round-robin between the active circuits on a connection, delivering one
-    cell from each in turn. When the value is positive, we prefer delivering
-    cells from whichever connection has the lowest weighted cell count, where
-    cells are weighted exponentially according to the supplied
-    CircuitPriorityHalflife value (in seconds). If this option is not set at
-    all, we use the behavior recommended in the current consensus
-    networkstatus. This is an advanced option; you generally shouldn't have
-    to mess with it. (Default: not set)
+    circuit's cell to deliver or relay next. It is delivered first to the
+    circuit that has the lowest weighted cell count, where cells are weighted
+    exponentially according to this value (in seconds). If the value is -1, it
+    is taken from the consensus if possible else it will fallback to the
+    default value of 30. Minimum: 1, Maximum: 2147483647. This can be defined
+    as a float value. This is an advanced option; you generally shouldn't have
+    to mess with it. (Default: -1)
 
 [[CountPrivateBandwidth]] **CountPrivateBandwidth** **0**|**1**::
     If this option is set, then Tor's rate-limiting applies not only to

+ 0 - 15
src/or/channel.c

@@ -2108,21 +2108,6 @@ channel_listener_dumpstats(int severity)
   }
 }
 
-/**
- * Set the cmux policy on all active channels.
- */
-void
-channel_set_cmux_policy_everywhere(circuitmux_policy_t *pol)
-{
-  if (!active_channels) return;
-
-  SMARTLIST_FOREACH_BEGIN(active_channels, channel_t *, curr) {
-    if (curr->cmux) {
-      circuitmux_set_policy(curr->cmux, pol);
-    }
-  } SMARTLIST_FOREACH_END(curr);
-}
-
 /**
  * Clean up channels.
  *

+ 0 - 3
src/or/channel.h

@@ -422,9 +422,6 @@ void channel_free_all(void);
 void channel_dumpstats(int severity);
 void channel_listener_dumpstats(int severity);
 
-/* Set the cmux policy on all active channels */
-void channel_set_cmux_policy_everywhere(circuitmux_policy_t *pol);
-
 #ifdef TOR_CHANNEL_INTERNAL_
 
 #ifdef CHANNEL_PRIVATE_

+ 2 - 3
src/or/channeltls.c

@@ -160,9 +160,8 @@ channel_tls_common_init(channel_tls_t *tlschan)
   chan->write_var_cell = channel_tls_write_var_cell_method;
 
   chan->cmux = circuitmux_alloc();
-  if (cell_ewma_enabled()) {
-    circuitmux_set_policy(chan->cmux, &ewma_policy);
-  }
+  /* We only have one policy for now so always set it to EWMA. */
+  circuitmux_set_policy(chan->cmux, &ewma_policy);
 }
 
 /**

+ 0 - 5
src/or/circuitlist.c

@@ -403,9 +403,6 @@ circuit_set_p_circid_chan(or_circuit_t *or_circ, circid_t id,
   circuit_set_circid_chan_helper(circ, CELL_DIRECTION_IN, id, chan);
 
   if (chan) {
-    tor_assert(bool_eq(or_circ->p_chan_cells.n,
-                       or_circ->next_active_on_p_chan));
-
     chan->timestamp_last_had_circuits = approx_time();
   }
 
@@ -428,8 +425,6 @@ circuit_set_n_circid_chan(circuit_t *circ, circid_t id,
   circuit_set_circid_chan_helper(circ, CELL_DIRECTION_OUT, id, chan);
 
   if (chan) {
-    tor_assert(bool_eq(circ->n_chan_cells.n, circ->next_active_on_n_chan));
-
     chan->timestamp_last_had_circuits = approx_time();
   }
 

+ 27 - 651
src/or/circuitmux.c

@@ -114,13 +114,6 @@ struct circuitmux_s {
    */
   chanid_circid_muxinfo_map_t *chanid_circid_map;
 
-  /*
-   * Double-linked ring of circuits with queued cells waiting for room to
-   * free up on this connection's outbuf.  Every time we pull cells from
-   * a circuit, we advance this pointer to the next circuit in the ring.
-   */
-  struct circuit_t *active_circuits_head, *active_circuits_tail;
-
   /** List of queued destroy cells */
   destroy_cell_queue_t destroy_cell_queue;
   /** Boolean: True iff the last cell to circuitmux_get_first_active_circuit
@@ -176,17 +169,6 @@ struct chanid_circid_muxinfo_t {
   circuit_muxinfo_t muxinfo;
 };
 
-/*
- * Internal-use #defines
- */
-
-#ifdef CMUX_PARANOIA
-#define circuitmux_assert_okay_paranoid(cmux) \
-  circuitmux_assert_okay(cmux)
-#else
-#define circuitmux_assert_okay_paranoid(cmux)
-#endif /* defined(CMUX_PARANOIA) */
-
 /*
  * Static function declarations
  */
@@ -199,21 +181,9 @@ chanid_circid_entry_hash(chanid_circid_muxinfo_t *a);
 static chanid_circid_muxinfo_t *
 circuitmux_find_map_entry(circuitmux_t *cmux, circuit_t *circ);
 static void
-circuitmux_make_circuit_active(circuitmux_t *cmux, circuit_t *circ,
-                               cell_direction_t direction);
+circuitmux_make_circuit_active(circuitmux_t *cmux, circuit_t *circ);
 static void
-circuitmux_make_circuit_inactive(circuitmux_t *cmux, circuit_t *circ,
-                                 cell_direction_t direction);
-static inline void
-circuitmux_move_active_circ_to_tail(circuitmux_t *cmux, circuit_t *circ,
-                                    cell_direction_t direction);
-static inline circuit_t **
-circuitmux_next_active_circ_p(circuitmux_t *cmux, circuit_t *circ);
-static inline circuit_t **
-circuitmux_prev_active_circ_p(circuitmux_t *cmux, circuit_t *circ);
-static void circuitmux_assert_okay_pass_one(circuitmux_t *cmux);
-static void circuitmux_assert_okay_pass_two(circuitmux_t *cmux);
-static void circuitmux_assert_okay_pass_three(circuitmux_t *cmux);
+circuitmux_make_circuit_inactive(circuitmux_t *cmux, circuit_t *circ);
 
 /* Static global variables */
 
@@ -222,119 +192,6 @@ static int64_t global_destroy_ctr = 0;
 
 /* Function definitions */
 
-/**
- * Linked list helpers
- */
-
-/**
- * Move an active circuit to the tail of the cmux's active circuits list;
- * used by circuitmux_notify_xmit_cells().
- */
-
-static inline void
-circuitmux_move_active_circ_to_tail(circuitmux_t *cmux, circuit_t *circ,
-                                    cell_direction_t direction)
-{
-  circuit_t **next_p = NULL, **prev_p = NULL;
-  circuit_t **next_prev = NULL, **prev_next = NULL;
-  circuit_t **tail_next = NULL;
-  or_circuit_t *or_circ = NULL;
-
-  tor_assert(cmux);
-  tor_assert(circ);
-
-  circuitmux_assert_okay_paranoid(cmux);
-
-  /* Figure out our next_p and prev_p for this cmux/direction */
-  if (direction) {
-    if (direction == CELL_DIRECTION_OUT) {
-      tor_assert(circ->n_mux == cmux);
-      next_p = &(circ->next_active_on_n_chan);
-      prev_p = &(circ->prev_active_on_n_chan);
-    } else {
-      or_circ = TO_OR_CIRCUIT(circ);
-      tor_assert(or_circ->p_mux == cmux);
-      next_p = &(or_circ->next_active_on_p_chan);
-      prev_p = &(or_circ->prev_active_on_p_chan);
-    }
-  } else {
-    if (circ->n_mux == cmux) {
-      next_p = &(circ->next_active_on_n_chan);
-      prev_p = &(circ->prev_active_on_n_chan);
-    } else {
-      or_circ = TO_OR_CIRCUIT(circ);
-      tor_assert(or_circ->p_mux == cmux);
-      next_p = &(or_circ->next_active_on_p_chan);
-      prev_p = &(or_circ->prev_active_on_p_chan);
-    }
-  }
-  tor_assert(next_p);
-  tor_assert(prev_p);
-
-  /* Check if this really is an active circuit */
-  if ((*next_p == NULL && *prev_p == NULL) &&
-      !(circ == cmux->active_circuits_head ||
-        circ == cmux->active_circuits_tail)) {
-    /* Not active, no-op */
-    return;
-  }
-
-  /* Check if this is already the tail */
-  if (circ == cmux->active_circuits_tail) return;
-
-  /* Okay, we have to move it; figure out next_prev and prev_next */
-  if (*next_p) next_prev = circuitmux_prev_active_circ_p(cmux, *next_p);
-  if (*prev_p) prev_next = circuitmux_next_active_circ_p(cmux, *prev_p);
-  /* Adjust the previous node's next pointer, if any */
-  if (prev_next) *prev_next = *next_p;
-  /* Otherwise, we were the head */
-  else cmux->active_circuits_head = *next_p;
-  /* Adjust the next node's previous pointer, if any */
-  if (next_prev) *next_prev = *prev_p;
-  /* We're out of the list; now re-attach at the tail */
-  /* Adjust our next and prev pointers */
-  *next_p = NULL;
-  *prev_p = cmux->active_circuits_tail;
-  /* Set the next pointer of the tail, or the head if none */
-  if (cmux->active_circuits_tail) {
-    tail_next = circuitmux_next_active_circ_p(cmux,
-                                              cmux->active_circuits_tail);
-    *tail_next = circ;
-  } else {
-    cmux->active_circuits_head = circ;
-  }
-  /* Set the tail to this circuit */
-  cmux->active_circuits_tail = circ;
-
-  circuitmux_assert_okay_paranoid(cmux);
-}
-
-static inline circuit_t **
-circuitmux_next_active_circ_p(circuitmux_t *cmux, circuit_t *circ)
-{
-  tor_assert(cmux);
-  tor_assert(circ);
-
-  if (circ->n_mux == cmux) return &(circ->next_active_on_n_chan);
-  else {
-    tor_assert(TO_OR_CIRCUIT(circ)->p_mux == cmux);
-    return &(TO_OR_CIRCUIT(circ)->next_active_on_p_chan);
-  }
-}
-
-static inline circuit_t **
-circuitmux_prev_active_circ_p(circuitmux_t *cmux, circuit_t *circ)
-{
-  tor_assert(cmux);
-  tor_assert(circ);
-
-  if (circ->n_mux == cmux) return &(circ->prev_active_on_n_chan);
-  else {
-    tor_assert(TO_OR_CIRCUIT(circ)->p_mux == cmux);
-    return &(TO_OR_CIRCUIT(circ)->prev_active_on_p_chan);
-  }
-}
-
 /**
  * Helper for chanid_circid_cell_count_map_t hash table: compare the channel
  * ID and circuit ID for a and b, and return less than, equal to, or greater
@@ -406,11 +263,6 @@ circuitmux_detach_all_circuits(circuitmux_t *cmux, smartlist_t *detached_out)
   circuit_t *circ = NULL;
 
   tor_assert(cmux);
-  /*
-   * Don't circuitmux_assert_okay_paranoid() here; this gets called when
-   * channels are being freed and have already been unregistered, so
-   * the channel ID lookups it does will fail.
-   */
 
   i = HT_START(chanid_circid_muxinfo_map, cmux->chanid_circid_map);
   while (i) {
@@ -435,7 +287,7 @@ circuitmux_detach_all_circuits(circuitmux_t *cmux, smartlist_t *detached_out)
              */
 
             if (to_remove->muxinfo.cell_count > 0) {
-              circuitmux_make_circuit_inactive(cmux, circ, CELL_DIRECTION_OUT);
+              circuitmux_make_circuit_inactive(cmux, circ);
             }
 
             /* Clear n_mux */
@@ -450,7 +302,7 @@ circuitmux_detach_all_circuits(circuitmux_t *cmux, smartlist_t *detached_out)
              */
 
             if (to_remove->muxinfo.cell_count > 0) {
-              circuitmux_make_circuit_inactive(cmux, circ, CELL_DIRECTION_IN);
+              circuitmux_make_circuit_inactive(cmux, circ);
             }
 
             /*
@@ -606,9 +458,7 @@ circuitmux_clear_policy(circuitmux_t *cmux)
   tor_assert(cmux);
 
   /* Internally, this is just setting policy to NULL */
-  if (cmux->policy) {
-    circuitmux_set_policy(cmux, NULL);
-  }
+  circuitmux_set_policy(cmux, NULL);
 }
 
 /**
@@ -944,7 +794,6 @@ circuitmux_attach_circuit,(circuitmux_t *cmux, circuit_t *circ,
   tor_assert(circ);
   tor_assert(direction == CELL_DIRECTION_IN ||
              direction == CELL_DIRECTION_OUT);
-  circuitmux_assert_okay_paranoid(cmux);
 
   /*
    * Figure out which channel we're using, and get the circuit's current
@@ -1002,10 +851,10 @@ circuitmux_attach_circuit,(circuitmux_t *cmux, circuit_t *circ,
      */
     if (hashent->muxinfo.cell_count > 0 && cell_count == 0) {
       --(cmux->n_active_circuits);
-      circuitmux_make_circuit_inactive(cmux, circ, direction);
+      circuitmux_make_circuit_inactive(cmux, circ);
     } else if (hashent->muxinfo.cell_count == 0 && cell_count > 0) {
       ++(cmux->n_active_circuits);
-      circuitmux_make_circuit_active(cmux, circ, direction);
+      circuitmux_make_circuit_active(cmux, circ);
     }
     cmux->n_cells -= hashent->muxinfo.cell_count;
     cmux->n_cells += cell_count;
@@ -1033,7 +882,7 @@ circuitmux_attach_circuit,(circuitmux_t *cmux, circuit_t *circ,
     hashent->muxinfo.cell_count = cell_count;
     hashent->muxinfo.direction = direction;
     /* Allocate policy specific circuit data if we need it */
-    if (cmux->policy && cmux->policy->alloc_circ_data) {
+    if (cmux->policy->alloc_circ_data) {
       /* Assert that we have the means to free policy-specific data */
       tor_assert(cmux->policy->free_circ_data);
       /* Allocate it */
@@ -1053,25 +902,14 @@ circuitmux_attach_circuit,(circuitmux_t *cmux, circuit_t *circ,
     if (direction == CELL_DIRECTION_OUT) circ->n_mux = cmux;
     else TO_OR_CIRCUIT(circ)->p_mux = cmux;
 
-    /* Make sure the next/prev pointers are NULL */
-    if (direction == CELL_DIRECTION_OUT) {
-      circ->next_active_on_n_chan = NULL;
-      circ->prev_active_on_n_chan = NULL;
-    } else {
-      TO_OR_CIRCUIT(circ)->next_active_on_p_chan = NULL;
-      TO_OR_CIRCUIT(circ)->prev_active_on_p_chan = NULL;
-    }
-
     /* Update counters */
     ++(cmux->n_circuits);
     if (cell_count > 0) {
       ++(cmux->n_active_circuits);
-      circuitmux_make_circuit_active(cmux, circ, direction);
+      circuitmux_make_circuit_active(cmux, circ);
     }
     cmux->n_cells += cell_count;
   }
-
-  circuitmux_assert_okay_paranoid(cmux);
 }
 
 /**
@@ -1095,7 +933,6 @@ circuitmux_detach_circuit,(circuitmux_t *cmux, circuit_t *circ))
   tor_assert(cmux);
   tor_assert(cmux->chanid_circid_map);
   tor_assert(circ);
-  circuitmux_assert_okay_paranoid(cmux);
 
   /* See if we have it for n_chan/n_circ_id */
   if (circ->n_chan) {
@@ -1133,7 +970,7 @@ circuitmux_detach_circuit,(circuitmux_t *cmux, circuit_t *circ))
     if (hashent->muxinfo.cell_count > 0) {
       --(cmux->n_active_circuits);
       /* This does policy notifies, so comes before freeing policy data */
-      circuitmux_make_circuit_inactive(cmux, circ, last_searched_direction);
+      circuitmux_make_circuit_inactive(cmux, circ);
     }
     cmux->n_cells -= hashent->muxinfo.cell_count;
 
@@ -1162,8 +999,6 @@ circuitmux_detach_circuit,(circuitmux_t *cmux, circuit_t *circ))
     /* Free the hash entry */
     tor_free(hashent);
   }
-
-  circuitmux_assert_okay_paranoid(cmux);
 }
 
 /**
@@ -1172,94 +1007,22 @@ circuitmux_detach_circuit,(circuitmux_t *cmux, circuit_t *circ))
  */
 
 static void
-circuitmux_make_circuit_active(circuitmux_t *cmux, circuit_t *circ,
-                               cell_direction_t direction)
+circuitmux_make_circuit_active(circuitmux_t *cmux, circuit_t *circ)
 {
-  circuit_t **next_active = NULL, **prev_active = NULL, **next_prev = NULL;
-  circuitmux_t *circuit_cmux = NULL;
-  chanid_circid_muxinfo_t *hashent = NULL;
-  channel_t *chan = NULL;
-  circid_t circ_id;
-  int already_active;
-
   tor_assert(cmux);
+  tor_assert(cmux->policy);
   tor_assert(circ);
-  tor_assert(direction == CELL_DIRECTION_OUT ||
-             direction == CELL_DIRECTION_IN);
-  /*
-   * Don't circuitmux_assert_okay_paranoid(cmux) here because the cell count
-   * already got changed and we have to update the list for it to be consistent
-   * again.
-   */
-
-  /* Get the right set of active list links for this direction */
-  if (direction == CELL_DIRECTION_OUT) {
-    next_active = &(circ->next_active_on_n_chan);
-    prev_active = &(circ->prev_active_on_n_chan);
-    circuit_cmux = circ->n_mux;
-    chan = circ->n_chan;
-    circ_id = circ->n_circ_id;
-  } else {
-    next_active = &(TO_OR_CIRCUIT(circ)->next_active_on_p_chan);
-    prev_active = &(TO_OR_CIRCUIT(circ)->prev_active_on_p_chan);
-    circuit_cmux = TO_OR_CIRCUIT(circ)->p_mux;
-    chan = TO_OR_CIRCUIT(circ)->p_chan;
-    circ_id = TO_OR_CIRCUIT(circ)->p_circ_id;
-  }
-
-  /* Assert that it is attached to this mux and a channel */
-  tor_assert(cmux == circuit_cmux);
-  tor_assert(chan != NULL);
-
-  /*
-   * Check if the circuit really was inactive; if it's active, at least one
-   * of the next_active and prev_active pointers will not be NULL, or this
-   * circuit will be either the head or tail of the list for this cmux.
-   */
-  already_active = (*prev_active != NULL || *next_active != NULL ||
-                    cmux->active_circuits_head == circ ||
-                    cmux->active_circuits_tail == circ);
-
-  /* If we're already active, log a warning and finish */
-  if (already_active) {
-    log_warn(LD_CIRC,
-             "Circuit %u on channel " U64_FORMAT " was already active",
-             (unsigned)circ_id, U64_PRINTF_ARG(chan->global_identifier));
-    return;
-  }
-
-  /*
-   * This is going at the head of the list; if the old head is not NULL,
-   * then its prev pointer should point to this.
-   */
-  *next_active = cmux->active_circuits_head; /* Next is old head */
-  *prev_active = NULL; /* Prev is NULL (this will be the head) */
-  if (cmux->active_circuits_head) {
-    /* The list had an old head; update its prev pointer */
-    next_prev =
-      circuitmux_prev_active_circ_p(cmux, cmux->active_circuits_head);
-    tor_assert(next_prev);
-    *next_prev = circ;
-  } else {
-    /* The list was empty; this becomes the tail as well */
-    cmux->active_circuits_tail = circ;
-  }
-  /* This becomes the new head of the list */
-  cmux->active_circuits_head = circ;
 
   /* Policy-specific notification */
-  if (cmux->policy &&
-      cmux->policy->notify_circ_active) {
+  if (cmux->policy->notify_circ_active) {
     /* Okay, we need to check the circuit for policy data now */
-    hashent = circuitmux_find_map_entry(cmux, circ);
+    chanid_circid_muxinfo_t *hashent = circuitmux_find_map_entry(cmux, circ);
     /* We should have found something */
     tor_assert(hashent);
     /* Notify */
     cmux->policy->notify_circ_active(cmux, cmux->policy_data,
                                      circ, hashent->muxinfo.policy_data);
   }
-
-  circuitmux_assert_okay_paranoid(cmux);
 }
 
 /**
@@ -1268,112 +1031,22 @@ circuitmux_make_circuit_active(circuitmux_t *cmux, circuit_t *circ,
  */
 
 static void
-circuitmux_make_circuit_inactive(circuitmux_t *cmux, circuit_t *circ,
-                                 cell_direction_t direction)
+circuitmux_make_circuit_inactive(circuitmux_t *cmux, circuit_t *circ)
 {
-  circuit_t **next_active = NULL, **prev_active = NULL;
-  circuit_t **next_prev = NULL, **prev_next = NULL;
-  circuitmux_t *circuit_cmux = NULL;
-  chanid_circid_muxinfo_t *hashent = NULL;
-  channel_t *chan = NULL;
-  circid_t circ_id;
-  int already_inactive;
-
   tor_assert(cmux);
+  tor_assert(cmux->policy);
   tor_assert(circ);
-  tor_assert(direction == CELL_DIRECTION_OUT ||
-             direction == CELL_DIRECTION_IN);
-  /*
-   * Don't circuitmux_assert_okay_paranoid(cmux) here because the cell count
-   * already got changed and we have to update the list for it to be consistent
-   * again.
-   */
-
-  /* Get the right set of active list links for this direction */
-  if (direction == CELL_DIRECTION_OUT) {
-    next_active = &(circ->next_active_on_n_chan);
-    prev_active = &(circ->prev_active_on_n_chan);
-    circuit_cmux = circ->n_mux;
-    chan = circ->n_chan;
-    circ_id = circ->n_circ_id;
-  } else {
-    next_active = &(TO_OR_CIRCUIT(circ)->next_active_on_p_chan);
-    prev_active = &(TO_OR_CIRCUIT(circ)->prev_active_on_p_chan);
-    circuit_cmux = TO_OR_CIRCUIT(circ)->p_mux;
-    chan = TO_OR_CIRCUIT(circ)->p_chan;
-    circ_id = TO_OR_CIRCUIT(circ)->p_circ_id;
-  }
-
-  /* Assert that it is attached to this mux and a channel */
-  tor_assert(cmux == circuit_cmux);
-  tor_assert(chan != NULL);
-
-  /*
-   * Check if the circuit really was active; if it's inactive, the
-   * next_active and prev_active pointers will be NULL and this circuit
-   * will not be the head or tail of the list for this cmux.
-   */
-  already_inactive = (*prev_active == NULL && *next_active == NULL &&
-                      cmux->active_circuits_head != circ &&
-                      cmux->active_circuits_tail != circ);
-
-  /* If we're already inactive, log a warning and finish */
-  if (already_inactive) {
-    log_warn(LD_CIRC,
-             "Circuit %d on channel " U64_FORMAT " was already inactive",
-             (unsigned)circ_id, U64_PRINTF_ARG(chan->global_identifier));
-    return;
-  }
-
-  /* Remove from the list; first get next_prev and prev_next */
-  if (*next_active) {
-    /*
-     * If there's a next circuit, its previous circuit becomes this
-     * circuit's previous circuit.
-     */
-    next_prev = circuitmux_prev_active_circ_p(cmux, *next_active);
-  } else {
-    /* Else, the tail becomes this circuit's previous circuit */
-    next_prev = &(cmux->active_circuits_tail);
-  }
-
-  /* Got next_prev, now prev_next */
-  if (*prev_active) {
-    /*
-     * If there's a previous circuit, its next circuit becomes this circuit's
-     * next circuit.
-     */
-    prev_next = circuitmux_next_active_circ_p(cmux, *prev_active);
-  } else {
-    /* Else, the head becomes this circuit's next circuit */
-    prev_next = &(cmux->active_circuits_head);
-  }
-
-  /* Assert that we got sensible values for the next/prev pointers */
-  tor_assert(next_prev != NULL);
-  tor_assert(prev_next != NULL);
-
-  /* Update the next/prev pointers - this removes circ from the list */
-  *next_prev = *prev_active;
-  *prev_next = *next_active;
-
-  /* Now null out prev_active/next_active */
-  *prev_active = NULL;
-  *next_active = NULL;
 
   /* Policy-specific notification */
-  if (cmux->policy &&
-      cmux->policy->notify_circ_inactive) {
+  if (cmux->policy->notify_circ_inactive) {
     /* Okay, we need to check the circuit for policy data now */
-    hashent = circuitmux_find_map_entry(cmux, circ);
+    chanid_circid_muxinfo_t *hashent = circuitmux_find_map_entry(cmux, circ);
     /* We should have found something */
     tor_assert(hashent);
     /* Notify */
     cmux->policy->notify_circ_inactive(cmux, cmux->policy_data,
                                        circ, hashent->muxinfo.policy_data);
   }
-
-  circuitmux_assert_okay_paranoid(cmux);
 }
 
 /**
@@ -1400,8 +1073,6 @@ circuitmux_set_num_cells(circuitmux_t *cmux, circuit_t *circ,
   tor_assert(cmux);
   tor_assert(circ);
 
-  circuitmux_assert_okay_paranoid(cmux);
-
   /* Search for this circuit's entry */
   hashent = circuitmux_find_map_entry(cmux, circ);
   /* Assert that we found one */
@@ -1412,7 +1083,7 @@ circuitmux_set_num_cells(circuitmux_t *cmux, circuit_t *circ,
   cmux->n_cells += n_cells;
 
   /* Do we need to notify a cmux policy? */
-  if (cmux->policy && cmux->policy->notify_set_n_cells) {
+  if (cmux->policy->notify_set_n_cells) {
     /* Call notify_set_n_cells */
     cmux->policy->notify_set_n_cells(cmux,
                                      cmux->policy_data,
@@ -1428,21 +1099,15 @@ circuitmux_set_num_cells(circuitmux_t *cmux, circuit_t *circ,
   if (hashent->muxinfo.cell_count > 0 && n_cells == 0) {
     --(cmux->n_active_circuits);
     hashent->muxinfo.cell_count = n_cells;
-    circuitmux_make_circuit_inactive(cmux, circ, hashent->muxinfo.direction);
+    circuitmux_make_circuit_inactive(cmux, circ);
   /* Is the old cell count == 0 and the new cell count > 0 ? */
   } else if (hashent->muxinfo.cell_count == 0 && n_cells > 0) {
     ++(cmux->n_active_circuits);
     hashent->muxinfo.cell_count = n_cells;
-    circuitmux_make_circuit_active(cmux, circ, hashent->muxinfo.direction);
+    circuitmux_make_circuit_active(cmux, circ);
   } else {
-    /*
-     * Update the entry cell count like this so we can put a
-     * circuitmux_assert_okay_paranoid inside make_circuit_(in)active() too.
-     */
     hashent->muxinfo.cell_count = n_cells;
   }
-
-  circuitmux_assert_okay_paranoid(cmux);
 }
 
 /*
@@ -1468,6 +1133,9 @@ circuitmux_get_first_active_circuit(circuitmux_t *cmux,
   circuit_t *circ = NULL;
 
   tor_assert(cmux);
+  tor_assert(cmux->policy);
+  /* This callback is mandatory. */
+  tor_assert(cmux->policy->pick_active_circuit);
   tor_assert(destroy_queue_out);
 
   *destroy_queue_out = NULL;
@@ -1486,14 +1154,7 @@ circuitmux_get_first_active_circuit(circuitmux_t *cmux,
     /* We also must have a cell available for this to be the case */
     tor_assert(cmux->n_cells > 0);
     /* Do we have a policy-provided circuit selector? */
-    if (cmux->policy && cmux->policy->pick_active_circuit) {
-      circ = cmux->policy->pick_active_circuit(cmux, cmux->policy_data);
-    }
-    /* Fall back on the head of the active circuits list */
-    if (!circ) {
-      tor_assert(cmux->active_circuits_head);
-      circ = cmux->active_circuits_head;
-    }
+    circ = cmux->policy->pick_active_circuit(cmux, cmux->policy_data);
     cmux->last_cell_was_destroy = 0;
   } else {
     tor_assert(cmux->n_cells == 0);
@@ -1517,7 +1178,6 @@ circuitmux_notify_xmit_cells(circuitmux_t *cmux, circuit_t *circ,
 
   tor_assert(cmux);
   tor_assert(circ);
-  circuitmux_assert_okay_paranoid(cmux);
 
   if (n_cells == 0) return;
 
@@ -1544,17 +1204,11 @@ circuitmux_notify_xmit_cells(circuitmux_t *cmux, circuit_t *circ,
   /* Adjust the mux cell counter */
   cmux->n_cells -= n_cells;
 
-  /* If we aren't making it inactive later, move it to the tail of the list */
-  if (!becomes_inactive) {
-    circuitmux_move_active_circ_to_tail(cmux, circ,
-                                        hashent->muxinfo.direction);
-  }
-
   /*
    * We call notify_xmit_cells() before making the circuit inactive if needed,
    * so the policy can always count on this coming in on an active circuit.
    */
-  if (cmux->policy && cmux->policy->notify_xmit_cells) {
+  if (cmux->policy->notify_xmit_cells) {
     cmux->policy->notify_xmit_cells(cmux, cmux->policy_data, circ,
                                     hashent->muxinfo.policy_data,
                                     n_cells);
@@ -1566,10 +1220,8 @@ circuitmux_notify_xmit_cells(circuitmux_t *cmux, circuit_t *circ,
    */
   if (becomes_inactive) {
     --(cmux->n_active_circuits);
-    circuitmux_make_circuit_inactive(cmux, circ, hashent->muxinfo.direction);
+    circuitmux_make_circuit_inactive(cmux, circ);
   }
-
-  circuitmux_assert_okay_paranoid(cmux);
 }
 
 /**
@@ -1592,282 +1244,6 @@ circuitmux_notify_xmit_destroy(circuitmux_t *cmux)
             I64_PRINTF_ARG(global_destroy_ctr));
 }
 
-/*
- * Circuitmux consistency checking assertions
- */
-
-/**
- * Check that circuitmux data structures are consistent and fail with an
- * assert if not.
- */
-
-void
-circuitmux_assert_okay(circuitmux_t *cmux)
-{
-  tor_assert(cmux);
-
-  /*
-   * Pass 1: iterate the hash table; for each entry:
-   *  a) Check that the circuit has this cmux for n_mux or p_mux
-   *  b) If the cell_count is > 0, set the mark bit; otherwise clear it
-   *  c) Also check activeness (cell_count > 0 should be active)
-   *  d) Count the number of circuits, active circuits and queued cells
-   *     and at the end check that they match the counters in the cmux.
-   *
-   * Pass 2: iterate the active circuits list; for each entry,
-   *         make sure the circuit is attached to this mux and appears
-   *         in the hash table.  Make sure the mark bit is 1, and clear
-   *         it in the hash table entry.  Consistency-check the linked
-   *         list pointers.
-   *
-   * Pass 3: iterate the hash table again; assert if any active circuits
-   *         (mark bit set to 1) are discovered that weren't cleared in pass 2
-   *         (don't appear in the linked list).
-   */
-
-  circuitmux_assert_okay_pass_one(cmux);
-  circuitmux_assert_okay_pass_two(cmux);
-  circuitmux_assert_okay_pass_three(cmux);
-}
-
-/**
- * Do the first pass of circuitmux_assert_okay(); see the comment in that
- * function.
- */
-
-static void
-circuitmux_assert_okay_pass_one(circuitmux_t *cmux)
-{
-  chanid_circid_muxinfo_t **i = NULL;
-  uint64_t chan_id;
-  channel_t *chan;
-  circid_t circ_id;
-  circuit_t *circ;
-  or_circuit_t *or_circ;
-  circuit_t **next_p, **prev_p;
-  unsigned int n_circuits, n_active_circuits, n_cells;
-
-  tor_assert(cmux);
-  tor_assert(cmux->chanid_circid_map);
-
-  /* Reset the counters */
-  n_circuits = n_active_circuits = n_cells = 0;
-  /* Start iterating the hash table */
-  i = HT_START(chanid_circid_muxinfo_map, cmux->chanid_circid_map);
-  while (i) {
-    /* Assert that the hash table entry isn't null */
-    tor_assert(*i);
-
-    /* Get the channel and circuit id */
-    chan_id = (*i)->chan_id;
-    circ_id = (*i)->circ_id;
-
-    /* Find the channel and circuit, assert that they exist */
-    chan = channel_find_by_global_id(chan_id);
-    tor_assert(chan);
-    circ = circuit_get_by_circid_channel_even_if_marked(circ_id, chan);
-    tor_assert(circ);
-
-    /* Assert that we know which direction this is going */
-    tor_assert((*i)->muxinfo.direction == CELL_DIRECTION_OUT ||
-               (*i)->muxinfo.direction == CELL_DIRECTION_IN);
-
-    if ((*i)->muxinfo.direction == CELL_DIRECTION_OUT) {
-      /* We should be n_mux on this circuit */
-      tor_assert(cmux == circ->n_mux);
-      tor_assert(chan == circ->n_chan);
-      /* Get next and prev for next test */
-      next_p = &(circ->next_active_on_n_chan);
-      prev_p = &(circ->prev_active_on_n_chan);
-    } else {
-      /* This should be an or_circuit_t and we should be p_mux */
-      or_circ = TO_OR_CIRCUIT(circ);
-      tor_assert(cmux == or_circ->p_mux);
-      tor_assert(chan == or_circ->p_chan);
-      /* Get next and prev for next test */
-      next_p = &(or_circ->next_active_on_p_chan);
-      prev_p = &(or_circ->prev_active_on_p_chan);
-    }
-
-    /*
-     * Should this circuit be active?  I.e., does the mux know about > 0
-     * cells on it?
-     */
-    const int circ_is_active = ((*i)->muxinfo.cell_count > 0);
-
-    /* It should be in the linked list iff it's active */
-    if (circ_is_active) {
-      /* Either we have a next link or we are the tail */
-      tor_assert(*next_p || (circ == cmux->active_circuits_tail));
-      /* Either we have a prev link or we are the head */
-      tor_assert(*prev_p || (circ == cmux->active_circuits_head));
-      /* Increment the active circuits counter */
-      ++n_active_circuits;
-    } else {
-      /* Shouldn't be in list, so no next or prev link */
-      tor_assert(!(*next_p));
-      tor_assert(!(*prev_p));
-      /* And can't be head or tail */
-      tor_assert(circ != cmux->active_circuits_head);
-      tor_assert(circ != cmux->active_circuits_tail);
-    }
-
-    /* Increment the circuits counter */
-    ++n_circuits;
-    /* Adjust the cell counter */
-    n_cells += (*i)->muxinfo.cell_count;
-
-    /* Set the mark bit to circ_is_active */
-    (*i)->muxinfo.mark = circ_is_active;
-
-    /* Advance to the next entry */
-    i = HT_NEXT(chanid_circid_muxinfo_map, cmux->chanid_circid_map, i);
-  }
-
-  /* Now check the counters */
-  tor_assert(n_cells == cmux->n_cells);
-  tor_assert(n_circuits == cmux->n_circuits);
-  tor_assert(n_active_circuits == cmux->n_active_circuits);
-}
-
-/**
- * Do the second pass of circuitmux_assert_okay(); see the comment in that
- * function.
- */
-
-static void
-circuitmux_assert_okay_pass_two(circuitmux_t *cmux)
-{
-  circuit_t *curr_circ, *prev_circ = NULL, *next_circ;
-  or_circuit_t *curr_or_circ;
-  uint64_t curr_chan_id;
-  circid_t curr_circ_id;
-  circuit_t **next_p, **prev_p;
-  channel_t *chan;
-  unsigned int n_active_circuits = 0;
-  chanid_circid_muxinfo_t search, *hashent = NULL;
-
-  tor_assert(cmux);
-  tor_assert(cmux->chanid_circid_map);
-
-  /*
-   * Walk the linked list of active circuits in cmux; keep track of the
-   * previous circuit seen for consistency checking purposes.  Count them
-   * to make sure the number in the linked list matches
-   * cmux->n_active_circuits.
-   */
-  curr_circ = cmux->active_circuits_head;
-  while (curr_circ) {
-    /* Reset some things */
-    chan = NULL;
-    curr_or_circ = NULL;
-    next_circ = NULL;
-    next_p = prev_p = NULL;
-    cell_direction_t direction;
-
-    /* Figure out if this is n_mux or p_mux */
-    if (cmux == curr_circ->n_mux) {
-      /* Get next_p and prev_p */
-      next_p = &(curr_circ->next_active_on_n_chan);
-      prev_p = &(curr_circ->prev_active_on_n_chan);
-      /* Get the channel */
-      chan = curr_circ->n_chan;
-      /* Get the circuit id */
-      curr_circ_id = curr_circ->n_circ_id;
-      /* Remember the direction */
-      direction = CELL_DIRECTION_OUT;
-    } else {
-      /* We must be p_mux and this must be an or_circuit_t */
-      curr_or_circ = TO_OR_CIRCUIT(curr_circ);
-      tor_assert(cmux == curr_or_circ->p_mux);
-      /* Get next_p and prev_p */
-      next_p = &(curr_or_circ->next_active_on_p_chan);
-      prev_p = &(curr_or_circ->prev_active_on_p_chan);
-      /* Get the channel */
-      chan = curr_or_circ->p_chan;
-      /* Get the circuit id */
-      curr_circ_id = curr_or_circ->p_circ_id;
-      /* Remember the direction */
-      direction = CELL_DIRECTION_IN;
-    }
-
-    /* Assert that we got a channel and get the channel ID */
-    tor_assert(chan);
-    curr_chan_id = chan->global_identifier;
-
-    /* Assert that prev_p points to last circuit we saw */
-    tor_assert(*prev_p == prev_circ);
-    /* If that's NULL, assert that we are the head */
-    if (!(*prev_p)) tor_assert(curr_circ == cmux->active_circuits_head);
-
-    /* Get the next circuit */
-    next_circ = *next_p;
-    /* If it's NULL, assert that we are the tail */
-    if (!(*next_p)) tor_assert(curr_circ == cmux->active_circuits_tail);
-
-    /* Now find the hash table entry for this circuit */
-    search.chan_id = curr_chan_id;
-    search.circ_id = curr_circ_id;
-    hashent = HT_FIND(chanid_circid_muxinfo_map, cmux->chanid_circid_map,
-                      &search);
-
-    /* Assert that we have one */
-    tor_assert(hashent);
-
-    /* Assert that the direction matches */
-    tor_assert(direction == hashent->muxinfo.direction);
-
-    /* Assert that the hash entry got marked in pass one */
-    tor_assert(hashent->muxinfo.mark);
-
-    /* Clear the mark */
-    hashent->muxinfo.mark = 0;
-
-    /* Increment the counter */
-    ++n_active_circuits;
-
-    /* Advance to the next active circuit and update prev_circ */
-    prev_circ = curr_circ;
-    curr_circ = next_circ;
-  }
-
-  /* Assert that the counter matches the cmux */
-  tor_assert(n_active_circuits == cmux->n_active_circuits);
-}
-
-/**
- * Do the third pass of circuitmux_assert_okay(); see the comment in that
- * function.
- */
-
-static void
-circuitmux_assert_okay_pass_three(circuitmux_t *cmux)
-{
-  chanid_circid_muxinfo_t **i = NULL;
-
-  tor_assert(cmux);
-  tor_assert(cmux->chanid_circid_map);
-
-  /* Start iterating the hash table */
-  i = HT_START(chanid_circid_muxinfo_map, cmux->chanid_circid_map);
-
-  /* Advance through each entry */
-  while (i) {
-    /* Assert that it isn't null */
-    tor_assert(*i);
-
-    /*
-     * Assert that this entry is not marked - i.e., that either we didn't
-     * think it should be active in pass one or we saw it in the active
-     * circuits linked list.
-     */
-    tor_assert(!((*i)->muxinfo.mark));
-
-    /* Advance to the next entry */
-    i = HT_NEXT(chanid_circid_muxinfo_map, cmux->chanid_circid_map, i);
-  }
-}
-
 /*DOCDOC */
 void
 circuitmux_append_destroy_cell(channel_t *chan,

+ 70 - 45
src/or/circuitmux_ewma.c

@@ -223,8 +223,6 @@ ewma_cmp_cmux(circuitmux_t *cmux_1, circuitmux_policy_data_t *pol_data_1,
  * has value ewma_scale_factor ** N.)
  */
 static double ewma_scale_factor = 0.1;
-/* DOCDOC ewma_enabled */
-static int ewma_enabled = 0;
 
 /*** EWMA circuitmux_policy_t method table ***/
 
@@ -243,6 +241,13 @@ circuitmux_policy_t ewma_policy = {
 
 /*** EWMA method implementations using the below EWMA helper functions ***/
 
+/** Compute and return the current cell_ewma tick. */
+static inline unsigned int
+cell_ewma_get_tick(void)
+{
+  return ((unsigned)approx_time() / EWMA_TICK_LEN);
+}
+
 /**
  * Allocate an ewma_policy_data_t and upcast it to a circuitmux_policy_data_t;
  * this is called when setting the policy on a circuitmux_t to ewma_policy.
@@ -612,59 +617,79 @@ cell_ewma_tick_from_timeval(const struct timeval *now,
   return res;
 }
 
-/** Tell the caller whether ewma_enabled is set */
-int
-cell_ewma_enabled(void)
+/* Default value for the CircuitPriorityHalflifeMsec consensus parameter in
+ * msec. */
+#define CMUX_PRIORITY_HALFLIFE_MSEC_DEFAULT 30000
+/* Minimum and maximum value for the CircuitPriorityHalflifeMsec consensus
+ * parameter. */
+#define CMUX_PRIORITY_HALFLIFE_MSEC_MIN 1
+#define CMUX_PRIORITY_HALFLIFE_MSEC_MAX INT32_MAX
+
+/* Return the value of the circuit priority halflife from the options if
+ * available or else from the consensus (in that order). If none can be found,
+ * a default value is returned.
+ *
+ * The source_msg points to a string describing from where the value was
+ * picked so it can be used for logging. */
+static double
+get_circuit_priority_halflife(const or_options_t *options,
+                              const networkstatus_t *consensus,
+                              const char **source_msg)
 {
-  return ewma_enabled;
-}
+  int32_t halflife_ms;
+  double halflife;
+  /* Compute the default value now. We might need it. */
+  double halflife_default =
+    ((double) CMUX_PRIORITY_HALFLIFE_MSEC_DEFAULT) / 1000.0;
 
-/** Compute and return the current cell_ewma tick. */
-unsigned int
-cell_ewma_get_tick(void)
-{
-  return ((unsigned)approx_time() / EWMA_TICK_LEN);
+  /* Try to get it from configuration file first. */
+  if (options && options->CircuitPriorityHalflife < EPSILON) {
+    halflife = options->CircuitPriorityHalflife;
+    *source_msg = "CircuitPriorityHalflife in configuration";
+    goto end;
+  }
+
+  /* Try to get the msec value from the consensus. */
+  halflife_ms = networkstatus_get_param(consensus,
+                                        "CircuitPriorityHalflifeMsec",
+                                        CMUX_PRIORITY_HALFLIFE_MSEC_DEFAULT,
+                                        CMUX_PRIORITY_HALFLIFE_MSEC_MIN,
+                                        CMUX_PRIORITY_HALFLIFE_MSEC_MAX);
+  halflife = ((double) halflife_ms) / 1000.0;
+  *source_msg = "CircuitPriorityHalflifeMsec in consensus";
+
+ end:
+  /* We should never go below the EPSILON else we would consider it disabled
+   * and we can't have that. */
+  if (halflife < EPSILON) {
+    log_warn(LD_CONFIG, "CircuitPriorityHalflife is too small (%f). "
+                        "Adjusting to the smallest value allowed: %f.",
+             halflife, halflife_default);
+    halflife = halflife_default;
+  }
+  return halflife;
 }
 
 /** Adjust the global cell scale factor based on <b>options</b> */
 void
-cell_ewma_set_scale_factor(const or_options_t *options,
-                           const networkstatus_t *consensus)
+cmux_ewma_set_options(const or_options_t *options,
+                      const networkstatus_t *consensus)
 {
-  int32_t halflife_ms;
   double halflife;
   const char *source;
-  if (options && options->CircuitPriorityHalflife >= -EPSILON) {
-    halflife = options->CircuitPriorityHalflife;
-    source = "CircuitPriorityHalflife in configuration";
-  } else if (consensus && (halflife_ms = networkstatus_get_param(
-                 consensus, "CircuitPriorityHalflifeMsec",
-                 -1, -1, INT32_MAX)) >= 0) {
-    halflife = ((double)halflife_ms)/1000.0;
-    source = "CircuitPriorityHalflifeMsec in consensus";
-  } else {
-    halflife = EWMA_DEFAULT_HALFLIFE;
-    source = "Default value";
-  }
 
-  if (halflife <= EPSILON) {
-    /* The cell EWMA algorithm is disabled. */
-    ewma_scale_factor = 0.1;
-    ewma_enabled = 0;
-    log_info(LD_OR,
-             "Disabled cell_ewma algorithm because of value in %s",
-             source);
-  } else {
-    /* convert halflife into halflife-per-tick. */
-    halflife /= EWMA_TICK_LEN;
-    /* compute per-tick scale factor. */
-    ewma_scale_factor = exp( LOG_ONEHALF / halflife );
-    ewma_enabled = 1;
-    log_info(LD_OR,
-             "Enabled cell_ewma algorithm because of value in %s; "
-             "scale factor is %f per %d seconds",
-             source, ewma_scale_factor, EWMA_TICK_LEN);
-  }
+  /* 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);
+
+  /* convert halflife into halflife-per-tick. */
+  halflife /= EWMA_TICK_LEN;
+  /* compute per-tick scale factor. */
+  ewma_scale_factor = exp( LOG_ONEHALF / halflife );
+  log_info(LD_OR,
+           "Enabled cell_ewma algorithm because of value in %s; "
+           "scale factor is %f per %d seconds",
+           source, ewma_scale_factor, EWMA_TICK_LEN);
 }
 
 /** Return the multiplier necessary to convert the value of a cell sent in

+ 3 - 4
src/or/circuitmux_ewma.h

@@ -12,13 +12,12 @@
 #include "or.h"
 #include "circuitmux.h"
 
+/* The public EWMA policy callbacks object. */
 extern circuitmux_policy_t ewma_policy;
 
 /* Externally visible EWMA functions */
-int cell_ewma_enabled(void);
-unsigned int cell_ewma_get_tick(void);
-void cell_ewma_set_scale_factor(const or_options_t *options,
-                                const networkstatus_t *consensus);
+void cmux_ewma_set_options(const or_options_t *options,
+                           const networkstatus_t *consensus);
 
 #endif /* !defined(TOR_CIRCUITMUX_EWMA_H) */
 

+ 2 - 11
src/or/config.c

@@ -264,7 +264,7 @@ static config_var_t option_vars_[] = {
   OBSOLETE("CircuitIdleTimeout"),
   V(CircuitsAvailableTimeout,    INTERVAL, "0"),
   V(CircuitStreamTimeout,        INTERVAL, "0"),
-  V(CircuitPriorityHalflife,     DOUBLE,  "-100.0"), /*negative:'Use default'*/
+  V(CircuitPriorityHalflife,     DOUBLE,  "-1.0"), /*negative:'Use default'*/
   V(ClientDNSRejectInternalAddresses, BOOL,"1"),
   V(ClientOnly,                  BOOL,     "0"),
   V(ClientPreferIPv6ORPort,      AUTOBOOL, "auto"),
@@ -1791,7 +1791,6 @@ options_act(const or_options_t *old_options)
   char *msg=NULL;
   const int transition_affects_workers =
     old_options && options_transition_affects_workers(old_options, options);
-  int old_ewma_enabled;
   const int transition_affects_guards =
     old_options && options_transition_affects_guards(old_options, options);
 
@@ -2065,16 +2064,8 @@ options_act(const or_options_t *old_options)
   if (accounting_is_enabled(options))
     configure_accounting(time(NULL));
 
-  old_ewma_enabled = cell_ewma_enabled();
   /* Change the cell EWMA settings */
-  cell_ewma_set_scale_factor(options, networkstatus_get_latest_consensus());
-  /* If we just enabled ewma, set the cmux policy on all active channels */
-  if (cell_ewma_enabled() && !old_ewma_enabled) {
-    channel_set_cmux_policy_everywhere(&ewma_policy);
-  } else if (!cell_ewma_enabled() && old_ewma_enabled) {
-    /* Turn it off everywhere */
-    channel_set_cmux_policy_everywhere(NULL);
-  }
+  cmux_ewma_set_options(options, networkstatus_get_latest_consensus());
 
   /* Update the BridgePassword's hashed version as needed.  We store this as a
    * digest so that we can do side-channel-proof comparisons on it.

+ 1 - 11
src/or/networkstatus.c

@@ -1768,7 +1768,6 @@ networkstatus_set_current_consensus(const char *consensus,
   consensus_waiting_for_certs_t *waiting = NULL;
   time_t current_valid_after = 0;
   int free_consensus = 1; /* Free 'c' at the end of the function */
-  int old_ewma_enabled;
   int checked_protocols_already = 0;
 
   if (flav < 0) {
@@ -2001,17 +2000,8 @@ networkstatus_set_current_consensus(const char *consensus,
     /* XXXXNM Microdescs: needs a non-ns variant. ???? NM*/
     update_consensus_networkstatus_fetch_time(now);
 
-    /* Update ewma and adjust policy if needed; first cache the old value */
-    old_ewma_enabled = cell_ewma_enabled();
     /* Change the cell EWMA settings */
-    cell_ewma_set_scale_factor(options, c);
-    /* If we just enabled ewma, set the cmux policy on all active channels */
-    if (cell_ewma_enabled() && !old_ewma_enabled) {
-      channel_set_cmux_policy_everywhere(&ewma_policy);
-    } else if (!cell_ewma_enabled() && old_ewma_enabled) {
-      /* Turn it off everywhere */
-      channel_set_cmux_policy_everywhere(NULL);
-    }
+    cmux_ewma_set_options(options, c);
 
     /* XXXX this call might be unnecessary here: can changing the
      * current consensus really alter our view of any OR's rate limits? */

+ 0 - 17
src/or/or.h

@@ -3162,15 +3162,6 @@ typedef struct circuit_t {
   /** Index in smartlist of all circuits (global_circuitlist). */
   int global_circuitlist_idx;
 
-  /** Next circuit in the doubly-linked ring of circuits waiting to add
-   * cells to n_conn.  NULL if we have no cells pending, or if we're not
-   * linked to an OR connection. */
-  struct circuit_t *next_active_on_n_chan;
-  /** Previous circuit in the doubly-linked ring of circuits waiting to add
-   * cells to n_conn.  NULL if we have no cells pending, or if we're not
-   * linked to an OR connection. */
-  struct circuit_t *prev_active_on_n_chan;
-
   /** Various statistics about cells being added to or removed from this
    * circuit's queues; used only if CELL_STATS events are enabled and
    * cleared after being sent to control port. */
@@ -3450,14 +3441,6 @@ struct onion_queue_t;
 typedef struct or_circuit_t {
   circuit_t base_;
 
-  /** Next circuit in the doubly-linked ring of circuits waiting to add
-   * cells to p_chan.  NULL if we have no cells pending, or if we're not
-   * linked to an OR connection. */
-  struct circuit_t *next_active_on_p_chan;
-  /** Previous circuit in the doubly-linked ring of circuits waiting to add
-   * cells to p_chan.  NULL if we have no cells pending, or if we're not
-   * linked to an OR connection. */
-  struct circuit_t *prev_active_on_p_chan;
   /** Pointer to an entry on the onion queue, if this circuit is waiting for a
    * chance to give an onionskin to a cpuworker. Used only in onion.c */
   struct onion_queue_t *onionqueue_entry;

+ 0 - 25
src/or/relay.c

@@ -2399,13 +2399,6 @@ circuit_consider_sending_sendme(circuit_t *circ, crypt_path_t *layer_hint)
   }
 }
 
-#ifdef ACTIVE_CIRCUITS_PARANOIA
-#define assert_cmux_ok_paranoid(chan) \
-     assert_circuit_mux_okay(chan)
-#else
-#define assert_cmux_ok_paranoid(chan)
-#endif /* defined(ACTIVE_CIRCUITS_PARANOIA) */
-
 /** The total number of cells we have allocated. */
 static size_t total_cells_allocated = 0;
 
@@ -2693,16 +2686,12 @@ update_circuit_on_cmux_(circuit_t *circ, cell_direction_t direction,
   }
   tor_assert(circuitmux_attached_circuit_direction(cmux, circ) == direction);
 
-  assert_cmux_ok_paranoid(chan);
-
   /* Update the number of cells we have for the circuit mux */
   if (direction == CELL_DIRECTION_OUT) {
     circuitmux_set_num_cells(cmux, circ, circ->n_chan_cells.n);
   } else {
     circuitmux_set_num_cells(cmux, circ, or_circ->p_chan_cells.n);
   }
-
-  assert_cmux_ok_paranoid(chan);
 }
 
 /** Remove all circuits from the cmux on <b>chan</b>.
@@ -2847,7 +2836,6 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max))
     }
     /* If it returns NULL, no cells left to send */
     if (!circ) break;
-    assert_cmux_ok_paranoid(chan);
 
     if (circ->n_chan == chan) {
       queue = &circ->n_chan_cells;
@@ -2951,8 +2939,6 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max))
   }
 
   /* Okay, we're done sending now */
-  assert_cmux_ok_paranoid(chan);
-
   return n_flushed;
 }
 
@@ -3103,17 +3089,6 @@ circuit_clear_cell_queue(circuit_t *circ, channel_t *chan)
     update_circuit_on_cmux(circ, direction);
 }
 
-/** Fail with an assert if the circuit mux on chan is corrupt
- */
-void
-assert_circuit_mux_okay(channel_t *chan)
-{
-  tor_assert(chan);
-  tor_assert(chan->cmux);
-
-  circuitmux_assert_okay(chan->cmux);
-}
-
 /** Return 1 if we shouldn't restart reading on this circuit, even if
  * we get a SENDME.  Else return 0.
 */

+ 0 - 1
src/or/relay.h

@@ -76,7 +76,6 @@ void destroy_cell_queue_append(destroy_cell_queue_t *queue,
 void channel_unlink_all_circuits(channel_t *chan, smartlist_t *detached_out);
 MOCK_DECL(int, channel_flush_from_first_active_circuit,
           (channel_t *chan, int max));
-void assert_circuit_mux_okay(channel_t *chan);
 void update_circuit_on_cmux_(circuit_t *circ, cell_direction_t direction,
                              const char *file, int lineno);
 #define update_circuit_on_cmux(circ, direction) \

+ 2 - 3
src/test/test_channel.c

@@ -281,6 +281,7 @@ new_fake_channel(void)
   chan->state = CHANNEL_STATE_OPEN;
 
   chan->cmux = circuitmux_alloc();
+  circuitmux_set_policy(chan->cmux, &ewma_policy);
 
   return chan;
 }
@@ -575,15 +576,13 @@ test_channel_outbound_cell(void *arg)
   channel_register(chan);
   tt_int_op(chan->registered, OP_EQ, 1);
   /* Set EWMA policy so we can pick it when flushing. */
-  channel_set_cmux_policy_everywhere(&ewma_policy);
+  circuitmux_set_policy(chan->cmux, &ewma_policy);
   tt_ptr_op(circuitmux_get_policy(chan->cmux), OP_EQ, &ewma_policy);
 
   /* Register circuit to the channel circid map which will attach the circuit
    * to the channel's cmux as well. */
   circuit_set_n_circid_chan(TO_CIRCUIT(circ), 42, chan);
   tt_int_op(channel_num_circuits(chan), OP_EQ, 1);
-  tt_assert(!TO_CIRCUIT(circ)->next_active_on_n_chan);
-  tt_assert(!TO_CIRCUIT(circ)->prev_active_on_n_chan);
   /* Test the cmux state. */
   tt_ptr_op(TO_CIRCUIT(circ)->n_mux, OP_EQ, chan->cmux);
   tt_int_op(circuitmux_is_circuit_attached(chan->cmux, TO_CIRCUIT(circ)),

+ 1 - 0
src/test/test_circuitlist.c

@@ -9,6 +9,7 @@
 #include "channel.h"
 #include "circuitbuild.h"
 #include "circuitlist.h"
+#include "circuitmux_ewma.h"
 #include "hs_circuitmap.h"
 #include "test.h"
 #include "log_test_helpers.h"

+ 2 - 0
src/test/test_circuitmux.c

@@ -7,6 +7,7 @@
 #include "or.h"
 #include "channel.h"
 #include "circuitmux.h"
+#include "circuitmux_ewma.h"
 #include "relay.h"
 #include "scheduler.h"
 #include "test.h"
@@ -45,6 +46,7 @@ test_cmux_destroy_cell_queue(void *arg)
   cmux = circuitmux_alloc();
   tt_assert(cmux);
   ch = new_fake_channel();
+  circuitmux_set_policy(cmux, &ewma_policy);
   ch->has_queued_writes = has_queued_writes;
   ch->wide_circ_ids = 1;