Allow any valid 'end' cell to mean a circuit was used successfully.

Also improve some log messages.

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 9b1236f34d91f4427b6abbaae31c29f38dafa583..c3a5827589f9b365782ad3ee3a94d22f16b90683 100644 (file)
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -1411,16 +1411,18 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason)
       // state.. Can we use that? Does optimistic data change this?
 
       log_info(LD_CIRC,
-            "Circuit closed without successful use for reason %d. "
+            "Circuit %d closed without successful use for reason %d. "
             "Circuit purpose %d currently %s.",
+            ocirc->global_identifier,
             reason, circ->purpose, circuit_state_to_string(circ->state));
       pathbias_count_unusable(ocirc);
     } else {
       if (reason & END_CIRC_REASON_FLAG_REMOTE) {
         /* Unused remote circ close reasons all could be bias */
         log_info(LD_CIRC,
-            "Circuit remote-closed without successful use for reason %d. "
+            "Circuit %d remote-closed without successful use for reason %d. "
             "Circuit purpose %d currently %s.",
+            ocirc->global_identifier,
             reason, circ->purpose, circuit_state_to_string(circ->state));
         pathbias_count_collapse(ocirc);
       } else if ((reason & ~END_CIRC_REASON_FLAG_REMOTE)
@@ -1432,8 +1434,9 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason)
         /* FIXME: Only count bias if the network is live?
          * What about clock jumps/suspends? */
         log_info(LD_CIRC,
-            "Circuit's channel closed without successful use for reason %d, "
+            "Circuit %d's channel closed without successful use for reason %d, "
             "channel reason %d. Circuit purpose %d currently %s.",
+            ocirc->global_identifier,
             reason, circ->n_chan->reason_for_closing,
             circ->purpose, circuit_state_to_string(circ->state));
         pathbias_count_collapse(ocirc);

diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 79bb54cbb7ec05c3221cadd79bab14498b0d872c..ca6060ca579399616ade368a4bc3bd8011448db6 100644 (file)
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -2178,14 +2178,19 @@ connection_ap_handshake_socks_reply(entry_connection_t *conn, char *reply,
 
   /* Flag this stream's circuit as having completed a stream successfully
    * (for path bias) */
-  if (status == SOCKS5_SUCCEEDED) {
+  if (status == SOCKS5_SUCCEEDED ||
+      endreason == END_STREAM_REASON_RESOLVEFAILED ||
+      endreason == END_STREAM_REASON_CONNECTREFUSED ||
+      endreason == END_STREAM_REASON_CONNRESET ||
+      endreason == END_STREAM_REASON_NOROUTE ||
+      endreason == END_STREAM_REASON_RESOURCELIMIT) {
     if(!conn->edge_.on_circuit ||
        !CIRCUIT_IS_ORIGIN(conn->edge_.on_circuit)) {
-      // XXX: Weird. We hit this a lot, and yet have no unusable_circs.
-      // Maybe during addrmaps/resolves?
-      log_warn(LD_BUG,
-               "(Harmless.) No origin circuit for successful SOCKS stream. "
-               "Reason: %d", endreason);
+      // DNS remaps can trigger this. So can failed hidden service
+      // lookups.
+      log_info(LD_BUG,
+               "(Harmless.) No origin circuit for successful SOCKS stream %ld. "
+               "Reason: %d", ENTRY_TO_CONN(conn)->global_identifier, endreason);
     } else {
       TO_ORIGIN_CIRCUIT(conn->edge_.on_circuit)->path_state
           = PATH_STATE_USE_SUCCEEDED;

diff --git a/src/or/relay.c b/src/or/relay.c
index 7f49299e2bd02debc01bda389e3148ce439cac28..1638dae4fd147c05be7b7b7fc4d41b8a1ab78b3f 100644 (file)
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -693,6 +693,14 @@ connection_ap_process_end_not_open(
   edge_connection_t *edge_conn = ENTRY_TO_EDGE_CONN(conn);
   (void) layer_hint; /* unused */
 
+  if (rh->length > 0) {
+    /* Path bias: If we get a valid reason code from the exit, 
+     * it wasn't due to tagging */
+    // XXX: This relies on recognized+digest being strong enough not
+    // to be spoofable.. Is that a valid assumption?
+    circ->path_state = PATH_STATE_USE_SUCCEEDED;
+  }
+
   if (rh->length > 0 && edge_reason_is_retriable(reason) &&
       /* avoid retry if rend */
       !connection_edge_is_rendezvous_stream(edge_conn)) {