Merge branch 'ticket961_squashed'

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".

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
+    Limits the max number of bytes sent and received within a set time period
-    period, or receive more than that number in the period. For example, with
+    using a given calculation rule (see: AccountingStart, AccountingRule).
-    AccountingMax set to 1 GByte, a server could send 900 MBytes and
+    Useful if you need to stay under a specific bandwidth. By default, the
-    receive 800 MBytes and continue running. It will only hibernate once
+    number used for calculation is the max of either the bytes sent or 
-    one of the two reaches 1 GByte. When the number of bytes gets low,
+    received. For example, with AccountingMax set to 1 GByte, a server
-    Tor will stop accepting new connections and circuits.  When the
+    could send 900 MBytes and receive 800 MBytes and continue running.
-    number of bytes is exhausted, Tor will hibernate until some
+    It will only hibernate once one of the two reaches 1 GByte. This can
-    time in the next accounting period. To prevent all servers from waking at
+    be changed to use the sum of the both bytes received and sent by setting
-    the same time, Tor will also wait until a random point in each period
+    the AccountingRule option to "sum" (total bandwidth in/out). When the
-    before waking up. If you have bandwidth cost issues, enabling hibernation
+    number of bytes remaining gets low, Tor will stop accepting new connections
-    is preferable to setting a low bandwidth, since it provides users with a
+    and circuits. When the number of bytes is exhausted, Tor will hibernate
-    collection of fast servers that are up some of the time, which is more
+    until some time in the next accounting period. To prevent all servers
-    useful than a set of slow servers that are always "available".
+    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

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"),
+  VAR("AccountingRule",          STRING,   AccountingRule_option,  "max"),
   V(AccountingStart,             STRING,   NULL),
   V(Address,                     STRING,   NULL),
   V(AllowDotExit,                BOOL,     "0"),
@@ -3111,6 +3112,16 @@ options_validate(or_options_t *old_options, or_options_t *options,
     }
   }
 
+  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'");
+  }
+
   if (options->HTTPProxy) { /* parse it now */
     if (tor_addr_port_lookup(options->HTTPProxy,
                         &options->HTTPProxyAddr, &options->HTTPProxyPort) < 0)

src/or/hibernate.c

@@ -410,6 +410,17 @@ 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 (get_options()->AccountingRule == ACCT_SUM)
+    return 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
  * per minute last interval (if we were up for at least 30 minutes),
  * or based on our declared bandwidth otherwise. */
@@ -421,6 +432,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 (get_options()->AccountingRule == ACCT_SUM)
+    max_configured *= 2;
 
 #define MIN_TIME_FOR_MEASUREMENT (1800)
 
@@ -439,8 +455,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,
+    uint64_t used = get_accounting_bytes();
-                        n_bytes_read_in_interval);
     expected = used / (n_seconds_active_in_interval / 60);
   } else {
     /* If we haven't gotten enough data last interval, set 'expected'
@@ -715,8 +730,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
+  return get_accounting_bytes() >= hard_limit;
-    || n_bytes_written_in_interval >= hard_limit;
 }
 
 /** Return true iff we have sent/received almost all the bytes we are willing
@@ -747,8 +761,7 @@ hibernate_soft_limit_reached(void)
 
   if (!soft_limit)
     return 0;
-  return n_bytes_read_in_interval >= soft_limit
+  return get_accounting_bytes() >= soft_limit;
-    || n_bytes_written_in_interval >= soft_limit;
 }
 
 /** Called when we get a SIGINT, or when bandwidth soft limit is
@@ -772,8 +785,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_at_soft_limit = get_accounting_bytes();
-                                n_bytes_written_in_interval);
   }
 
   /* close listeners. leave control listener(s). */
@@ -1003,6 +1015,14 @@ 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 (get_options()->AccountingRule == ACCT_SUM) {
+      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;
@@ -1010,6 +1030,7 @@ getinfo_helper_accounting(control_connection_t *conn,
         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);

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 */

src/or/or.h

@@ -3775,6 +3775,11 @@ typedef struct {
   uint64_t AccountingMax; /**< How many bytes do we allow per accounting
                            * interval before hibernation?  0 for "never
                            * hibernate." */
+  /** 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;

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 (get_options()->AccountingRule == ACCT_SUM)
+      acc_bytes /= 2;
     if (effective_bw >=
-        options->AccountingMax / interval_length) {
+        acc_bytes / interval_length) {
       new_choice = 0;
       reason = "AccountingMax enabled";
     }

src/or/status.c

@@ -145,10 +145,14 @@ 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);
+  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 (options->AccountingRule == ACCT_SUM)
+    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);
 

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 \

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 },

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 = ACCT_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 = ACCT_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
+};
+