diff options
| -rw-r--r-- | changes/bug6799 | 15 | ||||
| -rw-r--r-- | src/or/channel.c | 11 | ||||
| -rw-r--r-- | src/or/channel.h | 6 | ||||
| -rw-r--r-- | src/or/circuitlist.c | 10 | ||||
| -rw-r--r-- | src/or/connection.c | 4 | ||||
| -rw-r--r-- | src/or/connection_or.c | 5 | ||||
| -rw-r--r-- | src/or/main.c | 29 | ||||
| -rw-r--r-- | src/or/or.h | 2 |
8 files changed, 50 insertions, 32 deletions
diff --git a/changes/bug6799 b/changes/bug6799 index b50762bb0..14ba4ae0c 100644 --- a/changes/bug6799 +++ b/changes/bug6799 @@ -1,13 +1,20 @@ o Major features: - - Increate the base amount of time that a canonical connection + - Increase the base amount of time that a canonical connection (one that we have made to a known OR) is allowed to stay open from a 3 minutes to 15 minutes. This leaks less information about when circuits have closed, and avoids unnecessary overhead from renegotiating connections. Part of a fix for ticket 6799. - - Instead of closing connections at a fixed interval after their - last circuit closed, randomly add up to 50% to each connection's - maximum timout. This makes it harder to tell when the last + - Instead of closing connections after they have been idle for a + fixed interval, randomly add up to 50% to each connection's + maximum timeout. This makes it harder to tell when the last circuit closed by looking at when a connection closes. Part of a fix for ticket 6799. + + - Base connection idleness tests on the actual time elapsed since + the connection last had circuits, not on the time when we last + added non-padding. This also makes it harder to tell when the last + circuit closed by looking at when a connection closes. Part of a + fix for ticket 6799. + Incidentally fixes bug 12023; bugfix on 0.2.5.1-alpha. diff --git a/src/or/channel.c b/src/or/channel.c index 1270eace7..ce2d01201 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -117,7 +117,9 @@ HT_GENERATE(channel_idmap, channel_idmap_entry_s, node, channel_idmap_hash, static cell_queue_entry_t * cell_queue_entry_dup(cell_queue_entry_t *q); static void cell_queue_entry_free(cell_queue_entry_t *q, int handed_off); +#if 0 static int cell_queue_entry_is_padding(cell_queue_entry_t *q); +#endif static cell_queue_entry_t * cell_queue_entry_new_fixed(cell_t *cell); static cell_queue_entry_t * @@ -729,7 +731,7 @@ channel_init(channel_t *chan) chan->global_identifier = n_channels_allocated++; /* Init timestamp */ - chan->timestamp_last_added_nonpadding = time(NULL); + chan->timestamp_last_had_circuits = time(NULL); /* Init next_circ_id */ chan->next_circ_id = crypto_rand_int(1 << 15); @@ -1597,6 +1599,7 @@ cell_queue_entry_free(cell_queue_entry_t *q, int handed_off) tor_free(q); } +#if 0 /** * Check whether a cell queue entry is padding; this is a helper function * for channel_write_cell_queue_entry() @@ -1625,6 +1628,7 @@ cell_queue_entry_is_padding(cell_queue_entry_t *q) return 0; } +#endif /** * Allocate a new cell queue entry for a fixed-size cell @@ -1683,11 +1687,6 @@ channel_write_cell_queue_entry(channel_t *chan, cell_queue_entry_t *q) chan->state == CHANNEL_STATE_OPEN || chan->state == CHANNEL_STATE_MAINT); - /* Increment the timestamp unless it's padding */ - if (!cell_queue_entry_is_padding(q)) { - chan->timestamp_last_added_nonpadding = approx_time(); - } - /* Can we send it right out? If so, try */ if (TOR_SIMPLEQ_EMPTY(&chan->outgoing_queue) && chan->state == CHANNEL_STATE_OPEN) { diff --git a/src/or/channel.h b/src/or/channel.h index 2dca81705..be40a30cc 100644 --- a/src/or/channel.h +++ b/src/or/channel.h @@ -187,8 +187,10 @@ struct channel_s { time_t timestamp_recv; /* Cell received from lower layer */ time_t timestamp_xmit; /* Cell sent to lower layer */ - /* Timestamp for relay.c */ - time_t timestamp_last_added_nonpadding; + /** Timestamp for run_connection_housekeeping(). We update this once a + * second when we run housekeeping and find a circuit on this channel, and + * whenever we add a circuit to the channel. */ + time_t timestamp_last_had_circuits; /** Unique ID for measuring direct network status requests;vtunneled ones * come over a circuit_t, which has a dirreq_id field as well, but is a diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index c7b15e40b..3cb429be1 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -217,8 +217,11 @@ circuit_set_p_circid_chan(or_circuit_t *circ, circid_t id, circuit_set_circid_chan_helper(TO_CIRCUIT(circ), CELL_DIRECTION_IN, id, chan); - if (chan) + if (chan) { tor_assert(bool_eq(circ->p_chan_cells.n, circ->next_active_on_p_chan)); + + chan->timestamp_last_had_circuits = approx_time(); + } } /** Set the n_conn field of a circuit <b>circ</b>, along @@ -230,8 +233,11 @@ circuit_set_n_circid_chan(circuit_t *circ, circid_t id, { circuit_set_circid_chan_helper(circ, CELL_DIRECTION_OUT, id, chan); - if (chan) + if (chan) { tor_assert(bool_eq(circ->n_chan_cells.n, circ->next_active_on_n_chan)); + + chan->timestamp_last_had_circuits = approx_time(); + } } /** Change the state of <b>circ</b> to <b>state</b>, adding it to or removing diff --git a/src/or/connection.c b/src/or/connection.c index b967eacf2..d99a54c15 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -249,8 +249,6 @@ dir_connection_new(int socket_family) /** Allocate and return a new or_connection_t, initialized as by * connection_init(). * - * Set timestamp_last_added_nonpadding to now. - * * Assign a pseudorandom next_circ_id between 0 and 2**15. * * Initialize active_circuit_pqueue. @@ -264,8 +262,6 @@ or_connection_new(int socket_family) time_t now = time(NULL); connection_init(now, TO_CONN(or_conn), CONN_TYPE_OR, socket_family); - or_conn->timestamp_last_added_nonpadding = time(NULL); - connection_or_set_canonical(or_conn, 0); return or_conn; diff --git a/src/or/connection_or.c b/src/or/connection_or.c index f03b18ddf..3d239d480 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1929,9 +1929,6 @@ connection_or_write_cell_to_buf(const cell_t *cell, or_connection_t *conn) if (conn->base_.state == OR_CONN_STATE_OR_HANDSHAKING_V3) or_handshake_state_record_cell(conn, conn->handshake_state, cell, 0); - - if (cell->command != CELL_PADDING) - conn->timestamp_last_added_nonpadding = approx_time(); } /** Pack a variable-length <b>cell</b> into wire-format, and write it onto @@ -1952,8 +1949,6 @@ connection_or_write_var_cell_to_buf(const var_cell_t *cell, cell->payload_len, TO_CONN(conn)); if (conn->base_.state == OR_CONN_STATE_OR_HANDSHAKING_V3) or_handshake_state_record_var_cell(conn, conn->handshake_state, cell, 0); - if (cell->command != CELL_PADDING) - conn->timestamp_last_added_nonpadding = approx_time(); /* Touch the channel's active timestamp if there is one */ if (conn->chan) diff --git a/src/or/main.c b/src/or/main.c index 8a653ca40..70b1340cb 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1003,6 +1003,8 @@ run_connection_housekeeping(int i, time_t now) connection_t *conn = smartlist_get(connection_array, i); const or_options_t *options = get_options(); or_connection_t *or_conn; + channel_t *chan = NULL; + int have_any_circuits; int past_keepalive = now >= conn->timestamp_lastwritten + options->KeepalivePeriod; @@ -1050,8 +1052,19 @@ run_connection_housekeeping(int i, time_t now) tor_assert(conn->outbuf); #endif + chan = TLS_CHAN_TO_BASE(or_conn->chan); + tor_assert(chan); + + + if (channel_num_circuits(chan) != 0) { + have_any_circuits = 1; + chan->timestamp_last_had_circuits = now; + } else { + have_any_circuits = 0; + } + if (channel_is_bad_for_new_circs(TLS_CHAN_TO_BASE(or_conn->chan)) && - !connection_or_get_num_circuits(or_conn)) { + ! have_any_circuits) { /* It's bad for new circuits, and has no unmarked circuits on it: * mark it now. */ log_info(LD_OR, @@ -1070,19 +1083,21 @@ run_connection_housekeeping(int i, time_t now) connection_or_close_normally(TO_OR_CONN(conn), 0); } } else if (we_are_hibernating() && - !connection_or_get_num_circuits(or_conn) && + ! have_any_circuits && !connection_get_outbuf_len(conn)) { /* We're hibernating, there's no circuits, and nothing to flush.*/ log_info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) " "[Hibernating or exiting].", (int)conn->s,conn->address, conn->port); connection_or_close_normally(TO_OR_CONN(conn), 1); - } else if (!connection_or_get_num_circuits(or_conn) && - now >= or_conn->timestamp_last_added_nonpadding + - or_conn->idle_timeout) { + } else if (!have_any_circuits && + now - or_conn->idle_timeout >= chan->timestamp_last_had_circuits) { log_info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) " - "[idle %d].", (int)conn->s,conn->address, conn->port, - (int)(now - or_conn->timestamp_last_added_nonpadding)); + "[no circuits for %d; timeout %d; %scanonical].", + (int)conn->s, conn->address, conn->port, + (int)(now - chan->timestamp_last_had_circuits), + or_conn->idle_timeout, + or_conn->is_canonical ? "" : "non"); connection_or_close_normally(TO_OR_CONN(conn), 0); } else if ( now >= or_conn->timestamp_lastempty + options->KeepalivePeriod*10 && diff --git a/src/or/or.h b/src/or/or.h index 21ee1855c..3ed9f9f58 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1432,8 +1432,6 @@ typedef struct or_connection_t { * up, state information to do so. */ time_t timestamp_lastempty; /**< When was the outbuf last completely empty?*/ - time_t timestamp_last_added_nonpadding; /** When did we last add a - * non-padding cell to the outbuf? */ /* bandwidth* and *_bucket only used by ORs in OPEN state: */ int bandwidthrate; /**< Bytes/s added to the bucket. (OPEN ORs only.) */ |