April 2018 - tor-commits - lists.torproject.org

[tor/master] main: Use rescan_periodic_events in initialize_periodic_events_cb
by nickm＠torproject.org 23 Apr '18

23 Apr '18

commit f70fa67da6d22d7351bd8f50db0804c239ffa5d6 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Apr 17 18:55:28 2018 -0400 main: Use rescan_periodic_events in initialize_periodic_events_cb --- src/or/main.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/or/main.c b/src/or/main.c index 1c5ac198b..565131cb3 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -150,6 +150,7 @@ static int run_main_loop_until_done(void); static void process_signal(int sig); static void shutdown_did_not_work_callback(evutil_socket_t fd, short event, void *arg) ATTR_NORETURN; +static void rescan_periodic_events(const or_options_t *options); /********* START VARIABLES **********/ @@ -1478,18 +1479,9 @@ initialize_periodic_events_cb(evutil_socket_t fd, short events, void *data) (void) events; (void) data; - int roles = get_my_roles(get_options()); - tor_event_free(initialize_periodic_events_event); - for (int i = 0; periodic_events[i].name; ++i) { - periodic_event_item_t *item = &periodic_events[i]; - if (item->roles & roles) { - /* This is safe to be called on an already enabled event. */ - periodic_event_enable(item); - log_debug(LD_GENERAL, "Launching periodic event %s", item->name); - } - } + rescan_periodic_events(get_options()); } /** Set up all the members of periodic_events[], and configure them all to be @@ -1546,8 +1538,10 @@ rescan_periodic_events(const or_options_t *options) /* Enable the event if needed. It is safe to enable an event that was * already enabled. Same goes for disabling it. */ if (item->roles & roles) { + log_debug(LD_GENERAL, "Launching periodic event %s", item->name); periodic_event_enable(item); } else { + log_debug(LD_GENERAL, "Disabling periodic event %s", item->name); periodic_event_disable(item); } }

1 0

[tor/master] test: Add periodic events unit tests
by nickm＠torproject.org 23 Apr '18

23 Apr '18

commit 665e23c59a370157c5e3526e29590798b1933ed5 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Apr 18 14:50:07 2018 -0400 test: Add periodic events unit tests Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/main.c | 4 +- src/or/main.h | 5 + src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_periodic_event.c | 256 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 266 insertions(+), 2 deletions(-) diff --git a/src/or/main.c b/src/or/main.c index 0a796c0fa..595246bfc 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1351,7 +1351,7 @@ CALLBACK(write_stats_file); /* Now we declare an array of periodic_event_item_t for each periodic event */ #define CALLBACK(name, r) PERIODIC_EVENT(name, r) -static periodic_event_item_t periodic_events[] = { +STATIC periodic_event_item_t periodic_events[] = { /* Everyone needs to run those. */ CALLBACK(add_entropy, PERIODIC_EVENT_ROLE_ALL), CALLBACK(check_expired_networkstatus, PERIODIC_EVENT_ROLE_ALL), @@ -1440,7 +1440,7 @@ find_periodic_event(const char *name) /** Return a bitmask of the roles this tor instance is configured for using * the given options. */ -static int +STATIC int get_my_roles(const or_options_t *options) { tor_assert(options); diff --git a/src/or/main.h b/src/or/main.h index 5220b1321..57ac8573a 100644 --- a/src/or/main.h +++ b/src/or/main.h @@ -98,8 +98,13 @@ STATIC void init_connection_lists(void); STATIC void close_closeable_connections(void); STATIC void initialize_periodic_events(void); STATIC void teardown_periodic_events(void); +STATIC int get_my_roles(const or_options_t *options); #ifdef TOR_UNIT_TESTS extern smartlist_t *connection_array; + +/* We need the periodic_event_item_t definition. */ +#include "periodic.h" +extern periodic_event_item_t periodic_events[]; #endif #endif /* defined(MAIN_PRIVATE) */ diff --git a/src/test/include.am b/src/test/include.am index 474da3f88..b59517a13 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -144,6 +144,7 @@ src_test_test_SOURCES = \ src/test/test_oom.c \ src/test/test_oos.c \ src/test/test_options.c \ + src/test/test_periodic_event.c \ src/test/test_policy.c \ src/test/test_procmon.c \ src/test/test_proto_http.c \ diff --git a/src/test/test.c b/src/test/test.c index 422e181b9..2963f169c 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -863,6 +863,7 @@ struct testgroup_t testgroups[] = { { "oom/", oom_tests }, { "oos/", oos_tests }, { "options/", options_tests }, + { "periodic-event/" , periodic_event_tests }, { "policy/" , policy_tests }, { "procmon/", procmon_tests }, { "proto/http/", proto_http_tests }, diff --git a/src/test/test.h b/src/test/test.h index 1728831ed..15965ccaf 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -239,6 +239,7 @@ extern struct testcase_t nodelist_tests[]; extern struct testcase_t oom_tests[]; extern struct testcase_t oos_tests[]; extern struct testcase_t options_tests[]; +extern struct testcase_t periodic_event_tests[]; extern struct testcase_t policy_tests[]; extern struct testcase_t procmon_tests[]; extern struct testcase_t proto_http_tests[]; diff --git a/src/test/test_periodic_event.c b/src/test/test_periodic_event.c new file mode 100644 index 000000000..1a9f4351e --- /dev/null +++ b/src/test/test_periodic_event.c @@ -0,0 +1,256 @@ +/* Copyright (c) 2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file test_periodic_event.c + * \brief Test the periodic events that Tor uses for different roles. They are + * part of the libevent mainloop + */ + +#define CONFIG_PRIVATE +#define HS_SERVICE_PRIVATE +#define MAIN_PRIVATE + +#include "test.h" +#include "test_helpers.h" + +#include "or.h" +#include "config.h" +#include "hs_service.h" +#include "main.h" +#include "periodic.h" + +/** Helper function: This is replaced in some tests for the event callbacks so + * we don't actually go into the code path of those callbacks. */ +static int +dumb_event_fn(time_t now, const or_options_t *options) +{ + (void) now; + (void) options; + + /* Will get rescheduled in 300 seconds. It just can't be 0. */ + return 300; +} + +static void +register_dummy_hidden_service(hs_service_t *service) +{ + memset(service, 0, sizeof(hs_service_t)); + memset(&service->keys.identity_pk, 'A', sizeof(service->keys.identity_pk)); + (void) register_service(get_hs_service_map(), service); +} + +static void +test_pe_initialize(void *arg) +{ + (void) arg; + + /* Initialize the events but the callback won't get called since we would + * need to run the main loop and then wait for a second delaying the unit + * tests. Instead, we'll test the callback work indepedently elsewhere. */ + initialize_periodic_events(); + + /* Validate that all events have been set up. */ + for (int i = 0; periodic_events[i].name; ++i) { + periodic_event_item_t *item = &periodic_events[i]; + tt_assert(item->ev); + tt_assert(item->fn); + tt_u64_op(item->last_action_time, OP_EQ, 0); + /* Every event must have role(s) assign to it. This is done statically. */ + tt_u64_op(item->roles, OP_NE, 0); + tt_uint_op(periodic_event_is_enabled(item), OP_EQ, 0); + } + + done: + teardown_periodic_events(); +} + +static void +test_pe_launch(void *arg) +{ + hs_service_t service; + or_options_t *options; + + (void) arg; + + hs_init(); + + /* Hack: We'll set a dumb fn() of each events so they don't get called when + * dispatching them. We just want to test the state of the callbacks, not + * the whole code path. */ + for (int i = 0; periodic_events[i].name; ++i) { + periodic_event_item_t *item = &periodic_events[i]; + item->fn = dumb_event_fn; + } + + /* Lets make sure that before intialization, we can't scan the periodic + * events list and launch them. Lets try by being a Client. */ + options = get_options_mutable(); + options->SocksPort_set = 1; + periodic_events_on_new_options(options); + for (int i = 0; periodic_events[i].name; ++i) { + periodic_event_item_t *item = &periodic_events[i]; + tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0); + } + + initialize_periodic_events(); + + /* Now that we've initialized, rescan the list to launch. */ + periodic_events_on_new_options(options); + + for (int i = 0; periodic_events[i].name; ++i) { + periodic_event_item_t *item = &periodic_events[i]; + if (item->roles & PERIODIC_EVENT_ROLE_CLIENT) { + tt_int_op(periodic_event_is_enabled(item), OP_EQ, 1); + tt_u64_op(item->last_action_time, OP_NE, 0); + } else { + tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0); + tt_u64_op(item->last_action_time, OP_EQ, 0); + } + } + + /* Remove Client but become a Relay. */ + options->SocksPort_set = 0; + options->ORPort_set = 1; + periodic_events_on_new_options(options); + + for (int i = 0; periodic_events[i].name; ++i) { + periodic_event_item_t *item = &periodic_events[i]; + /* Only Client role should be disabled. */ + if (item->roles == PERIODIC_EVENT_ROLE_CLIENT) { + tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0); + /* Was previously enabled so they should never be to 0. */ + tt_u64_op(item->last_action_time, OP_NE, 0); + } + if (item->roles & PERIODIC_EVENT_ROLE_RELAY) { + tt_int_op(periodic_event_is_enabled(item), OP_EQ, 1); + tt_u64_op(item->last_action_time, OP_NE, 0); + } + /* Non Relay role should be disabled! */ + if (!(item->roles & PERIODIC_EVENT_ROLE_RELAY)) { + tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0); + } + } + + /* Disable everything and we'll enable them ALL. */ + options->SocksPort_set = 0; + options->ORPort_set = 0; + periodic_events_on_new_options(options); + + for (int i = 0; periodic_events[i].name; ++i) { + periodic_event_item_t *item = &periodic_events[i]; + tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0); + } + + /* Enable everything. */ + options->SocksPort_set = 1; options->ORPort_set = 1; + options->BridgeRelay = 1; options->AuthoritativeDir = 1; + options->V3AuthoritativeDir = 1; options->BridgeAuthoritativeDir = 1; + register_dummy_hidden_service(&service); + periodic_events_on_new_options(options); + /* Remove it now so the hs_free_all() doesn't try to free stack memory. */ + remove_service(get_hs_service_map(), &service); + + for (int i = 0; periodic_events[i].name; ++i) { + periodic_event_item_t *item = &periodic_events[i]; + tt_int_op(periodic_event_is_enabled(item), OP_EQ, 1); + } + + done: + hs_free_all(); +} + +static void +test_pe_get_roles(void *arg) +{ + int roles; + + (void) arg; + + /* Just so the HS global map exists. */ + hs_init(); + + or_options_t *options = get_options_mutable(); + tt_assert(options); + + /* Nothing configured, should be no roles. */ + roles = get_my_roles(options); + tt_int_op(roles, OP_EQ, 0); + + /* Indicate we have a SocksPort, roles should be come Client. */ + options->SocksPort_set = 1; + roles = get_my_roles(options); + tt_int_op(roles, OP_EQ, PERIODIC_EVENT_ROLE_CLIENT); + + /* Now, we'll add a ORPort so should now be a Relay + Client. */ + options->ORPort_set = 1; + roles = get_my_roles(options); + tt_int_op(roles, OP_EQ, + (PERIODIC_EVENT_ROLE_CLIENT | PERIODIC_EVENT_ROLE_RELAY)); + + /* Now add a Bridge. */ + options->BridgeRelay = 1; + roles = get_my_roles(options); + tt_int_op(roles, OP_EQ, + (PERIODIC_EVENT_ROLE_CLIENT | PERIODIC_EVENT_ROLE_RELAY | + PERIODIC_EVENT_ROLE_BRIDGE)); + tt_assert(roles & PERIODIC_EVENT_ROLE_ROUTER); + /* Unset client so we can solely test Router role. */ + options->SocksPort_set = 0; + roles = get_my_roles(options); + tt_int_op(roles, OP_EQ, PERIODIC_EVENT_ROLE_ROUTER); + + /* Reset options so we can test authorities. */ + options->SocksPort_set = 0; + options->ORPort_set = 0; + options->BridgeRelay = 0; + roles = get_my_roles(options); + tt_int_op(roles, OP_EQ, 0); + + /* Now upgrade to Dirauth. */ + options->AuthoritativeDir = 1; + options->V3AuthoritativeDir = 1; + roles = get_my_roles(options); + tt_int_op(roles, OP_EQ, PERIODIC_EVENT_ROLE_DIRAUTH); + tt_assert(roles & PERIODIC_EVENT_ROLE_AUTHORITIES); + + /* Now Bridge Authority. */ + options->V3AuthoritativeDir = 0; + options->BridgeAuthoritativeDir = 1; + roles = get_my_roles(options); + tt_int_op(roles, OP_EQ, PERIODIC_EVENT_ROLE_BRIDGEAUTH); + tt_assert(roles & PERIODIC_EVENT_ROLE_AUTHORITIES); + + /* Move that bridge auth to become a relay. */ + options->ORPort_set = 1; + roles = get_my_roles(options); + tt_int_op(roles, OP_EQ, + (PERIODIC_EVENT_ROLE_BRIDGEAUTH | PERIODIC_EVENT_ROLE_RELAY)); + tt_assert(roles & PERIODIC_EVENT_ROLE_AUTHORITIES); + + /* And now an Hidden service. */ + hs_service_t service; + register_dummy_hidden_service(&service); + roles = get_my_roles(options); + /* Remove it now so the hs_free_all() doesn't try to free stack memory. */ + remove_service(get_hs_service_map(), &service); + tt_int_op(roles, OP_EQ, + (PERIODIC_EVENT_ROLE_BRIDGEAUTH | PERIODIC_EVENT_ROLE_RELAY | + PERIODIC_EVENT_ROLE_HS_SERVICE)); + tt_assert(roles & PERIODIC_EVENT_ROLE_AUTHORITIES); + + done: + hs_free_all(); +} + +#define PE_TEST(name) \ + { #name, test_pe_## name , TT_FORK, NULL, NULL } + +struct testcase_t periodic_event_tests[] = { + PE_TEST(initialize), + PE_TEST(launch), + PE_TEST(get_roles), + + END_OF_TESTCASES +}; +

1 0

[tor/master] main: Update periodic events comment based on latest code
by nickm＠torproject.org 23 Apr '18

23 Apr '18

commit 87cb9ce90088bbe3e677804eca7c1ffa4f63a1cb Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Apr 18 10:25:39 2018 -0400 main: Update periodic events comment based on latest code Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/main.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/or/main.c b/src/or/main.c index 565131cb3..0a796c0fa 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1523,8 +1523,8 @@ teardown_periodic_events(void) periodic_events_initialized = 0; } -/** Do a pass at all our periodic events, destroy those we don't need anymore - * and enabled those we need now using the given options. */ +/** Do a pass at all our periodic events, disable those we don't need anymore + * and enable those we need now using the given options. */ static void rescan_periodic_events(const or_options_t *options) { @@ -1547,15 +1547,15 @@ rescan_periodic_events(const or_options_t *options) } } -/* We just got new options globally set, see if we need to destroy or setup +/* We just got new options globally set, see if we need to enabled or disable * periodic events. */ void periodic_events_on_new_options(const or_options_t *options) { - /* Only if we've already initialized once the events, teardown them all and - * reinitialize. It is just simpler that way instead of going through all - * currently enabled events and trying to destroy only the one that could be - * affected. */ + /* Only if we've already initialized the events, rescan the list which will + * enable or disable events depending on our roles. This will be called at + * bootup and we don't want this function to initialize the events because + * they aren't set up at this stage. */ if (periodic_events_initialized) { rescan_periodic_events(options); }

1 0

[tor/master] Merge remote-tracking branch 'dgoulet/ticket25762_034_05'
by nickm＠torproject.org 23 Apr '18

23 Apr '18

commit 192c7c8bf9a3c53d32e83557ebc62cf7cfa3b1ca Merge: 3527f4b8a 665e23c59 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Apr 23 11:02:05 2018 -0400 Merge remote-tracking branch 'dgoulet/ticket25762_034_05' src/or/config.c | 7 +- src/or/main.c | 192 ++++++++++++++++++++++--------- src/or/main.h | 7 ++ src/or/networkstatus.c | 2 +- src/or/networkstatus.h | 2 + src/or/periodic.c | 51 +++++++- src/or/periodic.h | 42 ++++++- src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_periodic_event.c | 256 +++++++++++++++++++++++++++++++++++++++++ 11 files changed, 502 insertions(+), 60 deletions(-)

1 0

[translation/tor-and-https_completed] Update translations for tor-and-https_completed
by translation＠torproject.org 23 Apr '18

23 Apr '18

commit 5e4a92e12e5fd25f87d115e164b0c86498d7a485 Author: Translation commit bot <translation(a)torproject.org> Date: Mon Apr 23 14:17:50 2018 +0000 Update translations for tor-and-https_completed --- ru.po | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ru.po b/ru.po index 88ba571c9..2b9e9bfac 100644 --- a/ru.po +++ b/ru.po @@ -8,8 +8,8 @@ msgid "" msgstr "" "Project-Id-Version: The Tor Project\n" "POT-Creation-Date: 2014-07-17 14:23+0000\n" -"PO-Revision-Date: 2018-04-12 19:08+0000\n" -"Last-Translator: Andrey\n" +"PO-Revision-Date: 2018-04-23 14:10+0000\n" +"Last-Translator: e Translators <jobayer.1612191(a)gmail.com>\n" "Language-Team: Russian (http://www.transifex.com/otf/torproject/language/ru/)\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n"

1 0

[translation/tor-and-https] Update translations for tor-and-https
by translation＠torproject.org 23 Apr '18

23 Apr '18

commit cd6b1dac321c356a570bee1ff519dee507572f4e Author: Translation commit bot <translation(a)torproject.org> Date: Mon Apr 23 14:17:45 2018 +0000 Update translations for tor-and-https --- ru.po | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ru.po b/ru.po index 88ba571c9..2b9e9bfac 100644 --- a/ru.po +++ b/ru.po @@ -8,8 +8,8 @@ msgid "" msgstr "" "Project-Id-Version: The Tor Project\n" "POT-Creation-Date: 2014-07-17 14:23+0000\n" -"PO-Revision-Date: 2018-04-12 19:08+0000\n" -"Last-Translator: Andrey\n" +"PO-Revision-Date: 2018-04-23 14:10+0000\n" +"Last-Translator: e Translators <jobayer.1612191(a)gmail.com>\n" "Language-Team: Russian (http://www.transifex.com/otf/torproject/language/ru/)\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n"

1 0

[tor/master] Refactor the "block the connection on bandwidth" logic
by nickm＠torproject.org 23 Apr '18

23 Apr '18

commit 488e2b00bf881b97bcc8e4bbe304845ff1d79a03 Author: Nick Mathewson <nickm(a)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 conn as needing to stop reading because bandwidth has been + * exhausted. If is_global_bw, 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 conn as needing to stop reading because write bandwidth has + * been exhausted. If is_global_bw, 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>

1 0

[tor/master] Refactor responsibility for checking global write bucket emptiness
by nickm＠torproject.org 23 Apr '18

23 Apr '18

commit 9af4cd6f31dd9cbfaee526f2042aa7fa501338ef Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Apr 17 12:05:39 2018 -0400 Refactor responsibility for checking global write bucket emptiness We used to do this 10x per second in connection_buckets_refill(); instead, we now do it when the bucket becomes empty. This change is part of the work of making connection_buckets_refill() obsolete. Closes ticket 25828; bugfix on 0.2.3.5-alpha. --- changes/bug25828 | 7 +++++++ src/or/connection.c | 28 ++++++++++++++++------------ 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/changes/bug25828 b/changes/bug25828 new file mode 100644 index 000000000..45cd1f4ae --- /dev/null +++ b/changes/bug25828 @@ -0,0 +1,7 @@ + o Minor bugfixes (bandwidth management): + - Consider ourselves "low on write bandwidth" if we have exhausted our + write bandwidth some time in the last second. This was the + documented behavior before, but the actual behavior was to change + this value every TokenBucketRefillInterval. Fixes bug 25828; bugfix on + 0.2.3.5-alpha. + diff --git a/src/or/connection.c b/src/or/connection.c index 631d665af..062c85c7f 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -2814,10 +2814,10 @@ connection_is_rate_limited(connection_t *conn) return 1; } -/** Did either global write bucket run dry last second? If so, - * we are likely to run dry again this second, so be stingy with the - * tokens we just put in. */ -static int write_buckets_empty_last_second = 0; +/** When was either global write bucket last empty? If this was recent, then + * we're probably low on bandwidth, and we should be stingy with our bandwidth + * usage. */ +static time_t write_buckets_last_empty_at = -100; /** How many seconds of no active local circuits will make the * connection revert to the "relayed" bandwidth class? */ @@ -2969,8 +2969,11 @@ global_write_bucket_low(connection_t *conn, size_t attempt, int priority) if (smaller_bucket < attempt) return 1; /* not enough space no matter the priority */ - if (write_buckets_empty_last_second) - return 1; /* we're already hitting our limits, no more please */ + { + const time_t diff = approx_time() - write_buckets_last_empty_at; + if (diff <= 1) + return 1; /* we're already hitting our limits, no more please */ + } if (priority == 1) { /* old-style v1 query */ /* Could we handle *two* of these requests within the next two seconds? */ @@ -3042,10 +3045,15 @@ connection_buckets_decrement(connection_t *conn, time_t now, if (!connection_is_rate_limited(conn)) return; /* local IPs are free */ + unsigned flags = 0; if (connection_counts_as_relayed_traffic(conn, now)) { - token_bucket_rw_dec(&global_relayed_bucket, num_read, num_written); + flags = token_bucket_rw_dec(&global_relayed_bucket, num_read, num_written); + } + flags |= token_bucket_rw_dec(&global_bucket, num_read, num_written); + + if (flags & TB_WRITE) { + write_buckets_last_empty_at = now; } - token_bucket_rw_dec(&global_bucket, num_read, num_written); if (connection_speaks_cells(conn) && conn->state == OR_CONN_STATE_OPEN) { or_connection_t *or_conn = TO_OR_CONN(conn); token_bucket_rw_dec(&or_conn->bucket, num_read, num_written); @@ -3185,10 +3193,6 @@ connection_bucket_refill_all(time_t now, uint32_t now_ts) { smartlist_t *conns = get_connection_array(); - write_buckets_empty_last_second = - token_bucket_rw_get_write(&global_bucket) <= 0 || - token_bucket_rw_get_write(&global_relayed_bucket) <= 0; - /* refill the global buckets */ token_bucket_rw_refill(&global_bucket, now_ts); token_bucket_rw_refill(&global_relayed_bucket, now_ts);

1 0

[tor/master] Rename connection_bucket_round_robin -> get_share
by nickm＠torproject.org 23 Apr '18

23 Apr '18

commit 993f5d284d3a61b7bc397ad3671dc0ebd44b891b Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Apr 17 11:42:14 2018 -0400 Rename connection_bucket_round_robin -> get_share There was nothing round_robinish about this function. --- src/or/connection.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index 78befee0c..1e308e9b4 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -2845,14 +2845,14 @@ connection_counts_as_relayed_traffic(connection_t *conn, time_t now) * write many of them or just a few; and conn_bucket (if * non-negative) provides an upper limit for our answer. */ static ssize_t -connection_bucket_round_robin(int base, int priority, - ssize_t global_bucket_val, ssize_t conn_bucket) +connection_bucket_get_share(int base, int priority, + ssize_t global_bucket_val, ssize_t conn_bucket) { ssize_t at_most; ssize_t num_bytes_high = (priority ? 32 : 16) * base; ssize_t num_bytes_low = (priority ? 4 : 2) * base; - /* Do a rudimentary round-robin so one circuit can't hog a connection. + /* Do a rudimentary limiting so one circuit can't hog a connection. * Pick at most 32 cells, at least 4 cells if possible, and if we're in * the middle pick 1/8 of the available bandwidth. */ at_most = global_bucket_val / 8; @@ -2899,8 +2899,8 @@ connection_bucket_read_limit(connection_t *conn, time_t now) global_bucket_val = MIN(global_bucket_val, relayed); } - return connection_bucket_round_robin(base, priority, - global_bucket_val, conn_bucket); + return connection_bucket_get_share(base, priority, + global_bucket_val, conn_bucket); } /** How many bytes at most can we write onto this connection? */ @@ -2931,8 +2931,8 @@ connection_bucket_write_limit(connection_t *conn, time_t now) global_bucket_val = MIN(global_bucket_val, relayed); } - return connection_bucket_round_robin(base, priority, - global_bucket_val, conn_bucket); + return connection_bucket_get_share(base, priority, + global_bucket_val, conn_bucket); } /** Return 1 if the global write buckets are low enough that we

1 0

[tor/master] Rename connection_bucket_refill to connection_bucket_refill_all
by nickm＠torproject.org 23 Apr '18

23 Apr '18

commit 1356d51af62f839bb29eb150e6cb37edcef24d4f Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Apr 17 11:46:23 2018 -0400 Rename connection_bucket_refill to connection_bucket_refill_all Also document its actual behavior --- src/or/connection.c | 5 +++-- src/or/connection.h | 4 ++-- src/or/main.c | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index 1e308e9b4..631d665af 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -3178,9 +3178,10 @@ connection_bucket_adjust(const or_options_t *options) } } -/** Time has passed; increment buckets appropriately. */ +/** Time has passed; increment buckets appropriately and re-enable formerly + * blocked connections. */ void -connection_bucket_refill(time_t now, uint32_t now_ts) +connection_bucket_refill_all(time_t now, uint32_t now_ts) { smartlist_t *conns = get_connection_array(); diff --git a/src/or/connection.h b/src/or/connection.h index 83e2bd543..a2dce2435 100644 --- a/src/or/connection.h +++ b/src/or/connection.h @@ -123,8 +123,8 @@ ssize_t connection_bucket_write_limit(connection_t *conn, time_t now); int global_write_bucket_low(connection_t *conn, size_t attempt, int priority); 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_bucket_refill_all(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); diff --git a/src/or/main.c b/src/or/main.c index e21ef24f8..3217e2d9b 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -2407,8 +2407,8 @@ refill_callback(periodic_timer_t *timer, void *arg) accounting_add_bytes(bytes_read, bytes_written, seconds_rolled_over); if (milliseconds_elapsed > 0) { - connection_bucket_refill((time_t)now.tv_sec, - monotime_coarse_get_stamp()); + connection_bucket_refill_all((time_t)now.tv_sec, + monotime_coarse_get_stamp()); } stats_prev_global_read_bucket = token_bucket_rw_get_read(&global_bucket);

1 0