nihilist/tor
1
0
Fork 0
mirror of https://gitlab.torproject.org/tpo/core/tor.git synced 2024-11-10 13:13:44 +01:00
Code Issues Packages Projects Releases Wiki Activity

channel: Remove unused write cell functions

Browse Source 
The channel_write_cell() and channel_write_var_cell() can't be possibly called
nor are used by tor. We only write on the connection outbuf packed cell coming
from the scheduler that takes them from the circuit queue.

This makes channel_write_packed_cell() the only usable function. It is
simplify and now returns a code value. The reason for this is that in the next
commit(s), we'll re-queue the cell onto the circuit queue if the write fails.

Finally, channel unit tests are being removed with this commit because they do
not match the new semantic. They will be re-written in future commits.

Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit is contained in:
David Goulet 2017-11-21 12:44:46 -05:00
parent e1c29a769c
commit 6d1ea7766b
3 changed files with 45 additions and 320 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

210
src/or/channel.c
View File

@ -148,9 +148,6 @@ HT_PROTOTYPE(channel_idmap, channel_idmap_entry_s, node, channel_idmap_hash,
HT_GENERATE2(channel_idmap, channel_idmap_entry_s, node, channel_idmap_hash,
channel_idmap_eq, 0.5, tor_reallocarray_, tor_free_)
static int is_destroy_cell(channel_t *chan,
const cell_queue_entry_t *q, circid_t *circid_out);
/* Functions to maintain the digest map */
static void channel_add_to_digest_map(channel_t *chan);
static void channel_remove_from_digest_map(channel_t *chan);
@ -161,10 +158,6 @@ channel_free_list(smartlist_t *channels, int mark_for_close);
static void
channel_listener_free_list(smartlist_t *channels, int mark_for_close);
static void channel_listener_force_free(channel_listener_t *chan_l);
static size_t channel_get_cell_queue_entry_size(channel_t *chan,
cell_queue_entry_t *q);
static void
channel_write_cell_queue_entry(channel_t *chan, cell_queue_entry_t *q);
/***********************************
* Channel state utility functions *
@ -1628,151 +1621,55 @@ channel_set_remote_end(channel_t *chan,
channel_add_to_digest_map(chan);
}
/**
* Ask how big the cell contained in a cell_queue_entry_t is
*/
static size_t
channel_get_cell_queue_entry_size(channel_t *chan, cell_queue_entry_t *q)
{
size_t rv = 0;
tor_assert(chan);
tor_assert(q);
switch (q->type) {
case CELL_QUEUE_FIXED:
rv = get_cell_network_size(chan->wide_circ_ids);
break;
case CELL_QUEUE_VAR:
rv = get_var_cell_header_size(chan->wide_circ_ids) +
(q->u.var.var_cell ? q->u.var.var_cell->payload_len : 0);
break;
case CELL_QUEUE_PACKED:
rv = get_cell_network_size(chan->wide_circ_ids);
break;
default:
tor_assert_nonfatal_unreached_once();
}
return rv;
}
/**
* Write to a channel based on a cell_queue_entry_t
*
* Given a cell_queue_entry_t filled out by the caller, try to send the cell
* and queue it if we can't.
*/
static void
channel_write_cell_queue_entry(channel_t *chan, cell_queue_entry_t *q)
static int
write_packed_cell(channel_t *chan, packed_cell_t *cell)
{
int result = 0;
int ret = -1;
size_t cell_bytes;
tor_assert(chan);
tor_assert(q);
tor_assert(cell);
/* Assert that the state makes sense for a cell write */
tor_assert(CHANNEL_CAN_HANDLE_CELLS(chan));
{
circid_t circ_id;
if (is_destroy_cell(chan, q, &circ_id)) {
if (packed_cell_is_destroy(chan, cell, &circ_id)) {
channel_note_destroy_not_pending(chan, circ_id);
}
}
/* For statistical purposes, figure out how big this cell is */
cell_bytes = channel_get_cell_queue_entry_size(chan, q);
cell_bytes = get_cell_network_size(chan->wide_circ_ids);
/* Can we send it right out? If so, try */
if (CHANNEL_IS_OPEN(chan)) {
/* Pick the right write function for this cell type and save the result */
switch (q->type) {
case CELL_QUEUE_FIXED:
tor_assert(chan->write_cell);
tor_assert(q->u.fixed.cell);
result = chan->write_cell(chan, q->u.fixed.cell);
break;
case CELL_QUEUE_PACKED:
tor_assert(chan->write_packed_cell);
tor_assert(q->u.packed.packed_cell);
result = chan->write_packed_cell(chan, q->u.packed.packed_cell);
break;
case CELL_QUEUE_VAR:
tor_assert(chan->write_var_cell);
tor_assert(q->u.var.var_cell);
result = chan->write_var_cell(chan, q->u.var.var_cell);
break;
default:
tor_assert(1);
}
/* Check if we got it out */
if (result > 0) {
/* Timestamp for transmission */
channel_timestamp_xmit(chan);
/* If we're here the queue is empty, so it's drained too */
channel_timestamp_drained(chan);
/* Update the counter */
++(chan->n_cells_xmitted);
chan->n_bytes_xmitted += cell_bytes;
}
if (!CHANNEL_IS_OPEN(chan)) {
goto done;
}
/* XXX: If the cell wasn't sent, we need to propagate the error back so we
* can put it back in the circuit queue. */
}
/** Write a generic cell type to a channel
*
* Write a generic cell to a channel. It is called by channel_write_cell(),
* channel_write_var_cell() and channel_write_packed_cell() in order to reduce
* code duplication. Notice that it takes cell as pointer of type void,
* this can be dangerous because no type check is performed.
*/
void
channel_write_cell_generic_(channel_t *chan, const char *cell_type,
void *cell, cell_queue_entry_t *q)
{
tor_assert(chan);
tor_assert(cell);
if (CHANNEL_IS_CLOSING(chan)) {
log_debug(LD_CHANNEL, "Discarding %c %p on closing channel %p with "
"global ID "U64_FORMAT, *cell_type, cell, chan,
U64_PRINTF_ARG(chan->global_identifier));
tor_free(cell);
return;
/* Write the cell on the connection's outbuf. */
if (chan->write_packed_cell(chan, cell) < 0) {
goto done;
}
log_debug(LD_CHANNEL,
"Writing %c %p to channel %p with global ID "
U64_FORMAT, *cell_type,
cell, chan, U64_PRINTF_ARG(chan->global_identifier));
/* Timestamp for transmission */
channel_timestamp_xmit(chan);
/* If we're here the queue is empty, so it's drained too */
channel_timestamp_drained(chan);
/* Update the counter */
++(chan->n_cells_xmitted);
chan->n_bytes_xmitted += cell_bytes;
/* Successfully sent the cell. */
ret = 0;
channel_write_cell_queue_entry(chan, q);
}
/**
* Write a cell to a channel
*
* Write a fixed-length cell to a channel using the write_cell() method.
* This is equivalent to the pre-channels connection_or_write_cell_to_buf();
* it is called by the transport-independent code to deliver a cell to a
* channel for transmission.
*/
void
channel_write_cell(channel_t *chan, cell_t *cell)
{
cell_queue_entry_t q;
q.type = CELL_QUEUE_FIXED;
q.u.fixed.cell = cell;
channel_write_cell_generic_(chan, "cell_t", cell, &q);
done:
return ret;
}
/**
@ -1782,32 +1679,24 @@ channel_write_cell(channel_t *chan, cell_t *cell)
* called by the transport-independent code to deliver a packed cell to a
* channel for transmission.
*/
void
channel_write_packed_cell(channel_t *chan, packed_cell_t *packed_cell)
int
channel_write_packed_cell(channel_t *chan, packed_cell_t *cell)
{
cell_queue_entry_t q;
q.type = CELL_QUEUE_PACKED;
q.u.packed.packed_cell = packed_cell;
channel_write_cell_generic_(chan, "packed_cell_t", packed_cell, &q);
}
tor_assert(chan);
tor_assert(cell);
/**
* Write a variable-length cell to a channel
*
* Write a variable-length cell to a channel using the write_cell() method.
* This is equivalent to the pre-channels
* connection_or_write_var_cell_to_buf(); it's called by the transport-
* independent code to deliver a var_cell to a channel for transmission.
*/
if (CHANNEL_IS_CLOSING(chan)) {
log_debug(LD_CHANNEL, "Discarding %p on closing channel %p with "
"global ID "U64_FORMAT, cell, chan,
U64_PRINTF_ARG(chan->global_identifier));
tor_free(cell);
return -1;
}
log_debug(LD_CHANNEL,
"Writing %p to channel %p with global ID "
U64_FORMAT, cell, chan, U64_PRINTF_ARG(chan->global_identifier));
void
channel_write_var_cell(channel_t *chan, var_cell_t *var_cell)
{
cell_queue_entry_t q;
q.type = CELL_QUEUE_VAR;
q.u.var.var_cell = var_cell;
channel_write_cell_generic_(chan, "var_cell_t", var_cell, &q);
return write_packed_cell(chan, cell);
}
/**
@ -2352,31 +2241,6 @@ packed_cell_is_destroy(channel_t *chan,
return 0;
}
/* DOCDOC */
static int
is_destroy_cell(channel_t *chan,
const cell_queue_entry_t *q, circid_t *circid_out)
{
*circid_out = 0;
switch (q->type) {
case CELL_QUEUE_FIXED:
if (q->u.fixed.cell->command == CELL_DESTROY) {
*circid_out = q->u.fixed.cell->circ_id;
return 1;
}
break;
case CELL_QUEUE_VAR:
if (q->u.var.var_cell->command == CELL_DESTROY) {
*circid_out = q->u.var.var_cell->circ_id;
return 1;
}
break;
case CELL_QUEUE_PACKED:
return packed_cell_is_destroy(chan, q->u.packed.packed_cell, circid_out);
}
return 0;
}
/**
* Send destroy cell on a channel
*

4
src/or/channel.h
View File

@ -406,9 +406,7 @@ channel_listener_state_to_string(channel_listener_state_t state);
/* Abstract channel operations */
void channel_mark_for_close(channel_t *chan);
void channel_write_cell(channel_t *chan, cell_t *cell);
void channel_write_packed_cell(channel_t *chan, packed_cell_t *cell);
void channel_write_var_cell(channel_t *chan, var_cell_t *cell);
int channel_write_packed_cell(channel_t *chan, packed_cell_t *cell);
void channel_listener_mark_for_close(channel_listener_t *chan_l);

151
src/test/test_channel.c
View File

@ -26,7 +26,6 @@ static cell_t * test_chan_last_seen_fixed_cell_ptr = NULL;
static int test_chan_var_cells_recved = 0;
static var_cell_t * test_chan_last_seen_var_cell_ptr = NULL;
static int test_cells_written = 0;
static int test_destroy_not_pending_calls = 0;
static int test_doesnt_want_writes_count = 0;
static int test_dumpstats_calls = 0;
static int test_has_waiting_cells_count = 0;
@ -37,8 +36,6 @@ static int dump_statistics_mock_matches = 0;
static void chan_test_channel_dump_statistics_mock(
channel_t *chan, int severity);
static void channel_note_destroy_not_pending_mock(channel_t *ch,
circid_t circid);
static void chan_test_cell_handler(channel_t *ch,
cell_t *cell);
static const char * chan_test_describe_transport(channel_t *ch);
@ -61,17 +58,6 @@ static void scheduler_channel_doesnt_want_writes_mock(channel_t *ch);
static void test_channel_dumpstats(void *arg);
static void test_channel_incoming(void *arg);
static void test_channel_lifecycle(void *arg);
static void test_channel_write(void *arg);
static void
channel_note_destroy_not_pending_mock(channel_t *ch,
circid_t circid)
{
(void)ch;
(void)circid;
++test_destroy_not_pending_calls;
}
static const char *
chan_test_describe_transport(channel_t *ch)
@ -447,7 +433,7 @@ static void
test_channel_dumpstats(void *arg)
{
channel_t *ch = NULL;
cell_t *cell = NULL;
packed_cell_t *p_cell = NULL;
int old_count;
(void)arg;
@ -513,12 +499,10 @@ test_channel_dumpstats(void *arg)
tt_assert(time(NULL) > ch->timestamp_created);
/* Put cells through it both ways to make the counters non-zero */
cell = tor_malloc_zero(sizeof(*cell));
make_fake_cell(cell);
p_cell = packed_cell_new();
test_chan_accept_cells = 1;
old_count = test_cells_written;
channel_write_cell(ch, cell);
cell = NULL;
channel_write_packed_cell(ch, p_cell);
tt_int_op(test_cells_written, OP_EQ, old_count + 1);
tt_assert(ch->n_bytes_xmitted > 0);
tt_assert(ch->n_cells_xmitted > 0);
@ -530,10 +514,8 @@ test_channel_dumpstats(void *arg)
tt_ptr_op(channel_get_cell_handler(ch), OP_EQ, chan_test_cell_handler);
tt_ptr_op(channel_get_var_cell_handler(ch), OP_EQ,
chan_test_var_cell_handler);
cell = tor_malloc_zero(sizeof(cell_t));
make_fake_cell(cell);
p_cell = packed_cell_new();
old_count = test_chan_fixed_cells_recved;
tor_free(cell);
tt_int_op(test_chan_fixed_cells_recved, OP_EQ, old_count + 1);
tt_assert(ch->n_bytes_recved > 0);
tt_assert(ch->n_cells_recved > 0);
@ -555,7 +537,6 @@ test_channel_dumpstats(void *arg)
ch = NULL;
done:
tor_free(cell);
free_fake_channel(ch);
UNMOCK(scheduler_channel_doesnt_want_writes);
@ -652,7 +633,7 @@ static void
test_channel_lifecycle(void *arg)
{
channel_t *ch1 = NULL, *ch2 = NULL;
cell_t *cell = NULL;
packed_cell_t *p_cell = NULL;
int old_count, init_doesnt_want_writes_count;
int init_releases_count;
@ -685,10 +666,9 @@ test_channel_lifecycle(void *arg)
tt_assert(ch1->registered);
/* Try to write a cell through (should queue) */
cell = tor_malloc_zero(sizeof(cell_t));
make_fake_cell(cell);
p_cell = packed_cell_new();
old_count = test_cells_written;
channel_write_cell(ch1, cell);
channel_write_packed_cell(ch1, p_cell);
tt_int_op(old_count, OP_EQ, test_cells_written);
/* Move it to OPEN and flush */
@ -915,122 +895,6 @@ test_channel_lifecycle_2(void *arg)
return;
}
static void
test_channel_write(void *arg)
{
channel_t *ch = NULL;
cell_t *cell = tor_malloc_zero(sizeof(cell_t));
packed_cell_t *packed_cell = NULL;
var_cell_t *var_cell =
tor_malloc_zero(sizeof(var_cell_t) + CELL_PAYLOAD_SIZE);
int old_count;
(void)arg;
packed_cell = packed_cell_new();
tt_assert(packed_cell);
ch = new_fake_channel();
tt_assert(ch);
make_fake_cell(cell);
make_fake_var_cell(var_cell);
/* Tell it to accept cells */
test_chan_accept_cells = 1;
old_count = test_cells_written;
channel_write_cell(ch, cell);
cell = NULL;
tt_assert(test_cells_written == old_count + 1);
channel_write_var_cell(ch, var_cell);
var_cell = NULL;
tt_assert(test_cells_written == old_count + 2);
channel_write_packed_cell(ch, packed_cell);
packed_cell = NULL;
tt_assert(test_cells_written == old_count + 3);
/* Now we test queueing; tell it not to accept cells */
test_chan_accept_cells = 0;
/* ...and keep it from trying to flush the queue */
ch->state = CHANNEL_STATE_MAINT;
/* Get a fresh cell */
cell = tor_malloc_zero(sizeof(cell_t));
make_fake_cell(cell);
old_count = test_cells_written;
channel_write_cell(ch, cell);
tt_assert(test_cells_written == old_count);
/*
* Now change back to open with channel_change_state() and assert that it
* gets drained from the queue.
*/
test_chan_accept_cells = 1;
channel_change_state_open(ch);
tt_assert(test_cells_written == old_count + 1);
/*
* Check the note destroy case
*/
cell = tor_malloc_zero(sizeof(cell_t));
make_fake_cell(cell);
cell->command = CELL_DESTROY;
/* Set up the mock */
MOCK(channel_note_destroy_not_pending,
channel_note_destroy_not_pending_mock);
old_count = test_destroy_not_pending_calls;
channel_write_cell(ch, cell);
tt_assert(test_destroy_not_pending_calls == old_count + 1);
/* Now send a non-destroy and check we don't call it */
cell = tor_malloc_zero(sizeof(cell_t));
make_fake_cell(cell);
channel_write_cell(ch, cell);
tt_assert(test_destroy_not_pending_calls == old_count + 1);
UNMOCK(channel_note_destroy_not_pending);
/*
* Now switch it to CLOSING so we can test the discard-cells case
* in the channel_write_*() functions.
*/
MOCK(scheduler_release_channel, scheduler_release_channel_mock);
channel_mark_for_close(ch);
UNMOCK(scheduler_release_channel);
/* Send cells that will drop in the closing state */
old_count = test_cells_written;
cell = tor_malloc_zero(sizeof(cell_t));
make_fake_cell(cell);
channel_write_cell(ch, cell);
cell = NULL;
tt_assert(test_cells_written == old_count);
var_cell = tor_malloc_zero(sizeof(var_cell_t) + CELL_PAYLOAD_SIZE);
make_fake_var_cell(var_cell);
channel_write_var_cell(ch, var_cell);
var_cell = NULL;
tt_assert(test_cells_written == old_count);
packed_cell = packed_cell_new();
channel_write_packed_cell(ch, packed_cell);
packed_cell = NULL;
tt_assert(test_cells_written == old_count);
done:
free_fake_channel(ch);
tor_free(var_cell);
tor_free(cell);
packed_cell_free(packed_cell);
return;
}
static void
test_channel_id_map(void *arg)
{
@ -1142,7 +1006,6 @@ struct testcase_t channel_tests[] = {
{ "incoming", test_channel_incoming, TT_FORK, NULL, NULL },
{ "lifecycle", test_channel_lifecycle, TT_FORK, NULL, NULL },
{ "lifecycle_2", test_channel_lifecycle_2, TT_FORK, NULL, NULL },
{ "write", test_channel_write, TT_FORK, NULL, NULL },
{ "id_map", test_channel_id_map, TT_FORK, NULL, NULL },
END_OF_TESTCASES
};