Merge branch 'ticket961_squashed'

9 files changed, 143 insertions, 17 deletions

diff --git a/src/or/config.c b/src/or/config.c
index b68990ca68..d620f585fe 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"),
+  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)
diff --git a/src/or/hibernate.c b/src/or/hibernate.c
index c433ac1be9..b3761cfabf 100644
--- a/src/or/hibernate.c
+++ b/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,
-                        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 +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
-    || 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 +761,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 +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_written_in_interval);
+    n_bytes_at_soft_limit = get_accounting_bytes();
   }
 
   /* close listeners. leave control listener(s). */
@@ -1003,13 +1015,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 (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;
+      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..54cee46ee3 100644
--- a/src/or/or.h
+++ b/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;
diff --git a/src/or/router.c b/src/or/router.c
index 4af8d262f9..dbe985ac78 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 (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";
     }
diff --git a/src/or/status.c b/src/or/status.c
index c4156d0cc3..daae1d71c6 100644
--- a/src/or/status.c
+++ b/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);
 
diff --git a/src/test/include.am b/src/test/include.am
index 5f88189222..4020ac4190 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..25908e942c
--- /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 = 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
+};
+