From 8512e337734a1eb8746df8500ec0f1f8b1fdc5e8 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Fri, 25 Dec 2009 05:42:35 -0600 Subject: [PATCH 1/8] Add BUILDTIMEOUT_SET event for CBT stress testing. --- doc/spec/control-spec.txt | 34 +++++++++++++++++++++++++- src/or/circuitbuild.c | 17 +++++++++++++ src/or/control.c | 50 ++++++++++++++++++++++++++++++++++++++- src/or/or.h | 17 +++++++++++-- 4 files changed, 114 insertions(+), 4 deletions(-) diff --git a/doc/spec/control-spec.txt b/doc/spec/control-spec.txt index 73f0b020a9..4057ffa741 100644 --- a/doc/spec/control-spec.txt +++ b/doc/spec/control-spec.txt @@ -219,7 +219,7 @@ "INFO" / "NOTICE" / "WARN" / "ERR" / "NEWDESC" / "ADDRMAP" / "AUTHDIR_NEWDESCS" / "DESCCHANGED" / "STATUS_GENERAL" / "STATUS_CLIENT" / "STATUS_SERVER" / "GUARD" / "NS" / "STREAM_BW" / - "CLIENTS_SEEN" / "NEWCONSENSUS" + "CLIENTS_SEEN" / "NEWCONSENSUS" / "BUILDTIMEOUT_SET" Any events *not* listed in the SETEVENTS line are turned off; thus, sending SETEVENTS with an empty body turns off all event reporting. @@ -1656,6 +1656,38 @@ [First added in 0.2.1.13-alpha] +4.1.16. New circuit buildtime has been set. + + The syntax is: + "650" SP "BUILDTIMEOUT_SET" SP Type SP "TOTAL_TIMES=" Total SP + "TIMEOUT_MS=" Timeout SP "XM=" Xm SP "ALPHA=" Alpha SP + "CUTOFF_QUANTILE=" Quantile CRLF + Type = "COMPUTED" / "RESET" / "SUSPENDED" / "DISCARD" / "RESUME" + Total = Integer count of timeouts stored + Timeout = Integer timeout in milliseconds + Xm = Estimated integer Pareto parameter Xm in milliseconds + Alpha = Estimated floating point Paredo paremter alpha + Quantile = Floating point CDF quantile cutoff point for this timeout + + A new circuit build timeout time has been set. If Type is "COMPUTED", + Tor has computed the value based on historical data. If Type is "RESET", + initialization or drastic network changes have caused Tor to reset + the timeout back to the default, to relearn again. If Type is + "SUSPENDED", Tor has detected a loss of network connectivity and has + temporarily changed the timeout value to the default until the network + recovers. If type is "DISCARD", Tor has decided to discard timeout + values that likely happened while the network was down. If type is + "RESUME", Tor has decided to resume timeout calculation. + + The Total value is the count of circuit build times Tor used in + computing this value. It is capped internally at the maximum number + of build times Tor stores (NCIRCUITS_TO_OBSERVE). + + The Timeout itself is provided in milliseconds. Internally, Tor rounds + this value to the nearest second before using it. + + [First added in 0.2.2.7-alpha] + 5. Implementation notes 5.1. Authentication diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 300da7eed0..b16e9a4d80 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -134,6 +134,7 @@ circuit_build_times_init(circuit_build_times_t *cbt) { memset(cbt, 0, sizeof(*cbt)); cbt->timeout_ms = circuit_build_times_get_initial_timeout(); + control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_RESET); } /** @@ -687,6 +688,9 @@ circuit_build_times_needs_circuits_now(circuit_build_times_t *cbt) /** * Called to indicate that the network showed some signs of liveness. + * + * This function is called every time we receive a cell. Avoid + * syscalls, events, and other high-intensity work. */ void circuit_build_times_network_is_live(circuit_build_times_t *cbt) @@ -760,6 +764,7 @@ circuit_build_times_network_check_live(circuit_build_times_t *cbt) /* Only discard NETWORK_NONLIVE_TIMEOUT_COUNT-1 because we stopped * counting after that */ circuit_build_times_rewind_history(cbt, NETWORK_NONLIVE_TIMEOUT_COUNT-1); + control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_DISCARD); } return 0; } else if (cbt->liveness.nonlive_timeouts >= NETWORK_NONLIVE_TIMEOUT_COUNT) { @@ -770,9 +775,17 @@ circuit_build_times_network_check_live(circuit_build_times_t *cbt) (long int)(now - cbt->liveness.network_last_live), tor_lround(circuit_build_times_get_initial_timeout()/1000)); cbt->timeout_ms = circuit_build_times_get_initial_timeout(); + cbt->liveness.net_suspended = 1; + control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_SUSPENDED); } return 0; + } else if (cbt->liveness.net_suspended) { + log_notice(LD_CIRC, + "Network activity has resumed. " + "Resuming circuit timeout calculations."); + cbt->liveness.net_suspended = 0; + control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_RESUME); } return 1; @@ -819,6 +832,8 @@ circuit_build_times_network_check_changed(circuit_build_times_t *cbt) cbt->timeout_ms = circuit_build_times_get_initial_timeout(); } + control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_RESET); + log_notice(LD_CIRC, "Network connection speed appears to have changed. Resetting " "timeout to %lds after %d timeouts and %d buildtimes.", @@ -893,6 +908,8 @@ circuit_build_times_set_timeout(circuit_build_times_t *cbt) cbt->timeout_ms = BUILD_TIMEOUT_MIN_VALUE; } + control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_COMPUTED); + log_info(LD_CIRC, "Set circuit build timeout to %lds (%lfms, Xm: %d, a: %lf) " "based on %d circuit times", tor_lround(cbt->timeout_ms/1000), diff --git a/src/or/control.c b/src/or/control.c index c08d6a2440..c34848d454 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -43,7 +43,8 @@ #define EVENT_STREAM_BANDWIDTH_USED 0x0014 #define EVENT_CLIENTS_SEEN 0x0015 #define EVENT_NEWCONSENSUS 0x0016 -#define _EVENT_MAX 0x0016 +#define EVENT_BUILDTIMEOUT_SET 0x0017 +#define _EVENT_MAX 0x0017 /* If _EVENT_MAX ever hits 0x0020, we need to make the mask wider. */ /** Bitfield: The bit 1<<e is set if any open control @@ -922,6 +923,8 @@ handle_control_setevents(control_connection_t *conn, uint32_t len, event_code = EVENT_CLIENTS_SEEN; else if (!strcasecmp(ev, "NEWCONSENSUS")) event_code = EVENT_NEWCONSENSUS; + else if (!strcasecmp(ev, "BUILDTIMEOUT_SET")) + event_code = EVENT_BUILDTIMEOUT_SET; else { connection_printf_to_buf(conn, "552 Unrecognized event \"%s\"\r\n", ev); @@ -3440,6 +3443,51 @@ control_event_newconsensus(const networkstatus_t *consensus) consensus->routerstatus_list, EVENT_NEWCONSENSUS, "NEWCONSENSUS"); } +/** Called when we compute a new circuitbuildtimeout */ +int +control_event_buildtimeout_set(const circuit_build_times_t *cbt, + buildtimeout_set_event_t type) +{ + const char *type_string = NULL; + double qnt = BUILDTIMEOUT_QUANTILE_CUTOFF; + + if (!control_event_is_interesting(EVENT_BUILDTIMEOUT_SET)) + return 0; + + switch (type) { + case BUILDTIMEOUT_SET_EVENT_COMPUTED: + type_string = "COMPUTED"; + break; + case BUILDTIMEOUT_SET_EVENT_RESET: + type_string = "RESET"; + qnt = 1.0; + break; + case BUILDTIMEOUT_SET_EVENT_SUSPENDED: + type_string = "SUSPENDED"; + qnt = 1.0; + break; + case BUILDTIMEOUT_SET_EVENT_DISCARD: + type_string = "DISCARD"; + qnt = 1.0; + break; + case BUILDTIMEOUT_SET_EVENT_RESUME: + type_string = "RESUME"; + break; + default: + type_string = "UNKNOWN"; + break; + } + + send_control_event(EVENT_BUILDTIMEOUT_SET, ALL_FORMATS, + "650 BUILDTIMEOUT_SET %s TOTAL_TIMES=%lu " + "TIMEOUT_MS=%lu XM=%lu ALPHA=%lf CUTOFF_QUANTILE=%lf\r\n", + type_string, (unsigned long)cbt->total_build_times, + (unsigned long)cbt->timeout_ms, + (unsigned long)cbt->Xm, cbt->alpha, qnt); + + return 0; +} + /** Called when a single local_routerstatus_t has changed: Sends an NS event * to any controller that cares. */ int diff --git a/src/or/or.h b/src/or/or.h index e0b86387fa..24cabb4300 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3008,7 +3008,7 @@ void entry_guards_free_all(void); #define MAX_SYNTHETIC_QUANTILE 0.985 /** Minimum circuits before estimating a timeout */ -#define MIN_CIRCUITS_TO_OBSERVE 500 +#define MIN_CIRCUITS_TO_OBSERVE 20 /** Total size of the circuit timeout history to accumulate. * 5000 is approx 1.5 weeks worth of continual-use circuits. */ @@ -3035,7 +3035,7 @@ typedef uint32_t build_time_t; #define BUILD_TIMES_TEST_FREQUENCY 60 /** Save state every 10 circuits */ -#define BUILD_TIMES_SAVE_STATE_EVERY 10 +#define BUILD_TIMES_SAVE_STATE_EVERY 1 /* Circuit Build Timeout network liveness constants */ @@ -3090,6 +3090,8 @@ typedef struct { int8_t timeouts_after_firsthop[RECENT_CIRCUITS]; /** Index into circular array. */ int after_firsthop_idx; + /** The network is not live. Timeout gathering is suspended */ + int net_suspended; } network_liveness_t; /** Structure for circuit build times history */ @@ -3583,6 +3585,15 @@ typedef enum or_conn_status_event_t { OR_CONN_EVENT_NEW = 4, } or_conn_status_event_t; +/** Used to indicate the type of a buildtime event */ +typedef enum buildtimeout_set_event_t { + BUILDTIMEOUT_SET_EVENT_COMPUTED = 0, + BUILDTIMEOUT_SET_EVENT_RESET = 1, + BUILDTIMEOUT_SET_EVENT_SUSPENDED = 2, + BUILDTIMEOUT_SET_EVENT_DISCARD = 3, + BUILDTIMEOUT_SET_EVENT_RESUME = 4 +} buildtimeout_set_event_t; + void control_update_global_event_mask(void); void control_adjust_event_log_severity(void); @@ -3648,6 +3659,8 @@ int control_event_server_status(int severity, const char *format, ...) CHECK_PRINTF(2,3); int control_event_guard(const char *nickname, const char *digest, const char *status); +int control_event_buildtimeout_set(const circuit_build_times_t *cbt, + buildtimeout_set_event_t type); int init_cookie_authentication(int enabled); smartlist_t *decode_hashed_passwords(config_line_t *passwords); From ac68704f07c2b70321b72e3b8665cb6d12da7e6b Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Tue, 29 Dec 2009 04:17:12 +0100 Subject: [PATCH 2/8] Allow "EXTENDCIRCUIT 0" to omit a path. --- doc/spec/control-spec.txt | 19 ++++++++------ src/or/control.c | 53 +++++++++++++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 18 deletions(-) diff --git a/doc/spec/control-spec.txt b/doc/spec/control-spec.txt index 4057ffa741..7d3312a5a8 100644 --- a/doc/spec/control-spec.txt +++ b/doc/spec/control-spec.txt @@ -606,15 +606,20 @@ 3.10. EXTENDCIRCUIT Sent from the client to the server. The format is: - "EXTENDCIRCUIT" SP CircuitID SP - ServerSpec *("," ServerSpec) - [SP "purpose=" Purpose] CRLF + "EXTENDCIRCUIT" SP CircuitID + [SP ServerSpec *("," ServerSpec) + SP "purpose=" Purpose] CRLF This request takes one of two forms: either the CircuitID is zero, in - which case it is a request for the server to build a new circuit according - to the specified path, or the CircuitID is nonzero, in which case it is a - request for the server to extend an existing circuit with that ID according - to the specified path. + which case it is a request for the server to build a new circuit, + or the CircuitID is nonzero, in which case it is a request for the + server to extend an existing circuit with that ID according to the + specified path. + + If the CircuitID is 0, the controller has the option of providing + a path for Tor to use to build the circuit. If it does not provide + a path, Tor will select one automatically from high capacity nodes + according to path-spec.txt. If CircuitID is 0 and "purpose=" is specified, then the circuit's purpose is set. Two choices are recognized: "general" and diff --git a/src/or/control.c b/src/or/control.c index c34848d454..a7e60d55f0 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -2055,27 +2055,58 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len, router_nicknames = smartlist_create(); - args = getargs_helper("EXTENDCIRCUIT", conn, body, 2, -1); + args = getargs_helper("EXTENDCIRCUIT", conn, body, 1, -1); if (!args) goto done; zero_circ = !strcmp("0", (char*)smartlist_get(args,0)); + + if (zero_circ) { + char *purp = NULL; + if (smartlist_len(args) == 2) { + // "EXTENDCIRCUIT 0 PURPOSE=foo" + purp = smartlist_get(args,1); + } else if (smartlist_len(args) == 3) { + // "EXTENDCIRCUIT 0 router1,router2 PURPOSE=foo" + purp = smartlist_get(args,2); + } + + if (purp && strcmpstart(purp, "purpose=") != 0) + purp = NULL; + + if (purp) { + intended_purpose = circuit_purpose_from_string(purp); + if (intended_purpose == CIRCUIT_PURPOSE_UNKNOWN) { + connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n", purp); + SMARTLIST_FOREACH(args, char *, cp, tor_free(cp)); + smartlist_free(args); + } + } + + if ((smartlist_len(args) == 1) || (purp && (smartlist_len(args) == 2))) { + // "EXTENDCIRCUIT 0" || EXTENDCIRCUIT 0 PURPOSE=foo" + circ = circuit_launch_by_router(intended_purpose, NULL, + CIRCLAUNCH_NEED_CAPACITY); + if (!circ) { + connection_write_str_to_buf("551 Couldn't start circuit\r\n", conn); + } else { + connection_printf_to_buf(conn, "250 EXTENDED %lu\r\n", + (unsigned long)circ->global_identifier); + } + goto done; + } + // "EXTENDCIRCUIT 0 router1,router2" || + // "EXTENDCIRCUIT 0 router1,router2 PURPOSE=foo" + } + if (!zero_circ && !(circ = get_circ(smartlist_get(args,0)))) { connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n", (char*)smartlist_get(args, 0)); + goto done; } + smartlist_split_string(router_nicknames, smartlist_get(args,1), ",", 0, 0); - if (zero_circ && smartlist_len(args)>2) { - char *purp = smartlist_get(args,2); - intended_purpose = circuit_purpose_from_string(purp); - if (intended_purpose == CIRCUIT_PURPOSE_UNKNOWN) { - connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n", purp); - SMARTLIST_FOREACH(args, char *, cp, tor_free(cp)); - smartlist_free(args); - goto done; - } - } SMARTLIST_FOREACH(args, char *, cp, tor_free(cp)); smartlist_free(args); if (!zero_circ && !circ) { From f459388c29d1a072e1809265ccd252671975edeb Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Mon, 11 Jan 2010 10:16:50 -0800 Subject: [PATCH 3/8] Add an event for a case where we drop guards. Also add a comment about an odd CBT timeout edgecase. --- src/or/circuitbuild.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index b16e9a4d80..c73f98bb9f 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -594,6 +594,9 @@ void circuit_build_times_add_timeout_worker(circuit_build_times_t *cbt, double quantile_cutoff) { + // XXX: This may be failing when the number of samples is small? + // Keep getting values for the largest timeout bucket over and over + // again... Probably because alpha is very very large in that case.. build_time_t gentime = circuit_build_times_generate_sample(cbt, quantile_cutoff, MAX_SYNTHETIC_QUANTILE); @@ -3313,6 +3316,7 @@ entry_guard_register_connect_status(const char *digest, int succeeded, "Removing from the list. %d/%d entry guards usable/new.", entry->nickname, buf, num_live_entry_guards()-1, smartlist_len(entry_guards)-1); + control_event_guard(entry->nickname, entry->identity, "DROPPED"); entry_guard_free(entry); smartlist_del_keeporder(entry_guards, idx); log_entry_guards(LOG_INFO); From 2258125e1af78983c2735e7f2b3d90690b45b328 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Thu, 21 Jan 2010 16:10:02 -0800 Subject: [PATCH 4/8] Move CBT params into consensus. --- src/or/circuitbuild.c | 200 ++++++++++++++++++++++++++++++----------- src/or/control.c | 6 +- src/or/networkstatus.c | 1 + src/or/or.h | 84 +++++++++-------- src/test/test.c | 35 ++++---- 5 files changed, 215 insertions(+), 111 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index c73f98bb9f..4ef6bfac36 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -79,6 +79,92 @@ static int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice); static void entry_guards_changed(void); +static int32_t +circuit_build_times_max_timeouts(void) +{ + int32_t num = networkstatus_get_param(NULL, "cbtmaxtimeouts", + CBT_DEFAULT_MAX_RECENT_TIMEOUT_COUNT); + return num; +} + +static int32_t +circuit_build_times_min_circs_to_observe(void) +{ + int32_t num = networkstatus_get_param(NULL, "cbtmincircs", + CBT_DEFAULT_MIN_CIRCUITS_TO_OBSERVE); + return num; +} + +double +circuit_build_times_quantile_cutoff(void) +{ + int32_t num = networkstatus_get_param(NULL, "cbtquantile", + CBT_DEFAULT_QUANTILE_CUTOFF); + return num/100.0; +} + +static int32_t +circuit_build_times_test_frequency(void) +{ + int32_t num = networkstatus_get_param(NULL, "cbttestfreq", + CBT_DEFAULT_TEST_FREQUENCY); + return num; +} + +static int32_t +circuit_build_times_min_timeout(void) +{ + int32_t num = networkstatus_get_param(NULL, "cbtmintimeout", + CBT_DEFAULT_TIMEOUT_MIN_VALUE); + return num; +} + +int32_t +circuit_build_times_initial_timeout(void) +{ + int32_t num = networkstatus_get_param(NULL, "cbtinitialtimeout", + CBT_DEFAULT_TIMEOUT_INITIAL_VALUE); + return num; +} + +static int32_t +circuit_build_times_recent_circuit_count(void) +{ + int32_t num = networkstatus_get_param(NULL, "cbtrecentcount", + CBT_DEFAULT_RECENT_CIRCUITS); + return num; +} + +void +circuit_build_times_new_consensus_params(circuit_build_times_t *cbt, + networkstatus_t *ns) +{ + int32_t num = networkstatus_get_param(ns, "cbtrecentcount", + CBT_DEFAULT_RECENT_CIRCUITS); + + if (num != cbt->liveness.num_recent_circs) { + int8_t *recent_circs; + log_notice(LD_CIRC, "Changing recent timeout size from %d to %d", + cbt->liveness.num_recent_circs, num); + + tor_assert(num > 0); + tor_assert(cbt->liveness.timeouts_after_firsthop); + recent_circs = tor_malloc_zero(sizeof(int8_t)*num); + memcpy(recent_circs, cbt->liveness.timeouts_after_firsthop, + sizeof(int8_t)*MIN(num, cbt->liveness.num_recent_circs)); + + // Adjust the index if it needs it. + if (num < cbt->liveness.num_recent_circs) { + cbt->liveness.after_firsthop_idx = MIN(num-1, + cbt->liveness.after_firsthop_idx); + } + + tor_free(cbt->liveness.timeouts_after_firsthop); + cbt->liveness.timeouts_after_firsthop = recent_circs; + cbt->liveness.num_recent_circs = num; + } +} + /** Make a note that we're running unit tests (rather than running Tor * itself), so we avoid clobbering our state file. */ void @@ -96,13 +182,13 @@ circuit_build_times_get_initial_timeout(void) double timeout; if (!unit_tests && get_options()->CircuitBuildTimeout) { timeout = get_options()->CircuitBuildTimeout*1000; - if (timeout < BUILD_TIMEOUT_MIN_VALUE) { + if (timeout < circuit_build_times_min_timeout()) { log_warn(LD_CIRC, "Config CircuitBuildTimeout too low. Setting to %ds", - BUILD_TIMEOUT_MIN_VALUE/1000); - timeout = BUILD_TIMEOUT_MIN_VALUE; + circuit_build_times_min_timeout()/1000); + timeout = circuit_build_times_min_timeout(); } } else { - timeout = BUILD_TIMEOUT_INITIAL_VALUE; + timeout = circuit_build_times_initial_timeout(); } return timeout; } @@ -133,6 +219,9 @@ void circuit_build_times_init(circuit_build_times_t *cbt) { memset(cbt, 0, sizeof(*cbt)); + cbt->liveness.num_recent_circs = circuit_build_times_recent_circuit_count(); + cbt->liveness.timeouts_after_firsthop = tor_malloc_zero(sizeof(int8_t)* + cbt->liveness.num_recent_circs); cbt->timeout_ms = circuit_build_times_get_initial_timeout(); control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_RESET); } @@ -162,10 +251,11 @@ circuit_build_times_rewind_history(circuit_build_times_t *cbt, int n) } cbt->build_times_idx -= n; - cbt->build_times_idx %= NCIRCUITS_TO_OBSERVE; + cbt->build_times_idx %= CBT_NCIRCUITS_TO_OBSERVE; for (i = 0; i < n; i++) { - cbt->circuit_build_times[(i+cbt->build_times_idx)%NCIRCUITS_TO_OBSERVE]=0; + cbt->circuit_build_times[(i+cbt->build_times_idx) + %CBT_NCIRCUITS_TO_OBSERVE]=0; } if (cbt->total_build_times > n) { @@ -189,7 +279,7 @@ circuit_build_times_rewind_history(circuit_build_times_t *cbt, int n) int circuit_build_times_add_time(circuit_build_times_t *cbt, build_time_t time) { - tor_assert(time <= BUILD_TIME_MAX); + tor_assert(time <= CBT_BUILD_TIME_MAX); if (time <= 0) { log_warn(LD_CIRC, "Circuit build time is %u!", time); return -1; @@ -199,11 +289,11 @@ circuit_build_times_add_time(circuit_build_times_t *cbt, build_time_t time) log_info(LD_CIRC, "Adding circuit build time %u", time); cbt->circuit_build_times[cbt->build_times_idx] = time; - cbt->build_times_idx = (cbt->build_times_idx + 1) % NCIRCUITS_TO_OBSERVE; - if (cbt->total_build_times < NCIRCUITS_TO_OBSERVE) + cbt->build_times_idx = (cbt->build_times_idx + 1) % CBT_NCIRCUITS_TO_OBSERVE; + if (cbt->total_build_times < CBT_NCIRCUITS_TO_OBSERVE) cbt->total_build_times++; - if ((cbt->total_build_times % BUILD_TIMES_SAVE_STATE_EVERY) == 0) { + if ((cbt->total_build_times % CBT_SAVE_STATE_EVERY) == 0) { /* Save state every n circuit builds */ if (!unit_tests && !get_options()->AvoidDiskWrites) or_state_mark_dirty(get_or_state(), 0); @@ -220,7 +310,7 @@ circuit_build_times_max(circuit_build_times_t *cbt) { int i = 0; build_time_t max_build_time = 0; - for (i = 0; i < NCIRCUITS_TO_OBSERVE; i++) { + for (i = 0; i < CBT_NCIRCUITS_TO_OBSERVE; i++) { if (cbt->circuit_build_times[i] > max_build_time) max_build_time = cbt->circuit_build_times[i]; } @@ -233,14 +323,14 @@ build_time_t circuit_build_times_min(circuit_build_times_t *cbt) { int i = 0; - build_time_t min_build_time = BUILD_TIME_MAX; - for (i = 0; i < NCIRCUITS_TO_OBSERVE; i++) { + build_time_t min_build_time = CBT_BUILD_TIME_MAX; + for (i = 0; i < CBT_NCIRCUITS_TO_OBSERVE; i++) { if (cbt->circuit_build_times[i] && /* 0 <-> uninitialized */ cbt->circuit_build_times[i] < min_build_time) min_build_time = cbt->circuit_build_times[i]; } - if (min_build_time == BUILD_TIME_MAX) { - log_warn(LD_CIRC, "No build times less than BUILD_TIME_MAX!"); + if (min_build_time == CBT_BUILD_TIME_MAX) { + log_warn(LD_CIRC, "No build times less than CBT_BUILD_TIME_MAX!"); } return min_build_time; } @@ -250,7 +340,7 @@ circuit_build_times_min(circuit_build_times_t *cbt) * Calculate and return a histogram for the set of build times. * * Returns an allocated array of histrogram bins representing - * the frequency of index*BUILDTIME_BIN_WIDTH millisecond + * the frequency of index*CBT_BIN_WIDTH millisecond * build times. Also outputs the number of bins in nbins. * * The return value must be freed by the caller. @@ -263,14 +353,14 @@ circuit_build_times_create_histogram(circuit_build_times_t *cbt, build_time_t max_build_time = circuit_build_times_max(cbt); int i, c; - *nbins = 1 + (max_build_time / BUILDTIME_BIN_WIDTH); + *nbins = 1 + (max_build_time / CBT_BIN_WIDTH); histogram = tor_malloc_zero(*nbins * sizeof(build_time_t)); // calculate histogram - for (i = 0; i < NCIRCUITS_TO_OBSERVE; i++) { + for (i = 0; i < CBT_NCIRCUITS_TO_OBSERVE; i++) { if (cbt->circuit_build_times[i] == 0) continue; /* 0 <-> uninitialized */ - c = (cbt->circuit_build_times[i] / BUILDTIME_BIN_WIDTH); + c = (cbt->circuit_build_times[i] / CBT_BIN_WIDTH); histogram[c]++; } @@ -278,7 +368,7 @@ circuit_build_times_create_histogram(circuit_build_times_t *cbt, } /** - * Return the most frequent build time (rounded to BUILDTIME_BIN_WIDTH ms). + * Return the most frequent build time (rounded to CBT_BIN_WIDTH ms). * * Ties go in favor of the slower time. */ @@ -296,7 +386,7 @@ circuit_build_times_mode(circuit_build_times_t *cbt) tor_free(histogram); - return max_bin*BUILDTIME_BIN_WIDTH+BUILDTIME_BIN_WIDTH/2; + return max_bin*CBT_BIN_WIDTH+CBT_BIN_WIDTH/2; } /** @@ -327,7 +417,7 @@ circuit_build_times_update_state(circuit_build_times_t *cbt, line->key = tor_strdup("CircuitBuildTimeBin"); line->value = tor_malloc(25); tor_snprintf(line->value, 25, "%d %d", - i*BUILDTIME_BIN_WIDTH+BUILDTIME_BIN_WIDTH/2, histogram[i]); + i*CBT_BIN_WIDTH+CBT_BIN_WIDTH/2, histogram[i]); next = &(line->next); } @@ -350,9 +440,9 @@ circuit_build_times_shuffle_and_store_array(circuit_build_times_t *cbt, int num_times) { int n = num_times; - if (num_times > NCIRCUITS_TO_OBSERVE) { + if (num_times > CBT_NCIRCUITS_TO_OBSERVE) { log_notice(LD_CIRC, "Decreasing circuit_build_times size from %d to %d", - num_times, NCIRCUITS_TO_OBSERVE); + num_times, CBT_NCIRCUITS_TO_OBSERVE); } /* This code can only be run on a compact array */ @@ -363,9 +453,9 @@ circuit_build_times_shuffle_and_store_array(circuit_build_times_t *cbt, raw_times[n] = tmp; } - /* Since the times are now shuffled, take a random NCIRCUITS_TO_OBSERVE - * subset (ie the first NCIRCUITS_TO_OBSERVE values) */ - for (n = 0; n < MIN(num_times, NCIRCUITS_TO_OBSERVE); n++) { + /* Since the times are now shuffled, take a random CBT_NCIRCUITS_TO_OBSERVE + * subset (ie the first CBT_NCIRCUITS_TO_OBSERVE values) */ + for (n = 0; n < MIN(num_times, CBT_NCIRCUITS_TO_OBSERVE); n++) { circuit_build_times_add_time(cbt, raw_times[n]); } } @@ -407,7 +497,7 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt, build_time_t ms; int ok; ms = (build_time_t)tor_parse_ulong(ms_str, 0, 0, - BUILD_TIME_MAX, &ok, NULL); + CBT_BUILD_TIME_MAX, &ok, NULL); if (!ok) { *msg = tor_strdup("Unable to parse circuit build times: " "Unparsable bin number"); @@ -453,7 +543,7 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt, circuit_build_times_shuffle_and_store_array(cbt, loaded_times, loaded_cnt); /* Verify that we didn't overwrite any indexes */ - for (i=0; i < NCIRCUITS_TO_OBSERVE; i++) { + for (i=0; i < CBT_NCIRCUITS_TO_OBSERVE; i++) { if (!cbt->circuit_build_times[i]) break; tot_values++; @@ -462,7 +552,7 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt, "Loaded %d/%d values from %d lines in circuit time histogram", tot_values, cbt->total_build_times, N); tor_assert(cbt->total_build_times == tot_values); - tor_assert(cbt->total_build_times <= NCIRCUITS_TO_OBSERVE); + tor_assert(cbt->total_build_times <= CBT_NCIRCUITS_TO_OBSERVE); circuit_build_times_set_timeout(cbt); tor_free(loaded_times); return *msg ? -1 : 0; @@ -489,7 +579,7 @@ circuit_build_times_update_alpha(circuit_build_times_t *cbt) * and less frechet-like. */ cbt->Xm = circuit_build_times_mode(cbt); - for (i=0; i< NCIRCUITS_TO_OBSERVE; i++) { + for (i=0; i< CBT_NCIRCUITS_TO_OBSERVE; i++) { if (!x[i]) { continue; } @@ -598,18 +688,18 @@ circuit_build_times_add_timeout_worker(circuit_build_times_t *cbt, // Keep getting values for the largest timeout bucket over and over // again... Probably because alpha is very very large in that case.. build_time_t gentime = circuit_build_times_generate_sample(cbt, - quantile_cutoff, MAX_SYNTHETIC_QUANTILE); + quantile_cutoff, CBT_MAX_SYNTHETIC_QUANTILE); if (gentime < (build_time_t)tor_lround(cbt->timeout_ms)) { log_warn(LD_CIRC, "Generated a synthetic timeout LESS than the current timeout: " "%ums vs %lfms using Xm: %d a: %lf, q: %lf", gentime, cbt->timeout_ms, cbt->Xm, cbt->alpha, quantile_cutoff); - } else if (gentime > BUILD_TIME_MAX) { + } else if (gentime > CBT_BUILD_TIME_MAX) { log_info(LD_CIRC, "Generated a synthetic timeout larger than the max: %u", gentime); - gentime = BUILD_TIME_MAX; + gentime = CBT_BUILD_TIME_MAX; } else { log_info(LD_CIRC, "Generated synthetic circuit build time %u for timeout", gentime); @@ -653,7 +743,7 @@ circuit_build_times_count_pretimeouts(circuit_build_times_t *cbt) ((double)cbt->pre_timeouts)/ (cbt->pre_timeouts+cbt->total_build_times); /* Make sure it doesn't exceed the synthetic max */ - timeout_quantile *= MAX_SYNTHETIC_QUANTILE; + timeout_quantile *= CBT_MAX_SYNTHETIC_QUANTILE; cbt->Xm = circuit_build_times_mode(cbt); tor_assert(cbt->Xm > 0); /* Use current timeout to get an estimate on alpha */ @@ -673,7 +763,7 @@ int circuit_build_times_needs_circuits(circuit_build_times_t *cbt) { /* Return true if < MIN_CIRCUITS_TO_OBSERVE */ - if (cbt->total_build_times < MIN_CIRCUITS_TO_OBSERVE) + if (cbt->total_build_times < circuit_build_times_min_circs_to_observe()) return 1; return 0; } @@ -686,7 +776,7 @@ int circuit_build_times_needs_circuits_now(circuit_build_times_t *cbt) { return circuit_build_times_needs_circuits(cbt) && - approx_time()-cbt->last_circ_at > BUILD_TIMES_TEST_FREQUENCY; + approx_time()-cbt->last_circ_at > circuit_build_times_test_frequency(); } /** @@ -712,7 +802,7 @@ circuit_build_times_network_circ_success(circuit_build_times_t *cbt) { cbt->liveness.timeouts_after_firsthop[cbt->liveness.after_firsthop_idx] = 0; cbt->liveness.after_firsthop_idx++; - cbt->liveness.after_firsthop_idx %= RECENT_CIRCUITS; + cbt->liveness.after_firsthop_idx %= cbt->liveness.num_recent_circs; } /** @@ -743,7 +833,7 @@ circuit_build_times_network_timeout(circuit_build_times_t *cbt, /* Count a one-hop timeout */ cbt->liveness.timeouts_after_firsthop[cbt->liveness.after_firsthop_idx]=1; cbt->liveness.after_firsthop_idx++; - cbt->liveness.after_firsthop_idx %= RECENT_CIRCUITS; + cbt->liveness.after_firsthop_idx %= cbt->liveness.num_recent_circs; } } @@ -758,7 +848,7 @@ int circuit_build_times_network_check_live(circuit_build_times_t *cbt) { time_t now = approx_time(); - if (cbt->liveness.nonlive_timeouts >= NETWORK_NONLIVE_DISCARD_COUNT) { + if (cbt->liveness.nonlive_timeouts >= CBT_NETWORK_NONLIVE_DISCARD_COUNT) { if (!cbt->liveness.nonlive_discarded) { cbt->liveness.nonlive_discarded = 1; log_notice(LD_CIRC, "Network is no longer live (too many recent " @@ -766,11 +856,13 @@ circuit_build_times_network_check_live(circuit_build_times_t *cbt) (long int)(now - cbt->liveness.network_last_live)); /* Only discard NETWORK_NONLIVE_TIMEOUT_COUNT-1 because we stopped * counting after that */ - circuit_build_times_rewind_history(cbt, NETWORK_NONLIVE_TIMEOUT_COUNT-1); + circuit_build_times_rewind_history(cbt, + CBT_NETWORK_NONLIVE_TIMEOUT_COUNT-1); control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_DISCARD); } return 0; - } else if (cbt->liveness.nonlive_timeouts >= NETWORK_NONLIVE_TIMEOUT_COUNT) { + } else if (cbt->liveness.nonlive_timeouts >= + CBT_NETWORK_NONLIVE_TIMEOUT_COUNT) { if (cbt->timeout_ms < circuit_build_times_get_initial_timeout()) { log_notice(LD_CIRC, "Network is flaky. No activity for %ld seconds. " @@ -812,19 +904,20 @@ circuit_build_times_network_check_changed(circuit_build_times_t *cbt) /* how many of our recent circuits made it to the first hop but then * timed out? */ - for (i = 0; i < RECENT_CIRCUITS; i++) { + for (i = 0; i < cbt->liveness.num_recent_circs; i++) { timeout_count += cbt->liveness.timeouts_after_firsthop[i]; } /* If 80% of our recent circuits are timing out after the first hop, * we need to re-estimate a new initial alpha and timeout. */ - if (timeout_count < MAX_RECENT_TIMEOUT_COUNT) { + if (timeout_count < circuit_build_times_max_timeouts()) { return 0; } circuit_build_times_reset(cbt); memset(cbt->liveness.timeouts_after_firsthop, 0, - sizeof(cbt->liveness.timeouts_after_firsthop)); + sizeof(*cbt->liveness.timeouts_after_firsthop)* + cbt->liveness.num_recent_circs); cbt->liveness.after_firsthop_idx = 0; /* Check to see if this has happened before. If so, double the timeout @@ -875,13 +968,14 @@ circuit_build_times_add_timeout(circuit_build_times_t *cbt, cbt->pre_timeouts++; log_info(LD_CIRC, "Not enough circuits yet to calculate a new build timeout." - " Need %d more.", - MIN_CIRCUITS_TO_OBSERVE-cbt->total_build_times); + " Need %d more.", circuit_build_times_min_circs_to_observe() + - cbt->total_build_times); return 0; } circuit_build_times_count_pretimeouts(cbt); - circuit_build_times_add_timeout_worker(cbt, BUILDTIMEOUT_QUANTILE_CUTOFF); + circuit_build_times_add_timeout_worker(cbt, + circuit_build_times_quantile_cutoff()); return 1; } @@ -893,7 +987,7 @@ circuit_build_times_add_timeout(circuit_build_times_t *cbt, void circuit_build_times_set_timeout(circuit_build_times_t *cbt) { - if (cbt->total_build_times < MIN_CIRCUITS_TO_OBSERVE) { + if (cbt->total_build_times < circuit_build_times_min_circs_to_observe()) { return; } @@ -901,14 +995,14 @@ circuit_build_times_set_timeout(circuit_build_times_t *cbt) circuit_build_times_update_alpha(cbt); cbt->timeout_ms = circuit_build_times_calculate_timeout(cbt, - BUILDTIMEOUT_QUANTILE_CUTOFF); + circuit_build_times_quantile_cutoff()); cbt->have_computed_timeout = 1; - if (cbt->timeout_ms < BUILD_TIMEOUT_MIN_VALUE) { + if (cbt->timeout_ms < circuit_build_times_min_timeout()) { log_warn(LD_CIRC, "Set buildtimeout to low value %lfms. Setting to %dms", - cbt->timeout_ms, BUILD_TIMEOUT_MIN_VALUE); - cbt->timeout_ms = BUILD_TIMEOUT_MIN_VALUE; + cbt->timeout_ms, circuit_build_times_min_timeout()); + cbt->timeout_ms = circuit_build_times_min_timeout(); } control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_COMPUTED); diff --git a/src/or/control.c b/src/or/control.c index a7e60d55f0..835c3be518 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -3480,13 +3480,13 @@ control_event_buildtimeout_set(const circuit_build_times_t *cbt, buildtimeout_set_event_t type) { const char *type_string = NULL; - double qnt = BUILDTIMEOUT_QUANTILE_CUTOFF; + double qnt = circuit_build_times_quantile_cutoff(); if (!control_event_is_interesting(EVENT_BUILDTIMEOUT_SET)) return 0; switch (type) { - case BUILDTIMEOUT_SET_EVENT_COMPUTED: + case BUILDTIMEOUT_SET_EVENT_COMPUTED: type_string = "COMPUTED"; break; case BUILDTIMEOUT_SET_EVENT_RESET: @@ -3513,7 +3513,7 @@ control_event_buildtimeout_set(const circuit_build_times_t *cbt, "650 BUILDTIMEOUT_SET %s TOTAL_TIMES=%lu " "TIMEOUT_MS=%lu XM=%lu ALPHA=%lf CUTOFF_QUANTILE=%lf\r\n", type_string, (unsigned long)cbt->total_build_times, - (unsigned long)cbt->timeout_ms, + (unsigned long)cbt->timeout_ms, (unsigned long)cbt->Xm, cbt->alpha, qnt); return 0; diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 507875ab2a..a507f1be2d 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1680,6 +1680,7 @@ networkstatus_set_current_consensus(const char *consensus, dirvote_recalculate_timing(get_options(), now); routerstatus_list_update_named_server_map(); cell_ewma_set_scale_factor(get_options(), current_consensus); + circuit_build_times_new_consensus_params(&circ_times, current_consensus); } if (!from_cache) { diff --git a/src/or/or.h b/src/or/or.h index 24cabb4300..434de7819e 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3002,49 +3002,27 @@ void entry_guards_free_all(void); /* Circuit Build Timeout "public" functions and structures. */ +/** Total size of the circuit timeout history to accumulate. + * 1000 is approx 2.5 days worth of continual-use circuits. */ +#define CBT_NCIRCUITS_TO_OBSERVE 1000 + /** Maximum quantile to use to generate synthetic timeouts. * We want to stay a bit short of 1.0, because longtail is * loooooooooooooooooooooooooooooooooooooooooooooooooooong. */ -#define MAX_SYNTHETIC_QUANTILE 0.985 - -/** Minimum circuits before estimating a timeout */ -#define MIN_CIRCUITS_TO_OBSERVE 20 - -/** Total size of the circuit timeout history to accumulate. - * 5000 is approx 1.5 weeks worth of continual-use circuits. */ -#define NCIRCUITS_TO_OBSERVE 5000 +#define CBT_MAX_SYNTHETIC_QUANTILE 0.985 /** Width of the histogram bins in milliseconds */ -#define BUILDTIME_BIN_WIDTH ((build_time_t)50) - -/** Cutoff point on the CDF for our timeout estimation. - * TODO: This should be moved to the consensus */ -#define BUILDTIMEOUT_QUANTILE_CUTOFF 0.8 +#define CBT_BIN_WIDTH ((build_time_t)50) /** A build_time_t is milliseconds */ typedef uint32_t build_time_t; -#define BUILD_TIME_MAX ((build_time_t)(INT32_MAX)) - -/** Lowest allowable value for CircuitBuildTimeout in milliseconds */ -#define BUILD_TIMEOUT_MIN_VALUE (3*1000) - -/** Initial circuit build timeout in milliseconds */ -#define BUILD_TIMEOUT_INITIAL_VALUE (60*1000) - -/** How often in seconds should we build a test circuit */ -#define BUILD_TIMES_TEST_FREQUENCY 60 +#define CBT_BUILD_TIME_MAX ((build_time_t)(INT32_MAX)) /** Save state every 10 circuits */ -#define BUILD_TIMES_SAVE_STATE_EVERY 1 +#define CBT_SAVE_STATE_EVERY 10 /* Circuit Build Timeout network liveness constants */ -/** - * How many circuits count as recent when considering if the - * connection has gone gimpy or changed. - */ -#define RECENT_CIRCUITS 20 - /** * Have we received a cell in the last N circ attempts? * @@ -3053,7 +3031,7 @@ typedef uint32_t build_time_t; * at which point we switch back to computing the timeout from * our saved history. */ -#define NETWORK_NONLIVE_TIMEOUT_COUNT (RECENT_CIRCUITS*3/20) +#define CBT_NETWORK_NONLIVE_TIMEOUT_COUNT (3) /** * This tells us when to toss out the last streak of N timeouts. @@ -3061,7 +3039,15 @@ typedef uint32_t build_time_t; * If instead we start getting cells, we switch back to computing the timeout * from our saved history. */ -#define NETWORK_NONLIVE_DISCARD_COUNT (NETWORK_NONLIVE_TIMEOUT_COUNT*2) +#define CBT_NETWORK_NONLIVE_DISCARD_COUNT (CBT_NETWORK_NONLIVE_TIMEOUT_COUNT*2) + +/* Circuit build times consensus parameters */ + +/** + * How many circuits count as recent when considering if the + * connection has gone gimpy or changed. + */ +#define CBT_DEFAULT_RECENT_CIRCUITS 20 /** * Maximum count of timeouts that finish the first hop in the past @@ -3070,10 +3056,28 @@ typedef uint32_t build_time_t; * This tells us to abandon timeout history and set * the timeout back to BUILD_TIMEOUT_INITIAL_VALUE. */ -#define MAX_RECENT_TIMEOUT_COUNT (RECENT_CIRCUITS*4/5) +#define CBT_DEFAULT_MAX_RECENT_TIMEOUT_COUNT (CBT_DEFAULT_RECENT_CIRCUITS*9/10) -#if MAX_RECENT_TIMEOUT_COUNT < 1 || NETWORK_NONLIVE_DISCARD_COUNT < 1 || \ - NETWORK_NONLIVE_TIMEOUT_COUNT < 1 +/** Minimum circuits before estimating a timeout */ +#define CBT_DEFAULT_MIN_CIRCUITS_TO_OBSERVE 100 + +/** Cutoff percentile on the CDF for our timeout estimation. */ +#define CBT_DEFAULT_QUANTILE_CUTOFF 80 +double circuit_build_times_quantile_cutoff(void); + +/** How often in seconds should we build a test circuit */ +#define CBT_DEFAULT_TEST_FREQUENCY 60 + +/** Lowest allowable value for CircuitBuildTimeout in milliseconds */ +#define CBT_DEFAULT_TIMEOUT_MIN_VALUE (2*1000) + +/** Initial circuit build timeout in milliseconds */ +#define CBT_DEFAULT_TIMEOUT_INITIAL_VALUE (60*1000) +int32_t circuit_build_times_initial_timeout(void); + +#if CBT_DEFAULT_MAX_RECENT_TIMEOUT_COUNT < 1 || \ + CBT_NETWORK_NONLIVE_DISCARD_COUNT < 1 || \ + CBT_NETWORK_NONLIVE_TIMEOUT_COUNT < 1 #error "RECENT_CIRCUITS is set too low." #endif @@ -3087,7 +3091,9 @@ typedef struct { int nonlive_discarded; /** Circular array of circuits that have made it to the first hop. Slot is * 1 if circuit timed out, 0 if circuit succeeded */ - int8_t timeouts_after_firsthop[RECENT_CIRCUITS]; + int8_t *timeouts_after_firsthop; + /** Number of elements allocated for the above array */ + int num_recent_circs; /** Index into circular array. */ int after_firsthop_idx; /** The network is not live. Timeout gathering is suspended */ @@ -3097,10 +3103,10 @@ typedef struct { /** Structure for circuit build times history */ typedef struct { /** The circular array of recorded build times in milliseconds */ - build_time_t circuit_build_times[NCIRCUITS_TO_OBSERVE]; + build_time_t circuit_build_times[CBT_NCIRCUITS_TO_OBSERVE]; /** Current index in the circuit_build_times circular array */ int build_times_idx; - /** Total number of build times accumulated. Maxes at NCIRCUITS_TO_OBSERVE */ + /** Total number of build times accumulated. Max CBT_NCIRCUITS_TO_OBSERVE */ int total_build_times; /** Information about the state of our local network connection */ network_liveness_t liveness; @@ -3132,6 +3138,8 @@ int circuit_build_times_add_time(circuit_build_times_t *cbt, int circuit_build_times_needs_circuits(circuit_build_times_t *cbt); int circuit_build_times_needs_circuits_now(circuit_build_times_t *cbt); void circuit_build_times_init(circuit_build_times_t *cbt); +void circuit_build_times_new_consensus_params(circuit_build_times_t *cbt, + networkstatus_t *ns); #ifdef CIRCUIT_PRIVATE double circuit_build_times_calculate_timeout(circuit_build_times_t *cbt, diff --git a/src/test/test.c b/src/test/test.c index 8b84290446..760558b65a 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -479,11 +479,12 @@ test_circuit_timeout(void) circuitbuild_running_unit_tests(); #define timeout0 (build_time_t)(30*1000.0) initial.Xm = 750; - circuit_build_times_initial_alpha(&initial, BUILDTIMEOUT_QUANTILE_CUTOFF, + circuit_build_times_initial_alpha(&initial, + CBT_DEFAULT_QUANTILE_CUTOFF/100.0, timeout0); do { int n = 0; - for (i=0; i < MIN_CIRCUITS_TO_OBSERVE; i++) { + for (i=0; i < CBT_DEFAULT_MIN_CIRCUITS_TO_OBSERVE; i++) { if (circuit_build_times_add_time(&estimate, circuit_build_times_generate_sample(&initial, 0, 1)) == 0) { n++; @@ -491,23 +492,23 @@ test_circuit_timeout(void) } circuit_build_times_update_alpha(&estimate); timeout1 = circuit_build_times_calculate_timeout(&estimate, - BUILDTIMEOUT_QUANTILE_CUTOFF); + CBT_DEFAULT_QUANTILE_CUTOFF/100.0); circuit_build_times_set_timeout(&estimate); log_warn(LD_CIRC, "Timeout is %lf, Xm is %d", timeout1, estimate.Xm); /* XXX: 5% distribution error may not be the right metric */ } while (fabs(circuit_build_times_cdf(&initial, timeout0) - circuit_build_times_cdf(&initial, timeout1)) > 0.05 /* 5% error */ - && estimate.total_build_times < NCIRCUITS_TO_OBSERVE); + && estimate.total_build_times < CBT_NCIRCUITS_TO_OBSERVE); - test_assert(estimate.total_build_times < NCIRCUITS_TO_OBSERVE); + test_assert(estimate.total_build_times < CBT_NCIRCUITS_TO_OBSERVE); circuit_build_times_update_state(&estimate, &state); test_assert(circuit_build_times_parse_state(&final, &state, &msg) == 0); circuit_build_times_update_alpha(&final); timeout2 = circuit_build_times_calculate_timeout(&final, - BUILDTIMEOUT_QUANTILE_CUTOFF); + CBT_DEFAULT_QUANTILE_CUTOFF/100.0); circuit_build_times_set_timeout(&final); log_warn(LD_CIRC, "Timeout is %lf, Xm is %d", timeout2, final.Xm); @@ -519,19 +520,19 @@ test_circuit_timeout(void) int build_times_idx = 0; int total_build_times = 0; - final.timeout_ms = BUILD_TIMEOUT_INITIAL_VALUE; - estimate.timeout_ms = BUILD_TIMEOUT_INITIAL_VALUE; + final.timeout_ms = CBT_DEFAULT_TIMEOUT_INITIAL_VALUE; + estimate.timeout_ms = CBT_DEFAULT_TIMEOUT_INITIAL_VALUE; - for (i = 0; i < RECENT_CIRCUITS*2; i++) { + for (i = 0; i < CBT_DEFAULT_RECENT_CIRCUITS*2; i++) { circuit_build_times_network_circ_success(&estimate); circuit_build_times_add_time(&estimate, circuit_build_times_generate_sample(&estimate, 0, - BUILDTIMEOUT_QUANTILE_CUTOFF)); + CBT_DEFAULT_QUANTILE_CUTOFF/100.0)); estimate.have_computed_timeout = 1; circuit_build_times_network_circ_success(&estimate); circuit_build_times_add_time(&final, circuit_build_times_generate_sample(&final, 0, - BUILDTIMEOUT_QUANTILE_CUTOFF)); + CBT_DEFAULT_QUANTILE_CUTOFF/100.0)); final.have_computed_timeout = 1; } @@ -544,7 +545,7 @@ test_circuit_timeout(void) build_times_idx = estimate.build_times_idx; total_build_times = estimate.total_build_times; - for (i = 0; i < NETWORK_NONLIVE_TIMEOUT_COUNT; i++) { + for (i = 0; i < CBT_NETWORK_NONLIVE_TIMEOUT_COUNT; i++) { test_assert(circuit_build_times_network_check_live(&estimate)); test_assert(circuit_build_times_network_check_live(&final)); @@ -559,12 +560,12 @@ test_circuit_timeout(void) test_assert(!circuit_build_times_network_check_live(&estimate)); test_assert(!circuit_build_times_network_check_live(&final)); - for ( ; i < NETWORK_NONLIVE_DISCARD_COUNT; i++) { + for ( ; i < CBT_NETWORK_NONLIVE_DISCARD_COUNT; i++) { if (circuit_build_times_add_timeout(&estimate, 0, (time_t)(approx_time()-estimate.timeout_ms/1000.0-1))) estimate.have_computed_timeout = 1; - if (i < NETWORK_NONLIVE_DISCARD_COUNT-1) { + if (i < CBT_NETWORK_NONLIVE_DISCARD_COUNT-1) { if (circuit_build_times_add_timeout(&final, 0, (time_t)(approx_time()-final.timeout_ms/1000.0-1))) final.have_computed_timeout = 1; @@ -587,11 +588,11 @@ test_circuit_timeout(void) circuit_build_times_network_is_live(&estimate); circuit_build_times_network_is_live(&final); - for (i = 0; i < MAX_RECENT_TIMEOUT_COUNT; i++) { + for (i = 0; i < CBT_DEFAULT_MAX_RECENT_TIMEOUT_COUNT; i++) { if (circuit_build_times_add_timeout(&estimate, 1, approx_time()-1)) estimate.have_computed_timeout = 1; - if (i < MAX_RECENT_TIMEOUT_COUNT-1) { + if (i < CBT_DEFAULT_MAX_RECENT_TIMEOUT_COUNT-1) { if (circuit_build_times_add_timeout(&final, 1, approx_time()-1)) final.have_computed_timeout = 1; } @@ -599,7 +600,7 @@ test_circuit_timeout(void) test_assert(estimate.liveness.after_firsthop_idx == 0); test_assert(final.liveness.after_firsthop_idx == - MAX_RECENT_TIMEOUT_COUNT-1); + CBT_DEFAULT_MAX_RECENT_TIMEOUT_COUNT-1); test_assert(circuit_build_times_network_check_live(&estimate)); test_assert(circuit_build_times_network_check_live(&final)); From 2b95d1c0ee463b278dbb8de008338687cd340e87 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Wed, 10 Feb 2010 12:25:06 -0800 Subject: [PATCH 5/8] Describe the recent timeouts reallocation behavior. --- src/or/circuitbuild.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 4ef6bfac36..08ee58416b 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -135,6 +135,12 @@ circuit_build_times_recent_circuit_count(void) return num; } +/** + * This function is called when we get a consensus update. + * + * It checks to see if we have changed any consensus parameters + * that require reallocation or discard of previous stats. + */ void circuit_build_times_new_consensus_params(circuit_build_times_t *cbt, networkstatus_t *ns) @@ -149,6 +155,19 @@ circuit_build_times_new_consensus_params(circuit_build_times_t *cbt, tor_assert(num > 0); tor_assert(cbt->liveness.timeouts_after_firsthop); + + /* + * Technically this is a circular array that we are reallocating + * and memcopying. However, since it only consists of either 1s + * or 0s, and is only used in a statistical test to determine when + * we should discard our history after a sufficient number of 1's + * have been reached, it is fine if order is not preserved or + * elements are lost. + * + * cbtrecentcount should only be changing in cases of severe network + * distress anyway, so memory correctness here is paramount over + * doing acrobatics to preserve the array. + */ recent_circs = tor_malloc_zero(sizeof(int8_t)*num); memcpy(recent_circs, cbt->liveness.timeouts_after_firsthop, sizeof(int8_t)*MIN(num, cbt->liveness.num_recent_circs)); From 54f1f2e5584a86b1cd687cba85cc8b9fa0fefc96 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Thu, 18 Feb 2010 09:40:15 -0800 Subject: [PATCH 6/8] Add changelog entry for CBT testing work. --- ChangeLog | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ChangeLog b/ChangeLog index 6ec19e6527..42f6aebbf7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -20,6 +20,7 @@ Changes in version 0.2.2.9-alpha - 2010-??-?? implemented in the past. - Avoid a bogus overlapped memcpy in tor_addr_copy(). Found by "memcpyfail". + - Emit an GUARD DROPPED event for a case we missed. o Code simplifications and refactoring: - Generate our manpage and HTML documentation using Asciidoc. This @@ -32,6 +33,18 @@ Changes in version 0.2.2.9-alpha - 2010-??-?? AUTHORS file had its content merged into the people page on the website. The roadmaps and design doc can now be found in the projects directory in svn. + - Enabled various circuit build timeout constants to be controlled + by consensus parameters. Also set better defaults for these + parameters based on experimentation on broadband and simulated + high latency links. + + o Minor features: + - The 'EXTENDCIRCUIT' control port command can now be used with + a circ id of 0 and no path. This will cause Tor to build a new + 'fast' general purpose circuit using its own path selection + algorithms. + - Added a BUILDTIMEOUT_SET control port event to describe changes + to the circuit build timeout. o Removed features: - Stop shipping parts of the website and the design paper in the From 2d95e02914c07f9186a1745e135d26b4da02cdb1 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 18 Feb 2010 12:32:11 -0500 Subject: [PATCH 7/8] Make more arguments in control.c properly case-insensitive. --- ChangeLog | 2 ++ src/or/control.c | 12 ++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index 42f6aebbf7..a25db73d56 100644 --- a/ChangeLog +++ b/ChangeLog @@ -21,6 +21,8 @@ Changes in version 0.2.2.9-alpha - 2010-??-?? - Avoid a bogus overlapped memcpy in tor_addr_copy(). Found by "memcpyfail". - Emit an GUARD DROPPED event for a case we missed. + - Make more fields in the controller protocol case-insensitive as + documented in control-spec.txt. o Code simplifications and refactoring: - Generate our manpage and HTML documentation using Asciidoc. This diff --git a/src/or/control.c b/src/or/control.c index 835c3be518..adf0611506 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -2003,12 +2003,12 @@ handle_control_getinfo(control_connection_t *conn, uint32_t len, static uint8_t circuit_purpose_from_string(const char *string) { - if (!strcmpstart(string, "purpose=")) + if (!strcasecmpstart(string, "purpose=")) string += strlen("purpose="); - if (!strcmp(string, "general")) + if (!strcasecmp(string, "general")) return CIRCUIT_PURPOSE_C_GENERAL; - else if (!strcmp(string, "controller")) + else if (!strcasecmp(string, "controller")) return CIRCUIT_PURPOSE_CONTROLLER; else return CIRCUIT_PURPOSE_UNKNOWN; @@ -2071,7 +2071,7 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len, purp = smartlist_get(args,2); } - if (purp && strcmpstart(purp, "purpose=") != 0) + if (purp && strcasecmpstart(purp, "purpose=") != 0) purp = NULL; if (purp) { @@ -2351,9 +2351,9 @@ handle_control_postdescriptor(control_connection_t *conn, uint32_t len, } } else if (!strcasecmpstart(option, "cache=")) { option += strlen("cache="); - if (!strcmp(option, "no")) + if (!strcasecmp(option, "no")) cache = 0; - else if (!strcmp(option, "yes")) + else if (!strcasecmp(option, "yes")) cache = 1; else { connection_printf_to_buf(conn, "552 Unknown cache request \"%s\"\r\n", From 061ffbd7d563d270a2e311f7c47bc434cf913a40 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 18 Feb 2010 12:47:42 -0500 Subject: [PATCH 8/8] Future-proof the control protocol by ignoring unrecognized keyword args --- ChangeLog | 2 ++ src/or/control.c | 66 ++++++++++++++++++++++++++++++------------------ 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/ChangeLog b/ChangeLog index a25db73d56..641aa74e5c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -47,6 +47,8 @@ Changes in version 0.2.2.9-alpha - 2010-??-?? algorithms. - Added a BUILDTIMEOUT_SET control port event to describe changes to the circuit build timeout. + - Future-proof the controller protocol a bit by ignoring keyword + arguments we do not recognize. o Removed features: - Stop shipping parts of the website and the design paper in the diff --git a/src/or/control.c b/src/or/control.c index adf0611506..9840ea6294 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -2040,6 +2040,31 @@ getargs_helper(const char *command, control_connection_t *conn, return NULL; } +/** Helper. Return the first element of sl at index start_at or + * higher that starts with prefix, case-insensitive. Return NULL if no + * such element exists. */ +static const char * +find_element_starting_with(smartlist_t *sl, int start_at, const char *prefix) +{ + int i; + for (i = start_at; i < smartlist_len(sl); ++i) { + const char *elt = smartlist_get(sl, i); + if (!strcasecmpstart(elt, prefix)) + return elt; + } + return NULL; +} + +/** Helper. Return true iff s is an argument that we should treat as a + * key-value pair. */ +static int +is_keyval_pair(const char *s) +{ + /* An argument is a key-value pair if it has an =, and it isn't of the form + * $fingeprint=name */ + return strchr(s, '=') && s[0] != '$'; +} + /** Called when we get an EXTENDCIRCUIT message. Try to extend the listed * circuit, and report success or failure. */ static int @@ -2062,17 +2087,7 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len, zero_circ = !strcmp("0", (char*)smartlist_get(args,0)); if (zero_circ) { - char *purp = NULL; - if (smartlist_len(args) == 2) { - // "EXTENDCIRCUIT 0 PURPOSE=foo" - purp = smartlist_get(args,1); - } else if (smartlist_len(args) == 3) { - // "EXTENDCIRCUIT 0 router1,router2 PURPOSE=foo" - purp = smartlist_get(args,2); - } - - if (purp && strcasecmpstart(purp, "purpose=") != 0) - purp = NULL; + const char *purp = find_element_starting_with(args, 1, "PURPOSE="); if (purp) { intended_purpose = circuit_purpose_from_string(purp); @@ -2083,8 +2098,9 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len, } } - if ((smartlist_len(args) == 1) || (purp && (smartlist_len(args) == 2))) { - // "EXTENDCIRCUIT 0" || EXTENDCIRCUIT 0 PURPOSE=foo" + if ((smartlist_len(args) == 1) || + (smartlist_len(args) >= 2 && is_keyval_pair(smartlist_get(args, 1)))) { + // "EXTENDCIRCUIT 0" || EXTENDCIRCUIT 0 foo=bar" circ = circuit_launch_by_router(intended_purpose, NULL, CIRCLAUNCH_NEED_CAPACITY); if (!circ) { @@ -2196,7 +2212,7 @@ handle_control_setcircuitpurpose(control_connection_t *conn, } { - char *purp = smartlist_get(args,1); + const char *purp = find_element_starting_with(args,1,"PURPOSE="); new_purpose = circuit_purpose_from_string(purp); if (new_purpose == CIRCUIT_PURPOSE_UNKNOWN) { connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n", purp); @@ -2241,9 +2257,9 @@ handle_control_attachstream(control_connection_t *conn, uint32_t len, } else if (!zero_circ && !(circ = get_circ(smartlist_get(args, 1)))) { connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n", (char*)smartlist_get(args, 1)); - } else if (circ && smartlist_len(args) > 2) { - char *hopstring = smartlist_get(args, 2); - if (!strcasecmpstart(hopstring, "HOP=")) { + } else if (circ) { + const char *hopstring = find_element_starting_with(args,2,"HOP="); + if (hopstring) { hopstring += strlen("HOP="); hop = (int) tor_parse_ulong(hopstring, 10, 0, INT_MAX, &hop_line_ok, NULL); @@ -2535,17 +2551,17 @@ handle_control_resolve(control_connection_t *conn, uint32_t len, args = smartlist_create(); smartlist_split_string(args, body, " ", SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); - if (smartlist_len(args) && - !strcasecmp(smartlist_get(args, 0), "mode=reverse")) { - char *cp = smartlist_get(args, 0); - smartlist_del_keeporder(args, 0); - tor_free(cp); - is_reverse = 1; + { + const char *modearg = find_element_starting_with(args, 0, "mode="); + if (modearg && !strcasecmp(modearg, "mode=reverse")) + is_reverse = 1; } failed = smartlist_create(); SMARTLIST_FOREACH(args, const char *, arg, { - if (dnsserv_launch_request(arg, is_reverse)<0) - smartlist_add(failed, (char*)arg); + if (!is_keyval_pair(arg)) { + if (dnsserv_launch_request(arg, is_reverse)<0) + smartlist_add(failed, (char*)arg); + } }); send_control_done(conn);