Be even more aggressive about separating local traffic from relayed

traffic when RelayBandwidthRate is set. (Refines proposal 111.)


svn:r10974

ChangeLog

@@ -7,6 +7,8 @@ Changes in version 0.2.0.3-alpha - 2007-07-29
     - New ConstrainedSockets option to set SO_SNDBUF and SO_RCVBUF on TCP
       sockets. Hopefully useful for Tor servers running on "vserver"
       accounts. (Patch from coderman.)
+    - Be even more aggressive about separating local traffic from relayed
+      traffic when RelayBandwidthRate is set. (Refines proposal 111.)
 
   o Security fixes:
     - Directory authorities now call routers Fast if their bandwidth is

doc/spec/proposals/111-local-traffic-priority.txt

@@ -118,20 +118,35 @@ Some options:
   (Gosh. How could UDP designs possibly be compatible with rate limiting
   with multiple bucket sizes?)
 
-  Option 4: ?
+  Option 4: put both classes of circuits over a single connection, and
+  keep track of the last time we read or wrote a high-priority cell. If
+  it's been less than N seconds, give the whole connection high priority,
+  else give the whole connection low priority.
+
+  Option 5: put both classes of circuits over a single connection, and
+  play a complex juggling game by periodically telling the remote side
+  what rate limits to set for that connection, so you end up giving
+  priority to the right connections but still stick to roughly your
+  intended bandwidthrate and relaybandwidthrate.
+
+  Option 6: ?
 
 Prognosis:
 
-  Of the above options, only option 2 can actually be built and achieve
-  what we want. So that's it by default, unless we can come up with
-  something better.
+  Nick really didn't like option 2 because of the partitioning questions.
+
+  I've put option 4 into place as of Tor 0.2.0.3-alpha.
 
-  In terms of implementation, it will be easy: just add a bit to
-  or_connection_t that specifies priority_traffic (used by the initiator
-  of the connection to ignore that connection when relaying a create
-  request), and another bit that specifies client_only (used by a
-  receiving Tor server so it can ignore that connection when sending
-  create requests).
+  In terms of implementation, it will be easy: just add a time_t to
+  or_connection_t that specifies client_used (used by the initiator
+  of the connection to rate limit it differently depending on how
+  recently the time_t was reset). We currently update client_used
+  in three places:
+    - command_process_relay_cell() when we receive a relay cell for
+      an origin circuit.
+    - relay_send_command_from_edge() when we send a relay cell for
+      an origin circuit.
+    - circuit_deliver_create_cell() when send a create cell.
+  We could probably remove the third case and it would still work,
+  but hey.
 
-[Not writing the rest of the proposal until we sort out which option
-we'll take.]

src/or/circuitbuild.c

@@ -502,7 +502,7 @@ circuit_deliver_create_cell(circuit_t *circ, uint8_t cell_type,
   append_cell_to_circuit_queue(circ, circ->n_conn, &cell, CELL_DIRECTION_OUT);
 
   /* mark it so it gets better rate limiting treatment. */
-  circ->n_conn->client_used = 1;
+  circ->n_conn->client_used = time(NULL);
 
   return 0;
 }

src/or/circuitlist.c

@@ -904,22 +904,6 @@ circuit_expire_all_dirty_circs(void)
   }
 }
 
-/** Return 1 if there are any origin circuits that use
- * <b>conn</b> as there first hop. Else return 0. */
-static int
-circuit_any_origin_circs_on_conn(or_connection_t *conn)
-{
-  circuit_t *circ;
-
-  for (circ=global_circuitlist; circ; circ = circ->next) {
-    if (CIRCUIT_IS_ORIGIN(circ) &&
-        !circ->marked_for_close &&
-        circ->n_conn == conn)
-      return 1;
-  }
-  return 0;
-}
-
 /** Mark <b>circ</b> to be closed next time we call
  * circuit_close_all_marked(). Do any cleanup needed:
  *   - If state is onionskin_pending, remove circ from the onion_pending
@@ -1044,12 +1028,7 @@ _circuit_mark_for_close(circuit_t *circ, int reason, int line,
   circ->marked_for_close = line;
   circ->marked_for_close_file = file;
 
-  if (CIRCUIT_IS_ORIGIN(circ)) {
-    if (circ->n_conn && circ->n_conn->client_used) {
-      circ->n_conn->client_used =
-        circuit_any_origin_circs_on_conn(circ->n_conn);
-    }
-  } else {
+  if (!CIRCUIT_IS_ORIGIN(circ)) {
     or_circuit_t *or_circ = TO_OR_CIRCUIT(circ);
     if (or_circ->rend_splice) {
       if (!or_circ->rend_splice->_base.marked_for_close) {

src/or/command.c

@@ -322,6 +322,12 @@ command_process_relay_cell(cell_t *cell, or_connection_t *conn)
     return;
   }
 
+  if (CIRCUIT_IS_ORIGIN(circ)) {
+    /* if we're a server and treating connections with recent local
+     * traffic better, then this is one of them. */
+    conn->client_used = time(NULL);
+  }
+
   if (!CIRCUIT_IS_ORIGIN(circ) &&
       cell->circ_id == TO_OR_CIRCUIT(circ)->p_circ_id)
     direction = CELL_DIRECTION_OUT;

src/or/connection.c

@@ -1372,14 +1372,19 @@ extern int global_relayed_read_bucket, global_relayed_write_bucket;
  * tokens we just put in. */
 static int write_buckets_empty_last_second = 0;
 
+/** How many seconds of no active local circuits will make the
+ * connection revert to the "relayed" bandwidth class? */
+#define CLIENT_IDLE_TIME_FOR_PRIORITY 30
+
 /** Return 1 if <b>conn</b> should use tokens from the "relayed"
  * bandwidth rates, else 0. Currently, only OR conns with bandwidth
  * class 1, and directory conns that are serving data out, count.
  */
 static int
-connection_counts_as_relayed_traffic(connection_t *conn)
+connection_counts_as_relayed_traffic(connection_t *conn, time_t now)
 {
-  if (conn->type == CONN_TYPE_OR && !TO_OR_CONN(conn)->client_used)
+  if (conn->type == CONN_TYPE_OR &&
+      TO_OR_CONN(conn)->client_used + CLIENT_IDLE_TIME_FOR_PRIORITY < now)
     return 1;
   if (conn->type == CONN_TYPE_DIR && DIR_CONN_IS_SERVER(conn))
     return 1;
@@ -1441,7 +1446,7 @@ connection_bucket_read_limit(connection_t *conn)
     return conn_bucket>=0 ? conn_bucket : 1<<14;
   }
 
-  if (connection_counts_as_relayed_traffic(conn) &&
+  if (connection_counts_as_relayed_traffic(conn, time(NULL)) &&
       global_relayed_read_bucket <= global_read_bucket)
     global_bucket = global_relayed_read_bucket;
 
@@ -1463,7 +1468,7 @@ connection_bucket_write_limit(connection_t *conn)
     return conn->outbuf_flushlen;
   }
 
-  if (connection_counts_as_relayed_traffic(conn) &&
+  if (connection_counts_as_relayed_traffic(conn, time(NULL)) &&
       global_relayed_write_bucket <= global_write_bucket)
     global_bucket = global_relayed_write_bucket;
 
@@ -1536,7 +1541,7 @@ connection_buckets_decrement(connection_t *conn, time_t now,
   if (num_written > 0)
     rep_hist_note_bytes_written(num_written, now);
 
-  if (connection_counts_as_relayed_traffic(conn)) {
+  if (connection_counts_as_relayed_traffic(conn, now)) {
     global_relayed_read_bucket -= num_read;
     global_relayed_write_bucket -= num_written;
   }
@@ -1555,7 +1560,7 @@ connection_consider_empty_read_buckets(connection_t *conn)
 
   if (global_read_bucket <= 0) {
     reason = "global read bucket exhausted. Pausing.";
-  } else if (connection_counts_as_relayed_traffic(conn) &&
+  } else if (connection_counts_as_relayed_traffic(conn, time(NULL)) &&
              global_relayed_read_bucket <= 0) {
     reason = "global relayed read bucket exhausted. Pausing.";
   } else if (connection_speaks_cells(conn) &&
@@ -1579,7 +1584,7 @@ connection_consider_empty_write_buckets(connection_t *conn)
 
   if (global_write_bucket <= 0) {
     reason = "global write bucket exhausted. Pausing.";
-  } else if (connection_counts_as_relayed_traffic(conn) &&
+  } else if (connection_counts_as_relayed_traffic(conn, time(NULL)) &&
              global_relayed_write_bucket <= 0) {
     reason = "global relayed write bucket exhausted. Pausing.";
 #if 0
@@ -1632,6 +1637,7 @@ connection_bucket_refill(int seconds_elapsed)
   or_options_t *options = get_options();
   smartlist_t *conns = get_connection_array();
   int relayrate, relayburst;
+  time_t now = time(NULL);
 
   if (options->RelayBandwidthRate) {
     relayrate = (int)options->RelayBandwidthRate;
@@ -1678,7 +1684,7 @@ connection_bucket_refill(int seconds_elapsed)
 
     if (conn->read_blocked_on_bw == 1 /* marked to turn reading back on now */
         && global_read_bucket > 0 /* and we're allowed to read */
-        && (!connection_counts_as_relayed_traffic(conn) ||
+        && (!connection_counts_as_relayed_traffic(conn, now) ||
             global_relayed_read_bucket > 0) /* even if we're relayed traffic */
         && (!connection_speaks_cells(conn) ||
             conn->state != OR_CONN_STATE_OPEN ||
@@ -1692,7 +1698,7 @@ connection_bucket_refill(int seconds_elapsed)
 
     if (conn->write_blocked_on_bw == 1
         && global_write_bucket > 0 /* and we're allowed to write */
-        && (!connection_counts_as_relayed_traffic(conn) ||
+        && (!connection_counts_as_relayed_traffic(conn, now) ||
             global_relayed_write_bucket > 0)) {
             /* even if we're relayed traffic */
       LOG_FN_CONN(conn, (LOG_DEBUG,LD_NET,

src/or/or.h

@@ -845,9 +845,9 @@ typedef struct or_connection_t {
 
   tor_tls_t *tls; /**< TLS connection state. */
   int tls_error; /**< Last tor_tls error code. */
-  /** Whether we are using this conn for any client traffic. If we're
-   * not, we can rate limit it further. */
-  uint8_t client_used:1;
+  /** When we last used this conn for any client traffic. If not
+   * recent, we can rate limit it further. */
+  time_t client_used;
 
   circ_id_type_t circ_id_type:2; /**< When we send CREATE cells along this
                                   * connection, which half of the space should

src/or/relay.c

@@ -505,6 +505,11 @@ relay_send_command_from_edge(uint16_t stream_id, circuit_t *circ,
   log_debug(LD_OR,"delivering %d cell %s.", relay_command,
             cell_direction == CELL_DIRECTION_OUT ? "forward" : "backward");
 
+  if (cell_direction == CELL_DIRECTION_OUT && circ->n_conn) {
+    /* if we're using relaybandwidthrate, this conn wants priority */
+    circ->n_conn->client_used = time(NULL);
+  }
+
   if (circuit_package_relay_cell(&cell, circ, cell_direction, cpath_layer)
       < 0) {
     log_warn(LD_BUG,"circuit_package_relay_cell failed. Closing.");