From 05d314f888dd96840d92fbcc513974487485f4cb Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 26 Apr 2018 14:20:31 -0400 Subject: [PATCH 1/2] main: Add mainloop callback event flags Implement the ability to set flags per events which influences the set up of the event. This commit only adds one flag which is "need network" meaning that the event is not enabled if tor has disabled the network or if hibernation mode. Signed-off-by: David Goulet --- changes/ticket25376_25762 | 10 +++++ src/or/main.c | 75 +++++++++++++++++++++------------- src/or/periodic.h | 15 ++++++- src/test/test_periodic_event.c | 4 ++ 4 files changed, 73 insertions(+), 31 deletions(-) create mode 100644 changes/ticket25376_25762 diff --git a/changes/ticket25376_25762 b/changes/ticket25376_25762 new file mode 100644 index 0000000000..b3ebf56d3b --- /dev/null +++ b/changes/ticket25376_25762 @@ -0,0 +1,10 @@ + o Major feature (main loop, CPU usage): + - Previously, tor would enable at startup all possible main loop event + regardless if it needed them. For instance, directory authorities + callbacks were fired up even for client only. We have now refactored this + whole interface to only enable the appropriate callbacks depending on what + are tor roles (client only, relay, hidden service, etc.). Furthermore, + these events now depend on DisableNetwork or the hibernation state in + order to enable them. This is a big step towards reducing client CPU usage + by reducing the amount of wake ups the daemon does. Closes ticket 25376 + and 25762. diff --git a/src/or/main.c b/src/or/main.c index a7896fafa5..c7bd720ccf 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1362,54 +1362,65 @@ CALLBACK(write_stats_file); #undef CALLBACK /* Now we declare an array of periodic_event_item_t for each periodic event */ -#define CALLBACK(name, r) PERIODIC_EVENT(name, r) +#define CALLBACK(name, r, f) PERIODIC_EVENT(name, r, f) 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), - CALLBACK(clean_caches, PERIODIC_EVENT_ROLE_ALL), - CALLBACK(fetch_networkstatus, PERIODIC_EVENT_ROLE_ALL), - CALLBACK(heartbeat, PERIODIC_EVENT_ROLE_ALL), - CALLBACK(launch_descriptor_fetches, PERIODIC_EVENT_ROLE_ALL), - CALLBACK(reset_padding_counts, PERIODIC_EVENT_ROLE_ALL), - CALLBACK(retry_listeners, PERIODIC_EVENT_ROLE_ALL), - CALLBACK(rotate_x509_certificate, PERIODIC_EVENT_ROLE_ALL), - CALLBACK(write_stats_file, PERIODIC_EVENT_ROLE_ALL), + CALLBACK(add_entropy, PERIODIC_EVENT_ROLE_ALL, 0), + CALLBACK(check_expired_networkstatus, PERIODIC_EVENT_ROLE_ALL, 0), + CALLBACK(clean_caches, PERIODIC_EVENT_ROLE_ALL, 0), + CALLBACK(fetch_networkstatus, PERIODIC_EVENT_ROLE_ALL, + PERIODIC_EVENT_FLAG_NEED_NET), + CALLBACK(heartbeat, PERIODIC_EVENT_ROLE_ALL, 0), + CALLBACK(launch_descriptor_fetches, PERIODIC_EVENT_ROLE_ALL, + PERIODIC_EVENT_FLAG_NEED_NET), + CALLBACK(reset_padding_counts, PERIODIC_EVENT_ROLE_ALL, 0), + CALLBACK(retry_listeners, PERIODIC_EVENT_ROLE_ALL, + PERIODIC_EVENT_FLAG_NEED_NET), + CALLBACK(rotate_x509_certificate, PERIODIC_EVENT_ROLE_ALL, 0), + CALLBACK(write_stats_file, PERIODIC_EVENT_ROLE_ALL, 0), /* Routers (bridge and relay) only. */ - CALLBACK(check_descriptor, PERIODIC_EVENT_ROLE_ROUTER), - CALLBACK(check_ed_keys, PERIODIC_EVENT_ROLE_ROUTER), - CALLBACK(check_for_reachability_bw, PERIODIC_EVENT_ROLE_ROUTER), - CALLBACK(check_onion_keys_expiry_time, PERIODIC_EVENT_ROLE_ROUTER), - CALLBACK(clean_consdiffmgr, PERIODIC_EVENT_ROLE_ROUTER), - CALLBACK(expire_old_ciruits_serverside, PERIODIC_EVENT_ROLE_ROUTER), - CALLBACK(retry_dns, PERIODIC_EVENT_ROLE_ROUTER), - CALLBACK(rotate_onion_key, PERIODIC_EVENT_ROLE_ROUTER), + CALLBACK(check_descriptor, PERIODIC_EVENT_ROLE_ROUTER, + PERIODIC_EVENT_FLAG_NEED_NET), + CALLBACK(check_ed_keys, PERIODIC_EVENT_ROLE_ROUTER, 0), + CALLBACK(check_for_reachability_bw, PERIODIC_EVENT_ROLE_ROUTER, + PERIODIC_EVENT_FLAG_NEED_NET), + CALLBACK(check_onion_keys_expiry_time, PERIODIC_EVENT_ROLE_ROUTER, 0), + CALLBACK(clean_consdiffmgr, PERIODIC_EVENT_ROLE_ROUTER, 0), + CALLBACK(expire_old_ciruits_serverside, PERIODIC_EVENT_ROLE_ROUTER, + PERIODIC_EVENT_FLAG_NEED_NET), + CALLBACK(retry_dns, PERIODIC_EVENT_ROLE_ROUTER, 0), + CALLBACK(rotate_onion_key, PERIODIC_EVENT_ROLE_ROUTER, 0), /* Authorities (bridge and directory) only. */ - CALLBACK(downrate_stability, PERIODIC_EVENT_ROLE_AUTHORITIES), - CALLBACK(launch_reachability_tests, PERIODIC_EVENT_ROLE_AUTHORITIES), - CALLBACK(save_stability, PERIODIC_EVENT_ROLE_AUTHORITIES), + CALLBACK(downrate_stability, PERIODIC_EVENT_ROLE_AUTHORITIES, 0), + CALLBACK(launch_reachability_tests, PERIODIC_EVENT_ROLE_AUTHORITIES, + PERIODIC_EVENT_FLAG_NEED_NET), + CALLBACK(save_stability, PERIODIC_EVENT_ROLE_AUTHORITIES, 0), /* Directory authority only. */ - CALLBACK(check_authority_cert, PERIODIC_EVENT_ROLE_DIRAUTH), + CALLBACK(check_authority_cert, PERIODIC_EVENT_ROLE_DIRAUTH, 0), /* Relay only. */ - CALLBACK(check_canonical_channels, PERIODIC_EVENT_ROLE_RELAY), - CALLBACK(check_dns_honesty, PERIODIC_EVENT_ROLE_RELAY), + CALLBACK(check_canonical_channels, PERIODIC_EVENT_ROLE_RELAY, + PERIODIC_EVENT_FLAG_NEED_NET), + CALLBACK(check_dns_honesty, PERIODIC_EVENT_ROLE_RELAY, + PERIODIC_EVENT_FLAG_NEED_NET), /* Hidden Service service only. */ - CALLBACK(hs_service, PERIODIC_EVENT_ROLE_HS_SERVICE), + CALLBACK(hs_service, PERIODIC_EVENT_ROLE_HS_SERVICE, + PERIODIC_EVENT_FLAG_NEED_NET), /* Bridge only. */ - CALLBACK(record_bridge_stats, PERIODIC_EVENT_ROLE_BRIDGE), + CALLBACK(record_bridge_stats, PERIODIC_EVENT_ROLE_BRIDGE, 0), /* Client only. */ - CALLBACK(rend_cache_failure_clean, PERIODIC_EVENT_ROLE_CLIENT), + CALLBACK(rend_cache_failure_clean, PERIODIC_EVENT_ROLE_CLIENT, 0), /* Bridge Authority only. */ - CALLBACK(write_bridge_ns, PERIODIC_EVENT_ROLE_BRIDGEAUTH), + CALLBACK(write_bridge_ns, PERIODIC_EVENT_ROLE_BRIDGEAUTH, 0), + END_OF_PERIODIC_EVENTS }; #undef CALLBACK @@ -1548,6 +1559,12 @@ rescan_periodic_events(const or_options_t *options) for (int i = 0; periodic_events[i].name; ++i) { periodic_event_item_t *item = &periodic_events[i]; + /* Handle the event flags. */ + if (net_is_disabled() && + (item->flags & PERIODIC_EVENT_FLAG_NEED_NET)) { + continue; + } + /* 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) { diff --git a/src/or/periodic.h b/src/or/periodic.h index dde27db5af..a044b34fdd 100644 --- a/src/or/periodic.h +++ b/src/or/periodic.h @@ -29,6 +29,15 @@ (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. + */ + +/* Indicate that the event needs the network meaning that if we are in + * DisableNetwork or hibernation mode, the event won't be enabled. This obey + * the net_is_disabled() check. */ +#define PERIODIC_EVENT_FLAG_NEED_NET (1U << 0) + /** Callback function for a periodic event to take action. The return value * influences the next time the function will get called. Return * PERIODIC_EVENT_NO_UPDATE to not update last_action_time and be polled @@ -49,13 +58,15 @@ typedef struct periodic_event_item_t { /* Bitmask of roles define above for which this event applies. */ uint32_t roles; + /* Bitmask of flags which can change the behavior of the event. */ + uint32_t flags; /* Indicate that this event has been enabled that is scheduled. */ unsigned int enabled : 1; } periodic_event_item_t; /** events will get their interval from first execution */ -#define PERIODIC_EVENT(fn, r) { fn##_callback, 0, NULL, #fn, r, 0 } -#define END_OF_PERIODIC_EVENTS { NULL, 0, NULL, NULL, 0, 0 } +#define PERIODIC_EVENT(fn, r, f) { fn##_callback, 0, NULL, #fn, r, f, 0 } +#define END_OF_PERIODIC_EVENTS { NULL, 0, NULL, NULL, 0, 0, 0 } /* Return true iff the given event was setup before thus is enabled to be * scheduled. */ diff --git a/src/test/test_periodic_event.c b/src/test/test_periodic_event.c index 1a9f4351ea..bebbb5e584 100644 --- a/src/test/test_periodic_event.c +++ b/src/test/test_periodic_event.c @@ -16,6 +16,7 @@ #include "or.h" #include "config.h" +#include "hibernate.h" #include "hs_service.h" #include "main.h" #include "periodic.h" @@ -74,6 +75,9 @@ test_pe_launch(void *arg) (void) arg; hs_init(); + /* We need to put tor in hibernation live state so the events requiring + * network gets enabled. */ + consider_hibernation(time(NULL)); /* 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 From d6903e9e87ffd39846b7cacebcfc8bae71f3ca5f Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 26 Apr 2018 15:00:19 -0400 Subject: [PATCH 2/2] hibernation: Rescan the event list on state change When we change the hibernation state, rescan the main loop event list because the new state might affect the events. Signed-off-by: David Goulet --- src/or/hibernate.c | 10 +++++++++- src/or/main.c | 3 +-- src/or/main.h | 1 + 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/or/hibernate.c b/src/or/hibernate.c index 9fed338555..c4ae63acc4 100644 --- a/src/or/hibernate.c +++ b/src/or/hibernate.c @@ -1111,10 +1111,18 @@ getinfo_helper_accounting(control_connection_t *conn, static void on_hibernate_state_change(hibernate_state_t prev_state) { - (void)prev_state; /* Should we do something with this? */ control_event_server_status(LOG_NOTICE, "HIBERNATION_STATUS STATUS=%s", hibernate_state_to_string(hibernate_state)); + + /* We are changing hibernation state, this can affect the main loop event + * list. Rescan it to update the events state. We do this whatever the new + * hibernation state because they can each possibly affect an event. The + * initial state means we are booting up so we shouldn't scan here because + * at this point the events in the list haven't been initialized. */ + if (prev_state != HIBERNATE_STATE_INITIAL) { + rescan_periodic_events(get_options()); + } } #ifdef TOR_UNIT_TESTS diff --git a/src/or/main.c b/src/or/main.c index c7bd720ccf..b5a7017135 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -150,7 +150,6 @@ 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 **********/ @@ -1549,7 +1548,7 @@ teardown_periodic_events(void) /** 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 +void rescan_periodic_events(const or_options_t *options) { tor_assert(options); diff --git a/src/or/main.h b/src/or/main.h index 33ef40ce40..2447339fb5 100644 --- a/src/or/main.h +++ b/src/or/main.h @@ -62,6 +62,7 @@ void reset_all_main_loop_timers(void); void reschedule_descriptor_update_check(void); void reschedule_directory_downloads(void); void mainloop_schedule_postloop_cleanup(void); +void rescan_periodic_events(const or_options_t *options); MOCK_DECL(long,get_uptime,(void)); MOCK_DECL(void,reset_uptime,(void));