commit a20c8a81d717852ad3a2bf261ec68efba692f0d7 Author: Nick Mathewson nickm@torproject.org Date: Mon Sep 19 16:14:28 2016 -0400

Migrate main data loop for set_bad_connections to use channel structures

This was the last user of our or_connections-by-ID map. It also had a tendency to be O(N) in cases that only had to be O(1). --- src/or/channel.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ src/or/channel.h | 2 ++ src/or/connection_or.c | 46 +++++++++++++--------------------------------- src/or/connection_or.h | 3 ++- src/or/entrynodes.c | 4 ++-- src/or/main.c | 2 +- 6 files changed, 68 insertions(+), 37 deletions(-)

diff --git a/src/or/channel.c b/src/or/channel.c index d484e71..4712891 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -4539,6 +4539,54 @@ channel_set_circid_type,(channel_t *chan, } }

+/** Helper for channel_update_bad_for_new_circs(): Perform the + * channel_update_bad_for_new_circs operation on all channels in <b>lst</b>, + * all of which MUST have the same RSA ID. (They MAY have different + * Ed25519 IDs.) */ +static void +channel_rsa_id_group_set_badness(struct channel_list_s *lst, int force) +{ + channel_t *chan; + + smartlist_t *or_conns = smartlist_new(); + TOR_LIST_FOREACH(chan, lst, next_with_same_id) { + channel_tls_t *chantls = BASE_CHAN_TO_TLS(chan); + or_connection_t *orconn = chantls->conn; + if (orconn) + smartlist_add(or_conns, orconn); + } + /*XXXX This function should really be about channels. 15056 */ + connection_or_group_set_badness_(or_conns, force); + smartlist_free(or_conns); +} + +/** Go through all the channels (or if <b>digest</b> is non-NULL, just + * the OR connections with that digest), and set the is_bad_for_new_circs + * flag based on the rules in connection_or_group_set_badness() (or just + * always set it if <b>force</b> is true). + */ +void +channel_update_bad_for_new_circs(const char *digest, int force) +{ + if (digest) { + channel_idmap_entry_t *ent; + channel_idmap_entry_t search; + memset(&search, 0, sizeof(search)); + memcpy(search.digest, digest, DIGEST_LEN); + ent = HT_FIND(channel_idmap, &channel_identity_map, &search); + if (ent) { + channel_rsa_id_group_set_badness(&ent->channel_list, force); + } + return; + } + + /* no digest; just look at everything. */ + channel_idmap_entry_t **iter; + HT_FOREACH(iter, channel_idmap, &channel_identity_map) { + channel_rsa_id_group_set_badness(&(*iter)->channel_list, force); + } +} + /** * Update the estimated number of bytes queued to transmit for this channel, * and notify the scheduler. The estimate includes both the channel queue and diff --git a/src/or/channel.h b/src/or/channel.h index 39a4d05..3f0bb37 100644 --- a/src/or/channel.h +++ b/src/or/channel.h @@ -601,6 +601,8 @@ void channel_listener_dump_statistics(channel_listener_t *chan_l, void channel_listener_dump_transport_statistics(channel_listener_t *chan_l, int severity);

+void channel_update_bad_for_new_circs(const char *digest, int force); + /* Flow control queries */ uint64_t channel_get_global_queue_estimate(void); int channel_num_cells_writeable(channel_t *chan); diff --git a/src/or/connection_or.c b/src/or/connection_or.c index d0cd9c0..ca5f300 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -961,7 +961,7 @@ connection_or_mark_bad_for_new_circs(or_connection_t *or_conn) * too old for new circuits? */ #define TIME_BEFORE_OR_CONN_IS_TOO_OLD (60*60*24*7)

-/** Given the head of the linked list for all the or_connections with a given +/** Given a list of all the or_connections with a given * identity, set elements of that list as is_bad_for_new_circs as * appropriate. Helper for connection_or_set_bad_connections(). * @@ -978,19 +978,19 @@ connection_or_mark_bad_for_new_circs(or_connection_t *or_conn) * See channel_is_better() in channel.c for our idea of what makes one OR * connection better than another. */ -static void -connection_or_group_set_badness(or_connection_t *head, int force) +void +connection_or_group_set_badness_(smartlist_t *group, int force) { - // XXXX 15056 we should make this about channels instead, so we - // can finally remove orconn_identity_map. - - or_connection_t *or_conn = NULL, *best = NULL; + /* XXXX this should be entirely about channels, not OR connections. 15056*/ + /* XXXX Look at Ed25519 ids too! 15056 */ + + or_connection_t *best = NULL; int n_old = 0, n_inprogress = 0, n_canonical = 0, n_other = 0; time_t now = time(NULL);

/* Pass 1: expire everything that's old, and see what the status of * everything else is. */ - for (or_conn = head; or_conn; or_conn = or_conn->next_with_same_id) { + SMARTLIST_FOREACH_BEGIN(group, or_connection_t *, or_conn) { if (or_conn->base_.marked_for_close || connection_or_is_bad_for_new_circs(or_conn)) continue; @@ -1014,11 +1014,11 @@ connection_or_group_set_badness(or_connection_t *head, int force) } else { ++n_other; } - } + } SMARTLIST_FOREACH_END(or_conn);

/* Pass 2: We know how about how good the best connection is. * expire everything that's worse, and find the very best if we can. */ - for (or_conn = head; or_conn; or_conn = or_conn->next_with_same_id) { + SMARTLIST_FOREACH_BEGIN(group, or_connection_t *, or_conn) { if (or_conn->base_.marked_for_close || connection_or_is_bad_for_new_circs(or_conn)) continue; /* This one doesn't need to be marked bad. */ @@ -1045,7 +1045,7 @@ connection_or_group_set_badness(or_connection_t *head, int force) 0)) { best = or_conn; } - } + } SMARTLIST_FOREACH_END(or_conn);

if (!best) return; @@ -1064,7 +1064,7 @@ connection_or_group_set_badness(or_connection_t *head, int force) * 0.1.2.x dies out, the first case will go away, and the second one is * "mostly harmless", so a fix can wait until somebody is bored. */ - for (or_conn = head; or_conn; or_conn = or_conn->next_with_same_id) { + SMARTLIST_FOREACH_BEGIN(group, or_connection_t *, or_conn) { if (or_conn->base_.marked_for_close || connection_or_is_bad_for_new_circs(or_conn) || or_conn->base_.state != OR_CONN_STATE_OPEN) @@ -1098,27 +1098,7 @@ connection_or_group_set_badness(or_connection_t *head, int force) connection_or_mark_bad_for_new_circs(or_conn); } } - } -} - -/** Go through all the OR connections (or if <b>digest</b> is non-NULL, just - * the OR connections with that digest), and set the is_bad_for_new_circs - * flag based on the rules in connection_or_group_set_badness() (or just - * always set it if <b>force</b> is true). - */ -void -connection_or_set_bad_connections(const char *digest, int force) -{ - if (!orconn_identity_map) - return; - - // XXXX This is just about the only remaining user of orconn_identity_map! - // XXXX If we kill it, we can yoink out the map. 15056. - - DIGESTMAP_FOREACH(orconn_identity_map, identity, or_connection_t *, conn) { - if (!digest || tor_memeq(digest, conn->identity_digest, DIGEST_LEN)) - connection_or_group_set_badness(conn, force); - } DIGESTMAP_FOREACH_END; + } SMARTLIST_FOREACH_END(or_conn); }

/** <b>conn</b> is in the 'connecting' state, and it failed to complete diff --git a/src/or/connection_or.h b/src/or/connection_or.h index da95718..b35bcdd 100644 --- a/src/or/connection_or.h +++ b/src/or/connection_or.h @@ -19,7 +19,6 @@ or_connection_t *connection_or_get_for_extend(const char *digest, const tor_addr_t *target_addr, const char **msg_out, int *launch_out); -void connection_or_set_bad_connections(const char *digest, int force);

void connection_or_block_renegotiation(or_connection_t *conn); int connection_or_reached_eof(or_connection_t *conn); @@ -111,5 +110,7 @@ void var_cell_free(var_cell_t *cell); /* DOCDOC */ #define MIN_LINK_PROTO_FOR_WIDE_CIRC_IDS 4

+void connection_or_group_set_badness_(smartlist_t *group, int force); + #endif

diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index d954477..c8215d3 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -15,13 +15,13 @@ #define ENTRYNODES_PRIVATE

#include "or.h" +#include "channel.h" #include "circpathbias.h" #include "circuitbuild.h" #include "circuitstats.h" #include "config.h" #include "confparse.h" #include "connection.h" -#include "connection_or.h" #include "control.h" #include "directory.h" #include "entrynodes.h" @@ -2749,7 +2749,7 @@ entries_retry_helper(const or_options_t *options, int act) * the node down and undermine the retry attempt. We mark even * the established conns, since if the network just came back * we'll want to attach circuits to fresh conns. */ - connection_or_set_bad_connections(node->identity, 1); + channel_update_bad_for_new_circs(node->identity, 1);

/* mark this entry node for retry */ router_set_status(node->identity, 1); diff --git a/src/or/main.c b/src/or/main.c index c10f627..30adb16 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1426,7 +1426,7 @@ run_scheduled_events(time_t now) }

/* 5. We do housekeeping for each connection... */ - connection_or_set_bad_connections(NULL, 0); + channel_update_bad_for_new_circs(NULL, 0); int i; for (i=0;i<smartlist_len(connection_array);i++) { run_connection_housekeeping(i, now);