commit 4834641dce809fbb6fd4a586a823446970d19c1d Author: vagrant <vagrant@precise32.(none)> Date: Wed Aug 21 11:44:28 2013 -0400

Make circ_times static and add accessor functions.

Change the global circ_times to a static variable and use accessor functions throughout the code, instead of accessing it directly. --- src/or/channel.c | 2 +- src/or/circuitbuild.c | 27 +++++++++++++++------------ src/or/circuitlist.c | 2 +- src/or/circuitstats.c | 21 +++++++++++++++++++-- src/or/circuitstats.h | 4 +++- src/or/circuituse.c | 37 ++++++++++++++++++++----------------- src/or/connection_or.c | 8 ++++---- src/or/entrynodes.c | 2 +- src/or/networkstatus.c | 3 ++- src/or/statefile.c | 5 +++-- 10 files changed, 69 insertions(+), 42 deletions(-)

diff --git a/src/or/channel.c b/src/or/channel.c index 48bbf79..8743437 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -2361,7 +2361,7 @@ channel_do_open_actions(channel_t *chan) started_here = channel_is_outgoing(chan);

if (started_here) { - circuit_build_times_network_is_live(&circ_times); + circuit_build_times_network_is_live(get_circuit_build_times()); rep_hist_note_connect_succeeded(chan->identity_digest, now); if (entry_guard_register_connect_status( chan->identity_digest, 1, 0, now) < 0) { diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 21fc2ae..c144f27 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -772,20 +772,23 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) * it off at, we probably had a suspend event along this codepath, * and we should discard the value. */ - if (timediff < 0 || timediff > 2*circ_times.close_ms+1000) { + if (timediff < 0 || timediff > 2*get_circuit_build_close_time()+1000) { log_notice(LD_CIRC, "Strange value for circuit build time: %ldmsec. " "Assuming clock jump. Purpose %d (%s)", timediff, circ->base_.purpose, circuit_purpose_to_string(circ->base_.purpose)); } else if (!circuit_build_times_disabled()) { /* Only count circuit times if the network is live */ - if (circuit_build_times_network_check_live(&circ_times)) { - circuit_build_times_add_time(&circ_times, (build_time_t)timediff); - circuit_build_times_set_timeout(&circ_times); + if (circuit_build_times_network_check_live( + get_circuit_build_times())) { + circuit_build_times_add_time(get_circuit_build_times(), + (build_time_t)timediff); + circuit_build_times_set_timeout(get_circuit_build_times()); }

if (circ->base_.purpose != CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) { - circuit_build_times_network_circ_success(&circ_times); + circuit_build_times_network_circ_success( + get_circuit_build_times()); } } } @@ -2273,7 +2276,7 @@ pathbias_measure_use_rate(entry_guard_t *guard) tor_lround(guard->unusable_circuits), tor_lround(guard->collapsed_circuits), tor_lround(guard->timeouts), - tor_lround(circ_times.close_ms/1000)); + tor_lround(get_circuit_build_close_time()/1000)); guard->path_bias_disabled = 1; guard->bad_since = approx_time(); entry_guards_changed(); @@ -2299,7 +2302,7 @@ pathbias_measure_use_rate(entry_guard_t *guard) tor_lround(guard->unusable_circuits), tor_lround(guard->collapsed_circuits), tor_lround(guard->timeouts), - tor_lround(circ_times.close_ms/1000)); + tor_lround(get_circuit_build_close_time()/1000)); } } else if (pathbias_get_use_success_count(guard)/guard->use_attempts < pathbias_get_notice_use_rate(options)) { @@ -2323,7 +2326,7 @@ pathbias_measure_use_rate(entry_guard_t *guard) tor_lround(guard->unusable_circuits), tor_lround(guard->collapsed_circuits), tor_lround(guard->timeouts), - tor_lround(circ_times.close_ms/1000)); + tor_lround(get_circuit_build_close_time()/1000)); } } } @@ -2379,7 +2382,7 @@ pathbias_measure_close_rate(entry_guard_t *guard) tor_lround(guard->unusable_circuits), tor_lround(guard->collapsed_circuits), tor_lround(guard->timeouts), - tor_lround(circ_times.close_ms/1000)); + tor_lround(get_circuit_build_close_time()/1000)); guard->path_bias_disabled = 1; guard->bad_since = approx_time(); entry_guards_changed(); @@ -2405,7 +2408,7 @@ pathbias_measure_close_rate(entry_guard_t *guard) tor_lround(guard->unusable_circuits), tor_lround(guard->collapsed_circuits), tor_lround(guard->timeouts), - tor_lround(circ_times.close_ms/1000)); + tor_lround(get_circuit_build_close_time()/1000)); } } else if (pathbias_get_close_success_count(guard)/guard->circ_attempts < pathbias_get_warn_rate(options)) { @@ -2430,7 +2433,7 @@ pathbias_measure_close_rate(entry_guard_t *guard) tor_lround(guard->unusable_circuits), tor_lround(guard->collapsed_circuits), tor_lround(guard->timeouts), - tor_lround(circ_times.close_ms/1000)); + tor_lround(get_circuit_build_close_time()/1000)); } } else if (pathbias_get_close_success_count(guard)/guard->circ_attempts < pathbias_get_notice_rate(options)) { @@ -2453,7 +2456,7 @@ pathbias_measure_close_rate(entry_guard_t *guard) tor_lround(guard->unusable_circuits), tor_lround(guard->collapsed_circuits), tor_lround(guard->timeouts), - tor_lround(circ_times.close_ms/1000)); + tor_lround(get_circuit_build_close_time()/1000)); } } } diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 69c4c4b..1edea85 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -678,7 +678,7 @@ origin_circuit_new(void)

init_circuit_base(TO_CIRCUIT(circ));

- circ_times.last_circ_at = approx_time(); + get_circuit_build_times()->last_circ_at = approx_time();

return circ; } diff --git a/src/or/circuitstats.c b/src/or/circuitstats.c index 95129f9..88ed350 100644 --- a/src/or/circuitstats.c +++ b/src/or/circuitstats.c @@ -26,8 +26,7 @@ // vary in their own latency. The downside of this is that guards // can change frequently, so we'd be building a lot more circuits // most likely. -/* XXXX024 Make this static; add accessor functions. */ -circuit_build_times_t circ_times; +static circuit_build_times_t circ_times;

#ifdef TOR_UNIT_TESTS /** If set, we're running the unit tests: we should avoid clobbering @@ -37,6 +36,24 @@ static int unit_tests = 0; #define unit_tests 0 #endif

+circuit_build_times_t * +get_circuit_build_times(void) +{ + return &circ_times; +} + +double +get_circuit_build_close_time(void) +{ + return circ_times.close_ms; +} + +double +get_circuit_build_timeout(void) +{ + return circ_times.timeout_ms; +} + /** * This function decides if CBT learning should be disabled. It returns * true if one or more of the following four conditions are met: diff --git a/src/or/circuitstats.h b/src/or/circuitstats.h index 53ba2b2..2381bf9 100644 --- a/src/or/circuitstats.h +++ b/src/or/circuitstats.h @@ -12,7 +12,9 @@ #ifndef TOR_CIRCUITSTATS_H #define TOR_CIRCUITSTATS_H

-extern circuit_build_times_t circ_times; +circuit_build_times_t *get_circuit_build_times(void); +double get_circuit_build_close_time(void); +double get_circuit_build_timeout(void);

int circuit_build_times_disabled(void); int circuit_build_times_enough_to_compute(circuit_build_times_t *cbt); diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 7beaa4e..57079f0 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -442,12 +442,12 @@ circuit_expire_building(void) * RTTs = 4a + 3b + 2c * RTTs = 9h */ - SET_CUTOFF(general_cutoff, circ_times.timeout_ms); - SET_CUTOFF(begindir_cutoff, circ_times.timeout_ms); + SET_CUTOFF(general_cutoff, get_circuit_build_timeout()); + SET_CUTOFF(begindir_cutoff, get_circuit_build_timeout());

/* > 3hop circs seem to have a 1.0 second delay on their cannibalized * 4th hop. */ - SET_CUTOFF(fourhop_cutoff, circ_times.timeout_ms * (10/6.0) + 1000); + SET_CUTOFF(fourhop_cutoff, get_circuit_build_timeout() * (10/6.0) + 1000);

/* CIRCUIT_PURPOSE_C_ESTABLISH_REND behaves more like a RELAY cell. * Use the stream cutoff (more or less). */ @@ -456,20 +456,20 @@ circuit_expire_building(void) /* Be lenient with cannibalized circs. They already survived the official * CBT, and they're usually not performance-critical. */ SET_CUTOFF(cannibalized_cutoff, - MAX(circ_times.close_ms*(4/6.0), + MAX(get_circuit_build_close_time()*(4/6.0), options->CircuitStreamTimeout * 1000) + 1000);

/* Intro circs have an extra round trip (and are also 4 hops long) */ - SET_CUTOFF(c_intro_cutoff, circ_times.timeout_ms * (14/6.0) + 1000); + SET_CUTOFF(c_intro_cutoff, get_circuit_build_timeout() * (14/6.0) + 1000);

/* Server intro circs have an extra round trip */ - SET_CUTOFF(s_intro_cutoff, circ_times.timeout_ms * (9/6.0) + 1000); + SET_CUTOFF(s_intro_cutoff, get_circuit_build_timeout() * (9/6.0) + 1000);

- SET_CUTOFF(close_cutoff, circ_times.close_ms); - SET_CUTOFF(extremely_old_cutoff, circ_times.close_ms*2 + 1000); + SET_CUTOFF(close_cutoff, get_circuit_build_close_time()); + SET_CUTOFF(extremely_old_cutoff, get_circuit_build_close_time()*2 + 1000);

SET_CUTOFF(hs_extremely_old_cutoff, - MAX(circ_times.close_ms*2 + 1000, + MAX(get_circuit_build_close_time()*2 + 1000, options->SocksTimeout * 1000));

TOR_LIST_FOREACH(next_circ, circuit_get_global_list(), head) { @@ -545,12 +545,14 @@ circuit_expire_building(void) * was a timeout, and the timeout value needs to reset if we * see enough of them. Note this means we also need to avoid * double-counting below, too. */ - circuit_build_times_count_timeout(&circ_times, first_hop_succeeded); + circuit_build_times_count_timeout(get_circuit_build_times(), + first_hop_succeeded); TO_ORIGIN_CIRCUIT(victim)->relaxed_timeout = 1; } continue; } else { static ratelim_t relax_timeout_limit = RATELIM_INIT(3600); + const double build_close_ms = get_circuit_build_close_time(); log_fn_ratelim(&relax_timeout_limit, LOG_NOTICE, LD_CIRC, "No circuits are opened. Relaxed timeout for circuit %d " "(a %s %d-hop circuit in state %s with channel state %s) to " @@ -561,7 +563,8 @@ circuit_expire_building(void) TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len, circuit_state_to_string(victim->state), channel_state_to_string(victim->n_chan->state), - (long)circ_times.close_ms, num_live_entry_guards(0)); + (long)build_close_ms, + num_live_entry_guards(0)); } }

@@ -641,7 +644,7 @@ circuit_expire_building(void) }

if (circuit_timeout_want_to_count_circ(TO_ORIGIN_CIRCUIT(victim)) && - circuit_build_times_enough_to_compute(&circ_times)) { + circuit_build_times_enough_to_compute(get_circuit_build_times())) { /* Circuits are allowed to last longer for measurement. * Switch their purpose and wait. */ if (victim->purpose != CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) { @@ -655,7 +658,7 @@ circuit_expire_building(void) * have a timeout. We also want to avoid double-counting * already "relaxed" circuits, which are counted above. */ if (!TO_ORIGIN_CIRCUIT(victim)->relaxed_timeout) { - circuit_build_times_count_timeout(&circ_times, + circuit_build_times_count_timeout(get_circuit_build_times(), first_hop_succeeded); } continue; @@ -673,10 +676,10 @@ circuit_expire_building(void) (long)(now.tv_sec - victim->timestamp_began.tv_sec), victim->purpose, circuit_purpose_to_string(victim->purpose)); - } else if (circuit_build_times_count_close(&circ_times, + } else if (circuit_build_times_count_close(get_circuit_build_times(), first_hop_succeeded, victim->timestamp_created.tv_sec)) { - circuit_build_times_set_timeout(&circ_times); + circuit_build_times_set_timeout(get_circuit_build_times()); } } } @@ -939,7 +942,7 @@ circuit_predict_and_launch_new(void) * we can still build circuits preemptively as needed. */ if (num < MAX_UNUSED_OPEN_CIRCUITS-2 && get_options()->LearnCircuitBuildTimeout && - circuit_build_times_needs_circuits_now(&circ_times)) { + circuit_build_times_needs_circuits_now(get_circuit_build_times())) { flags = CIRCLAUNCH_NEED_CAPACITY; log_info(LD_CIRC, "Have %d clean circs need another buildtime test circ.", num); @@ -1075,7 +1078,7 @@ circuit_expire_old_circuits_clientside(void) cutoff = now;

if (get_options()->LearnCircuitBuildTimeout && - circuit_build_times_needs_circuits(&circ_times)) { + circuit_build_times_needs_circuits(get_circuit_build_times())) { /* Circuits should be shorter lived if we need more of them * for learning a good build timeout */ cutoff.tv_sec -= IDLE_TIMEOUT_WHILE_LEARNING; diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 31fd6d6..b7e72fd 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1749,7 +1749,7 @@ connection_tls_finish_handshake(or_connection_t *conn) digest_rcvd) < 0) return -1;

- circuit_build_times_network_is_live(&circ_times); + circuit_build_times_network_is_live(get_circuit_build_times());

if (tor_tls_used_v1_handshake(conn->tls)) { conn->link_proto = 1; @@ -1783,7 +1783,7 @@ connection_or_launch_v3_or_handshake(or_connection_t *conn) tor_assert(connection_or_nonopen_was_started_here(conn)); tor_assert(tor_tls_received_v3_certificate(conn->tls));

- circuit_build_times_network_is_live(&circ_times); + circuit_build_times_network_is_live(get_circuit_build_times());

connection_or_change_state(conn, OR_CONN_STATE_OR_HANDSHAKING_V3); if (connection_init_or_handshake_state(conn, 1) < 0) @@ -2016,7 +2016,7 @@ connection_or_process_cells_from_inbuf(or_connection_t *conn) if (conn->chan) channel_timestamp_active(TLS_CHAN_TO_BASE(conn->chan));

- circuit_build_times_network_is_live(&circ_times); + circuit_build_times_network_is_live(get_circuit_build_times()); channel_tls_handle_var_cell(var_cell, conn); var_cell_free(var_cell); } else { @@ -2032,7 +2032,7 @@ connection_or_process_cells_from_inbuf(or_connection_t *conn) if (conn->chan) channel_timestamp_active(TLS_CHAN_TO_BASE(conn->chan));

- circuit_build_times_network_is_live(&circ_times); + circuit_build_times_network_is_live(get_circuit_build_times()); connection_fetch_from_buf(buf, cell_network_size, TO_CONN(conn));

/* retrieve cell info from buf (create the host-order struct from the diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index cadc70e..b3764a9 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -2279,6 +2279,6 @@ entry_guards_free_all(void) clear_bridge_list(); smartlist_free(bridge_list); bridge_list = NULL; - circuit_build_times_free_timeouts(&circ_times); + circuit_build_times_free_timeouts(get_circuit_build_times()); }

diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 3f995a9..8b4a791 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1822,7 +1822,8 @@ networkstatus_set_current_consensus(const char *consensus, * current consensus really alter our view of any OR's rate limits? */ connection_or_update_token_buckets(get_connection_array(), options);

- circuit_build_times_new_consensus_params(&circ_times, current_consensus); + circuit_build_times_new_consensus_params(get_circuit_build_times(), + current_consensus); }

if (directory_caches_dir_info(options)) { diff --git a/src/or/statefile.c b/src/or/statefile.c index 8736c35..4b61fde 100644 --- a/src/or/statefile.c +++ b/src/or/statefile.c @@ -238,7 +238,8 @@ or_state_set(or_state_t *new_state) tor_free(err); ret = -1; } - if (circuit_build_times_parse_state(&circ_times, global_state) < 0) { + if (circuit_build_times_parse_state( + get_circuit_build_times(),global_state) < 0) { ret = -1; } return ret; @@ -405,7 +406,7 @@ or_state_save(time_t now) * to avoid redundant writes. */ entry_guards_update_state(global_state); rep_hist_update_state(global_state); - circuit_build_times_update_state(&circ_times, global_state); + circuit_build_times_update_state(get_circuit_build_times(), global_state); if (accounting_is_enabled(get_options())) accounting_run_housekeeping(now);