cmux: Make EWMA policy mandatory

To achieve this, a default value for the CircuitPriorityHalflife option was
needed. We still look in the options and then the consensus but in case no
value can be found, the default CircuitPriorityHalflifeMsec=30000 is used. It
it the value we've been using since 0.2.4.4-alpha.

This means that EWMA, our only policy, can not be disabled anymore fallbacking
to the round robin algorithm. Unneeded code to control that is removed in this
commit.

Part of #25268

Signed-off-by: David Goulet <dgoulet@torproject.org>

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.
  *

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_

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()) {
+  /* We only have one policy for now so always set it to EWMA. */
   circuitmux_set_policy(chan->cmux, &ewma_policy);
-  }
 }
 
 /**

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 ***/
 
@@ -612,13 +610,6 @@ cell_ewma_tick_from_timeval(const struct timeval *now,
   return res;
 }
 
-/** Tell the caller whether ewma_enabled is set */
-int
-cell_ewma_enabled(void)
-{
-  return ewma_enabled;
-}
-
 /** Compute and return the current cell_ewma tick. */
 unsigned int
 cell_ewma_get_tick(void)
@@ -626,45 +617,79 @@ cell_ewma_get_tick(void)
   return ((unsigned)approx_time() / EWMA_TICK_LEN);
 }
 
+/* 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)
+{
+  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;
+
+  /* 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)
 {
-  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) {
+  /* Both options and consensus can be NULL. This assures us to either get a
-    /* The cell EWMA algorithm is disabled. */
+   * valid configured value or the default one. */
-    ewma_scale_factor = 0.1;
+  halflife = get_circuit_priority_halflife(options, consensus, &source);
-    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);
-  }
 }
 
 /** Return the multiplier necessary to convert the value of a cell sent in

src/or/circuitmux_ewma.h

@@ -15,7 +15,6 @@
 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);

src/or/config.c

@@ -1799,7 +1799,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);
 
@@ -2073,16 +2072,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);
-  }
 
   /* 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.

src/or/networkstatus.c

@@ -1779,7 +1779,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) {
@@ -2003,17 +2002,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);
-    }
 
     /* XXXX this call might be unnecessary here: can changing the
      * current consensus really alter our view of any OR's rate limits? */

src/test/test_channel.c

@@ -575,7 +575,7 @@ 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