r9060@totoro: nickm | 2006-10-17 11:12:48 -0400

Apply patch from Mike Perry: add more reasons for circuit destroys. (Slightly tweaked to avoid allocating a number for an "internal" reason.)


svn:r8739

src/or/circuitbuild.c

@@ -305,8 +305,7 @@ circuit_establish_circuit(uint8_t purpose, extend_info_t *info,
 
   if (onion_pick_cpath_exit(circ, info) < 0 ||
       onion_populate_cpath(circ) < 0) {
-    /* XXX should there be a 'couldn't build a path' reason? */
-    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL);
+    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_NOPATH);
     return NULL;
   }
 

src/or/circuitlist.c

@@ -780,7 +780,7 @@ circuit_mark_all_unused_circs(void)
     if (CIRCUIT_IS_ORIGIN(circ) &&
         !circ->marked_for_close &&
         !circ->timestamp_dirty)
-      circuit_mark_for_close(circ, END_CIRC_REASON_REQUESTED);
+      circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED);
   }
 }
 
@@ -843,7 +843,7 @@ _circuit_mark_for_close(circuit_t *circ, int reason, int line,
                file, line, circ->purpose);
     }
     reason = END_CIRC_REASON_NONE;
-  } else if (CIRCUIT_IS_ORIGIN(circ) && reason != END_CIRC_REASON_NONE) {
+  } else if (CIRCUIT_IS_ORIGIN(circ) && reason < _END_CIRC_REASON_MIN) {
     /* We don't send reasons when closing circuits at the origin, but we want
      * to track them anyway so we can give them to the controller. */
     reason = END_CIRC_REASON_NONE;

src/or/circuituse.c

@@ -265,8 +265,7 @@ circuit_expire_building(time_t now)
                circuit_state_to_string(victim->state), victim->purpose);
 
     circuit_log_path(LOG_INFO,LD_CIRC,TO_ORIGIN_CIRCUIT(victim));
-    /* XXXX Should there be a timeout reason? CONNECTFAILED isn't right. */
-    circuit_mark_for_close(victim, END_CIRC_REASON_CONNECTFAILED);
+    circuit_mark_for_close(victim, END_CIRC_REASON_TIMEOUT);
   }
 }
 
@@ -584,8 +583,7 @@ circuit_expire_old_circuits(time_t now)
       log_debug(LD_CIRC, "Closing n_circ_id %d (dirty %d secs ago, purp %d)",
                 circ->n_circ_id, (int)(now - circ->timestamp_dirty),
                 circ->purpose);
-      /* XXXX Should there be a timeout reason? REQUESTED isn't right. */
-      circuit_mark_for_close(circ, END_CIRC_REASON_REQUESTED);
+      circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED);
     } else if (!circ->timestamp_dirty &&
                circ->state == CIRCUIT_STATE_OPEN &&
                circ->purpose == CIRCUIT_PURPOSE_C_GENERAL) {
@@ -593,8 +591,7 @@ circuit_expire_old_circuits(time_t now)
         log_debug(LD_CIRC,
                   "Closing circuit that has been unused for %d seconds.",
                   (int)(now - circ->timestamp_created));
-        /* XXXX Should there be a timeout reason? REQUESTED isn't right. */
-        circuit_mark_for_close(circ, END_CIRC_REASON_REQUESTED);
+        circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED);
       }
     }
   }

src/or/command.c

@@ -379,18 +379,12 @@ command_process_destroy_cell(cell_t *cell, or_connection_t *conn)
     circuit_set_n_circid_orconn(circ, 0, NULL);
     if (CIRCUIT_IS_ORIGIN(circ)) {
       /* Prevent arbitrary destroys from going unnoticed by controller */
-      /* XXXX Not quite right; what we want is to tell the controller the
-       *      exact reason that we were asked to close, but tell it that we
-       *      closed because we were asked. Anything else is not accurate.
-       *      OR_CONN_CLOSED is certainly wrong, since a destroy doesn't mean
-       *      that the underlying OR connection got closed. -NM */
-#if 0
       if (reason == END_CIRC_AT_ORIGIN ||
           reason == END_CIRC_REASON_NONE ||
+          reason == END_CIRC_REASON_FINISHED ||
           reason == END_CIRC_REASON_REQUESTED) {
-        reason = END_CIRC_REASON_OR_CONN_CLOSED;
+        reason = END_CIRC_REASON_DESTROYED;
       }
-#endif
       circuit_mark_for_close(circ, reason);
     } else {
       char payload[1];

src/or/config.c

@@ -3666,6 +3666,7 @@ static const struct {
   { "1.1",  LE_11 },
   { "1.1a", LE_11A },
   { "1.1b", LE_11B },
+  { "1.2",  LE_12 },
   { NULL, 0 }
 };
 

src/or/control.c

@@ -2825,6 +2825,16 @@ circuit_end_reason_to_string(int reason)
       return "REASON=OR_IDENTITY";
     case END_CIRC_REASON_OR_CONN_CLOSED:
       return "REASON=OR_CONN_CLOSED";
+    case END_CIRC_REASON_FINISHED:
+      return "REASON=FINISHED";
+    case END_CIRC_REASON_TIMEOUT:
+      return "REASON=TIMEOUT";
+    case END_CIRC_REASON_DESTROYED:
+      return "REASON=DESTROYED";
+    case END_CIRC_REASON_NOPATH:
+      return "REASON=NOPATH";
+    case END_CIRC_REASON_NOSUCHSERVICE:
+      return "REASON=NOSUCHSERVICE";
     default:
       log_warn(LD_BUG, "Unrecognized reason code %d", (int)reason);
       return NULL;

src/or/or.h

@@ -494,7 +494,9 @@ typedef enum {
 #define RESOLVED_TYPE_ERROR 0xF1
 
 /* DOCDOC We should document the meaning of these. */
-#define END_CIRC_AT_ORIGIN           -1
+/* Negative reasons are internal */
+#define END_CIRC_REASON_NOPATH          -2
+#define END_CIRC_AT_ORIGIN              -1
 #define _END_CIRC_REASON_MIN            0
 #define END_CIRC_REASON_NONE            0
 #define END_CIRC_REASON_TORPROTOCOL     1
@@ -505,7 +507,11 @@ typedef enum {
 #define END_CIRC_REASON_CONNECTFAILED   6
 #define END_CIRC_REASON_OR_IDENTITY     7
 #define END_CIRC_REASON_OR_CONN_CLOSED  8
-#define _END_CIRC_REASON_MAX            8
+#define END_CIRC_REASON_FINISHED        9
+#define END_CIRC_REASON_TIMEOUT         10
+#define END_CIRC_REASON_DESTROYED       11
+#define END_CIRC_REASON_NOSUCHSERVICE   12
+#define _END_CIRC_REASON_MAX            12
 
 /** Length of 'y' portion of 'y.onion' URL. */
 #define REND_SERVICE_ID_LEN 16

src/or/rendclient.c

@@ -211,7 +211,7 @@ rend_client_introduction_acked(origin_circuit_t *circ,
     }
     /* close the circuit: we won't need it anymore. */
     circ->_base.purpose = CIRCUIT_PURPOSE_C_INTRODUCE_ACKED;
-    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_AT_ORIGIN);
+    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_FINISHED);
   } else {
     /* It's a NAK; the introduction point didn't relay our request. */
     circ->_base.purpose = CIRCUIT_PURPOSE_C_INTRODUCING;

src/or/rendmid.c

@@ -90,7 +90,7 @@ rend_mid_establish_intro(or_circuit_t *circ, const char *request,
   while ((c = circuit_get_intro_point(pk_digest))) {
     log_info(LD_REND, "Replacing old circuit for service %s",
              safe_str(serviceid));
-    circuit_mark_for_close(TO_CIRCUIT(c), END_CIRC_REASON_REQUESTED);
+    circuit_mark_for_close(TO_CIRCUIT(c), END_CIRC_REASON_FINISHED);
     /* Now it's marked, and it won't be returned next time. */
   }
 

src/or/rendservice.c

@@ -738,8 +738,7 @@ rend_service_intro_has_opened(origin_circuit_t *circuit)
   if (!service) {
     log_warn(LD_REND, "Unrecognized service ID %s on introduction circuit %d.",
              serviceid, circuit->_base.n_circ_id);
-    /* XXXX Add a no-such-servicer reason? */
-    reason = END_CIRC_REASON_CONNECTFAILED;
+    reason = END_CIRC_REASON_NOSUCHSERVICE;
     goto err;
   }