diff options
| -rw-r--r-- | changes/decouple_conn_attach | 6 | ||||
| -rw-r--r-- | src/or/circuituse.c | 8 | ||||
| -rw-r--r-- | src/or/connection_edge.c | 162 | ||||
| -rw-r--r-- | src/or/connection_edge.h | 6 | ||||
| -rw-r--r-- | src/or/main.c | 6 | ||||
| -rw-r--r-- | src/or/rendclient.c | 11 |
6 files changed, 165 insertions, 34 deletions
diff --git a/changes/decouple_conn_attach b/changes/decouple_conn_attach new file mode 100644 index 0000000000..6167b4e932 --- /dev/null +++ b/changes/decouple_conn_attach @@ -0,0 +1,6 @@ + o Code simplification and refactorings: + - Decouple the list of streams needing to be attached to circuits + from the overall connection list. This change makes it possible to + attach streams quickly while both simplifying Tor's callgraph and + avoiding O(N) scans of the entire connection list. Closes ticket + 17590. diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 00340fd689..5b24425877 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1123,7 +1123,7 @@ circuit_build_needed_circs(time_t now) * don't require an exit circuit, review in #13814. * This allows HSs to function in a consensus without exits. */ if (router_have_consensus_path() != CONSENSUS_PATH_UNKNOWN) - connection_ap_attach_pending(); + connection_ap_rescan_and_attach_pending(); /* make sure any hidden services have enough intro points * HS intro point streams only require an internal circuit */ @@ -1475,7 +1475,7 @@ circuit_has_opened(origin_circuit_t *circ) case CIRCUIT_PURPOSE_C_ESTABLISH_REND: rend_client_rendcirc_has_opened(circ); /* Start building an intro circ if we don't have one yet. */ - connection_ap_attach_pending(); + connection_ap_attach_pending(1); /* This isn't a call to circuit_try_attaching_streams because a * circuit in _C_ESTABLISH_REND state isn't connected to its * hidden service yet, thus we can't attach streams to it yet, @@ -1537,14 +1537,14 @@ void circuit_try_attaching_streams(origin_circuit_t *circ) { /* Attach streams to this circuit if we can. */ - connection_ap_attach_pending(); + connection_ap_attach_pending(1); /* The call to circuit_try_clearing_isolation_state here will do * nothing and return 0 if we didn't attach any streams to circ * above. */ if (circuit_try_clearing_isolation_state(circ)) { /* Maybe *now* we can attach some streams to this circuit. */ - connection_ap_attach_pending(); + connection_ap_attach_pending(1); } } diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 729ef8a4c7..67e594a6a9 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -503,6 +503,16 @@ connection_edge_finished_connecting(edge_connection_t *edge_conn) return connection_edge_process_inbuf(edge_conn, 1); } +/** A list of all the entry_connection_t * objects that are not marked + * for close, and are in AP_CONN_STATE_CIRCUIT_WAIT. + * + * (Right now, we check in several places to make sure that this list is + * correct. When it's incorrect, we'll fix it, and log a BUG message.) + */ +static smartlist_t *pending_entry_connections = NULL; + +static int untried_pending_connections = 0; + /** Common code to connection_(ap|exit)_about_to_close. */ static void connection_edge_about_to_close(edge_connection_t *edge_conn) @@ -514,6 +524,27 @@ connection_edge_about_to_close(edge_connection_t *edge_conn) conn->marked_for_close_file, conn->marked_for_close); tor_fragile_assert(); } + + if (TO_CONN(edge_conn)->type != CONN_TYPE_AP || + PREDICT_UNLIKELY(NULL == pending_entry_connections)) + return; + + entry_connection_t *entry_conn = EDGE_TO_ENTRY_CONN(edge_conn); + + if (TO_CONN(edge_conn)->state == AP_CONN_STATE_CIRCUIT_WAIT) { + smartlist_remove(pending_entry_connections, entry_conn); + } + +#if 1 + /* Check to make sure that this isn't in pending_entry_connections if it + * didn't actually belong there. */ + if (TO_CONN(edge_conn)->type == CONN_TYPE_AP && + smartlist_contains(pending_entry_connections, entry_conn)) { + log_warn(LD_BUG, "What was %p doing in pending_entry_connections???", + entry_conn); + smartlist_remove(pending_entry_connections, entry_conn); + } +#endif } /** Called when we're about to finally unlink and free an AP (client) @@ -711,26 +742,114 @@ connection_ap_expire_beginning(void) } SMARTLIST_FOREACH_END(base_conn); } -/** Tell any AP streams that are waiting for a new circuit to try again, - * either attaching to an available circ or launching a new one. +/** + * As connection_ap_attach_pending, but first scans the entire connection + * array to see if any elements are missing. */ void -connection_ap_attach_pending(void) +connection_ap_rescan_and_attach_pending(void) { entry_connection_t *entry_conn; smartlist_t *conns = get_connection_array(); + + if (PREDICT_UNLIKELY(NULL == pending_entry_connections)) + pending_entry_connections = smartlist_new(); + SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn) { if (conn->marked_for_close || conn->type != CONN_TYPE_AP || conn->state != AP_CONN_STATE_CIRCUIT_WAIT) continue; + entry_conn = TO_ENTRY_CONN(conn); + if (! smartlist_contains(pending_entry_connections, entry_conn)) { + log_warn(LD_BUG, "Found a connection %p that was supposed to be " + "in pending_entry_connections, but wasn't. No worries; " + "adding it.", + pending_entry_connections); + untried_pending_connections = 1; + smartlist_add(pending_entry_connections, entry_conn); + } + + } SMARTLIST_FOREACH_END(conn); + + connection_ap_attach_pending(1); +} + +/** Tell any AP streams that are listed as waiting for a new circuit to try + * again, either attaching to an available circ or launching a new one. + * + * If <b>retry</b> is false, only check the list if it contains at least one + * streams that we have not yet tried to attach to a circuit. + */ +void +connection_ap_attach_pending(int retry) +{ + if (PREDICT_UNLIKELY(!pending_entry_connections)) { + return; + } + + if (untried_pending_connections == 0 && !retry) + return; + + SMARTLIST_FOREACH_BEGIN(pending_entry_connections, + entry_connection_t *, entry_conn) { + connection_t *conn = ENTRY_TO_CONN(entry_conn); + if (conn->marked_for_close) { + SMARTLIST_DEL_CURRENT(pending_entry_connections, entry_conn); + continue; + } + if (conn->state != AP_CONN_STATE_CIRCUIT_WAIT) { + log_warn(LD_BUG, "%p is no longer in circuit_wait. Why is it on " + "pending_entry_connections?", entry_conn); + SMARTLIST_DEL_CURRENT(pending_entry_connections, entry_conn); + continue; + } + if (connection_ap_handshake_attach_circuit(entry_conn) < 0) { if (!conn->marked_for_close) connection_mark_unattached_ap(entry_conn, END_STREAM_REASON_CANT_ATTACH); } - } SMARTLIST_FOREACH_END(conn); + + if (conn->marked_for_close || + conn->type != CONN_TYPE_AP || + conn->state != AP_CONN_STATE_CIRCUIT_WAIT) { + SMARTLIST_DEL_CURRENT(pending_entry_connections, entry_conn); + } + + } SMARTLIST_FOREACH_END(entry_conn); + + untried_pending_connections = 0; +} + +/** Mark <b>entry_conn</b> as needing to get attached to a circuit. + * + * And <b>entry_conn</b> must be in AP_CONN_STATE_CIRCUIT_WAIT, + * should not already be pending a circuit. The circuit will get + * launched or the connection will get attached the next time we + * call connection_ap_attach_pending(). + */ +void +connection_ap_mark_as_pending_circuit(entry_connection_t *entry_conn) +{ + connection_t *conn = ENTRY_TO_CONN(entry_conn); + tor_assert(conn->state == AP_CONN_STATE_CIRCUIT_WAIT); + if (conn->marked_for_close) + return; + + if (PREDICT_UNLIKELY(NULL == pending_entry_connections)) + pending_entry_connections = smartlist_new(); + + if (PREDICT_UNLIKELY(smartlist_contains(pending_entry_connections, + entry_conn))) { + log_warn(LD_BUG, "What?? pending_entry_connections already contains %p!", + entry_conn); + return; + } + + untried_pending_connections = 1; + smartlist_add(pending_entry_connections, entry_conn); } /** Tell any AP streams that are waiting for a one-hop tunnel to @@ -851,12 +970,12 @@ connection_ap_detach_retriable(entry_connection_t *conn, * a tunneled directory connection, then just attach it. */ ENTRY_TO_CONN(conn)->state = AP_CONN_STATE_CIRCUIT_WAIT; circuit_detach_stream(TO_CIRCUIT(circ),ENTRY_TO_EDGE_CONN(conn)); - return connection_ap_handshake_attach_circuit(conn); + connection_ap_mark_as_pending_circuit(conn); } else { ENTRY_TO_CONN(conn)->state = AP_CONN_STATE_CONTROLLER_WAIT; circuit_detach_stream(TO_CIRCUIT(circ),ENTRY_TO_EDGE_CONN(conn)); - return 0; } + return 0; } /** Check if <b>conn</b> is using a dangerous port. Then warn and/or @@ -1454,10 +1573,12 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, /* If we were given a circuit to attach to, try to attach. Otherwise, * try to find a good one and attach to that. */ int rv; - if (circ) - rv = connection_ap_handshake_attach_chosen_circuit(conn, circ, cpath); - else - rv = connection_ap_handshake_attach_circuit(conn); + if (circ) { + rv = connection_ap_handshake_attach_chosen_circuit(conn, circ, cpath); + } else { + connection_ap_mark_as_pending_circuit(conn); + rv = 0; + } /* If the above function returned 0 then we're waiting for a circuit. * if it returned 1, we're attached. Both are okay. But if it returned @@ -1564,11 +1685,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, /* We have the descriptor so launch a connection to the HS. */ base_conn->state = AP_CONN_STATE_CIRCUIT_WAIT; log_info(LD_REND, "Descriptor is here. Great."); - if (connection_ap_handshake_attach_circuit(conn) < 0) { - if (!base_conn->marked_for_close) - connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH); - return -1; - } + connection_ap_mark_as_pending_circuit(conn); return 0; } @@ -2324,12 +2441,7 @@ connection_ap_make_link(connection_t *partner, control_event_stream_status(conn, STREAM_EVENT_NEW, 0); /* attaching to a dirty circuit is fine */ - if (connection_ap_handshake_attach_circuit(conn) < 0) { - if (!base_conn->marked_for_close) - connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH); - return NULL; - } - + connection_ap_mark_as_pending_circuit(conn); log_info(LD_APP,"... application connection created and linked."); return conn; } @@ -3478,3 +3590,11 @@ circuit_clear_isolation(origin_circuit_t *circ) circ->socks_username_len = circ->socks_password_len = 0; } +/** Free all storage held in module-scoped variables for connection_edge.c */ +void +connection_edge_free_all(void) +{ + untried_pending_connections = 0; + smartlist_free(pending_entry_connections); + pending_entry_connections = NULL; +} diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h index 7c0b9c0767..521e759aed 100644 --- a/src/or/connection_edge.h +++ b/src/or/connection_edge.h @@ -64,7 +64,9 @@ int connection_edge_is_rendezvous_stream(edge_connection_t *conn); int connection_ap_can_use_exit(const entry_connection_t *conn, const node_t *exit); void connection_ap_expire_beginning(void); -void connection_ap_attach_pending(void); +void connection_ap_rescan_and_attach_pending(void); +void connection_ap_attach_pending(int retry); +void connection_ap_mark_as_pending_circuit(entry_connection_t *entry_conn); void connection_ap_fail_onehop(const char *failed_digest, cpath_build_state_t *build_state); void circuit_discard_optional_exit_enclaves(extend_info_t *info); @@ -100,6 +102,8 @@ int connection_edge_update_circuit_isolation(const entry_connection_t *conn, void circuit_clear_isolation(origin_circuit_t *circ); streamid_t get_unique_stream_id_by_circ(origin_circuit_t *circ); +void connection_edge_free_all(void); + /** @name Begin-cell flags * * These flags are used in RELAY_BEGIN cells to change the default behavior diff --git a/src/or/main.c b/src/or/main.c index 7aeacd138d..c9007b9798 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -2516,6 +2516,11 @@ run_main_loop_once(void) } } + /* This will be pretty fast if nothing new is pending. Note that this gets + * called once per libevent loop, which will make it happen once per group + * of events that fire, or once per second. */ + connection_ap_attach_pending(0); + return 1; } @@ -3090,6 +3095,7 @@ tor_free_all(int postfork) channel_tls_free_all(); channel_free_all(); connection_free_all(); + connection_edge_free_all(); scheduler_free_all(); memarea_clear_freelist(); nodelist_free_all(); diff --git a/src/or/rendclient.c b/src/or/rendclient.c index a39e518e99..b8a4b2ab9b 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -52,7 +52,7 @@ rend_client_introcirc_has_opened(origin_circuit_t *circ) tor_assert(circ->cpath); log_info(LD_REND,"introcirc is open"); - connection_ap_attach_pending(); + connection_ap_attach_pending(1); } /** Send the establish-rendezvous cell along a rendezvous circuit. if @@ -1107,7 +1107,7 @@ rend_client_rendezvous_acked(origin_circuit_t *circ, const uint8_t *request, * than trying to attach them all. See comments bug 743. */ /* If we already have the introduction circuit built, make sure we send * the INTRODUCE cell _now_ */ - connection_ap_attach_pending(); + connection_ap_attach_pending(1); return 0; } @@ -1226,12 +1226,7 @@ rend_client_desc_trynow(const char *query) base_conn->timestamp_lastread = now; base_conn->timestamp_lastwritten = now; - if (connection_ap_handshake_attach_circuit(conn) < 0) { - /* it will never work */ - log_warn(LD_REND,"Rendezvous attempt failed. Closing."); - if (!base_conn->marked_for_close) - connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH); - } + connection_ap_mark_as_pending_circuit(conn); } else { /* 404, or fetch didn't get that far */ log_notice(LD_REND,"Closing stream for '%s.onion': hidden service is " "unavailable (try again later).", |