diff options
| -rw-r--r-- | changes/bug24119 | 4 | ||||
| -rw-r--r-- | src/or/channel.c | 70 | ||||
| -rw-r--r-- | src/or/connection_or.c | 43 | ||||
| -rw-r--r-- | src/or/connection_or.h | 3 |
4 files changed, 82 insertions, 38 deletions
diff --git a/changes/bug24119 b/changes/bug24119 new file mode 100644 index 0000000000..5014257602 --- /dev/null +++ b/changes/bug24119 @@ -0,0 +1,4 @@ + o Code simplification and refactoring: + - Rewrite channel_rsa_id_group_set_badness to reduce temporary memory + allocations with large numbers of OR connections (e.g. relays). Closes + ticket 24119. diff --git a/src/or/channel.c b/src/or/channel.c index 0b5a7fde90..7590ba8a5b 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -4726,6 +4726,16 @@ channel_set_circid_type,(channel_t *chan, } } +static int +channel_sort_by_ed25519_identity(const void **a_, const void **b_) +{ + const channel_t *a = *a_, + *b = *b_; + return fast_memcmp(&a->ed25519_identity.pubkey, + &b->ed25519_identity.pubkey, + sizeof(a->ed25519_identity.pubkey)); +} + /** 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 @@ -4734,44 +4744,52 @@ static void channel_rsa_id_group_set_badness(struct channel_list_s *lst, int force) { /*XXXX This function should really be about channels. 15056 */ - channel_t *chan; + channel_t *chan = TOR_LIST_FIRST(lst); + + if (!chan) + return; + + /* if there is only one channel, don't bother looping */ + if (PREDICT_LIKELY(!TOR_LIST_NEXT(chan, next_with_same_id))) { + connection_or_single_set_badness_( + time(NULL), BASE_CHAN_TO_TLS(chan)->conn, force); + return; + } + + smartlist_t *channels = smartlist_new(); - /* First, get a minimal list of the ed25519 identites */ - smartlist_t *ed_identities = smartlist_new(); TOR_LIST_FOREACH(chan, lst, next_with_same_id) { - uint8_t *id_copy = - tor_memdup(&chan->ed25519_identity.pubkey, DIGEST256_LEN); - smartlist_add(ed_identities, id_copy); + if (BASE_CHAN_TO_TLS(chan)->conn) { + smartlist_add(channels, chan); + } } - smartlist_sort_digests256(ed_identities); - smartlist_uniq_digests256(ed_identities); - /* Now, for each Ed identity, build a smartlist and find the best entry on - * it. */ + smartlist_sort(channels, channel_sort_by_ed25519_identity); + + const ed25519_public_key_t *common_ed25519_identity = NULL; + /* it would be more efficient to do a slice, but this case is rare */ smartlist_t *or_conns = smartlist_new(); - SMARTLIST_FOREACH_BEGIN(ed_identities, const uint8_t *, ed_id) { - TOR_LIST_FOREACH(chan, lst, next_with_same_id) { - channel_tls_t *chantls = BASE_CHAN_TO_TLS(chan); - if (tor_memneq(ed_id, &chan->ed25519_identity.pubkey, DIGEST256_LEN)) - continue; - or_connection_t *orconn = chantls->conn; - if (orconn) { - tor_assert(orconn->chan == chantls); - smartlist_add(or_conns, orconn); - } + SMARTLIST_FOREACH_BEGIN(channels, channel_t *, channel) { + if (!common_ed25519_identity) + common_ed25519_identity = &channel->ed25519_identity; + + if (! ed25519_pubkey_eq(&channel->ed25519_identity, + common_ed25519_identity)) { + connection_or_group_set_badness_(or_conns, force); + smartlist_clear(or_conns); + common_ed25519_identity = &channel->ed25519_identity; } - connection_or_group_set_badness_(or_conns, force); - smartlist_clear(or_conns); - } SMARTLIST_FOREACH_END(ed_id); + smartlist_add(or_conns, BASE_CHAN_TO_TLS(channel)->conn); + } SMARTLIST_FOREACH_END(channel); + + connection_or_group_set_badness_(or_conns, force); /* XXXX 15056 we may want to do something special with connections that have * no set Ed25519 identity! */ smartlist_free(or_conns); - - SMARTLIST_FOREACH(ed_identities, uint8_t *, ed_id, tor_free(ed_id)); - smartlist_free(ed_identities); + smartlist_free(channels); } /** Go through all the channels (or if <b>digest</b> is non-NULL, just diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 7af1f2b645..fdf1b2ebb1 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -965,6 +965,36 @@ 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) +/** Expire an or_connection if it is too old. Helper for + * connection_or_group_set_badness_ and fast path for + * channel_rsa_id_group_set_badness. + * + * Returns 1 if the connection was already expired, else 0. + */ +int +connection_or_single_set_badness_(time_t now, + or_connection_t *or_conn, + int force) +{ + /* XXXX this function should also be about channels? */ + if (or_conn->base_.marked_for_close || + connection_or_is_bad_for_new_circs(or_conn)) + return 1; + + if (force || + or_conn->base_.timestamp_created + TIME_BEFORE_OR_CONN_IS_TOO_OLD + < now) { + log_info(LD_OR, + "Marking OR conn to %s:%d as too old for new circuits " + "(fd "TOR_SOCKET_T_FORMAT", %d secs old).", + or_conn->base_.address, or_conn->base_.port, or_conn->base_.s, + (int)(now - or_conn->base_.timestamp_created)); + connection_or_mark_bad_for_new_circs(or_conn); + } + + return 0; +} + /** 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(). @@ -995,19 +1025,8 @@ connection_or_group_set_badness_(smartlist_t *group, int force) /* Pass 1: expire everything that's old, and see what the status of * everything else is. */ 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)) + if (connection_or_single_set_badness_(now, or_conn, force)) continue; - if (force || - or_conn->base_.timestamp_created + TIME_BEFORE_OR_CONN_IS_TOO_OLD - < now) { - log_info(LD_OR, - "Marking OR conn to %s:%d as too old for new circuits " - "(fd "TOR_SOCKET_T_FORMAT", %d secs old).", - or_conn->base_.address, or_conn->base_.port, or_conn->base_.s, - (int)(now - or_conn->base_.timestamp_created)); - connection_or_mark_bad_for_new_circs(or_conn); - } if (connection_or_is_bad_for_new_circs(or_conn)) { ++n_old; diff --git a/src/or/connection_or.h b/src/or/connection_or.h index ee66b7c528..644df5c2c9 100644 --- a/src/or/connection_or.h +++ b/src/or/connection_or.h @@ -112,6 +112,9 @@ void var_cell_free(var_cell_t *cell); #define MIN_LINK_PROTO_FOR_CHANNEL_PADDING 5 #define MAX_LINK_PROTO MIN_LINK_PROTO_FOR_CHANNEL_PADDING +int connection_or_single_set_badness_(time_t now, + or_connection_t *or_conn, + int force); void connection_or_group_set_badness_(smartlist_t *group, int force); #ifdef TOR_UNIT_TESTS |