[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor/master] Launch sufficient circuits to satisfy pending isolated streams
commit 20c0581a7935369fecb6c62b7cf5c7c244cdb533
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date: Thu Jul 7 10:40:23 2011 -0400
Launch sufficient circuits to satisfy pending isolated streams
Our old "do we need to launch a circuit for stream S" logic was,
more or less, that if we had a pending circuit that could handle S,
we didn't need to launch a new one.
But now that we have streams isolated from one another, we need
something stronger here: It's possible that some pending C can
handle either S1 or S2, but not both.
This patch reuses the existing isolation logic for a simple
solution: when we decide during circuit launching that some pending
C would satisfy stream S1, we "hypothetically" mark C as though S1
had been connected to it. Now if S2 is incompatible with S1, it
won't be something that can attach to C, and so we'll launch a new
stream.
When the circuit becomes OPEN for the first time (with no streams
attached to it), we reset the circuit's isolation status. I'm not
too sure about this part: I wanted some way to be sure that, if all
streams that would have used a circuit die before the circuit is
done, the circuit can still get used. But I worry that this
approach could also lead to us launching too many circuits. Careful
thought needed here.
---
src/or/circuitlist.c | 18 ++++++++++++++++--
src/or/circuituse.c | 13 +++++++++++--
src/or/connection_edge.c | 35 +++++++++++++++++++++++++++++++++++
src/or/connection_edge.h | 1 +
src/or/or.h | 24 +++++++++++++++++++-----
5 files changed, 82 insertions(+), 9 deletions(-)
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 9f68880..6f17697 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -210,9 +210,23 @@ circuit_set_state(circuit_t *circ, uint8_t state)
/* add to waiting-circuit list. */
smartlist_add(circuits_pending_or_conns, circ);
}
- if (state == CIRCUIT_STATE_OPEN)
- tor_assert(!circ->n_conn_onionskin);
+
circ->state = state;
+
+ if (state == CIRCUIT_STATE_OPEN) {
+ tor_assert(!circ->n_conn_onionskin);
+ if (CIRCUIT_IS_ORIGIN(circ)) {
+ origin_circuit_t *origin_circ = TO_ORIGIN_CIRCUIT(circ);
+ if (origin_circ->isolation_values_set &&
+ !origin_circ->isolation_any_streams_attached) {
+ /* If we have any isolation information set on this circuit,
+ * but we never attached any streams to it, then all of the
+ * isolation information was hypothetical. Clear it.
+ */
+ circuit_clear_isolation(origin_circ);
+ }
+ }
+ }
}
/** Add <b>circ</b> to the global list of circuits. This is called only from
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 19a6234..93098e5 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -1457,12 +1457,20 @@ circuit_get_open_circ_or_launch(edge_connection_t *conn,
rend_client_rendcirc_has_opened(circ);
}
}
- }
- if (!circ)
+ } /* endif (!circ) */
+ if (circ) {
+ /* Mark the circuit with the isolation fields for this connection.
+ * When the circuit arrives, we'll clear these flags: this is
+ * just some internal bookkeeping to make sure that we have
+ * launched enough circuits.
+ */
+ connection_edge_update_circuit_isolation(conn, circ, 0);
+ } else {
log_info(LD_APP,
"No safe circuit (purpose %d) ready for edge "
"connection; delaying.",
desired_circuit_purpose);
+ }
*circp = circ;
return 0;
}
@@ -1509,6 +1517,7 @@ link_apconn_to_circ(edge_connection_t *apconn, origin_circuit_t *circ,
apconn->cpath_layer = circ->cpath->prev;
}
+ circ->isolation_any_streams_attached = 1;
connection_edge_update_circuit_isolation(apconn, circ, 0);
}
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index ce555ed..20f01b1 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -3414,3 +3414,38 @@ connection_edge_update_circuit_isolation(const edge_connection_t *conn,
}
}
+/**
+ * Clear the isolation settings on <b>circ</b>.
+ *
+ * This only works on an open circuit that has never had a stream attached to
+ * it, and whose isolation settings are hypothetical. (We set hypothetical
+ * isolation settings on circuits as we're launching them, so that we
+ * know whether they can handle more streams or whether we need to launch
+ * even more circuits. We clear the flags once the circuits are open,
+ * in case the streams that made us launch the circuits have closed
+ * since we began launching the circuits.)
+ */
+void
+circuit_clear_isolation(origin_circuit_t *circ)
+{
+ if (circ->isolation_any_streams_attached) {
+ log_warn(LD_BUG, "Tried to clear the isolation status of a dirty circuit");
+ return;
+ }
+ if (TO_CIRCUIT(circ)->state != CIRCUIT_STATE_OPEN) {
+ log_warn(LD_BUG, "Tried to clear the isolation status of a non-open "
+ "circuit");
+ return;
+ }
+
+ circ->isolation_values_set = 0;
+ circ->isolation_flags_mixed = 0;
+ circ->client_proto_type = 0;
+ circ->client_proto_socksver = 0;
+ circ->dest_port = 0;
+ tor_addr_make_unspec(&circ->client_addr);
+ tor_free(circ->dest_address);
+ circ->session_group = -1;
+ circ->nym_epoch = 0;
+}
+
diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h
index ddcf8cc..8bd308d 100644
--- a/src/or/connection_edge.h
+++ b/src/or/connection_edge.h
@@ -110,6 +110,7 @@ int connection_edge_compatible_with_circuit(const edge_connection_t *conn,
int connection_edge_update_circuit_isolation(const edge_connection_t *conn,
origin_circuit_t *circ,
int dry_run);
+void circuit_clear_isolation(origin_circuit_t *circ);
#endif
diff --git a/src/or/or.h b/src/or/or.h
index 9cf508c..97418f5 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2461,9 +2461,19 @@ typedef struct origin_circuit_t {
/* XXXX NM This can get re-used after 2**32 circuits. */
uint32_t global_identifier;
- /** True if we have attached at least one stream to this circuit, thereby
- * setting the isolation paramaters for this circuit. */
+ /** True if we have associated one stream to this circuit, thereby setting
+ * the isolation paramaters for this circuit. Note that this doesn't
+ * necessarily mean that we've <em>attached</em> any streams to the circuit:
+ * we may only have marked up this circuit during the launch process.
+ */
unsigned int isolation_values_set : 1;
+ /** True iff any stream has <em>ever</em> been attached to this circuit.
+ *
+ * In a better world we could use timestamp_dirty for this, but
+ * timestamp_dirty is far too overloaded at the moment.
+ */
+ unsigned int isolation_any_streams_attached : 1;
+
/** A bitfield of ISO_* flags for every isolation field such that this
* circuit has had streams with more than one value for that field
* attached to it. */
@@ -2471,11 +2481,15 @@ typedef struct origin_circuit_t {
/** @name Isolation parameters
*
- * If any streams have been attached to this circuit (isolation_values_set
- * == 1), and all streams attached to the circuit have had the same value
- * for some field ((isolation_flags_mixed & ISO_FOO) == 0), then these
+ * If any streams have been associated with this circ (isolation_values_set
+ * == 1), and all streams associated with the circuit have had the same
+ * value for some field ((isolation_flags_mixed & ISO_FOO) == 0), then these
* elements hold the value for that field.
*
+ * Note again that "associated" is not the same as "attached": we
+ * preliminarily associate streams with a circuit while the circuit is being
+ * launched, so that we can tell whether we need to launch more circuits.
+ *
* @{
*/
uint8_t client_proto_type;
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits