[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tor/master] Add new "ALL" and "NET_PARTICIPANT" roles for periodic events



commit b9a88bd53ae79a29c292275381bc7dbaa3804034
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
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:



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits