Merge remote-tracking branch 'public/bug8235-diagnostic-rebased' into maint-0.2.4

changes/bug8235-diagnosing

@@ -0,0 +1,5 @@
+ o Minor features (diagnostic)
+   - If the state file's path bias counts are invalid (presumably from a
+     buggy tor prior to 0.2.4.10-alpha), make them correct.
+   - Add additional checks and log messages to the scaling of Path Bias
+     counts, in case there still are remaining issues with scaling.

src/or/circuitbuild.c

@@ -1734,6 +1734,13 @@ pathbias_count_use_success(origin_circuit_t *circ)
       guard->use_successes++;
       entry_guards_changed();
 
+      if (guard->use_attempts < guard->use_successes) {
+        log_notice(LD_BUG, "Unexpectedly high use successes counts (%f/%f) "
+                 "for guard %s=%s",
+                 guard->use_successes, guard->use_attempts,
+                 guard->nickname, hex_str(guard->identity, DIGEST_LEN));
+      }
+
       log_debug(LD_CIRC,
                 "Marked circuit %d (%f/%f) as used successfully for guard "
                 "%s ($%s).",
@@ -2481,6 +2488,9 @@ pathbias_scale_close_rates(entry_guard_t *guard)
     int opened_built = pathbias_count_circs_in_states(guard,
                         PATH_STATE_BUILD_SUCCEEDED,
                         PATH_STATE_USE_FAILED);
+    /* Verify that the counts are sane before and after scaling */
+    int counts_are_sane = (guard->circ_attempts >= guard->circ_successes);
+
     guard->circ_attempts -= opened_attempts;
     guard->circ_successes -= opened_built;
 
@@ -2502,6 +2512,16 @@ pathbias_scale_close_rates(entry_guard_t *guard)
              guard->circ_successes, guard->successful_circuits_closed,
              guard->circ_attempts, opened_built, opened_attempts,
              guard->nickname, hex_str(guard->identity, DIGEST_LEN));
+
+    /* Have the counts just become invalid by this scaling attempt? */
+    if (counts_are_sane && guard->circ_attempts < guard->circ_successes) {
+      log_notice(LD_BUG,
+               "Scaling has mangled pathbias counts to %f/%f (%d/%d open) "
+               "for guard %s ($%s)",
+               guard->circ_successes, guard->circ_attempts, opened_built,
+               opened_attempts, guard->nickname,
+               hex_str(guard->identity, DIGEST_LEN));
+    }
   }
 }
 
@@ -2524,6 +2544,9 @@ pathbias_scale_use_rates(entry_guard_t *guard)
     double scale_ratio = pathbias_get_scale_ratio(options);
     int opened_attempts = pathbias_count_circs_in_states(guard,
             PATH_STATE_USE_ATTEMPTED, PATH_STATE_USE_SUCCEEDED);
+    /* Verify that the counts are sane before and after scaling */
+    int counts_are_sane = (guard->use_attempts >= guard->use_successes);
+
     guard->use_attempts -= opened_attempts;
 
     guard->use_attempts *= scale_ratio;
@@ -2535,6 +2558,17 @@ pathbias_scale_use_rates(entry_guard_t *guard)
              "Scaled pathbias use counts to %f/%f (%d open) for guard %s ($%s)",
              guard->use_successes, guard->use_attempts, opened_attempts,
              guard->nickname, hex_str(guard->identity, DIGEST_LEN));
+
+    /* Have the counts just become invalid by this scaling attempt? */
+    if (counts_are_sane && guard->use_attempts < guard->use_successes) {
+      log_notice(LD_BUG,
+               "Scaling has mangled pathbias usage counts to %f/%f "
+               "(%d open) for guard %s ($%s)",
+               guard->circ_successes, guard->circ_attempts,
+               opened_attempts, guard->nickname,
+               hex_str(guard->identity, DIGEST_LEN));
+    }
+
     entry_guards_changed();
   }
 }

src/or/entrynodes.c

@@ -1211,6 +1211,21 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg)
         continue;
       }
 
+      if (use_cnt < success_cnt) {
+        int severity = LOG_INFO;
+        /* If this state file was written by a Tor that would have
+         * already fixed it, then the overcounting bug is still there.. */
+        if (tor_version_as_new_as(state_version, "0.2.4.12-alpha")) {
+          severity = LOG_NOTICE;
+        }
+        log_fn(severity, LD_BUG,
+                   "State file contains unexpectedly high usage success "
+                   "counts %lf/%lf for Guard %s ($%s)",
+                   success_cnt, use_cnt,
+                   node->nickname, hex_str(node->identity, DIGEST_LEN));
+        success_cnt = use_cnt;
+      }
+
       node->use_attempts = use_cnt;
       node->use_successes = success_cnt;
 
@@ -1261,6 +1276,21 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg)
         unusable = 0;
       }
 
+      if (hop_cnt < success_cnt) {
+        int severity = LOG_INFO;
+        /* If this state file was written by a Tor that would have
+         * already fixed it, then the overcounting bug is still there.. */
+        if (tor_version_as_new_as(state_version, "0.2.4.12-alpha")) {
+          severity = LOG_NOTICE;
+        }
+        log_fn(severity, LD_BUG,
+                "State file contains unexpectedly high success counts "
+                "%lf/%lf for Guard %s ($%s)",
+                success_cnt, hop_cnt,
+                node->nickname, hex_str(node->identity, DIGEST_LEN));
+        success_cnt = hop_cnt;
+      }
+
       node->circ_attempts = hop_cnt;
       node->circ_successes = success_cnt;