commit b9a88bd53ae79a29c292275381bc7dbaa3804034 Author: Nick Mathewson <nickm@torproject.org> Date: Tue Nov 6 07:27:31 2018 -0500 Add new "ALL" and "NET_PARTICIPANT" roles for periodic events The previous "ALL" role was the OR of a bunch of other roles, which is a mistake: it's better if "ALL" means "all". The "NET_PARTICIPANT" role refers to the anything that is actively building circuits, downloading directory information, and participating in the Tor network. For now, it is set to !net_is_disabled(), but we're going to use it to implement a new "extra dormant mode". Closes ticket 28336. --- src/core/mainloop/mainloop.c | 36 ++++++++++++++++++++++--------- src/core/mainloop/periodic.h | 7 +++--- src/test/test_periodic_event.c | 49 ++++++++++++++++++++++++++---------------- 3 files changed, 60 insertions(+), 32 deletions(-) diff --git a/src/core/mainloop/mainloop.c b/src/core/mainloop/mainloop.c index 42f6fb50c..3c3f441a9 100644 --- a/src/core/mainloop/mainloop.c +++ b/src/core/mainloop/mainloop.c @@ -1377,16 +1377,27 @@ CALLBACK(write_stats_file); STATIC periodic_event_item_t periodic_events[] = { /* Everyone needs to run those. */ CALLBACK(add_entropy, ALL, 0), - CALLBACK(check_expired_networkstatus, ALL, 0), - CALLBACK(clean_caches, ALL, 0), - CALLBACK(fetch_networkstatus, ALL, 0), CALLBACK(heartbeat, ALL, 0), - CALLBACK(launch_descriptor_fetches, ALL, FL(NEED_NET)), - CALLBACK(reset_padding_counts, ALL, 0), + + /* XXXX Do we have a reason to do this on a callback? */ CALLBACK(retry_listeners, ALL, FL(NEED_NET)), - CALLBACK(save_state, ALL, 0), - CALLBACK(rotate_x509_certificate, ALL, 0), - CALLBACK(write_stats_file, ALL, 0), + + /* We need to do these if we're participating in the Tor network. */ + CALLBACK(check_expired_networkstatus, NET_PARTICIPANT, 0), + CALLBACK(fetch_networkstatus, NET_PARTICIPANT, 0), + CALLBACK(launch_descriptor_fetches, NET_PARTICIPANT, FL(NEED_NET)), + CALLBACK(rotate_x509_certificate, NET_PARTICIPANT, 0), + + /* We need to do these if we're participating in the Tor network, and + * immediately before we stop. */ + CALLBACK(clean_caches, NET_PARTICIPANT, FL(FLUSH_ON_DISABLE)), + CALLBACK(save_state, NET_PARTICIPANT, FL(FLUSH_ON_DISABLE)), + + /* XXXX investigate this. */ + CALLBACK(write_stats_file, NET_PARTICIPANT, FL(FLUSH_ON_DISABLE)), + + /* XXXX investigate this. */ + CALLBACK(reset_padding_counts, NET_PARTICIPANT, FL(FLUSH_ON_DISABLE)), /* Routers (bridge and relay) only. */ CALLBACK(check_descriptor, ROUTER, FL(NEED_NET)), @@ -1418,7 +1429,8 @@ STATIC periodic_event_item_t periodic_events[] = { CALLBACK(record_bridge_stats, BRIDGE, 0), /* Client only. */ - CALLBACK(rend_cache_failure_clean, CLIENT, 0), + /* XXXX this could be restricted to CLIENT even. */ + CALLBACK(rend_cache_failure_clean, NET_PARTICIPANT, FL(FLUSH_ON_DISABLE)), /* Bridge Authority only. */ CALLBACK(write_bridge_ns, BRIDGEAUTH, 0), @@ -1477,7 +1489,7 @@ get_my_roles(const or_options_t *options) { tor_assert(options); - int roles = 0; + int roles = PERIODIC_EVENT_ROLE_ALL; int is_bridge = options->BridgeRelay; int is_relay = server_mode(options); int is_dirauth = authdir_mode_v3(options); @@ -1492,6 +1504,9 @@ get_my_roles(const or_options_t *options) options->ControlPort_set || options->OwningControllerFD != UINT64_MAX; + /* We actually want a better definition here for our work on dormancy. */ + int is_net_participant = ! net_is_disabled(); + if (is_bridge) roles |= PERIODIC_EVENT_ROLE_BRIDGE; if (is_client) roles |= PERIODIC_EVENT_ROLE_CLIENT; if (is_relay) roles |= PERIODIC_EVENT_ROLE_RELAY; @@ -1499,6 +1514,7 @@ get_my_roles(const or_options_t *options) if (is_bridgeauth) roles |= PERIODIC_EVENT_ROLE_BRIDGEAUTH; if (is_hidden_service) roles |= PERIODIC_EVENT_ROLE_HS_SERVICE; if (is_dirserver) roles |= PERIODIC_EVENT_ROLE_DIRSERVER; + if (is_net_participant) roles |= PERIODIC_EVENT_ROLE_NET_PARTICIPANT; return roles; } diff --git a/src/core/mainloop/periodic.h b/src/core/mainloop/periodic.h index 7c71be7bc..23459ff2b 100644 --- a/src/core/mainloop/periodic.h +++ b/src/core/mainloop/periodic.h @@ -16,6 +16,9 @@ #define PERIODIC_EVENT_ROLE_HS_SERVICE (1U << 5) #define PERIODIC_EVENT_ROLE_DIRSERVER (1U << 6) +#define PERIODIC_EVENT_ROLE_NET_PARTICIPANT (1U << 7) +#define PERIODIC_EVENT_ROLE_ALL (1U << 8) + /* Helper macro to make it a bit less annoying to defined groups of roles that * are often used. */ @@ -25,10 +28,6 @@ /* Authorities that is both bridge and directory. */ #define PERIODIC_EVENT_ROLE_AUTHORITIES \ (PERIODIC_EVENT_ROLE_BRIDGEAUTH | PERIODIC_EVENT_ROLE_DIRAUTH) -/* All roles. */ -#define PERIODIC_EVENT_ROLE_ALL \ - (PERIODIC_EVENT_ROLE_AUTHORITIES | PERIODIC_EVENT_ROLE_CLIENT | \ - PERIODIC_EVENT_ROLE_HS_SERVICE | PERIODIC_EVENT_ROLE_ROUTER) /* * Event flags which can change the behavior of an event. diff --git a/src/test/test_periodic_event.c b/src/test/test_periodic_event.c index 86dedd85d..f63adf8e3 100644 --- a/src/test/test_periodic_event.c +++ b/src/test/test_periodic_event.c @@ -19,6 +19,7 @@ #include "feature/hibernate/hibernate.h" #include "feature/hs/hs_service.h" #include "core/mainloop/mainloop.h" +#include "core/mainloop/netstatus.h" #include "core/mainloop/periodic.h" /** Helper function: This is replaced in some tests for the event callbacks so @@ -59,7 +60,9 @@ test_pe_initialize(void *arg) 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); + int should_be_enabled = (item->roles & PERIODIC_EVENT_ROLE_ALL) && + !(item->flags & PERIODIC_EVENT_FLAG_NEED_NET); + tt_uint_op(periodic_event_is_enabled(item), OP_EQ, should_be_enabled); } done: @@ -106,13 +109,12 @@ test_pe_launch(void *arg) /* Now that we've initialized, rescan the list to launch. */ periodic_events_on_new_options(options); + int mask = PERIODIC_EVENT_ROLE_CLIENT|PERIODIC_EVENT_ROLE_ALL| + PERIODIC_EVENT_ROLE_NET_PARTICIPANT; 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); - } else { - tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0); - } + int should_be_enabled = !!(item->roles & mask); + tt_int_op(periodic_event_is_enabled(item), OP_EQ, should_be_enabled); // enabled or not, the event has not yet been run. tt_u64_op(item->last_action_time, OP_EQ, 0); } @@ -124,7 +126,8 @@ test_pe_launch(void *arg) unsigned roles = get_my_roles(options); tt_uint_op(roles, OP_EQ, - PERIODIC_EVENT_ROLE_RELAY|PERIODIC_EVENT_ROLE_DIRSERVER); + PERIODIC_EVENT_ROLE_RELAY|PERIODIC_EVENT_ROLE_DIRSERVER| + PERIODIC_EVENT_ROLE_ALL|PERIODIC_EVENT_ROLE_NET_PARTICIPANT); for (int i = 0; periodic_events[i].name; ++i) { periodic_event_item_t *item = &periodic_events[i]; @@ -144,17 +147,21 @@ test_pe_launch(void *arg) /* Disable everything and we'll enable them ALL. */ options->SocksPort_set = 0; options->ORPort_set = 0; + options->DisableNetwork = 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); + int should_be_enabled = (item->roles & PERIODIC_EVENT_ROLE_ALL) && + !(item->flags & PERIODIC_EVENT_FLAG_NEED_NET); + tt_int_op(periodic_event_is_enabled(item), OP_EQ, should_be_enabled); } /* Enable everything. */ options->SocksPort_set = 1; options->ORPort_set = 1; options->BridgeRelay = 1; options->AuthoritativeDir = 1; options->V3AuthoritativeDir = 1; options->BridgeAuthoritativeDir = 1; + options->DisableNetwork = 0; register_dummy_hidden_service(&service); periodic_events_on_new_options(options); /* Note down the reference because we need to remove this service from the @@ -188,41 +195,46 @@ test_pe_get_roles(void *arg) or_options_t *options = get_options_mutable(); tt_assert(options); + const int ALL = PERIODIC_EVENT_ROLE_ALL; + /* Nothing configured, should be no roles. */ + tt_assert(net_is_disabled()); roles = get_my_roles(options); - tt_int_op(roles, OP_EQ, 0); + tt_int_op(roles, OP_EQ, ALL); /* 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); + tt_int_op(roles, OP_EQ, PERIODIC_EVENT_ROLE_CLIENT|ALL); /* 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 | - PERIODIC_EVENT_ROLE_DIRSERVER)); + PERIODIC_EVENT_ROLE_DIRSERVER | ALL)); /* 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 | PERIODIC_EVENT_ROLE_DIRSERVER)); + PERIODIC_EVENT_ROLE_BRIDGE | PERIODIC_EVENT_ROLE_DIRSERVER | + ALL)); 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 | PERIODIC_EVENT_ROLE_DIRSERVER); + PERIODIC_EVENT_ROLE_ROUTER | PERIODIC_EVENT_ROLE_DIRSERVER | + ALL); /* 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); + tt_int_op(roles, OP_EQ, ALL); /* Now upgrade to Dirauth. */ options->DirPort_set = 1; @@ -230,7 +242,7 @@ test_pe_get_roles(void *arg) options->V3AuthoritativeDir = 1; roles = get_my_roles(options); tt_int_op(roles, OP_EQ, - PERIODIC_EVENT_ROLE_DIRAUTH|PERIODIC_EVENT_ROLE_DIRSERVER); + PERIODIC_EVENT_ROLE_DIRAUTH|PERIODIC_EVENT_ROLE_DIRSERVER|ALL); tt_assert(roles & PERIODIC_EVENT_ROLE_AUTHORITIES); /* Now Bridge Authority. */ @@ -238,7 +250,7 @@ test_pe_get_roles(void *arg) options->BridgeAuthoritativeDir = 1; roles = get_my_roles(options); tt_int_op(roles, OP_EQ, - PERIODIC_EVENT_ROLE_BRIDGEAUTH|PERIODIC_EVENT_ROLE_DIRSERVER); + PERIODIC_EVENT_ROLE_BRIDGEAUTH|PERIODIC_EVENT_ROLE_DIRSERVER|ALL); tt_assert(roles & PERIODIC_EVENT_ROLE_AUTHORITIES); /* Move that bridge auth to become a relay. */ @@ -246,7 +258,7 @@ test_pe_get_roles(void *arg) roles = get_my_roles(options); tt_int_op(roles, OP_EQ, (PERIODIC_EVENT_ROLE_BRIDGEAUTH | PERIODIC_EVENT_ROLE_RELAY - | PERIODIC_EVENT_ROLE_DIRSERVER)); + | PERIODIC_EVENT_ROLE_DIRSERVER|ALL)); tt_assert(roles & PERIODIC_EVENT_ROLE_AUTHORITIES); /* And now an Hidden service. */ @@ -257,7 +269,8 @@ test_pe_get_roles(void *arg) 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 | PERIODIC_EVENT_ROLE_DIRSERVER)); + PERIODIC_EVENT_ROLE_HS_SERVICE | PERIODIC_EVENT_ROLE_DIRSERVER | + ALL)); tt_assert(roles & PERIODIC_EVENT_ROLE_AUTHORITIES); done: