Revise all calls to connection_edge_end to avoid sending MISC, and to take...

Revise all calls to connection_edge_end to avoid sending MISC, and to take errno into account where possible.


svn:r3720

doc/TODO

@@ -72,15 +72,20 @@ R . HTTPS proxy for OR CONNECT stuff. (For outgoing SSL connections to
   o Changes for forward compatibility
     o If a version is later than the last in its series, but a version
       in the next series is recommended, that doesn't mean it's bad.
-  - Do end reasons better
+N . Do end reasons better
     . Start using RESOURCELIMIT more.
-    . Try to use MISC a lot less.
-      - bug: if the exit node fails to create a socket (e.g. because it
+    o Try to use MISC a lot less.
+      o bug: if the exit node fails to create a socket (e.g. because it
         has too many open), we will get a generic stream end response.
-      . niels's "did it fail because conn refused or timeout or what"
+        o Fix on platforms with set_max_file_descriptors.
+      o niels's "did it fail because conn refused or timeout or what"
         relay end feature.
     o Realize that unrecognized end reasons are probably features rather than
       bugs. (backport to 009x)
+    - Check for anyplace where we can close an edge connection without
+      sending an end; see if we should send an end.
+    - Get some kind of sane return code from package_raw_inbuf, or maybe
+      push the work of sending the end cell deeper into package_raw_inbuf.
 N . Feed end reason back into SOCK5 as reasonable.
 R o cache .foo.exit names better, or differently, or not.
 N - make !advertised_server_mode() ORs fetch dirs less often.

src/or/connection.c

@@ -913,9 +913,7 @@ loop_again:
     /* There's a read error; kill the connection.*/
     connection_close_immediate(conn); /* Don't flush; connection is dead. */
     if (CONN_IS_EDGE(conn)) {
-      connection_edge_end(conn, (char)(connection_state_is_open(conn) ?
-                  END_STREAM_REASON_MISC : END_STREAM_REASON_CONNECTREFUSED),
-                  conn->cpath_layer);
+      connection_edge_end_errno(conn, conn->cpath_layer);
     }
     connection_mark_for_close(conn);
     return -1;
@@ -1083,7 +1081,7 @@ int connection_handle_write(connection_t *conn) {
     if (getsockopt(conn->s, SOL_SOCKET, SO_ERROR, (void*)&e, &len) < 0) {
       log_fn(LOG_WARN,"getsockopt() syscall failed?! Please report to tor-ops.");
       if (CONN_IS_EDGE(conn))
-        connection_edge_end(conn, END_STREAM_REASON_MISC, conn->cpath_layer);
+        connection_edge_end_errno(conn, conn->cpath_layer);
       connection_mark_for_close(conn);
       return -1;
     }
@@ -1092,8 +1090,7 @@ int connection_handle_write(connection_t *conn) {
       if (!ERRNO_IS_CONN_EINPROGRESS(e)) {
         log_fn(LOG_INFO,"in-progress connect failed. Removing.");
         if (CONN_IS_EDGE(conn))
-          connection_edge_end(conn, END_STREAM_REASON_CONNECTREFUSED,
-                              conn->cpath_layer);
+          connection_edge_end_errno(conn, conn->cpath_layer);
 
         connection_close_immediate(conn);
         connection_mark_for_close(conn);
@@ -1157,11 +1154,9 @@ int connection_handle_write(connection_t *conn) {
   } else {
     result = flush_buf(conn->s, conn->outbuf, &conn->outbuf_flushlen);
     if (result < 0) {
-      /* XXXX Is this right? -NM
       if (CONN_IS_EDGE(conn))
-        connection_edge_end(conn, END_STREAM_REASON_MISC,
-                            conn->cpath_layer);
-      */
+        connection_edge_end_errno(conn, conn->cpath_layer);
+
       connection_close_immediate(conn); /* Don't flush; connection is dead. */
       conn->has_sent_end = 1;
       connection_mark_for_close(conn);

src/or/connection_edge.c

@@ -78,6 +78,7 @@ int connection_edge_process_inbuf(connection_t *conn, int package_partial) {
     case AP_CONN_STATE_OPEN:
     case EXIT_CONN_STATE_OPEN:
       if (connection_edge_package_raw_inbuf(conn, package_partial) < 0) {
+        /* XXXX We can't tell *why* package failed. -NM */
         connection_edge_end(conn, END_STREAM_REASON_MISC, conn->cpath_layer);
         connection_mark_for_close(conn);
         return -1;
@@ -96,7 +97,7 @@ int connection_edge_process_inbuf(connection_t *conn, int package_partial) {
 #ifdef TOR_FRAGILE
   tor_assert(0);
 #endif
-  connection_edge_end(conn, END_STREAM_REASON_MISC, conn->cpath_layer);
+  connection_edge_end(conn, END_STREAM_REASON_INTERNAL, conn->cpath_layer);
   connection_mark_for_close(conn);
   return -1;
 }
@@ -161,6 +162,17 @@ connection_edge_end(connection_t *conn, char reason, crypt_path_t *cpath_layer)
   return 0;
 }
 
+/** DOCDOC **/
+int
+connection_edge_end_errno(connection_t *conn, crypt_path_t *cpath_layer)
+{
+  uint8_t reason;
+  tor_assert(conn);
+  reason = (uint8_t)errno_to_end_reason(tor_socket_errno(conn->s));
+  return connection_edge_end(conn, reason, cpath_layer);
+}
+
+
 /** Connection <b>conn</b> has finished writing and has no bytes left on
  * its outbuf.
  *
@@ -1036,7 +1048,7 @@ int connection_exit_begin_conn(cell_t *cell, circuit_t *circ) {
   /* default to failed, change in dns_resolve if it turns out not to fail */
 
   if (we_are_hibernating()) {
-    connection_edge_end(n_stream, END_STREAM_REASON_EXITPOLICY, n_stream->cpath_layer);
+    connection_edge_end(n_stream, END_STREAM_REASON_HIBERNATING, n_stream->cpath_layer);
     connection_free(n_stream);
     return 0;
   }
@@ -1156,8 +1168,7 @@ connection_exit_connect(connection_t *conn) {
   log_fn(LOG_DEBUG,"about to try connecting");
   switch (connection_connect(conn, conn->address, addr, port)) {
     case -1:
-      connection_edge_end(conn, END_STREAM_REASON_CONNECTREFUSED,
-                          conn->cpath_layer);
+      connection_edge_end_errno(conn, conn->cpath_layer);
       circuit_detach_stream(circuit_get_by_conn(conn), conn);
       connection_free(conn);
       return;

src/or/dns.c

@@ -154,7 +154,7 @@ static void purge_expired_resolves(uint32_t now) {
         /* Connections should only be pending if they have no socket. */
         tor_assert(pend->conn->s == -1);
         pendconn = pend->conn;
-        connection_edge_end(pendconn, END_STREAM_REASON_MISC,
+        connection_edge_end(pendconn, END_STREAM_REASON_TIMEOUT,
                             pendconn->cpath_layer);
         circuit_detach_stream(circuit_get_by_conn(pendconn), pendconn);
         connection_free(pendconn);

src/or/hibernate.c

@@ -739,7 +739,8 @@ hibernate_go_dormant(time_t now) {
          (conn = connection_get_by_type(CONN_TYPE_AP)) ||
          (conn = connection_get_by_type(CONN_TYPE_EXIT))) {
     if (CONN_IS_EDGE(conn))
-      connection_edge_end(conn, END_STREAM_REASON_MISC, conn->cpath_layer);
+      connection_edge_end(conn, END_STREAM_REASON_HIBERNATING,
+                          conn->cpath_layer);
     log_fn(LOG_INFO,"Closing conn type %d", conn->type);
     connection_mark_for_close(conn);
   }

src/or/or.h

@@ -425,6 +425,7 @@ typedef enum {
 #define END_STREAM_REASON_INTERNAL 10
 #define END_STREAM_REASON_RESOURCELIMIT 11
 #define END_STREAM_REASON_CONNRESET 12
+#define END_STREAM_REASON_TORPROTOCOL 13
 
 #define RESOLVED_TYPE_IPV4 4
 #define RESOLVED_TYPE_IPV6 6
@@ -1286,6 +1287,7 @@ int connection_edge_reached_eof(connection_t *conn);
 int connection_edge_process_inbuf(connection_t *conn, int package_partial);
 int connection_edge_destroy(uint16_t circ_id, connection_t *conn);
 int connection_edge_end(connection_t *conn, char reason, crypt_path_t *cpath_layer);
+int connection_edge_end_errno(connection_t *conn, crypt_path_t *cpath_layer);
 int connection_edge_finished_flushing(connection_t *conn);
 int connection_edge_finished_connecting(connection_t *conn);
 
@@ -1527,9 +1529,7 @@ int connection_edge_send_command(connection_t *fromconn, circuit_t *circ,
 int connection_edge_package_raw_inbuf(connection_t *conn, int package_partial);
 void connection_edge_consider_sending_sendme(connection_t *conn);
 socks5_reply_status_t connection_edge_end_reason_sock5_response(char *payload, uint16_t length);
-int errno_to_end_reasaon(int e);
-
-
+int errno_to_end_reason(int e);
 
 extern uint64_t stats_n_data_cells_packaged;
 extern uint64_t stats_n_data_bytes_packaged;

src/or/relay.c

@@ -474,6 +474,7 @@ connection_edge_end_reason_str(char *payload, uint16_t length) {
     case END_STREAM_REASON_INTERNAL:       return "internal error at server";
     case END_STREAM_REASON_RESOURCELIMIT:  return "server out of resources";
     case END_STREAM_REASON_CONNRESET:      return "connection reset";
+    case END_STREAM_REASON_TORPROTOCOL:    return "Tor protocol error";
     default:
       log_fn(LOG_WARN,"Reason for ending (%d) not recognized.",*payload);
       return "unknown";
@@ -509,6 +510,8 @@ connection_edge_end_reason_sock5_response(char *payload, uint16_t length) {
       return SOCKS5_GENERAL_ERROR;
     case END_STREAM_REASON_CONNRESET:
       return SOCKS5_CONNECTION_REFUSED;
+    case END_STREAM_REASON_TORPROTOCOL:
+      return SOCKS5_GENERAL_ERROR;
     default:
       log_fn(LOG_WARN,"Reason for ending (%d) not recognized.",*payload);
       return SOCKS5_GENERAL_ERROR;
@@ -524,7 +527,7 @@ connection_edge_end_reason_sock5_response(char *payload, uint16_t length) {
 #endif
 
 int
-errno_to_end_reasaon(int e)
+errno_to_end_reason(int e)
 {
   switch (e) {
     E_CASE(EPIPE):
@@ -668,7 +671,7 @@ connection_edge_process_relay_cell_not_open(
       addr = ntohl(get_uint32(cell->payload+RELAY_HEADER_SIZE));
       if (!addr) {
         log_fn(LOG_INFO,"...but it claims the IP address was 0.0.0.0. Closing.");
-        connection_edge_end(conn, END_STREAM_REASON_MISC, conn->cpath_layer);
+        connection_edge_end(conn, END_STREAM_REASON_TORPROTOCOL, conn->cpath_layer);
         connection_mark_for_close(conn);
         return 0;
       }
@@ -680,6 +683,7 @@ connection_edge_process_relay_cell_not_open(
     conn->socks_request->has_finished = 1;
     /* handle anything that might have queued */
     if (connection_edge_package_raw_inbuf(conn, 1) < 0) {
+      /* XXXX we can't tell why package failed. -NM */
       connection_edge_end(conn, END_STREAM_REASON_MISC, conn->cpath_layer);
       connection_mark_for_close(conn);
       return 0;
@@ -710,7 +714,7 @@ connection_edge_process_relay_cell_not_open(
 
   log_fn(LOG_WARN,"Got an unexpected relay command %d, in state %d (%s). Closing.",
          rh->command, conn->state, conn_state_to_string[conn->type][conn->state]);
-  connection_edge_end(conn, END_STREAM_REASON_MISC, conn->cpath_layer);
+  connection_edge_end(conn, END_STREAM_REASON_TORPROTOCOL, conn->cpath_layer);
   connection_mark_for_close(conn);
   return -1;
 }
@@ -771,7 +775,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
       if (( layer_hint && --layer_hint->deliver_window < 0) ||
           (!layer_hint && --circ->deliver_window < 0)) {
         log_fn(LOG_WARN,"(relay data) circ deliver_window below 0. Killing.");
-        connection_edge_end(conn, END_STREAM_REASON_MISC, conn->cpath_layer);
+        connection_edge_end(conn, END_STREAM_REASON_TORPROTOCOL, conn->cpath_layer);
         connection_mark_for_close(conn);
         return -1;
       }