commit 488e2b00bf881b97bcc8e4bbe304845ff1d79a03 Author: Nick Mathewson nickm@torproject.org Date: Tue Apr 17 11:39:16 2018 -0400

Refactor the "block the connection on bandwidth" logic

Right now, this patch just introduces and exposes some new functions. Later, these functions will get a little more complexity. --- src/or/connection.c | 49 +++++++++++++++++++++++++++++++++++++++---------- src/or/connection.h | 4 ++++ src/or/main.c | 15 +++++++++------ src/or/or.h | 1 + 4 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/src/or/connection.c b/src/or/connection.c index 957398985..78befee0c 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -772,8 +772,8 @@ connection_close_immediate(connection_t *conn) connection_unregister_events(conn);

/* Prevent the event from getting unblocked. */ - conn->read_blocked_on_bw = - conn->write_blocked_on_bw = 0; + conn->read_blocked_on_bw = 0; + conn->write_blocked_on_bw = 0;

if (SOCKET_OK(conn->s)) tor_close_socket(conn->s); @@ -3052,9 +3052,37 @@ connection_buckets_decrement(connection_t *conn, time_t now, } }

+/** + * Mark <b>conn</b> as needing to stop reading because bandwidth has been + * exhausted. If <b>is_global_bw</b>, it is closing because global bandwidth + * limit has been exhausted. Otherwise, it is closing because its own + * bandwidth limit has been exhausted. + */ +void +connection_read_bw_exhausted(connection_t *conn, bool is_global_bw) +{ + (void)is_global_bw; + conn->read_blocked_on_bw = 1; + connection_stop_reading(conn); +} + +/** + * Mark <b>conn</b> as needing to stop reading because write bandwidth has + * been exhausted. If <b>is_global_bw</b>, it is closing because global + * bandwidth limit has been exhausted. Otherwise, it is closing because its + * own bandwidth limit has been exhausted. +*/ +void +connection_write_bw_exhausted(connection_t *conn, bool is_global_bw) +{ + (void)is_global_bw; + conn->write_blocked_on_bw = 1; + connection_stop_reading(conn); +} + /** If we have exhausted our global buckets, or the buckets for conn, * stop reading. */ -static void +void connection_consider_empty_read_buckets(connection_t *conn) { const char *reason; @@ -3062,6 +3090,7 @@ connection_consider_empty_read_buckets(connection_t *conn) if (!connection_is_rate_limited(conn)) return; /* Always okay. */

+ bool is_global = true; if (token_bucket_rw_get_read(&global_bucket) <= 0) { reason = "global read bucket exhausted. Pausing."; } else if (connection_counts_as_relayed_traffic(conn, approx_time()) && @@ -3071,17 +3100,17 @@ connection_consider_empty_read_buckets(connection_t *conn) conn->state == OR_CONN_STATE_OPEN && token_bucket_rw_get_read(&TO_OR_CONN(conn)->bucket) <= 0) { reason = "connection read bucket exhausted. Pausing."; + is_global = false; } else return; /* all good, no need to stop it */

LOG_FN_CONN(conn, (LOG_DEBUG, LD_NET, "%s", reason)); - conn->read_blocked_on_bw = 1; - connection_stop_reading(conn); + connection_read_bw_exhausted(conn, is_global); }

/** If we have exhausted our global buckets, or the buckets for conn, * stop writing. */ -static void +void connection_consider_empty_write_buckets(connection_t *conn) { const char *reason; @@ -3089,6 +3118,7 @@ connection_consider_empty_write_buckets(connection_t *conn) if (!connection_is_rate_limited(conn)) return; /* Always okay. */

+ bool is_global = true; if (token_bucket_rw_get_write(&global_bucket) <= 0) { reason = "global write bucket exhausted. Pausing."; } else if (connection_counts_as_relayed_traffic(conn, approx_time()) && @@ -3098,12 +3128,12 @@ connection_consider_empty_write_buckets(connection_t *conn) conn->state == OR_CONN_STATE_OPEN && token_bucket_rw_get_write(&TO_OR_CONN(conn)->bucket) <= 0) { reason = "connection write bucket exhausted. Pausing."; + is_global = false; } else return; /* all good, no need to stop it */

LOG_FN_CONN(conn, (LOG_DEBUG, LD_NET, "%s", reason)); - conn->write_blocked_on_bw = 1; - connection_stop_writing(conn); + connection_write_bw_exhausted(conn, is_global); }

/** Initialize the global buckets to the values configured in the @@ -3768,8 +3798,7 @@ connection_handle_write_impl(connection_t *conn, int force) /* Make sure to avoid a loop if the receive buckets are empty. */ log_debug(LD_NET,"wanted read."); if (!connection_is_reading(conn)) { - connection_stop_writing(conn); - conn->write_blocked_on_bw = 1; + connection_write_bw_exhausted(conn, true); /* we'll start reading again when we get more tokens in our * read bucket; then we'll start writing again too. */ diff --git a/src/or/connection.h b/src/or/connection.h index cfe31c372..83e2bd543 100644 --- a/src/or/connection.h +++ b/src/or/connection.h @@ -125,6 +125,10 @@ void connection_bucket_init(void); void connection_bucket_adjust(const or_options_t *options); void connection_bucket_refill(time_t now, uint32_t now_ts); +void connection_read_bw_exhausted(connection_t *conn, bool is_global_bw); +void connection_write_bw_exhausted(connection_t *conn, bool is_global_bw); +void connection_consider_empty_read_buckets(connection_t *conn); +void connection_consider_empty_write_buckets(connection_t *conn);

int connection_handle_read(connection_t *conn);

diff --git a/src/or/main.c b/src/or/main.c index a852d3273..e21ef24f8 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1025,19 +1025,22 @@ conn_close_if_marked(int i) * busy Libevent loops where we keep ending up here and returning * 0 until we are no longer blocked on bandwidth. */ - if (connection_is_writing(conn)) { - conn->write_blocked_on_bw = 1; - connection_stop_writing(conn); + connection_consider_empty_read_buckets(conn); + connection_consider_empty_write_buckets(conn); + + /* Make sure that consider_empty_buckets really disabled the + * connection: */ + if (BUG(connection_is_writing(conn))) { + connection_write_bw_exhausted(conn, true); } - if (connection_is_reading(conn)) { + if (BUG(connection_is_reading(conn))) { /* XXXX+ We should make this code unreachable; if a connection is * marked for close and flushing, there is no point in reading to it * at all. Further, checking at this point is a bit of a hack: it * would make much more sense to react in * connection_handle_read_impl, or to just stop reading in * mark_and_flush */ - conn->read_blocked_on_bw = 1; - connection_stop_reading(conn); + connection_read_bw_exhausted(conn, true/* kludge. */); } } return 0; diff --git a/src/or/or.h b/src/or/or.h index c5a039e93..e27f25197 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -57,6 +57,7 @@ #ifdef HAVE_TIME_H #include <time.h> #endif +#include <stdbool.h>

#ifdef _WIN32 #include <winsock2.h>