From 4f184415cc462214427627df0edfa897e555d5e8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Apr 2018 17:37:09 -0400 Subject: Start refactoring dirvote_act() towards self-scheduling This change should have no behavioral effect: it just uses macros to describe the current control flow. --- src/or/dirauth/dirvote.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/or/dirauth/dirvote.c b/src/or/dirauth/dirvote.c index dc978f26e9..90f8d22272 100644 --- a/src/or/dirauth/dirvote.c +++ b/src/or/dirauth/dirvote.c @@ -2729,7 +2729,9 @@ get_detached_signatures_from_pending_consensuses(pending_consensus_t *pending, return signatures; } -/** Entry point: Take whatever voting actions are pending as of now. */ +/** + * Entry point: Take whatever voting actions are pending as of now. + */ void dirvote_act(const or_options_t *options, time_t now) { @@ -2750,33 +2752,36 @@ dirvote_act(const or_options_t *options, time_t now) tor_free(keys); dirvote_recalculate_timing(options, now); } - if (voting_schedule.voting_starts < now && !voting_schedule.have_voted) { + +#define IF_TIME_FOR_NEXT_ACTION(when_field, done_field) \ + if (voting_schedule.when_field < now && !voting_schedule.done_field) do { +#define ENDIF } while(0); + + IF_TIME_FOR_NEXT_ACTION(voting_starts, have_voted) { log_notice(LD_DIR, "Time to vote."); dirvote_perform_vote(); voting_schedule.have_voted = 1; - } - if (voting_schedule.fetch_missing_votes < now && - !voting_schedule.have_fetched_missing_votes) { + } ENDIF + IF_TIME_FOR_NEXT_ACTION(fetch_missing_votes, have_fetched_missing_votes) { log_notice(LD_DIR, "Time to fetch any votes that we're missing."); dirvote_fetch_missing_votes(); voting_schedule.have_fetched_missing_votes = 1; - } - if (voting_schedule.voting_ends < now && - !voting_schedule.have_built_consensus) { + } ENDIF + IF_TIME_FOR_NEXT_ACTION(voting_ends, have_built_consensus) { log_notice(LD_DIR, "Time to compute a consensus."); dirvote_compute_consensuses(); /* XXXX We will want to try again later if we haven't got enough * votes yet. Implement this if it turns out to ever happen. */ voting_schedule.have_built_consensus = 1; - } - if (voting_schedule.fetch_missing_signatures < now && - !voting_schedule.have_fetched_missing_signatures) { + } ENDIF + IF_TIME_FOR_NEXT_ACTION(fetch_missing_signatures, + have_fetched_missing_signatures) { log_notice(LD_DIR, "Time to fetch any signatures that we're missing."); dirvote_fetch_missing_signatures(); voting_schedule.have_fetched_missing_signatures = 1; - } - if (voting_schedule.interval_starts < now && - !voting_schedule.have_published_consensus) { + } ENDIF + IF_TIME_FOR_NEXT_ACTION(interval_starts, + have_published_consensus) { log_notice(LD_DIR, "Time to publish the consensus and discard old votes"); dirvote_publish_consensus(); dirvote_clear_votes(0); @@ -2787,7 +2792,10 @@ dirvote_act(const or_options_t *options, time_t now) /* XXXX We will want to try again later if we haven't got enough * signatures yet. Implement this if it turns out to ever happen. */ dirvote_recalculate_timing(options, now); - } + } ENDIF + +#undef ENDIF +#undef IF_TIME_FOR_NEXT_ACTION } /** A vote networkstatus_t and its unparsed body: held around so we can -- cgit v1.2.3-54-g00ecf From 9870497f9df909c6a48a6f6c1e82171e99aa33a4 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Apr 2018 17:42:43 -0400 Subject: Update dirvote_act() to return the time of its next action. This is remarkably simple, given the macros in the last commit. --- src/or/dirauth/dirvote.c | 19 +++++++++++++++---- src/or/dirauth/dirvote.h | 5 +++-- 2 files changed, 18 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/or/dirauth/dirvote.c b/src/or/dirauth/dirvote.c index 90f8d22272..36f328d6c5 100644 --- a/src/or/dirauth/dirvote.c +++ b/src/or/dirauth/dirvote.c @@ -2731,12 +2731,14 @@ get_detached_signatures_from_pending_consensuses(pending_consensus_t *pending, /** * Entry point: Take whatever voting actions are pending as of now. + * + * Return the time at which the next action should be taken. */ -void +time_t dirvote_act(const or_options_t *options, time_t now) { if (!authdir_mode_v3(options)) - return; + return TIME_MAX; tor_assert_nonfatal(voting_schedule.voting_starts); /* If we haven't initialized this object through this codeflow, we need to * recalculate the timings to match our vote. The reason to do that is if we @@ -2754,8 +2756,13 @@ dirvote_act(const or_options_t *options, time_t now) } #define IF_TIME_FOR_NEXT_ACTION(when_field, done_field) \ - if (voting_schedule.when_field < now && !voting_schedule.done_field) do { -#define ENDIF } while(0); + if (! voting_schedule.done_field) { \ + if (voting_schedule.when_field > now) { \ + return voting_schedule.when_field; \ + } else { +#define ENDIF \ + } \ + } IF_TIME_FOR_NEXT_ACTION(voting_starts, have_voted) { log_notice(LD_DIR, "Time to vote."); @@ -2792,8 +2799,12 @@ dirvote_act(const or_options_t *options, time_t now) /* XXXX We will want to try again later if we haven't got enough * signatures yet. Implement this if it turns out to ever happen. */ dirvote_recalculate_timing(options, now); + return voting_schedule.voting_starts; } ENDIF + tor_assert_nonfatal_unreached(); + return now + 1; + #undef ENDIF #undef IF_TIME_FOR_NEXT_ACTION } diff --git a/src/or/dirauth/dirvote.h b/src/or/dirauth/dirvote.h index f69e872c8e..7294962925 100644 --- a/src/or/dirauth/dirvote.h +++ b/src/or/dirauth/dirvote.h @@ -96,7 +96,7 @@ */ #ifdef HAVE_MODULE_DIRAUTH -void dirvote_act(const or_options_t *options, time_t now); +time_t dirvote_act(const or_options_t *options, time_t now); void dirvote_free_all(void); void dirvote_parse_sr_commits(networkstatus_t *ns, smartlist_t *tokens); @@ -114,11 +114,12 @@ int dirvote_add_signatures(const char *detached_signatures_body, #else /* HAVE_MODULE_DIRAUTH */ -static inline void +static inline time_t dirvote_act(const or_options_t *options, time_t now) { (void) options; (void) now; + return TIME_MAX; } static inline void -- cgit v1.2.3-54-g00ecf From 6868398b69f9651ad4d15892f19470500ef031d3 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 27 Apr 2018 09:50:07 -0400 Subject: Move responsibility for voting into a separate periodic callback. Closes ticket25937. --- changes/ticket25937 | 9 +++++++++ src/or/main.c | 36 ++++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 changes/ticket25937 (limited to 'src') diff --git a/changes/ticket25937 b/changes/ticket25937 new file mode 100644 index 0000000000..7c49fac708 --- /dev/null +++ b/changes/ticket25937 @@ -0,0 +1,9 @@ + o Minor features (mainloop): + - Move responsibility for + consensus voting + from a once-per-second callback to a callback that is only scheduled as + needed. Once enough items are removed from our once-per-second + callback, we can eliminate it entirely to conserve CPU when idle. + Closes ticket + 25937. + diff --git a/src/or/main.c b/src/or/main.c index b5ddfe6f23..8bc89817a4 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1341,6 +1341,7 @@ CALLBACK(check_for_reachability_bw); CALLBACK(check_onion_keys_expiry_time); CALLBACK(clean_caches); CALLBACK(clean_consdiffmgr); +CALLBACK(dirvote); CALLBACK(downrate_stability); CALLBACK(expire_old_ciruits_serverside); CALLBACK(fetch_networkstatus); @@ -1402,6 +1403,7 @@ STATIC periodic_event_item_t periodic_events[] = { /* Directory authority only. */ CALLBACK(check_authority_cert, PERIODIC_EVENT_ROLE_DIRAUTH, 0), + CALLBACK(dirvote, PERIODIC_EVENT_ROLE_DIRAUTH, PERIODIC_EVENT_FLAG_NEED_NET), /* Relay only. */ CALLBACK(check_canonical_channels, PERIODIC_EVENT_ROLE_RELAY, @@ -1718,10 +1720,6 @@ run_scheduled_events(time_t now) accounting_run_housekeeping(now); } - if (authdir_mode_v3(options)) { - dirvote_act(options, now); - } - /* 3a. Every second, we examine pending circuits and prune the * ones which have been pending for more than a few seconds. * We do this before step 4, so it can try building more if @@ -1973,6 +1971,36 @@ check_authority_cert_callback(time_t now, const or_options_t *options) return CHECK_V3_CERTIFICATE_INTERVAL; } +/** + * Scheduled callback: Run directory-authority voting functionality. + * + * The schedule is a bit complicated here, so dirvote_act() manages the + * schedule itself. + **/ +static int +dirvote_callback(time_t now, const or_options_t *options) +{ + if (!authdir_mode_v3(options)) { + tor_assert_nonfatal_unreached(); + return 3600; + } + + time_t next = dirvote_act(options, now); + if (BUG(next == TIME_MAX)) { + /* This shouldn't be returned unless we called dirvote_act() without + * being an authority. If it happens, maybe our configuration will + * fix itself in an hour or so? */ + return 3600; + } + if (BUG(next <= now)) { + /* This case shouldn't be possible, since "next" is computed by + * dirvote_act() based on the value of "now" we give it. */ + return 1; + } else { + return next - now; + } +} + /** * Periodic callback: If our consensus is too old, recalculate whether * we can actually use it. -- cgit v1.2.3-54-g00ecf From 234e317ef17de111a48c8bb6dba9e84d346afe25 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 27 Apr 2018 09:55:05 -0400 Subject: Ensure that voting is rescheduled whenever the schedule changes. --- src/or/config.c | 5 ++++- src/or/main.c | 12 ++++++++++++ src/or/main.h | 1 + src/or/networkstatus.c | 1 + 4 files changed, 18 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/or/config.c b/src/or/config.c index 3719ac8847..1c2b4cf107 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1,3 +1,4 @@ + /* Copyright (c) 2001 Matej Pfajfar. * Copyright (c) 2001-2004, Roger Dingledine. * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. @@ -2329,8 +2330,10 @@ options_act(const or_options_t *old_options) /* We may need to reschedule some directory stuff if our status changed. */ if (old_options) { - if (authdir_mode_v3(options) && !authdir_mode_v3(old_options)) + if (authdir_mode_v3(options) && !authdir_mode_v3(old_options)) { dirvote_recalculate_timing(options, time(NULL)); + reschedule_dirvote(options); + } if (!bool_eq(directory_fetches_dir_info_early(options), directory_fetches_dir_info_early(old_options)) || !bool_eq(directory_fetches_dir_info_later(options), diff --git a/src/or/main.c b/src/or/main.c index 8bc89817a4..0708e647f1 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1436,6 +1436,7 @@ STATIC periodic_event_item_t periodic_events[] = { * can access them by name. We also keep them inside periodic_events[] * so that we can implement "reset all timers" in a reasonable way. */ static periodic_event_item_t *check_descriptor_event=NULL; +static periodic_event_item_t *dirvote_event=NULL; static periodic_event_item_t *fetch_networkstatus_event=NULL; static periodic_event_item_t *launch_descriptor_fetches_event=NULL; static periodic_event_item_t *check_dns_honesty_event=NULL; @@ -1535,6 +1536,7 @@ initialize_periodic_events(void) STMT_BEGIN name ## _event = find_periodic_event( #name ); STMT_END NAMED_CALLBACK(check_descriptor); + NAMED_CALLBACK(dirvote); NAMED_CALLBACK(fetch_networkstatus); NAMED_CALLBACK(launch_descriptor_fetches); NAMED_CALLBACK(check_dns_honesty); @@ -2001,6 +2003,16 @@ dirvote_callback(time_t now, const or_options_t *options) } } +/** Reschedule the directory-authority voting event. Run this whenever the + * schedule has changed. */ +void +reschedule_dirvote(const or_options_t *options) +{ + if (periodic_events_initialized && authdir_mode_v3(options)) { + periodic_event_reschedule(dirvote_event); + } +} + /** * Periodic callback: If our consensus is too old, recalculate whether * we can actually use it. diff --git a/src/or/main.h b/src/or/main.h index 836dbf1cad..a312b51e05 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 reschedule_or_state_save(void); +void reschedule_dirvote(const or_options_t *options); void mainloop_schedule_postloop_cleanup(void); void rescan_periodic_events(const or_options_t *options); diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 5ca320d284..1267d9d6bc 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -2001,6 +2001,7 @@ networkstatus_set_current_consensus(const char *consensus, * object so we can use the timings in there needed by some subsystems * such as hidden service and shared random. */ dirvote_recalculate_timing(options, now); + reschedule_dirvote(options); nodelist_set_consensus(c); -- cgit v1.2.3-54-g00ecf From a73603653a5b54260705b40c7d71bc38faaf6436 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 30 Apr 2018 10:16:40 -0400 Subject: Reschedule voting callback when any cfg option affecting it changes. --- src/or/config.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/or/config.c b/src/or/config.c index 1c2b4cf107..2b35138b6e 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -747,6 +747,8 @@ static int options_transition_affects_workers( const or_options_t *old_options, const or_options_t *new_options); static int options_transition_affects_descriptor( const or_options_t *old_options, const or_options_t *new_options); +static int options_transition_affects_dirauth_timing( + const or_options_t *old_options, const or_options_t *new_options); static int normalize_nickname_list(config_line_t **normalized_out, const config_line_t *lst, const char *name, char **msg); @@ -1746,6 +1748,32 @@ options_transition_affects_guards(const or_options_t *old_options, return 0; } +/** + * Return true if changing the configuration from old to new + * affects the timing of the voting subsystem + */ +static int +options_transition_affects_dirauth_timing(const or_options_t *old_options, + const or_options_t *new_options) +{ + tor_assert(old_options); + tor_assert(new_options); + + if (authdir_mode_v3(old_options) != authdir_mode_v3(new_options)) + return 1; + if (! authdir_mode_v3(new_options)) + return 0; + YES_IF_CHANGED_INT(V3AuthVotingInterval); + YES_IF_CHANGED_INT(V3AuthVoteDelay); + YES_IF_CHANGED_INT(V3AuthDistDelay); + YES_IF_CHANGED_INT(TestingV3AuthInitialVotingInterval); + YES_IF_CHANGED_INT(TestingV3AuthInitialVoteDelay); + YES_IF_CHANGED_INT(TestingV3AuthInitialDistDelay); + YES_IF_CHANGED_INT(TestingV3AuthVotingStartOffset); + + return 0; +} + /** Fetch the active option list, and take actions based on it. All of the * things we do should survive being done repeatedly. If present, * old_options contains the previous value of the options. @@ -2330,7 +2358,7 @@ options_act(const or_options_t *old_options) /* We may need to reschedule some directory stuff if our status changed. */ if (old_options) { - if (authdir_mode_v3(options) && !authdir_mode_v3(old_options)) { + if (options_transition_affects_dirauth_timing(old_options, options)) { dirvote_recalculate_timing(options, time(NULL)); reschedule_dirvote(options); } -- cgit v1.2.3-54-g00ecf From 4a559e996055d4ad8aeb1be7aece036fad94a4e9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 1 May 2018 10:56:11 -0400 Subject: Refactor to use safe_timer_diff. --- src/or/main.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/or/main.c b/src/or/main.c index 0708e647f1..c03e80dc03 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1994,13 +1994,7 @@ dirvote_callback(time_t now, const or_options_t *options) * fix itself in an hour or so? */ return 3600; } - if (BUG(next <= now)) { - /* This case shouldn't be possible, since "next" is computed by - * dirvote_act() based on the value of "now" we give it. */ - return 1; - } else { - return next - now; - } + return safe_timer_diff(now, next); } /** Reschedule the directory-authority voting event. Run this whenever the @@ -2046,14 +2040,8 @@ save_state_callback(time_t now, const or_options_t *options) const time_t next_write = get_or_state()->next_write; if (next_write == TIME_MAX) { return 86400; - } else if (BUG(next_write <= now)) { - /* This can't happen due to clock jumps, since the value of next_write - * is based on the same "now" that we passed to or_state_save(). - */ - return PERIODIC_EVENT_NO_UPDATE; - } else { - return (int)(next_write - now); } + return safe_timer_diff(now, next_write); } /** Reschedule the event for saving the state file. -- cgit v1.2.3-54-g00ecf