From 8527a2996675d9502551ccdd4224036a45aae47b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 23 Sep 2014 08:34:22 -0400 Subject: [PATCH 1/3] Add an "AccountingRule" feature to permit limiting bw usage by read+write Patch from "chobe". Closes ticket 961. --- doc/tor.1.txt | 38 ++++++++++++------- src/or/config.c | 5 +++ src/or/hibernate.c | 50 +++++++++++++++++-------- src/or/hibernate.h | 1 + src/or/or.h | 4 ++ src/or/router.c | 7 +++- src/or/status.c | 7 +++- src/test/include.am | 1 + src/test/test.c | 2 + src/test/test_accounting.c | 76 ++++++++++++++++++++++++++++++++++++++ 10 files changed, 161 insertions(+), 30 deletions(-) create mode 100644 src/test/test_accounting.c diff --git a/doc/tor.1.txt b/doc/tor.1.txt index d6b14329d1..c9172b85be 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -1635,19 +1635,31 @@ is non-zero): to 0 will disable the heartbeat. (Default: 6 hours) [[AccountingMax]] **AccountingMax** __N__ **bytes**|**KBytes**|**MBytes**|**GBytes**|**KBits**|**MBits**|**GBits**|**TBytes**:: - Never send more than the specified number of bytes in a given accounting - period, or receive more than that number in the period. For example, with - AccountingMax set to 1 GByte, a server could send 900 MBytes and - receive 800 MBytes and continue running. It will only hibernate once - one of the two reaches 1 GByte. When the number of bytes gets low, - Tor will stop accepting new connections and circuits. When the - number of bytes is exhausted, Tor will hibernate until some - time in the next accounting period. To prevent all servers from waking at - the same time, Tor will also wait until a random point in each period - before waking up. If you have bandwidth cost issues, enabling hibernation - is preferable to setting a low bandwidth, since it provides users with a - collection of fast servers that are up some of the time, which is more - useful than a set of slow servers that are always "available". + Limits the max number of bytes sent and received within a set time period + using a given calculation rule (see: AccountingStart, AccountingRule). + Useful if you need to stay under a specific bandwidth. By default, the + number used for calculation is the max of either the bytes sent or + received. For example, with AccountingMax set to 1 GByte, a server + could send 900 MBytes and receive 800 MBytes and continue running. + It will only hibernate once one of the two reaches 1 GByte. This can + be changed to use the sum of the both bytes received and sent by setting + the AccountingRule option to "sum" (total bandwidth in/out). When the + number of bytes remaining gets low, Tor will stop accepting new connections + and circuits. When the number of bytes is exhausted, Tor will hibernate + until some time in the next accounting period. To prevent all servers + from waking at the same time, Tor will also wait until a random point + in each period before waking up. If you have bandwidth cost issues, + enabling hibernation is preferable to setting a low bandwidth, since + it provides users with a collection of fast servers that are up some + of the time, which is more useful than a set of slow servers that are + always "available". + +[[AccountingRule]] **AccountingRule** **sum**|**max**:: + How we determine when our AccountingMax has been reached (when we + should hibernate) during a time interval. Set to "max" to calculate + using the higher of either the sent or received bytes (this is the + default functionality). Set to "sum" to calculate using the sent + plus received bytes. [[AccountingStart]] **AccountingStart** **day**|**week**|**month** [__day__] __HH:MM__:: Specify how long accounting periods last. If **month** is given, each diff --git a/src/or/config.c b/src/or/config.c index 16acec791c..c7c61549ae 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -126,6 +126,7 @@ static config_abbrev_t option_abbrevs_[] = { */ static config_var_t option_vars_[] = { V(AccountingMax, MEMUNIT, "0 bytes"), + V(AccountingRule, STRING, "max"), V(AccountingStart, STRING, NULL), V(Address, STRING, NULL), V(AllowDotExit, BOOL, "0"), @@ -3108,6 +3109,10 @@ options_validate(or_options_t *old_options, or_options_t *options, "risky: they will all turn off at the same time, which may " "alert observers that they are being run by the same party."); } + if (options->AccountingRule && + strcmp(options->AccountingRule, "sum") != 0 && + strcmp(options->AccountingRule, "max") != 0) + REJECT("AccountingRule must be 'sum' or 'max'"); } if (options->HTTPProxy) { /* parse it now */ diff --git a/src/or/hibernate.c b/src/or/hibernate.c index c433ac1be9..6978f47aef 100644 --- a/src/or/hibernate.c +++ b/src/or/hibernate.c @@ -410,6 +410,16 @@ configure_accounting(time_t now) accounting_set_wakeup_time(); } +/** Return the relevant number of bytes sent/received this interval + * based on the set AccountingRule */ +static uint64_t +get_accounting_bytes(void) +{ + if (strcmp(get_options()->AccountingRule, "sum") == 0) + return n_bytes_read_in_interval+n_bytes_written_in_interval; + return MAX(n_bytes_read_in_interval, n_bytes_written_in_interval); +} + /** Set expected_bandwidth_usage based on how much we sent/received * per minute last interval (if we were up for at least 30 minutes), * or based on our declared bandwidth otherwise. */ @@ -421,6 +431,11 @@ update_expected_bandwidth(void) uint64_t max_configured = (options->RelayBandwidthRate > 0 ? options->RelayBandwidthRate : options->BandwidthRate) * 60; + /* max_configured is the larger of bytes read and bytes written + * If we are accounting based on sum, worst case is both are + * at max, doubling the expected sum of bandwidth */ + if (strcmp(get_options()->AccountingRule, "sum") == 0) + max_configured *= 2; #define MIN_TIME_FOR_MEASUREMENT (1800) @@ -439,8 +454,7 @@ update_expected_bandwidth(void) * doesn't know to store soft-limit info. Just take rate at which * we were reading/writing in the last interval as our expected rate. */ - uint64_t used = MAX(n_bytes_written_in_interval, - n_bytes_read_in_interval); + uint64_t used = get_accounting_bytes(); expected = used / (n_seconds_active_in_interval / 60); } else { /* If we haven't gotten enough data last interval, set 'expected' @@ -715,8 +729,7 @@ hibernate_hard_limit_reached(void) uint64_t hard_limit = get_options()->AccountingMax; if (!hard_limit) return 0; - return n_bytes_read_in_interval >= hard_limit - || n_bytes_written_in_interval >= hard_limit; + return get_accounting_bytes() >= hard_limit; } /** Return true iff we have sent/received almost all the bytes we are willing @@ -747,8 +760,7 @@ hibernate_soft_limit_reached(void) if (!soft_limit) return 0; - return n_bytes_read_in_interval >= soft_limit - || n_bytes_written_in_interval >= soft_limit; + return get_accounting_bytes() >= soft_limit; } /** Called when we get a SIGINT, or when bandwidth soft limit is @@ -772,8 +784,7 @@ hibernate_begin(hibernate_state_t new_state, time_t now) hibernate_state == HIBERNATE_STATE_LIVE) { soft_limit_hit_at = now; n_seconds_to_hit_soft_limit = n_seconds_active_in_interval; - n_bytes_at_soft_limit = MAX(n_bytes_read_in_interval, - n_bytes_written_in_interval); + n_bytes_at_soft_limit = get_accounting_bytes(); } /* close listeners. leave control listener(s). */ @@ -1003,13 +1014,22 @@ getinfo_helper_accounting(control_connection_t *conn, U64_PRINTF_ARG(n_bytes_written_in_interval)); } else if (!strcmp(question, "accounting/bytes-left")) { uint64_t limit = get_options()->AccountingMax; - uint64_t read_left = 0, write_left = 0; - if (n_bytes_read_in_interval < limit) - read_left = limit - n_bytes_read_in_interval; - if (n_bytes_written_in_interval < limit) - write_left = limit - n_bytes_written_in_interval; - tor_asprintf(answer, U64_FORMAT" "U64_FORMAT, - U64_PRINTF_ARG(read_left), U64_PRINTF_ARG(write_left)); + if (strcmp(get_options()->AccountingRule, "sum") == 0) { + uint64_t total_left = 0; + uint64_t total_bytes = get_accounting_bytes(); + if (total_bytes < limit) + total_left = limit - total_bytes; + tor_asprintf(answer, U64_FORMAT" "U64_FORMAT, + U64_PRINTF_ARG(total_left), U64_PRINTF_ARG(total_left)); + } else { + uint64_t read_left = 0, write_left = 0; + if (n_bytes_read_in_interval < limit) + read_left = limit - n_bytes_read_in_interval; + if (n_bytes_written_in_interval < limit) + write_left = limit - n_bytes_written_in_interval; + tor_asprintf(answer, U64_FORMAT" "U64_FORMAT, + U64_PRINTF_ARG(read_left), U64_PRINTF_ARG(write_left)); + } } else if (!strcmp(question, "accounting/interval-start")) { *answer = tor_malloc(ISO_TIME_LEN+1); format_iso_time(*answer, interval_start_time); diff --git a/src/or/hibernate.h b/src/or/hibernate.h index 38ecb75129..799b582543 100644 --- a/src/or/hibernate.h +++ b/src/or/hibernate.h @@ -28,6 +28,7 @@ void consider_hibernation(time_t now); int getinfo_helper_accounting(control_connection_t *conn, const char *question, char **answer, const char **errmsg); +uint64_t get_accounting_max_total(void); #ifdef HIBERNATE_PRIVATE /** Possible values of hibernate_state */ diff --git a/src/or/or.h b/src/or/or.h index b2b0d5f7ab..20cfa5be36 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3775,6 +3775,10 @@ typedef struct { uint64_t AccountingMax; /**< How many bytes do we allow per accounting * interval before hibernation? 0 for "never * hibernate." */ + char *AccountingRule; /**< How do we determine when our AccountingMax + * has been reached? + * "max" for when in or out reaches AccountingMax + * "sum for when in plus out reaches AccountingMax */ /** Base64-encoded hash of accepted passwords for the control system. */ config_line_t *HashedControlPassword; diff --git a/src/or/router.c b/src/or/router.c index 4af8d262f9..96d16bb326 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1080,6 +1080,7 @@ decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port) * they're confused or to get statistics. */ int interval_length = accounting_get_interval_length(); uint32_t effective_bw = get_effective_bwrate(options); + uint64_t acc_bytes; if (!interval_length) { log_warn(LD_BUG, "An accounting interval is not allowed to be zero " "seconds long. Raising to 1."); @@ -1090,8 +1091,12 @@ decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port) "accounting interval length %d", effective_bw, U64_PRINTF_ARG(options->AccountingMax), interval_length); + + acc_bytes = options->AccountingMax; + if (strcmp(options->AccountingRule, "sum") == 0) + acc_bytes /= 2; if (effective_bw >= - options->AccountingMax / interval_length) { + acc_bytes / interval_length) { new_choice = 0; reason = "AccountingMax enabled"; } diff --git a/src/or/status.c b/src/or/status.c index c4156d0cc3..4158df5ad5 100644 --- a/src/or/status.c +++ b/src/or/status.c @@ -145,10 +145,15 @@ log_accounting(const time_t now, const or_options_t *options) or_state_t *state = get_or_state(); char *acc_rcvd = bytes_to_usage(state->AccountingBytesReadInInterval); char *acc_sent = bytes_to_usage(state->AccountingBytesWrittenInInterval); - char *acc_max = bytes_to_usage(options->AccountingMax); + const char *acc_rule = options->AccountingRule; + uint64_t acc_bytes = options->AccountingMax; + char *acc_max; time_t interval_end = accounting_get_end_time(); char end_buf[ISO_TIME_LEN + 1]; char *remaining = NULL; + if (strcmp(acc_rule, "sum") == 0) + acc_bytes *= 2; + acc_max = bytes_to_usage(acc_bytes); format_local_iso_time(end_buf, interval_end); remaining = secs_to_uptime(interval_end - now); diff --git a/src/test/include.am b/src/test/include.am index 77c92f12f8..03755a4fa0 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -34,6 +34,7 @@ src_test_test_SOURCES = \ src/test/test_logging.c \ src/test/test_microdesc.c \ src/test/test_oom.c \ + src/test/test_accounting.c \ src/test/test_options.c \ src/test/test_pt.c \ src/test/test_relaycell.c \ diff --git a/src/test/test.c b/src/test/test.c index cfbe203d2e..d2813ed42a 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1305,6 +1305,7 @@ extern struct testcase_t hs_tests[]; extern struct testcase_t nodelist_tests[]; extern struct testcase_t routerkeys_tests[]; extern struct testcase_t oom_tests[]; +extern struct testcase_t accounting_tests[]; extern struct testcase_t policy_tests[]; extern struct testcase_t status_tests[]; extern struct testcase_t routerset_tests[]; @@ -1337,6 +1338,7 @@ static struct testgroup_t testgroups[] = { { "nodelist/", nodelist_tests }, { "routerkeys/", routerkeys_tests }, { "oom/", oom_tests }, + { "accounting/", accounting_tests }, { "policy/" , policy_tests }, { "status/" , status_tests }, { "routerset/" , routerset_tests }, diff --git a/src/test/test_accounting.c b/src/test/test_accounting.c new file mode 100644 index 0000000000..a1a2ec643e --- /dev/null +++ b/src/test/test_accounting.c @@ -0,0 +1,76 @@ +#include "or.h" +#include "test.h" +#define HIBERNATE_PRIVATE +#include "hibernate.h" +#include "config.h" +#define STATEFILE_PRIVATE +#include "statefile.h" + +#define NS_MODULE accounting + +#define NS_SUBMODULE limits + +/* + * Test to make sure accounting triggers hibernation + * correctly with both sum or max rules set + */ + +static or_state_t *or_state; +NS_DECL(or_state_t *, get_or_state, (void)); +static or_state_t * +NS(get_or_state)(void) +{ + return or_state; +} + +static void +test_accounting_limits(void *arg) +{ + or_options_t *options = get_options_mutable(); + time_t fake_time = time(NULL); + (void) arg; + + NS_MOCK(get_or_state); + or_state = or_state_new(); + + options->AccountingMax = 100; + options->AccountingRule = tor_strdup("max"); + + tor_assert(accounting_is_enabled(options)); + configure_accounting(fake_time); + + accounting_add_bytes(10, 0, 1); + fake_time += 1; + consider_hibernation(fake_time); + tor_assert(we_are_hibernating() == 0); + + accounting_add_bytes(90, 0, 1); + fake_time += 1; + consider_hibernation(fake_time); + tor_assert(we_are_hibernating() == 1); + + options->AccountingMax = 200; + options->AccountingRule = tor_strdup("sum"); + + accounting_add_bytes(0, 10, 1); + fake_time += 1; + consider_hibernation(fake_time); + tor_assert(we_are_hibernating() == 0); + + accounting_add_bytes(0, 90, 1); + fake_time += 1; + consider_hibernation(fake_time); + tor_assert(we_are_hibernating() == 1); + goto done; + done: + NS_UNMOCK(get_or_state); + or_state_free(or_state); +} + +#undef NS_SUBMODULE + +struct testcase_t accounting_tests[] = { + { "bwlimits", test_accounting_limits, TT_FORK, NULL, NULL }, + END_OF_TESTCASES +}; + From 4903ab1caaf0b2631e6091b58b31eaff0c9f8424 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 23 Sep 2014 08:46:35 -0400 Subject: [PATCH 2/3] Avoid frequent strcmp() calls for AccountingRule Generally, we don't like to parse the same thing over and over; it's best IMO to do it once at the start of the code. --- src/or/config.c | 14 ++++++++++---- src/or/hibernate.c | 9 +++++---- src/or/or.h | 9 +++++---- src/or/router.c | 2 +- src/or/status.c | 3 +-- src/test/test_accounting.c | 4 ++-- 6 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index c7c61549ae..8035afbc79 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -126,7 +126,7 @@ static config_abbrev_t option_abbrevs_[] = { */ static config_var_t option_vars_[] = { V(AccountingMax, MEMUNIT, "0 bytes"), - V(AccountingRule, STRING, "max"), + VAR("AccountingRule", STRING, AccountingRule_option, "max"), V(AccountingStart, STRING, NULL), V(Address, STRING, NULL), V(AllowDotExit, BOOL, "0"), @@ -3109,9 +3109,15 @@ options_validate(or_options_t *old_options, or_options_t *options, "risky: they will all turn off at the same time, which may " "alert observers that they are being run by the same party."); } - if (options->AccountingRule && - strcmp(options->AccountingRule, "sum") != 0 && - strcmp(options->AccountingRule, "max") != 0) + } + + options->AccountingRule = ACCT_MAX; + if (options->AccountingRule_option) { + if (!strcmp(options->AccountingRule_option, "sum")) + options->AccountingRule = ACCT_SUM; + else if (!strcmp(options->AccountingRule_option, "max")) + options->AccountingRule = ACCT_MAX; + else REJECT("AccountingRule must be 'sum' or 'max'"); } diff --git a/src/or/hibernate.c b/src/or/hibernate.c index 6978f47aef..b3761cfabf 100644 --- a/src/or/hibernate.c +++ b/src/or/hibernate.c @@ -415,9 +415,10 @@ configure_accounting(time_t now) static uint64_t get_accounting_bytes(void) { - if (strcmp(get_options()->AccountingRule, "sum") == 0) + if (get_options()->AccountingRule == ACCT_SUM) return n_bytes_read_in_interval+n_bytes_written_in_interval; - return MAX(n_bytes_read_in_interval, n_bytes_written_in_interval); + else + return MAX(n_bytes_read_in_interval, n_bytes_written_in_interval); } /** Set expected_bandwidth_usage based on how much we sent/received @@ -434,7 +435,7 @@ update_expected_bandwidth(void) /* max_configured is the larger of bytes read and bytes written * If we are accounting based on sum, worst case is both are * at max, doubling the expected sum of bandwidth */ - if (strcmp(get_options()->AccountingRule, "sum") == 0) + if (get_options()->AccountingRule == ACCT_SUM) max_configured *= 2; #define MIN_TIME_FOR_MEASUREMENT (1800) @@ -1014,7 +1015,7 @@ getinfo_helper_accounting(control_connection_t *conn, U64_PRINTF_ARG(n_bytes_written_in_interval)); } else if (!strcmp(question, "accounting/bytes-left")) { uint64_t limit = get_options()->AccountingMax; - if (strcmp(get_options()->AccountingRule, "sum") == 0) { + if (get_options()->AccountingRule == ACCT_SUM) { uint64_t total_left = 0; uint64_t total_bytes = get_accounting_bytes(); if (total_bytes < limit) diff --git a/src/or/or.h b/src/or/or.h index 20cfa5be36..54cee46ee3 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3775,10 +3775,11 @@ typedef struct { uint64_t AccountingMax; /**< How many bytes do we allow per accounting * interval before hibernation? 0 for "never * hibernate." */ - char *AccountingRule; /**< How do we determine when our AccountingMax - * has been reached? - * "max" for when in or out reaches AccountingMax - * "sum for when in plus out reaches AccountingMax */ + /** How do we determine when our AccountingMax has been reached? + * "max" for when in or out reaches AccountingMax + * "sum for when in plus out reaches AccountingMax */ + char *AccountingRule_option; + enum { ACCT_MAX, ACCT_SUM } AccountingRule; /** Base64-encoded hash of accepted passwords for the control system. */ config_line_t *HashedControlPassword; diff --git a/src/or/router.c b/src/or/router.c index 96d16bb326..dbe985ac78 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1093,7 +1093,7 @@ decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port) interval_length); acc_bytes = options->AccountingMax; - if (strcmp(options->AccountingRule, "sum") == 0) + if (get_options()->AccountingRule == ACCT_SUM) acc_bytes /= 2; if (effective_bw >= acc_bytes / interval_length) { diff --git a/src/or/status.c b/src/or/status.c index 4158df5ad5..daae1d71c6 100644 --- a/src/or/status.c +++ b/src/or/status.c @@ -145,13 +145,12 @@ log_accounting(const time_t now, const or_options_t *options) or_state_t *state = get_or_state(); char *acc_rcvd = bytes_to_usage(state->AccountingBytesReadInInterval); char *acc_sent = bytes_to_usage(state->AccountingBytesWrittenInInterval); - const char *acc_rule = options->AccountingRule; uint64_t acc_bytes = options->AccountingMax; char *acc_max; time_t interval_end = accounting_get_end_time(); char end_buf[ISO_TIME_LEN + 1]; char *remaining = NULL; - if (strcmp(acc_rule, "sum") == 0) + if (options->AccountingRule == ACCT_SUM) acc_bytes *= 2; acc_max = bytes_to_usage(acc_bytes); format_local_iso_time(end_buf, interval_end); diff --git a/src/test/test_accounting.c b/src/test/test_accounting.c index a1a2ec643e..25908e942c 100644 --- a/src/test/test_accounting.c +++ b/src/test/test_accounting.c @@ -34,7 +34,7 @@ test_accounting_limits(void *arg) or_state = or_state_new(); options->AccountingMax = 100; - options->AccountingRule = tor_strdup("max"); + options->AccountingRule = ACCT_MAX; tor_assert(accounting_is_enabled(options)); configure_accounting(fake_time); @@ -50,7 +50,7 @@ test_accounting_limits(void *arg) tor_assert(we_are_hibernating() == 1); options->AccountingMax = 200; - options->AccountingRule = tor_strdup("sum"); + options->AccountingRule = ACCT_SUM; accounting_add_bytes(0, 10, 1); fake_time += 1; From f15e5aedbc6298f4fad4ef6cff755591a44a7109 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 23 Sep 2014 08:57:22 -0400 Subject: [PATCH 3/3] Changes file for ticket 961 --- changes/ticket961 | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changes/ticket961 diff --git a/changes/ticket961 b/changes/ticket961 new file mode 100644 index 0000000000..018f26554d --- /dev/null +++ b/changes/ticket961 @@ -0,0 +1,5 @@ + o Minor features: + - A new AccountingRule option lets you set whether you'd like the + AccountingMax value to be applied separately to inbound and + outbound traffic, or applied to the sum of inbound and outbound + traffic. Resolves ticket 961. Patch by "chobe".