[tor-commits] [tor/master] cmux: Make EWMA policy mandatory
nickm at torproject.org
nickm at torproject.org
Mon Mar 19 10:03:53 UTC 2018
commit 6b1dba214db3058b143bbb4d4c4bdfee32d100f1
Author: David Goulet <dgoulet at torproject.org>
Date: Thu Feb 15 13:45:21 2018 -0500
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 at torproject.org>
---
src/or/channel.c | 15 -------
src/or/channel.h | 3 --
src/or/channeltls.c | 5 +--
src/or/circuitmux_ewma.c | 105 +++++++++++++++++++++++++++++------------------
src/or/circuitmux_ewma.h | 1 -
src/or/config.c | 9 ----
src/or/networkstatus.c | 10 -----
src/test/test_channel.c | 2 +-
8 files changed, 68 insertions(+), 82 deletions(-)
diff --git a/src/or/channel.c b/src/or/channel.c
index ff1cfde2a..a9483ee02 100644
--- a/src/or/channel.c
+++ b/src/or/channel.c
@@ -2109,21 +2109,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.
*
* This gets called periodically from run_scheduled_events() in main.c;
diff --git a/src/or/channel.h b/src/or/channel.h
index 0af5aed41..6cf8cd7f7 100644
--- a/src/or/channel.h
+++ b/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_
diff --git a/src/or/channeltls.c b/src/or/channeltls.c
index 9000703b0..54d94f610 100644
--- a/src/or/channeltls.c
+++ b/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);
}
/**
diff --git a/src/or/circuitmux_ewma.c b/src/or/circuitmux_ewma.c
index fde2d22a8..d9ee8d3ef 100644
--- a/src/or/circuitmux_ewma.c
+++ b/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) {
- /* 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
diff --git a/src/or/circuitmux_ewma.h b/src/or/circuitmux_ewma.h
index 8f4e57865..4e9e512df 100644
--- a/src/or/circuitmux_ewma.h
+++ b/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);
diff --git a/src/or/config.c b/src/or/config.c
index c246bd00e..0ac777921 100644
--- a/src/or/config.c
+++ b/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.
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 31ecb2098..80cdc9e5b 100644
--- a/src/or/networkstatus.c
+++ b/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? */
diff --git a/src/test/test_channel.c b/src/test/test_channel.c
index bdc9d32f7..392759768 100644
--- a/src/test/test_channel.c
+++ b/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
More information about the tor-commits
mailing list