patch from mike perry to a) stop overloading guards as much, and

b) raise the max-believable-bandwidth to 10MB/s.


svn:r11258

doc/spec/path-spec.txt

@@ -182,21 +182,19 @@ of their choices.
    proportional to its advertised bandwidth [the smaller of the 'rate' and
    'observed' arguments to the "bandwidth" element in its descriptor].  If a
    router's advertised bandwidth is greater than MAX_BELIEVABLE_BANDWIDTH
-   (1.5 MB/s), we clip to that value.
+   (10 MB/s), we clip to that value.
 
    For non-exit positions on "fast" circuits, we pick routers as above, but
    we weight the clipped advertised bandwidth of Exit-flagged nodes depending
    on the fraction of bandwidth available from non-Exit nodes.  Call the
    total clipped advertised bandwidth for Exit nodes under consideration E,
-   and the total clipped advertised bandwidth for non-Exit nodes under
    and the total clipped advertised bandwidth for all nodes under
    consideration T.  If E<T/3, we do not consider Exit-flagged nodes.
-   Otherwise, we weight their bandwidth with the factor 1-T/(3E). This
+   Otherwise, we weight their bandwidth with the factor (E-T/3)/E. This 
    ensures that bandwidth is evenly distributed over nodes in 3-hop paths.
 
-   Otherwise, we weight their bandwidth with the factor (E-N/2)/(N+E-N/2) ==
-   (2E - N)/(2E + N).  This ensures that bandwidth is evenly distributed over
-   nodes in 3-hop paths.
+   Similarly, guard nodes are weighted by the factor (G-T/3)/G, and not
+   considered for non-guard positions if this value is less than 0.
 
    Additionally, we may be building circuits with one or more requests in
    mind.  Each kind of request puts certain constraints on paths:

src/or/circuitbuild.c

@@ -1264,7 +1264,7 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime,
     smartlist_subtract(sl,excludedexits);
     if (options->StrictExitNodes || smartlist_overlap(sl,preferredexits))
       smartlist_intersect(sl,preferredexits);
-    router = routerlist_sl_choose_by_bandwidth(sl, 1);
+    router = routerlist_sl_choose_by_bandwidth(sl, 1, 0);
   } else {
     /* Either there are no pending connections, or no routers even seem to
      * possibly support any of them.  Choose a router at random that satisfies
@@ -1308,7 +1308,7 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime,
         smartlist_intersect(sl,preferredexits);
         /* XXX sometimes the above results in null, when the requested
          * exit node is down. we should pick it anyway. */
-      router = routerlist_sl_choose_by_bandwidth(sl, 1);
+      router = routerlist_sl_choose_by_bandwidth(sl, 1, 0);
       if (router)
         break;
     }

src/or/config.c

@@ -4355,8 +4355,10 @@ or_state_validate(or_state_t *old_state, or_state_t *state,
       log_warn(LD_GENERAL, "Can't parse Tor version '%s' from your state "
                "file. Proceeding anyway.", state->TorVersion);
     } else { /* take action based on v */
-      if (tor_version_as_new_as(state->TorVersion, "0.1.1.10-alpha") &&
-          !tor_version_as_new_as(state->TorVersion, "0.1.1.16-rc-cvs")) {
+      if ((tor_version_as_new_as(state->TorVersion, "0.1.1.10-alpha") &&
+          !tor_version_as_new_as(state->TorVersion, "0.1.2.17-dev"))
+          || (tor_version_as_new_as(state->TorVersion, "0.2.0.0-alpha") &&
+          !tor_version_as_new_as(state->TorVersion, "0.2.0.6-alpha-dev"))) {
         log_notice(LD_CONFIG, "Detected state file from buggy version '%s'. "
                    "Enabling workaround to choose working entry guards.",
                    state->TorVersion);

src/or/or.h

@@ -3404,7 +3404,8 @@ routerinfo_t *router_find_exact_exit_enclave(const char *address,
 int router_is_unreliable(routerinfo_t *router, int need_uptime,
                          int need_capacity, int need_guard);
 uint32_t router_get_advertised_bandwidth(routerinfo_t *router);
-routerinfo_t *routerlist_sl_choose_by_bandwidth(smartlist_t *sl, int for_exit);
+routerinfo_t *routerlist_sl_choose_by_bandwidth(smartlist_t *sl, int for_exit,
+                                                int for_guard);
 routerstatus_t *routerstatus_sl_choose_by_bandwidth(smartlist_t *sl);
 
 routerinfo_t *router_choose_random_node(const char *preferred,

src/or/rephist.c

@@ -66,7 +66,6 @@ typedef struct or_history_t {
   /** If nonzero, we have been unable to connect since this time. */
   time_t down_since;
 
-
   /* === For MTBF tracking: */
   /** Weighted sum total of all times that this router has been online.
    */

src/or/routerlist.c

@@ -1262,7 +1262,7 @@ router_get_advertised_bandwidth(routerinfo_t *router)
 
 /** Do not weight any declared bandwidth more than this much when picking
  * routers by bandwidth. */
-#define MAX_BELIEVABLE_BANDWIDTH 1500000 /* 1.5 MB/sec */
+#define MAX_BELIEVABLE_BANDWIDTH 10000000 /* 10 MB/sec */
 
 /** Helper function:
  * choose a random element of smartlist <b>sl</b>, weighted by
@@ -1272,24 +1272,38 @@ router_get_advertised_bandwidth(routerinfo_t *router)
  * routerinfo_t's. Otherwise it's a list of routerstatus_t's.
  *
  * If <b>for_exit</b>, we're picking an exit node: consider all nodes'
- * bandwidth equally regardless of their Exit status.  If not <b>for_exit</b>,
+ * bandwidth equally regardless of their Exit status, since there may be
+ * some in the list because they exit to obscure ports. If not <b>for_exit</b>,
  * we're picking a non-exit node: weight exit-node's bandwidth less
  * depending on the smallness of the fraction of Exit-to-total bandwidth.
+ * Beware: this flag is often abused to force uniform selection when
+ * we are not actually choosing exits!
+ *
+ * If <b>for_guard</b>, we're picking a guard node: consider all guard's
+ * bandwidth equally. Otherwise, weight guards proportionally less.
+ *
  */
 static void *
-smartlist_choose_by_bandwidth(smartlist_t *sl, int for_exit, int statuses)
+smartlist_choose_by_bandwidth(smartlist_t *sl, int for_exit, int for_guard,
+                              int statuses)
 {
   unsigned int i;
   routerinfo_t *router;
-  routerstatus_t *status;
+  routerstatus_t *status=NULL;
   int32_t *bandwidths;
   int is_exit;
+  int is_guard;
   uint64_t total_nonexit_bw = 0, total_exit_bw = 0, total_bw = 0;
+  uint64_t total_nonguard_bw = 0, total_guard_bw = 0;
   uint64_t rand_bw, tmp;
   double exit_weight;
+  double guard_weight;
   int n_unknown = 0;
   bitarray_t *exit_bits;
-  int include_exits = 1;
+  bitarray_t *guard_bits;
+
+  /* Can't choose exit and guard at same time */
+  tor_assert(!(for_exit && for_guard));
 
   /* First count the total bandwidth weight, and make a list
    * of each value.  <0 means "unknown; no routerinfo."  We use the
@@ -1297,6 +1311,7 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, int for_exit, int statuses)
    * and whether it was an exit (-x)&2.  Yes, it's a hack. */
   bandwidths = tor_malloc(sizeof(int32_t)*smartlist_len(sl));
   exit_bits = bitarray_init_zero(smartlist_len(sl));
+  guard_bits = bitarray_init_zero(smartlist_len(sl));
 
   /* Iterate over all the routerinfo_t or routerstatus_t, and */
   for (i = 0; i < (unsigned)smartlist_len(sl); ++i) {
@@ -1309,25 +1324,48 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, int for_exit, int statuses)
       status = smartlist_get(sl, i);
       router = router_get_by_digest(status->identity_digest);
       is_exit = status->is_exit;
+      is_guard = status->is_possible_guard;
       if (router) {
         this_bw = router_get_advertised_bandwidth(router);
       } else { /* guess */
         is_known = 0;
         flags = status->is_fast ? 1 : 0;
         flags |= is_exit ? 2 : 0;
+        flags |= is_guard ? 4 : 0;
       }
     } else {
       router = smartlist_get(sl, i);
       is_exit = router->is_exit;
+      is_guard = router->is_possible_guard;
       this_bw = router_get_advertised_bandwidth(router);
     }
     if (is_exit)
       bitarray_set(exit_bits, i);
+    if (is_guard)
+      bitarray_set(guard_bits, i);
     /* if they claim something huge, don't believe it */
-    if (this_bw > MAX_BELIEVABLE_BANDWIDTH)
+    if (this_bw > MAX_BELIEVABLE_BANDWIDTH) {
+      char fp[HEX_DIGEST_LEN+1];
+      if (status) {
+          base16_encode(fp, sizeof(fp),
+                  status->identity_digest, DIGEST_LEN);
+      } else if (router) {
+          base16_encode(fp, sizeof(fp),
+                  router->cache_info.identity_digest, DIGEST_LEN);
+      }
+      log_notice(LD_DIR,
+          "Bandwidth %d for router %s (%s) exceeds allowed max %d, capping",
+          this_bw, router ? router->nickname : "(null)",
+          status || router ? fp : "0",
+          MAX_BELIEVABLE_BANDWIDTH);
       this_bw = MAX_BELIEVABLE_BANDWIDTH;
+    }
     if (is_known) {
       bandwidths[i] = (int32_t) this_bw; // safe since MAX_BELIEVABLE<INT32_MAX
+      if (is_guard)
+        total_guard_bw += this_bw;
+      else
+        total_nonguard_bw += this_bw;
       if (is_exit)
         total_exit_bw += this_bw;
       else
@@ -1356,55 +1394,78 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, int for_exit, int statuses)
       if (bw>=0)
         continue;
       is_exit = ((-bw)&2);
+      is_guard = ((-bw)&4);
       bandwidths[i] = ((-bw)&1) ? avg_fast : avg_slow;
       if (is_exit)
         total_exit_bw += bandwidths[i];
       else
         total_nonexit_bw += bandwidths[i];
+      if (is_guard)
+        total_guard_bw += bandwidths[i];
+      else
+        total_nonguard_bw += bandwidths[i];
     }
   }
 
   /* If there's no bandwidth at all, pick at random. */
   if (!(total_exit_bw+total_nonexit_bw)) {
     tor_free(bandwidths);
+    tor_free(exit_bits);
+    tor_free(guard_bits);
     return smartlist_choose(sl);
   }
 
-  /* Figure out how to weight exits. */
-  if (for_exit) {
-    /* If we're choosing an exit node, exit bandwidth counts fully. */
-    exit_weight = 1.0;
-    total_bw = total_exit_bw + total_nonexit_bw;
-  } else {
+  /* Figure out how to weight exits and guards */
+  {
     double all_bw = U64_TO_DBL(total_exit_bw+total_nonexit_bw);
     double exit_bw = U64_TO_DBL(total_exit_bw);
+    double guard_bw = U64_TO_DBL(total_guard_bw);
     /*
      * For detailed derivation of this formula, see
      *   http://archives.seul.org/or/dev/Jul-2007/msg00056.html
      */
-    exit_weight = 1.0 - all_bw/(3.0*exit_bw);
-    if (exit_weight <= 0.0) {
-      include_exits = 0;
+    if (for_exit)
+      exit_weight = 1.0;
+    else
+      exit_weight = 1.0 - all_bw/(3.0*exit_bw);
+
+    if (for_guard)
+      guard_weight = 1.0;
+    else
+      guard_weight = 1.0 - all_bw/(3.0*guard_bw);
+
+    if (exit_weight <= 0.0)
       exit_weight = 0.0;
-      total_bw = total_nonexit_bw;
-    } else {
-      total_bw = 0;
-      for (i=0; i < (unsigned)smartlist_len(sl); i++) {
-        is_exit = bitarray_is_set(exit_bits, i);
-        if (is_exit)
-          total_bw += ((uint64_t)(bandwidths[i] * exit_weight));
-        else
-          total_bw += bandwidths[i];
-      }
+
+    if (guard_weight <= 0.0)
+      guard_weight = 0.0;
+
+    total_bw = 0;
+    for (i=0; i < (unsigned)smartlist_len(sl); i++) {
+      is_exit = bitarray_is_set(exit_bits, i);
+      is_guard = bitarray_is_set(guard_bits, i);
+      if (is_exit && is_guard)
+        total_bw += ((uint64_t)(bandwidths[i] * exit_weight * guard_weight));
+      else if (is_guard)
+        total_bw += ((uint64_t)(bandwidths[i] * guard_weight));
+      else if (is_exit)
+        total_bw += ((uint64_t)(bandwidths[i] * exit_weight));
+      else
+        total_bw += bandwidths[i];
     }
   }
-  /*
-  log_debug(LD_CIRC, "Total bw = "U64_FORMAT", total exit bw = "U64_FORMAT
-            ", total nonexit bw = "U64_FORMAT", exit weight = %lf "
-            "(for exit == %d)",
-            U64_PRINTF_ARG(total_bw), U64_PRINTF_ARG(total_exit_bw),
-            U64_PRINTF_ARG(total_nonexit_bw), exit_weight, for_exit);
-  */
+  log_debug(LD_CIRC, "Total bw = "U64_FORMAT
+            ", exit bw = "U64_FORMAT
+            ", nonexit bw = "U64_FORMAT", exit weight = %lf "
+            "(for exit == %d)"
+            ", guard bw = "U64_FORMAT
+            ", nonguard bw = "U64_FORMAT", guard weight = %lf "
+            "(for guard == %d)",
+            U64_PRINTF_ARG(total_bw), 
+            U64_PRINTF_ARG(total_exit_bw), U64_PRINTF_ARG(total_nonexit_bw), 
+            exit_weight, for_exit,
+            U64_PRINTF_ARG(total_guard_bw), U64_PRINTF_ARG(total_nonguard_bw), 
+            guard_weight, for_guard);
 
   /* Almost done: choose a random value from the bandwidth weights. */
   rand_bw = crypto_rand_uint64(total_bw);
@@ -1413,11 +1474,18 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, int for_exit, int statuses)
   tmp = 0;
   for (i=0; i < (unsigned)smartlist_len(sl); i++) {
     is_exit = bitarray_is_set(exit_bits, i);
-    if (is_exit) {
-      if (include_exits)
-        tmp += ((uint64_t)(bandwidths[i] * exit_weight));
-    } else
+    is_guard = bitarray_is_set(guard_bits, i);
+
+    /* Weights can be 0 if not counting guards/exits */
+    if (is_exit && is_guard)
+      tmp += ((uint64_t)(bandwidths[i] * exit_weight * guard_weight));
+    else if (is_guard)
+      tmp += ((uint64_t)(bandwidths[i] * guard_weight));
+    else if (is_exit)
+      tmp += ((uint64_t)(bandwidths[i] * exit_weight));
+    else 
       tmp += bandwidths[i];
+
     if (tmp >= rand_bw)
       break;
   }
@@ -1427,10 +1495,13 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, int for_exit, int statuses)
     tor_fragile_assert();
     --i;
     log_warn(LD_BUG, "Round-off error in computing bandwidth had an effect on "
-             " which router we chose.  Please tell the developers.");
+             " which router we chose. Please tell the developers. "
+             U64_FORMAT " " U64_FORMAT " " U64_FORMAT, U64_PRINTF_ARG(tmp),
+             U64_PRINTF_ARG(rand_bw), U64_PRINTF_ARG(total_bw));
   }
   tor_free(bandwidths);
   tor_free(exit_bits);
+  tor_free(guard_bits);
   return smartlist_get(sl, i);
 }
 
@@ -1438,9 +1509,9 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, int for_exit, int statuses)
  * the advertised bandwidth of each router.
  */
 routerinfo_t *
-routerlist_sl_choose_by_bandwidth(smartlist_t *sl, int for_exit)
+routerlist_sl_choose_by_bandwidth(smartlist_t *sl, int for_exit, int for_guard)
 {
-  return smartlist_choose_by_bandwidth(sl, for_exit, 0);
+  return smartlist_choose_by_bandwidth(sl, for_exit, for_guard, 0);
 }
 
 /** Choose a random element of status list <b>sl</b>, weighted by
@@ -1449,7 +1520,8 @@ routerlist_sl_choose_by_bandwidth(smartlist_t *sl, int for_exit)
 routerstatus_t *
 routerstatus_sl_choose_by_bandwidth(smartlist_t *sl)
 {
-  return smartlist_choose_by_bandwidth(sl, 1, 1);
+  /* We are choosing neither exit nor guard here. Weight accordingly. */
+  return smartlist_choose_by_bandwidth(sl, 0, 0, 1);
 }
 
 /** Return a random running router from the routerlist.  If any node
@@ -1506,7 +1578,8 @@ router_choose_random_node(const char *preferred,
       smartlist_subtract(sl,excludedsmartlist);
 
     if (need_capacity || need_guard)
-      choice = routerlist_sl_choose_by_bandwidth(sl, weight_for_exit);
+      choice = routerlist_sl_choose_by_bandwidth(sl, weight_for_exit,
+                                                 need_guard);
     else
       choice = smartlist_choose(sl);