#6341: connection_or_flush_from_first_active_circuit() does wrong thing when
ewma_enabled
-----------------------+----------------------------------------------------
Reporter: arma | Owner:
Type: defect | Status: needs_review
Priority: normal | Milestone: Tor: 0.2.3.x-final
Component: Tor Relay | Version:
Keywords: | Parent:
Points: | Actualpoints:
-----------------------+----------------------------------------------------
Comment(by arma):
Hm. I think the description of the bug and patch is wrong.
If we check !ewma_enabled, we're actually causing ourselves to run *more*
code than we ran before.
Specifically, we now run all this:
{{{
tor_assert(*next_circ_on_conn_p(circ,conn));
assert_active_circuits_ok_paranoid(conn);
conn->active_circuits = *next_circ_on_conn_p(circ, conn);
/* Is the cell queue low enough to unblock all the streams that are
waiting
* to write to this circuit? */
if (streams_blocked && queue->n <= CELL_QUEUE_LOWWATER_SIZE)
set_streams_blocked_on_circ(circ, conn, 0, 0); /* unblock streams */
/* Did we just run out of cells on this circuit's queue? */
if (queue->n == 0) {
log_debug(LD_GENERAL, "Made a circuit inactive.");
make_circuit_inactive_on_conn(circ, conn);
}
}}}
Setting conn->active_circuits should be irrelevant but harmless. We should
look very carefully to make sure *next_circ_on_conn_p(circ, conn) is never
going to be NULL (or we're *introducing* an assert trigger case).
Being willing to call set_streams_blocked_on_circ() is the main feature of
this bugfix I believe. Right now we don't unblock streams when we should.
So this could be a performance fix. I have no idea if it fixes performance
in any noticeable way; I've been toying with the idea of asking Rob to run
a Shadow simulation with and without this patch.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/6341#comment:13>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online