From a20c8a81d717852ad3a2bf261ec68efba692f0d7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 19 Sep 2016 16:14:28 -0400 Subject: [PATCH] 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 d484e71259..4712891857 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 lst, + * 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 digest 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 force 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 39a4d05fba..3f0bb37af5 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 d0cd9c00c7..ca5f300095 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 digest 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 force 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); } /** conn 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 da95718ac9..b35bcddca6 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 d95447770f..c8215d3910 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 c10f62724a..30adb16f4e 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