Merge branch 'ticket25376_034_031_squashed'

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.

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

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 **********/
 
@@ -1362,54 +1361,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
@@ -1538,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);
@@ -1548,6 +1558,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) {

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));

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 <b>last_action_time</b> 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. */

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