commit 16f9861b22751bc90666fe1836b8cf740630447a Author: Andrea Shepard andrea@torproject.org Date: Wed Jun 12 22:22:21 2013 -0700

Add destroy balance tracking and logging to circuitmux --- src/or/channel.c | 1 - src/or/circuitlist.c | 5 +++-- src/or/circuitmux.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/or/circuitmux.h | 1 + src/or/relay.c | 2 ++ 5 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/src/or/channel.c b/src/or/channel.c index e327bda..33a3210 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -2652,7 +2652,6 @@ is_destroy_cell(channel_t *chan, return 0; }

- /** * Send destroy cell on a channel * diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index deb45b7..e50ab60 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -1717,8 +1717,9 @@ assert_circuit_ok(const circuit_t *c) if (or_circ && or_circ->p_chan) { if (or_circ->p_circ_id) { /* ibid */ - circuit_t *c2 = circuit_get_by_circid_channel_impl(or_circ->p_circ_id, - or_circ->p_chan, NULL); + circuit_t *c2 = + circuit_get_by_circid_channel_impl(or_circ->p_circ_id, + or_circ->p_chan, NULL); tor_assert(c == c2); } } diff --git a/src/or/circuitmux.c b/src/or/circuitmux.c index 198e518..a6256f8 100644 --- a/src/or/circuitmux.c +++ b/src/or/circuitmux.c @@ -127,6 +127,10 @@ struct circuitmux_s { * cells completely. */ unsigned int last_cell_was_destroy : 1; + /** Destroy counter: increment this when a destroy gets queued, decrement + * when we unqueue it, so we can test to make sure they don't starve. + */ + int64_t destroy_ctr;

/* * Circuitmux policy; if this is non-NULL, it can override the built- @@ -206,6 +210,11 @@ static void circuitmux_assert_okay_pass_one(circuitmux_t *cmux); static void circuitmux_assert_okay_pass_two(circuitmux_t *cmux); static void circuitmux_assert_okay_pass_three(circuitmux_t *cmux);

+/* Static global variables */ + +/** Count the destroy balance to debug destroy queue logic */ +static int64_t global_destroy_ctr = 0; + /* Function definitions */

/** @@ -521,6 +530,25 @@ circuitmux_free(circuitmux_t *cmux) tor_free(cmux->chanid_circid_map); }

+ /* + * We're throwing away some destroys; log the counter and + * adjust the global counter by the queue size. + */ + if (cmux->destroy_cell_queue.n > 0) { + cmux->destroy_ctr -= cmux->destroy_cell_queue.n; + global_destroy_ctr -= cmux->destroy_cell_queue.n; + log_debug(LD_CIRC, + "Freeing cmux at %p with %u queued destroys; the last cmux " + "destroy balance was %ld, global is %ld\n", + cmux, cmux->destroy_cell_queue.n, + cmux->destroy_ctr, global_destroy_ctr); + } else { + log_debug(LD_CIRC, + "Freeing cmux at %p with no queued destroys, the cmux destroy " + "balance was %ld, global is %ld\n", + cmux, cmux->destroy_ctr, global_destroy_ctr); + } + cell_queue_clear(&cmux->destroy_cell_queue);

tor_free(cmux); @@ -1502,6 +1530,24 @@ circuitmux_notify_xmit_cells(circuitmux_t *cmux, circuit_t *circ, circuitmux_assert_okay_paranoid(cmux); }

+/** + * Notify the circuitmux that a destroy was sent, so we can update + * the counter. + */ + +void +circuitmux_notify_xmit_destroy(circuitmux_t *cmux) +{ + tor_assert(cmux); + + --(cmux->destroy_ctr); + --(global_destroy_ctr); + log_debug(LD_CIRC, + "Cmux at %p sent a destroy, cmux counter is now %ld, " + "global counter is now %ld\n", + cmux, cmux->destroy_ctr, global_destroy_ctr); +} + /* * Circuitmux consistency checking assertions */ @@ -1798,6 +1844,14 @@ circuitmux_append_destroy_cell(channel_t *chan, cell_queue_append_packed_copy(&cmux->destroy_cell_queue, &cell, chan->wide_circ_ids, 0);

+ /* Destroy entering the queue, update counters */ + ++(cmux->destroy_ctr); + ++global_destroy_ctr; + log_debug(LD_CIRC, + "Cmux at %p queued a destroy for circ %u, " + "cmux counter is now %ld, global counter is now %ld\n", + cmux, circ_id, cmux->destroy_ctr, global_destroy_ctr); + /* XXXX Duplicate code from append_cell_to_circuit_queue */ if (!channel_has_queued_writes(chan)) { /* There is no data at all waiting to be sent on the outbuf. Add a @@ -1808,3 +1862,4 @@ circuitmux_append_destroy_cell(channel_t *chan, channel_flush_from_first_active_circuit(chan, 1); } } + diff --git a/src/or/circuitmux.h b/src/or/circuitmux.h index da62196..9ff29de 100644 --- a/src/or/circuitmux.h +++ b/src/or/circuitmux.h @@ -124,6 +124,7 @@ circuit_t * circuitmux_get_first_active_circuit(circuitmux_t *cmux, cell_queue_t **destroy_queue_out); void circuitmux_notify_xmit_cells(circuitmux_t *cmux, circuit_t *circ, unsigned int n_cells); +void circuitmux_notify_xmit_destroy(circuitmux_t *cmux);

/* Circuit interface */ void circuitmux_attach_circuit(circuitmux_t *cmux, circuit_t *circ, diff --git a/src/or/relay.c b/src/or/relay.c index ec86026..46bfc44 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -2358,6 +2358,8 @@ channel_flush_from_first_active_circuit(channel_t *chan, int max) tor_assert(destroy_queue->n > 0); cell = cell_queue_pop(destroy_queue); channel_write_packed_cell(chan, cell); + /* Update the cmux destroy counter */ + circuitmux_notify_xmit_destroy(cmux); cell = NULL; ++n_flushed; continue;