diff options
| -rw-r--r-- | changes/bug10870 | 6 | ||||
| -rw-r--r-- | src/or/or.h | 36 | ||||
| -rw-r--r-- | src/or/relay.c | 205 |
3 files changed, 43 insertions, 204 deletions
diff --git a/changes/bug10870 b/changes/bug10870 new file mode 100644 index 000000000..d8a00f402 --- /dev/null +++ b/changes/bug10870 @@ -0,0 +1,6 @@ + o Code simplification and refactoring: + - Remove data structures which were introduced to implement the + CellStatistics option: they are now redundant with the addition + of timestamp to the regular packed_cell_t data structure, which + we did in 0.2.4.18-rc in order to resolve #9093. Fixes bug + 10870.
\ No newline at end of file diff --git a/src/or/or.h b/src/or/or.h index b63b1ffcb..15cda284c 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1121,48 +1121,12 @@ typedef struct packed_cell_t { * bits truncated) when this cell was inserted. */ } packed_cell_t; -/* XXXX This next structure may be obsoleted by inserted_time in - * packed_cell_t */ - -/** Number of cells added to a circuit queue including their insertion - * time on 10 millisecond detail; used for buffer statistics. */ -typedef struct insertion_time_elem_t { - struct insertion_time_elem_t *next; /**< Next element in queue. */ - uint32_t insertion_time; /**< When were cells inserted (in 10 ms steps - * starting at 0:00 of the current day)? */ - unsigned counter; /**< How many cells were inserted? */ -} insertion_time_elem_t; - -/** Queue of insertion times. */ -typedef struct insertion_time_queue_t { - struct insertion_time_elem_t *first; /**< First element in queue. */ - struct insertion_time_elem_t *last; /**< Last element in queue. */ -} insertion_time_queue_t; - -/** Number of cells with the same command consecutively added to a circuit - * queue; used for cell statistics only if CELL_STATS events are enabled. */ -typedef struct insertion_command_elem_t { - struct insertion_command_elem_t *next; /**< Next element in queue. */ - /** Which command did these consecutively added cells have? */ - uint8_t command; - unsigned counter; /**< How many cells were inserted? */ -} insertion_command_elem_t; - -/** Queue of insertion commands. */ -typedef struct insertion_command_queue_t { - struct insertion_command_elem_t *first; /**< First element in queue. */ - struct insertion_command_elem_t *last; /**< Last element in queue. */ -} insertion_command_queue_t; - /** A queue of cells on a circuit, waiting to be added to the * or_connection_t's outbuf. */ typedef struct cell_queue_t { /** Linked list of packed_cell_t*/ TOR_SIMPLEQ_HEAD(cell_simpleq, packed_cell_t) head; int n; /**< The number of cells in the queue. */ - insertion_time_queue_t *insertion_times; /**< Insertion times of cells. */ - /** Commands of inserted cells. */ - insertion_command_queue_t *insertion_commands; } cell_queue_t; /** Beginning of a RELAY cell payload. */ diff --git a/src/or/relay.c b/src/or/relay.c index b5e4ff7cc..94016b49f 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -2047,14 +2047,6 @@ static size_t total_cells_allocated = 0; /** A memory pool to allocate packed_cell_t objects. */ static mp_pool_t *cell_pool = NULL; -/** Memory pool to allocate insertion_time_elem_t objects used for cell - * statistics. */ -static mp_pool_t *it_pool = NULL; - -/** Memory pool to allocate insertion_command_elem_t objects used for cell - * statistics if CELL_STATS events are enabled. */ -static mp_pool_t *ic_pool = NULL; - /** Allocate structures to hold cells. */ void init_cell_pool(void) @@ -2073,14 +2065,6 @@ free_cell_pool(void) mp_pool_destroy(cell_pool); cell_pool = NULL; } - if (it_pool) { - mp_pool_destroy(it_pool); - it_pool = NULL; - } - if (ic_pool) { - mp_pool_destroy(ic_pool); - ic_pool = NULL; - } } /** Free excess storage in cell pool. */ @@ -2153,57 +2137,6 @@ cell_queue_append(cell_queue_t *queue, packed_cell_t *cell) ++queue->n; } -/** Append command of type <b>command</b> in direction to <b>queue</b> for - * CELL_STATS event. */ -static void -cell_command_queue_append(cell_queue_t *queue, uint8_t command) -{ - insertion_command_queue_t *ic_queue = queue->insertion_commands; - if (!ic_pool) - ic_pool = mp_pool_new(sizeof(insertion_command_elem_t), 1024); - if (!ic_queue) { - ic_queue = tor_malloc_zero(sizeof(insertion_command_queue_t)); - queue->insertion_commands = ic_queue; - } - if (ic_queue->last && ic_queue->last->command == command) { - ic_queue->last->counter++; - } else { - insertion_command_elem_t *elem = mp_pool_get(ic_pool); - elem->next = NULL; - elem->command = command; - elem->counter = 1; - if (ic_queue->last) { - ic_queue->last->next = elem; - ic_queue->last = elem; - } else { - ic_queue->first = ic_queue->last = elem; - } - } -} - -/** Retrieve oldest command from <b>queue</b> and write it to - * <b>command</b> for CELL_STATS event. Return 0 for success, -1 - * otherwise. */ -static int -cell_command_queue_pop(uint8_t *command, cell_queue_t *queue) -{ - int res = -1; - insertion_command_queue_t *ic_queue = queue->insertion_commands; - if (ic_queue && ic_queue->first) { - insertion_command_elem_t *ic_elem = ic_queue->first; - ic_elem->counter--; - if (ic_elem->counter < 1) { - ic_queue->first = ic_elem->next; - if (ic_elem == ic_queue->last) - ic_queue->last = NULL; - mp_pool_release(ic_elem); - } - *command = ic_elem->command; - res = 0; - } - return res; -} - /** Append a newly allocated copy of <b>cell</b> to the end of the * <b>exitward</b> (or app-ward) <b>queue</b> of <b>circ</b>. If * <b>use_stats</b> is true, record statistics about the cell. @@ -2215,52 +2148,12 @@ cell_queue_append_packed_copy(circuit_t *circ, cell_queue_t *queue, { struct timeval now; packed_cell_t *copy = packed_cell_copy(cell, wide_circ_ids); + (void)circ; + (void)exitward; + (void)use_stats; tor_gettimeofday_cached(&now); copy->inserted_time = (uint32_t)tv_to_msec(&now); - /* Remember the time when this cell was put in the queue. */ - /*XXXX This may be obsoleted by inserted_time */ - if ((get_options()->CellStatistics || - get_options()->TestingEnableCellStatsEvent) && use_stats) { - uint32_t added; - insertion_time_queue_t *it_queue = queue->insertion_times; - if (!it_pool) - it_pool = mp_pool_new(sizeof(insertion_time_elem_t), 1024); - -#define SECONDS_IN_A_DAY 86400L - added = (uint32_t)(((now.tv_sec % SECONDS_IN_A_DAY) * 100L) - + ((uint32_t)now.tv_usec / (uint32_t)10000L)); - if (!it_queue) { - it_queue = tor_malloc_zero(sizeof(insertion_time_queue_t)); - queue->insertion_times = it_queue; - } - if (it_queue->last && it_queue->last->insertion_time == added) { - it_queue->last->counter++; - } else { - insertion_time_elem_t *elem = mp_pool_get(it_pool); - elem->next = NULL; - elem->insertion_time = added; - elem->counter = 1; - if (it_queue->last) { - it_queue->last->next = elem; - it_queue->last = elem; - } else { - it_queue->first = it_queue->last = elem; - } - } - } - /* Remember that we added a cell to the queue, and remember the cell - * command. */ - if (get_options()->TestingEnableCellStatsEvent && circ) { - testing_cell_stats_entry_t *ent = - tor_malloc_zero(sizeof(testing_cell_stats_entry_t)); - ent->command = cell->command; - ent->exitward = exitward; - if (!circ->testing_cell_stats) - circ->testing_cell_stats = smartlist_new(); - smartlist_add(circ->testing_cell_stats, ent); - cell_command_queue_append(queue, cell->command); - } cell_queue_append(queue, copy); } @@ -2283,14 +2176,6 @@ cell_queue_clear(cell_queue_t *queue) } TOR_SIMPLEQ_INIT(&queue->head); queue->n = 0; - if (queue->insertion_times) { - while (queue->insertion_times->first) { - insertion_time_elem_t *elem = queue->insertion_times->first; - queue->insertion_times->first = elem->next; - mp_pool_release(elem); - } - tor_free(queue->insertion_times); - } } /** Extract and return the cell at the head of <b>queue</b>; return NULL if @@ -2311,9 +2196,7 @@ cell_queue_pop(cell_queue_t *queue) size_t packed_cell_mem_cost(void) { - return sizeof(packed_cell_t) + MP_POOL_ITEM_OVERHEAD + - get_options()->CellStatistics ? - (sizeof(insertion_time_elem_t)+MP_POOL_ITEM_OVERHEAD) : 0; + return sizeof(packed_cell_t) + MP_POOL_ITEM_OVERHEAD; } /** Check whether we've got too much space used for cells. If so, @@ -2443,6 +2326,17 @@ set_streams_blocked_on_circ(circuit_t *circ, channel_t *chan, return n; } +/** Extract the command from a packed cell. */ +static uint8_t +packed_cell_get_command(const packed_cell_t *cell, int wide_circ_ids) +{ + if (wide_circ_ids) { + return get_uint8(cell->body+4); + } else { + return get_uint8(cell->body+2); + } +} + /** Pull as many cells as possible (but no more than <b>max</b>) from the * queue of the first active circuit on <b>chan</b>, and write them to * <b>chan</b>->outbuf. Return the number of cells written. Advance @@ -2504,55 +2398,30 @@ channel_flush_from_first_active_circuit(channel_t *chan, int max) /* Calculate the exact time that this cell has spent in the queue. */ if (get_options()->CellStatistics || get_options()->TestingEnableCellStatsEvent) { + uint32_t msec_waiting; struct timeval tvnow; - uint32_t flushed; - uint32_t cell_waiting_time; - insertion_time_queue_t *it_queue = queue->insertion_times; tor_gettimeofday_cached(&tvnow); - flushed = (uint32_t)((tvnow.tv_sec % SECONDS_IN_A_DAY) * 100L + - (uint32_t)tvnow.tv_usec / (uint32_t)10000L); - if (!it_queue || !it_queue->first) { - log_info(LD_GENERAL, "Cannot determine insertion time of cell. " - "Looks like the CellStatistics option was " - "recently enabled."); - } else { - insertion_time_elem_t *elem = it_queue->first; - cell_waiting_time = - (uint32_t)((flushed * 10L + SECONDS_IN_A_DAY * 1000L - - elem->insertion_time * 10L) % - (SECONDS_IN_A_DAY * 1000L)); -#undef SECONDS_IN_A_DAY - elem->counter--; - if (elem->counter < 1) { - it_queue->first = elem->next; - if (elem == it_queue->last) - it_queue->last = NULL; - mp_pool_release(elem); - } - if (get_options()->CellStatistics && !CIRCUIT_IS_ORIGIN(circ)) { - or_circ = TO_OR_CIRCUIT(circ); - or_circ->total_cell_waiting_time += cell_waiting_time; - or_circ->processed_cells++; - } - if (get_options()->TestingEnableCellStatsEvent) { - uint8_t command; - if (cell_command_queue_pop(&command, queue) < 0) { - log_info(LD_GENERAL, "Cannot determine command of cell. " - "Looks like the CELL_STATS event was " - "recently enabled."); - } else { - testing_cell_stats_entry_t *ent = - tor_malloc_zero(sizeof(testing_cell_stats_entry_t)); - ent->command = command; - ent->waiting_time = (unsigned int)cell_waiting_time / 10; - ent->removed = 1; - if (circ->n_chan == chan) - ent->exitward = 1; - if (!circ->testing_cell_stats) - circ->testing_cell_stats = smartlist_new(); - smartlist_add(circ->testing_cell_stats, ent); - } - } + msec_waiting = ((uint32_t)tv_to_msec(&tvnow)) - cell->inserted_time; + + if (get_options()->CellStatistics && !CIRCUIT_IS_ORIGIN(circ)) { + or_circ = TO_OR_CIRCUIT(circ); + or_circ->total_cell_waiting_time += msec_waiting; + or_circ->processed_cells++; + } + + if (get_options()->TestingEnableCellStatsEvent) { + uint8_t command = packed_cell_get_command(cell, chan->wide_circ_ids); + + testing_cell_stats_entry_t *ent = + tor_malloc_zero(sizeof(testing_cell_stats_entry_t)); + ent->command = command; + ent->waiting_time = msec_waiting / 10; + ent->removed = 1; + if (circ->n_chan == chan) + ent->exitward = 1; + if (!circ->testing_cell_stats) + circ->testing_cell_stats = smartlist_new(); + smartlist_add(circ->testing_cell_stats, ent); } } |