diff --git a/changes/ticket40704 b/changes/ticket40704 new file mode 100644 index 0000000000..b1a83488da --- /dev/null +++ b/changes/ticket40704 @@ -0,0 +1,6 @@ + o Minor feature (relay): + - Two new consensus parameters are added to control the wait time in queue + of the onionskins. One of them is the torrc MaxOnionQueueDelay options + which supersedes the consensus parameter. Closes ticket 40704. + - Change a hardcoded value for the maximum of per CPU tasks into a + consensus parameter. diff --git a/src/app/config/config.c b/src/app/config/config.c index a3382c761b..7443d6bcb3 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -549,7 +549,7 @@ static const config_var_t option_vars_[] = { V(MaxConsensusAgeForDiffs, INTERVAL, "0 seconds"), VAR("MaxMemInQueues", MEMUNIT, MaxMemInQueues_raw, "0"), OBSOLETE("MaxOnionsPending"), - V(MaxOnionQueueDelay, MSEC_INTERVAL, "1750 msec"), + V(MaxOnionQueueDelay, MSEC_INTERVAL, "0"), V(MaxUnparseableDescSizeToLog, MEMUNIT, "10 MB"), VPORT(MetricsPort), V(MetricsPortPolicy, LINELIST, NULL), diff --git a/src/core/mainloop/cpuworker.c b/src/core/mainloop/cpuworker.c index ab970259b5..3ad556a184 100644 --- a/src/core/mainloop/cpuworker.c +++ b/src/core/mainloop/cpuworker.c @@ -32,6 +32,7 @@ #include "feature/relay/onion_queue.h" #include "feature/stats/rephist.h" #include "feature/relay/router.h" +#include "feature/nodelist/networkstatus.h" #include "lib/evloop/workqueue.h" #include "core/crypto/onion_crypto.h" @@ -75,8 +76,42 @@ worker_state_free_void(void *arg) static replyqueue_t *replyqueue = NULL; static threadpool_t *threadpool = NULL; -static int total_pending_tasks = 0; -static int max_pending_tasks = 128; +static uint32_t total_pending_tasks = 0; +static uint32_t max_pending_tasks = 128; + +/** Return the consensus parameter max pending tasks per CPU. */ +static uint32_t +get_max_pending_tasks_per_cpu(const networkstatus_t *ns) +{ +/* Total voodoo. Can we make this more sensible? Maybe, that is why we made it + * a consensus parameter so our future self can figure out this magic. */ +#define MAX_PENDING_TASKS_PER_CPU_DEFAULT 64 +#define MAX_PENDING_TASKS_PER_CPU_MIN 1 +#define MAX_PENDING_TASKS_PER_CPU_MAX INT32_MAX + + return networkstatus_get_param(ns, "max_pending_tasks_per_cpu", + MAX_PENDING_TASKS_PER_CPU_DEFAULT, + MAX_PENDING_TASKS_PER_CPU_MIN, + MAX_PENDING_TASKS_PER_CPU_MAX); +} + +/** Set the max pending tasks per CPU worker. This uses the consensus to check + * for the allowed number per CPU. The ns parameter can be NULL as in that no + * consensus is available at the time of setting this value. */ +static void +set_max_pending_tasks(const networkstatus_t *ns) +{ + max_pending_tasks = + get_num_cpus(get_options()) * get_max_pending_tasks_per_cpu(ns); +} + +/** Called when the consensus has changed. */ +void +cpuworker_consensus_has_changed(const networkstatus_t *ns) +{ + tor_assert(ns); + set_max_pending_tasks(ns); +} /** Initialize the cpuworker subsystem. It is OK to call this more than once * during Tor's lifetime. @@ -106,8 +141,7 @@ cpu_init(void) tor_assert(r == 0); } - /* Total voodoo. Can we make this more sensible? */ - max_pending_tasks = get_num_cpus(get_options()) * 64; + set_max_pending_tasks(NULL); } /** Magic numbers to make sure our cpuworker_requests don't grow any diff --git a/src/core/mainloop/cpuworker.h b/src/core/mainloop/cpuworker.h index 9e589a3004..94545af586 100644 --- a/src/core/mainloop/cpuworker.h +++ b/src/core/mainloop/cpuworker.h @@ -12,8 +12,13 @@ #ifndef TOR_CPUWORKER_H #define TOR_CPUWORKER_H +#include "feature/nodelist/networkstatus_st.h" + void cpu_init(void); void cpuworkers_rotate_keyinfo(void); + +void cpuworker_consensus_has_changed(const networkstatus_t *ns); + struct workqueue_entry_t; enum workqueue_reply_t; enum workqueue_priority_t; diff --git a/src/feature/nodelist/networkstatus.c b/src/feature/nodelist/networkstatus.c index d53c6cc3a8..b994cfabc4 100644 --- a/src/feature/nodelist/networkstatus.c +++ b/src/feature/nodelist/networkstatus.c @@ -40,6 +40,7 @@ #include "core/or/or.h" #include "app/config/config.h" #include "core/mainloop/connection.h" +#include "core/mainloop/cpuworker.h" #include "core/mainloop/mainloop.h" #include "core/mainloop/netstatus.h" #include "core/or/channel.h" @@ -83,6 +84,7 @@ #include "feature/nodelist/routerlist.h" #include "feature/nodelist/torcert.h" #include "feature/relay/dns.h" +#include "feature/relay/onion_queue.h" #include "feature/relay/routermode.h" #include "lib/crypt_ops/crypto_rand.h" #include "lib/crypt_ops/crypto_util.h" @@ -1667,6 +1669,8 @@ notify_before_networkstatus_changes(const networkstatus_t *old_c, relay_consensus_has_changed(new_c); hs_dos_consensus_has_changed(new_c); rep_hist_consensus_has_changed(new_c); + cpuworker_consensus_has_changed(new_c); + onion_consensus_has_changed(new_c); } /* Called after a new consensus has been put in the global state. It is safe diff --git a/src/feature/relay/onion_queue.c b/src/feature/relay/onion_queue.c index c4d55dc510..6d1a6de15c 100644 --- a/src/feature/relay/onion_queue.c +++ b/src/feature/relay/onion_queue.c @@ -38,6 +38,22 @@ #include "core/or/or_circuit_st.h" #include "core/or/channel.h" +/** Onion queue default, max and min. */ + +/* In seconds. */ +#define ONION_QUEUE_WAIT_CUTOFF_DEFAULT 5 +#define ONION_QUEUE_WAIT_CUTOFF_MIN 0 +#define ONION_QUEUE_WAIT_CUTOFF_MAX INT32_MAX + +/* In msec. */ +#define ONION_QUEUE_MAX_DELAY_DEFAULT 1750 +#define ONION_QUEUE_MAX_DELAY_MIN 1 +#define ONION_QUEUE_MAX_DELAY_MAX INT32_MAX + +#define NUM_NTORS_PER_TAP_DEFAULT 10 +#define NUM_NTORS_PER_TAP_MIN 1 +#define NUM_NTORS_PER_TAP_MAX 100000 + /** Type for a linked list of circuits that are waiting for a free CPU worker * to process a waiting onion handshake. */ typedef struct onion_queue_t { @@ -48,9 +64,6 @@ typedef struct onion_queue_t { time_t when_added; } onion_queue_t; -/** 5 seconds on the onion queue til we just send back a destroy */ -#define ONIONQUEUE_WAIT_CUTOFF 5 - TOR_TAILQ_HEAD(onion_queue_head_t, onion_queue_t); typedef struct onion_queue_head_t onion_queue_head_t; @@ -68,9 +81,38 @@ static onion_queue_head_t ol_list[MAX_QUEUE_IDX+1] = /** Number of entries of each type currently in each element of ol_list[]. */ static int ol_entries[MAX_QUEUE_IDX+1]; -static int num_ntors_per_tap(void); static void onion_queue_entry_remove(onion_queue_t *victim); +/** Consensus parameters. */ +static int32_t ns_num_ntors_per_tap = NUM_NTORS_PER_TAP_DEFAULT; +static time_t ns_onion_queue_wait_cutoff = ONION_QUEUE_WAIT_CUTOFF_DEFAULT; +static uint32_t ns_onion_queue_max_delay = ONION_QUEUE_MAX_DELAY_DEFAULT; + +/** Return the number of ntors per tap from the cached parameter. */ +static inline int32_t +get_num_ntors_per_tap(void) +{ + return ns_num_ntors_per_tap; +} + +/** Return the onion queue wait cutoff value from the cached parameter. */ +static inline time_t +get_onion_queue_wait_cutoff(void) +{ + return ns_onion_queue_wait_cutoff; +} + +/** Return the max onion queue delay value either from the torrc options (if + * the user explicitly set it) else from the cached parameter. */ +static inline uint32_t +get_onion_queue_max_delay(const or_options_t *options) +{ + if (options && options->MaxOnionQueueDelay > 0) { + return options->MaxOnionQueueDelay; + } + return ns_onion_queue_max_delay; +} + /** * We combine ntorv3 and ntor into the same queue, so we must * use this function to covert the cell type to a queue index. @@ -103,6 +145,7 @@ have_room_for_onionskin(uint16_t type) { const or_options_t *options = get_options(); int num_cpus; + uint64_t max_onion_queue_delay; uint64_t tap_usec, ntor_usec; uint64_t ntor_during_tap_usec, tap_during_ntor_usec; @@ -110,6 +153,8 @@ have_room_for_onionskin(uint16_t type) if (ol_entries[type] < 50) return 1; num_cpus = get_num_cpus(options); + max_onion_queue_delay = get_onion_queue_max_delay(options); + /* Compute how many microseconds we'd expect to need to clear all * onionskins in various combinations of the queues. */ @@ -127,32 +172,30 @@ have_room_for_onionskin(uint16_t type) * process while draining the ntor queue? */ tap_during_ntor_usec = estimated_usec_for_onionskins( MIN(ol_entries[ONION_HANDSHAKE_TYPE_TAP], - ol_entries[ONION_HANDSHAKE_TYPE_NTOR] / num_ntors_per_tap()), + ol_entries[ONION_HANDSHAKE_TYPE_NTOR] / get_num_ntors_per_tap()), ONION_HANDSHAKE_TYPE_TAP) / num_cpus; /* How long would it take to process the ntor cells that we expect to * process while draining the tap queue? */ ntor_during_tap_usec = estimated_usec_for_onionskins( MIN(ol_entries[ONION_HANDSHAKE_TYPE_NTOR], - ol_entries[ONION_HANDSHAKE_TYPE_TAP] * num_ntors_per_tap()), + ol_entries[ONION_HANDSHAKE_TYPE_TAP] * get_num_ntors_per_tap()), ONION_HANDSHAKE_TYPE_NTOR) / num_cpus; /* See whether that exceeds MaxOnionQueueDelay. If so, we can't queue * this. */ if (type == ONION_HANDSHAKE_TYPE_NTOR && - (ntor_usec + tap_during_ntor_usec) / 1000 > - (uint64_t)options->MaxOnionQueueDelay) + (ntor_usec + tap_during_ntor_usec) / 1000 > max_onion_queue_delay) return 0; if (type == ONION_HANDSHAKE_TYPE_TAP && - (tap_usec + ntor_during_tap_usec) / 1000 > - (uint64_t)options->MaxOnionQueueDelay) + (tap_usec + ntor_during_tap_usec) / 1000 > max_onion_queue_delay) return 0; /* If we support the ntor handshake, then don't let TAP handshakes use * more than 2/3 of the space on the queue. */ if (type == ONION_HANDSHAKE_TYPE_TAP && - tap_usec / 1000 > (uint64_t)options->MaxOnionQueueDelay * 2 / 3) + tap_usec / 1000 > max_onion_queue_delay * 2 / 3) return 0; return 1; @@ -222,7 +265,7 @@ onion_pending_add(or_circuit_t *circ, create_cell_t *onionskin) /* cull elderly requests. */ while (1) { onion_queue_t *head = TOR_TAILQ_FIRST(&ol_list[queue_idx]); - if (now - head->when_added < (time_t)ONIONQUEUE_WAIT_CUTOFF) + if (now - head->when_added < get_onion_queue_wait_cutoff()) break; circ = head->circ; @@ -237,24 +280,6 @@ onion_pending_add(or_circuit_t *circ, create_cell_t *onionskin) return 0; } -/** Return a fairness parameter, to prefer processing NTOR style - * handshakes but still slowly drain the TAP queue so we don't starve - * it entirely. */ -static int -num_ntors_per_tap(void) -{ -#define DEFAULT_NUM_NTORS_PER_TAP 10 -#define MIN_NUM_NTORS_PER_TAP 1 -#define MAX_NUM_NTORS_PER_TAP 100000 - - int result = networkstatus_get_param(NULL, "NumNTorsPerTAP", - DEFAULT_NUM_NTORS_PER_TAP, - MIN_NUM_NTORS_PER_TAP, - MAX_NUM_NTORS_PER_TAP); - tor_assert(result > 0); - return result; -} - /** Choose which onion queue we'll pull from next. If one is empty choose * the other; if they both have elements, load balance across them but * favoring NTOR. */ @@ -278,7 +303,7 @@ decide_next_handshake_type(void) * once tap is rare. We should reevaluate whether we like this decision * once tap gets more rare. */ if (ol_entries[ONION_HANDSHAKE_TYPE_NTOR] && - recently_chosen_ntors <= num_ntors_per_tap()) + recently_chosen_ntors <= get_num_ntors_per_tap()) ++recently_chosen_ntors; return ONION_HANDSHAKE_TYPE_NTOR; /* no taps? try ntor */ @@ -286,7 +311,7 @@ decide_next_handshake_type(void) /* They both have something queued. Pick ntor if we haven't done that * too much lately. */ - if (++recently_chosen_ntors <= num_ntors_per_tap()) { + if (++recently_chosen_ntors <= get_num_ntors_per_tap()) { return ONION_HANDSHAKE_TYPE_NTOR; } @@ -396,3 +421,28 @@ clear_pending_onions(void) } memset(ol_entries, 0, sizeof(ol_entries)); } + +/** Consensus has changed, update the cached parameters. */ +void +onion_consensus_has_changed(const networkstatus_t *ns) +{ + tor_assert(ns); + + ns_onion_queue_max_delay = + networkstatus_get_param(ns, "MaxOnionQueueDelay", + ONION_QUEUE_MAX_DELAY_DEFAULT, + ONION_QUEUE_MAX_DELAY_MIN, + ONION_QUEUE_MAX_DELAY_MAX); + + ns_onion_queue_wait_cutoff = + networkstatus_get_param(ns, "onion_queue_wait_cutoff", + ONION_QUEUE_WAIT_CUTOFF_DEFAULT, + ONION_QUEUE_WAIT_CUTOFF_MIN, + ONION_QUEUE_WAIT_CUTOFF_MAX); + + ns_num_ntors_per_tap = + networkstatus_get_param(ns, "NumNTorsPerTAP", + NUM_NTORS_PER_TAP_DEFAULT, + NUM_NTORS_PER_TAP_MIN, + NUM_NTORS_PER_TAP_MAX); +} diff --git a/src/feature/relay/onion_queue.h b/src/feature/relay/onion_queue.h index 5ac1b1b280..0c2b97c2b0 100644 --- a/src/feature/relay/onion_queue.h +++ b/src/feature/relay/onion_queue.h @@ -14,6 +14,8 @@ struct create_cell_t; +void onion_consensus_has_changed(const networkstatus_t *ns); + int onion_pending_add(or_circuit_t *circ, struct create_cell_t *onionskin); or_circuit_t *onion_next_task(struct create_cell_t **onionskin_out); int onion_num_pending(uint16_t handshake_type); diff --git a/src/lib/thread/numcpus.c b/src/lib/thread/numcpus.c index e0ddb69931..40fac7dbe4 100644 --- a/src/lib/thread/numcpus.c +++ b/src/lib/thread/numcpus.c @@ -53,10 +53,10 @@ compute_num_cpus_impl(void) cpus = cpus_onln; } else if (cpus_onln > 0 && cpus_conf > 0) { if (cpus_onln < cpus_conf) { - log_notice(LD_GENERAL, "I think we have %ld CPUS, but only %ld of them " - "are available. Telling Tor to only use %ld. You can over" - "ride this with the NumCPUs option", - cpus_conf, cpus_onln, cpus_onln); + log_info(LD_GENERAL, "I think we have %ld CPUS, but only %ld of them " + "are available. Telling Tor to only use %ld. You can over" + "ride this with the NumCPUs option", + cpus_conf, cpus_onln, cpus_onln); } cpus = cpus_onln; }