From 2ae24d003d1d12e8e202748c4398d7438e4a65d9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 17 Mar 2021 08:45:37 -0400 Subject: Add a MinTimeToReportBandwidth option; make it 0 for testing networks. This option changes the time for which a bandwidth measurement period must have been in progress before we include it when reporting our observed bandwidth in our descriptors. Without this option, we only consider a time period towards our maximum if it has been running for a full day. Obviously, that's unacceptable for testing networks, where we'd like to get results as soon as possible. For non-testing networks, I've put a (somewhat arbitrary) 2-hour minimum on the option, since there are traffic analysis concerns with immediate reporting here. Closes #40337. --- changes/ticket40337 | 12 ++++++++++++ doc/man/tor.1.txt | 7 +++++++ src/app/config/config.c | 8 ++++++++ src/app/config/config.h | 7 +++++++ src/app/config/or_options_st.h | 4 ++++ src/app/config/testnet.inc | 1 + src/feature/stats/bwhist.c | 19 ++++++++++++++----- src/feature/stats/bwhist.h | 2 +- src/test/test_relay.c | 4 ++-- 9 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 changes/ticket40337 diff --git a/changes/ticket40337 b/changes/ticket40337 new file mode 100644 index 0000000000..f8f136a740 --- /dev/null +++ b/changes/ticket40337 @@ -0,0 +1,12 @@ + o Minor features (bandwidth reporting): + - Relays can now use the MinTimeToReportBandwidth option to change + the smallest amount of time over which they're willing to report + their observed maximum bandwidth. Previously, this was fixed + at 1 day. For safety, values under 2 hours are only supported on + testing networks. Part of a fix for ticket 40337. + + o Minor features (testing): + - Relays on testing networks now report their observed bandwidths + immediately from startup. Previously, they waited + until they had been running for a full day. Closes ticket + 40337. diff --git a/doc/man/tor.1.txt b/doc/man/tor.1.txt index b57c6ec70a..af4ee494ef 100644 --- a/doc/man/tor.1.txt +++ b/doc/man/tor.1.txt @@ -2448,6 +2448,13 @@ is non-zero): If we have more onionskins queued for processing than we can process in this amount of time, reject new ones. (Default: 1750 msec) +[[MinTimeToReportBandwidth]] **MinTimeToReportBandwidth** __N__ **seconds**|**minutes**|**hours**:: + Do not report our measurements for our maximum observed bandwidth for any + time period that has lasted for less than this amount of time. + Setting this option too low can enable traffic analysis, and is + not permitted except on testing networks. Values over 1 day have + no effect. (Default: 1 day) + [[MyFamily]] **MyFamily** __fingerprint__,__fingerprint__,...:: Declare that this Tor relay is controlled or administered by a group or organization identical or similar to that of the other relays, defined by diff --git a/src/app/config/config.c b/src/app/config/config.c index fa74907b3d..9011f36735 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -563,6 +563,7 @@ static const config_var_t option_vars_[] = { V(MaxUnparseableDescSizeToLog, MEMUNIT, "10 MB"), VPORT(MetricsPort), V(MetricsPortPolicy, LINELIST, NULL), + V(MinTimeToReportBandwidth, INTERVAL, "1 day"), VAR("MyFamily", LINELIST, MyFamily_lines, NULL), V(NewCircuitPeriod, INTERVAL, "30 seconds"), OBSOLETE("NamingAuthoritativeDirectory"), @@ -3711,6 +3712,13 @@ options_validate_cb(const void *old_options_, void *options_, char **msg) options->HeartbeatPeriod = MIN_HEARTBEAT_PERIOD; } + if (options->MinTimeToReportBandwidth < MIN_MIN_TIME_TO_REPORT_BW && + !options->TestingTorNetwork) { + log_warn(LD_CONFIG, "MinTimeToReportBandwidth is too short; " + "raising to %d seconds.", MIN_MIN_TIME_TO_REPORT_BW); + options->MinTimeToReportBandwidth = MIN_MIN_TIME_TO_REPORT_BW; + } + if (options->KeepalivePeriod < 1) REJECT("KeepalivePeriod option must be positive."); diff --git a/src/app/config/config.h b/src/app/config/config.h index e95ef4a728..74e6942eb5 100644 --- a/src/app/config/config.h +++ b/src/app/config/config.h @@ -24,6 +24,13 @@ * expose more information than we're comfortable with. */ #define MIN_HEARTBEAT_PERIOD (30*60) +/** + * Lowest allowable value for MinTimeToReportBandwidth on a non-testing + * network; if this is too low we might report detail that is too + * fine-grained. + **/ +#define MIN_MIN_TIME_TO_REPORT_BW (2*60*60) + /** Maximum default value for MaxMemInQueues, in bytes. */ #if SIZEOF_VOID_P >= 8 #define MAX_DEFAULT_MEMORY_QUEUE_SIZE (UINT64_C(8) << 30) diff --git a/src/app/config/or_options_st.h b/src/app/config/or_options_st.h index 4364f145ed..efecc85d66 100644 --- a/src/app/config/or_options_st.h +++ b/src/app/config/or_options_st.h @@ -1082,6 +1082,10 @@ struct or_options_t { /** List of policy allowed to query the Metrics port. */ struct config_line_t *MetricsPortPolicy; + /** How far must we be into the current bandwidth-measurement period to + * report bandwidth observations from this period? */ + int MinTimeToReportBandwidth; + /** * Configuration objects for individual modules. * diff --git a/src/app/config/testnet.inc b/src/app/config/testnet.inc index 00b307782b..527e0d00b1 100644 --- a/src/app/config/testnet.inc +++ b/src/app/config/testnet.inc @@ -19,6 +19,7 @@ { "TestingV3AuthInitialDistDelay", "20 seconds" }, { "TestingAuthDirTimeToLearnReachability", "0 minutes" }, { "MinUptimeHidServDirectoryV2", "0 minutes" }, +{ "MinTimeToReportBandwidth", "0 seconds" }, { "TestingServerDownloadInitialDelay", "0" }, { "TestingClientDownloadInitialDelay", "0" }, { "TestingServerConsensusDownloadInitialDelay", "0" }, diff --git a/src/feature/stats/bwhist.c b/src/feature/stats/bwhist.c index 7cbc5f60a6..55a8f7c747 100644 --- a/src/feature/stats/bwhist.c +++ b/src/feature/stats/bwhist.c @@ -206,16 +206,24 @@ bwhist_note_dir_bytes_read(uint64_t num_bytes, time_t when) add_obs(dir_read_array, when, num_bytes); } -/** Helper: Return the largest value in b->maxima. (This is equal to the +/** + * Helper: Return the largest value in b->maxima. (This is equal to the * most bandwidth used in any NUM_SECS_ROLLING_MEASURE period for the last * NUM_SECS_BW_SUM_IS_VALID seconds.) + * + * Also include the current period if we have been observing it for + * at least min_observation_time seconds. */ STATIC uint64_t -find_largest_max(bw_array_t *b) +find_largest_max(bw_array_t *b, int min_observation_time) { int i; uint64_t max; - max=0; + time_t period_start = b->next_period - NUM_SECS_BW_SUM_INTERVAL; + if (b->cur_obs_time > period_start + min_observation_time) + max = b->max_total; + else + max = 0; for (i=0; imaxima[i]>max) max = b->maxima[i]; @@ -233,8 +241,9 @@ MOCK_IMPL(int, bwhist_bandwidth_assess,(void)) { uint64_t w,r; - r = find_largest_max(read_array); - w = find_largest_max(write_array); + int min_obs_time = get_options()->MinTimeToReportBandwidth; + r = find_largest_max(read_array, min_obs_time); + w = find_largest_max(write_array, min_obs_time); if (r>w) return (int)(((double)w)/NUM_SECS_ROLLING_MEASURE); else diff --git a/src/feature/stats/bwhist.h b/src/feature/stats/bwhist.h index f88b951447..01055df720 100644 --- a/src/feature/stats/bwhist.h +++ b/src/feature/stats/bwhist.h @@ -28,7 +28,7 @@ int bwhist_load_state(struct or_state_t *state, char **err); #ifdef BWHIST_PRIVATE typedef struct bw_array_t bw_array_t; -STATIC uint64_t find_largest_max(bw_array_t *b); +STATIC uint64_t find_largest_max(bw_array_t *b, int min_observation_time); STATIC void commit_max(bw_array_t *b); STATIC void advance_obs(bw_array_t *b); STATIC bw_array_t *bw_array_new(void); diff --git a/src/test/test_relay.c b/src/test/test_relay.c index b287f0d38b..8ed29b6282 100644 --- a/src/test/test_relay.c +++ b/src/test/test_relay.c @@ -98,7 +98,7 @@ test_relay_close_circuit(void *arg) tt_int_op(new_count, OP_EQ, old_count + 1); /* Ensure our write totals are 0 */ - tt_u64_op(find_largest_max(write_array), OP_EQ, 0); + tt_u64_op(find_largest_max(write_array, 86400), OP_EQ, 0); /* Mark the circuit for close */ circuit_mark_for_close(TO_CIRCUIT(orcirc), 0); @@ -107,7 +107,7 @@ test_relay_close_circuit(void *arg) advance_obs(write_array); commit_max(write_array); /* Check for two cells plus overhead */ - tt_u64_op(find_largest_max(write_array), OP_EQ, + tt_u64_op(find_largest_max(write_array, 86400), OP_EQ, 2*(get_cell_network_size(nchan->wide_circ_ids) +TLS_PER_CELL_OVERHEAD)); -- cgit v1.2.3-54-g00ecf From 9d7fca2306dc09097a24e225de59fb1ade7c9e34 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 11 May 2021 15:49:00 -0400 Subject: Make MinTimeToReportBandwidth a testing-only option (and rename it) --- changes/ticket40337 | 5 +++-- doc/man/tor.1.txt | 12 +++++------- src/app/config/config.c | 10 ++-------- src/app/config/config.h | 7 ------- src/app/config/or_options_st.h | 2 +- src/app/config/testnet.inc | 2 +- src/feature/stats/bwhist.c | 2 +- 7 files changed, 13 insertions(+), 27 deletions(-) diff --git a/changes/ticket40337 b/changes/ticket40337 index f8f136a740..02ea29ffc1 100644 --- a/changes/ticket40337 +++ b/changes/ticket40337 @@ -1,5 +1,6 @@ - o Minor features (bandwidth reporting): - - Relays can now use the MinTimeToReportBandwidth option to change + o Minor features (testing): + - On a testing network, relays can now use the + TestingMinTimeToReportBandwidth option to change the smallest amount of time over which they're willing to report their observed maximum bandwidth. Previously, this was fixed at 1 day. For safety, values under 2 hours are only supported on diff --git a/doc/man/tor.1.txt b/doc/man/tor.1.txt index af4ee494ef..209900832f 100644 --- a/doc/man/tor.1.txt +++ b/doc/man/tor.1.txt @@ -2448,13 +2448,6 @@ is non-zero): If we have more onionskins queued for processing than we can process in this amount of time, reject new ones. (Default: 1750 msec) -[[MinTimeToReportBandwidth]] **MinTimeToReportBandwidth** __N__ **seconds**|**minutes**|**hours**:: - Do not report our measurements for our maximum observed bandwidth for any - time period that has lasted for less than this amount of time. - Setting this option too low can enable traffic analysis, and is - not permitted except on testing networks. Values over 1 day have - no effect. (Default: 1 day) - [[MyFamily]] **MyFamily** __fingerprint__,__fingerprint__,...:: Declare that this Tor relay is controlled or administered by a group or organization identical or similar to that of the other relays, defined by @@ -3602,6 +3595,11 @@ The following options are used for running a testing Tor network. Minimum value for the Fast flag. Overrides the ordinary minimum taken from the consensus when TestingTorNetwork is set. (Default: 0.) +[[TestingMinTimeToReportBandwidth]] **TestingMinTimeToReportBandwidth** __N__ **seconds**|**minutes**|**hours**:: + Do not report our measurements for our maximum observed bandwidth for any + time period that has lasted for less than this amount of time. + Values over 1 day have no effect. (Default: 1 day) + [[TestingServerConsensusDownloadInitialDelay]] **TestingServerConsensusDownloadInitialDelay** __N__:: Initial delay in seconds for when servers should download consensuses. Changing this requires that **TestingTorNetwork** is set. (Default: 0) diff --git a/src/app/config/config.c b/src/app/config/config.c index 9011f36735..88da7a0aa7 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -563,7 +563,7 @@ static const config_var_t option_vars_[] = { V(MaxUnparseableDescSizeToLog, MEMUNIT, "10 MB"), VPORT(MetricsPort), V(MetricsPortPolicy, LINELIST, NULL), - V(MinTimeToReportBandwidth, INTERVAL, "1 day"), + V(TestingMinTimeToReportBandwidth, INTERVAL, "1 day"), VAR("MyFamily", LINELIST, MyFamily_lines, NULL), V(NewCircuitPeriod, INTERVAL, "30 seconds"), OBSOLETE("NamingAuthoritativeDirectory"), @@ -3712,13 +3712,6 @@ options_validate_cb(const void *old_options_, void *options_, char **msg) options->HeartbeatPeriod = MIN_HEARTBEAT_PERIOD; } - if (options->MinTimeToReportBandwidth < MIN_MIN_TIME_TO_REPORT_BW && - !options->TestingTorNetwork) { - log_warn(LD_CONFIG, "MinTimeToReportBandwidth is too short; " - "raising to %d seconds.", MIN_MIN_TIME_TO_REPORT_BW); - options->MinTimeToReportBandwidth = MIN_MIN_TIME_TO_REPORT_BW; - } - if (options->KeepalivePeriod < 1) REJECT("KeepalivePeriod option must be positive."); @@ -3994,6 +3987,7 @@ options_validate_cb(const void *old_options_, void *options_, char **msg) CHECK_DEFAULT(TestingSigningKeySlop); CHECK_DEFAULT(TestingAuthKeySlop); CHECK_DEFAULT(TestingLinkKeySlop); + CHECK_DEFAULT(TestingMinTimeToReportBandwidth); or_options_free(dflt_options); } #undef CHECK_DEFAULT diff --git a/src/app/config/config.h b/src/app/config/config.h index 74e6942eb5..e95ef4a728 100644 --- a/src/app/config/config.h +++ b/src/app/config/config.h @@ -24,13 +24,6 @@ * expose more information than we're comfortable with. */ #define MIN_HEARTBEAT_PERIOD (30*60) -/** - * Lowest allowable value for MinTimeToReportBandwidth on a non-testing - * network; if this is too low we might report detail that is too - * fine-grained. - **/ -#define MIN_MIN_TIME_TO_REPORT_BW (2*60*60) - /** Maximum default value for MaxMemInQueues, in bytes. */ #if SIZEOF_VOID_P >= 8 #define MAX_DEFAULT_MEMORY_QUEUE_SIZE (UINT64_C(8) << 30) diff --git a/src/app/config/or_options_st.h b/src/app/config/or_options_st.h index efecc85d66..440c987365 100644 --- a/src/app/config/or_options_st.h +++ b/src/app/config/or_options_st.h @@ -1084,7 +1084,7 @@ struct or_options_t { /** How far must we be into the current bandwidth-measurement period to * report bandwidth observations from this period? */ - int MinTimeToReportBandwidth; + int TestingMinTimeToReportBandwidth; /** * Configuration objects for individual modules. diff --git a/src/app/config/testnet.inc b/src/app/config/testnet.inc index 527e0d00b1..039454a0d0 100644 --- a/src/app/config/testnet.inc +++ b/src/app/config/testnet.inc @@ -19,7 +19,7 @@ { "TestingV3AuthInitialDistDelay", "20 seconds" }, { "TestingAuthDirTimeToLearnReachability", "0 minutes" }, { "MinUptimeHidServDirectoryV2", "0 minutes" }, -{ "MinTimeToReportBandwidth", "0 seconds" }, +{ "TestingMinTimeToReportBandwidth", "0 seconds" }, { "TestingServerDownloadInitialDelay", "0" }, { "TestingClientDownloadInitialDelay", "0" }, { "TestingServerConsensusDownloadInitialDelay", "0" }, diff --git a/src/feature/stats/bwhist.c b/src/feature/stats/bwhist.c index 55a8f7c747..06ad48e5c3 100644 --- a/src/feature/stats/bwhist.c +++ b/src/feature/stats/bwhist.c @@ -241,7 +241,7 @@ MOCK_IMPL(int, bwhist_bandwidth_assess,(void)) { uint64_t w,r; - int min_obs_time = get_options()->MinTimeToReportBandwidth; + int min_obs_time = get_options()->TestingMinTimeToReportBandwidth; r = find_largest_max(read_array, min_obs_time); w = find_largest_max(write_array, min_obs_time); if (r>w) -- cgit v1.2.3-54-g00ecf From 265cca935aefd1a3bdbe6abc964858e10fb4a029 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 11 May 2021 15:54:14 -0400 Subject: Ignore MAX_BANDWIDTH_CHANGE_FREQ on testing networks. Part of the ever-growing 40337 fix. --- changes/ticket40337 | 3 +++ src/feature/relay/router.c | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/changes/ticket40337 b/changes/ticket40337 index 02ea29ffc1..1c86fc4c99 100644 --- a/changes/ticket40337 +++ b/changes/ticket40337 @@ -11,3 +11,6 @@ immediately from startup. Previously, they waited until they had been running for a full day. Closes ticket 40337. + - Relays on testing networks no longer rate-limit how frequently + they are willing to report new bandwidth measurements. Part of a fix + for ticket 40337. diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 2696b8633b..c95f36aa8b 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -2599,7 +2599,10 @@ check_descriptor_bandwidth_changed(time_t now) if ((prev != cur && (!prev || !cur)) || cur > (prev * BANDWIDTH_CHANGE_FACTOR) || cur < (prev / BANDWIDTH_CHANGE_FACTOR) ) { - if (last_changed+MAX_BANDWIDTH_CHANGE_FREQ < now || !prev) { + const bool change_recent_enough = + last_changed+MAX_BANDWIDTH_CHANGE_FREQ < now; + const bool testing_network = get_options()->TestingTorNetwork; + if (change_recent_enough || testing_network || !prev) { log_info(LD_GENERAL, "Measured bandwidth has changed; rebuilding descriptor."); mark_my_descriptor_dirty("bandwidth has changed"); -- cgit v1.2.3-54-g00ecf