September 2017 - tor-commits - lists.torproject.org

[tor/master] test: Fix unit tests with latest scheduler changes
by nickm＠torproject.org 15 Sep '17

15 Sep '17

commit 0d0b677b6427573b87736da00e638911fac7ef7d Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Sep 14 15:26:12 2017 -0400 test: Fix unit tests with latest scheduler changes Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/scheduler.c | 10 +++++++++ src/test/test_options.c | 3 ++- src/test/test_scheduler.c | 54 +++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/src/or/scheduler.c b/src/or/scheduler.c index af4d9e5f6..acb6801ab 100644 --- a/src/or/scheduler.c +++ b/src/or/scheduler.c @@ -273,6 +273,16 @@ select_scheduler(void) { const char *chosen_sched_type = NULL; +#ifdef TOR_UNIT_TESTS + /* This is hella annoying to set in the options for every test that passes + * through the scheduler and there are many so if we don't explicitely have + * a list of types set, just put the vanilla one. */ + if (get_options()->SchedulerTypes_ == NULL) { + the_scheduler = get_vanilla_scheduler(); + return; + } +#endif + /* This list is ordered that is first entry has the first priority. Thus, as * soon as we find a scheduler type that we can use, we use it and stop. */ SMARTLIST_FOREACH_BEGIN(get_options()->SchedulerTypes_, int *, type) { diff --git a/src/test/test_options.c b/src/test/test_options.c index 507fc6b9e..9a899d910 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -401,7 +401,8 @@ fixed_get_uname(void) "ClientUseIPv4 1\n" \ "VirtualAddrNetworkIPv4 127.192.0.0/10\n" \ "VirtualAddrNetworkIPv6 [FE80::]/10\n" \ - "UseEntryGuards 1\n" + "UseEntryGuards 1\n" \ + "Schedulers Vanilla\n" typedef struct { or_options_t *old_opt; diff --git a/src/test/test_scheduler.c b/src/test/test_scheduler.c index 633a08922..963c7927a 100644 --- a/src/test/test_scheduler.c +++ b/src/test/test_scheduler.c @@ -43,8 +43,32 @@ mock_get_options(void) } static void +cleanup_scheduler_options(void) +{ + if (mocked_options.SchedulerTypes_) { + SMARTLIST_FOREACH(mocked_options.SchedulerTypes_, int *, i, tor_free(i)); + smartlist_free(mocked_options.SchedulerTypes_); + mocked_options.SchedulerTypes_ = NULL; + } +} + +static void +set_scheduler_options(int val) +{ + int *type; + + if (mocked_options.SchedulerTypes_ == NULL) { + mocked_options.SchedulerTypes_ = smartlist_new(); + } + type = tor_malloc_zero(sizeof(int)); + *type = val; + smartlist_add(mocked_options.SchedulerTypes_, type); +} + +static void clear_options(void) { + cleanup_scheduler_options(); memset(&mocked_options, 0, sizeof(mocked_options)); } @@ -385,7 +409,7 @@ update_socket_info_impl_mock(socket_table_ent_t *ent) } static void -perform_channel_state_tests(int KISTSchedRunInterval) +perform_channel_state_tests(int KISTSchedRunInterval, int sched_type) { channel_t *ch1 = NULL, *ch2 = NULL; int old_count; @@ -394,6 +418,7 @@ perform_channel_state_tests(int KISTSchedRunInterval) MOCK(get_options, mock_get_options); clear_options(); mocked_options.KISTSchedRunInterval = KISTSchedRunInterval; + set_scheduler_options(sched_type); /* Set up libevent and scheduler */ mock_event_init(); @@ -501,6 +526,7 @@ perform_channel_state_tests(int KISTSchedRunInterval) UNMOCK(scheduler_compare_channels); UNMOCK(tor_libevent_get_base); UNMOCK(get_options); + cleanup_scheduler_options(); return; } @@ -600,6 +626,7 @@ test_scheduler_loop_vanilla(void *arg) /* setup options so we're sure about what sched we are running */ MOCK(get_options, mock_get_options); clear_options(); + set_scheduler_options(SCHEDULER_VANILLA); mocked_options.KISTSchedRunInterval = -1; /* Set up libevent and scheduler */ @@ -761,6 +788,7 @@ test_scheduler_loop_vanilla(void *arg) done: tor_free(ch1); tor_free(ch2); + cleanup_scheduler_options(); UNMOCK(channel_flush_some_cells); UNMOCK(scheduler_compare_channels); @@ -772,6 +800,11 @@ static void test_scheduler_loop_kist(void *arg) { (void) arg; + +#ifndef HAVE_KIST_SUPPORT + return; +#endif + channel_t *ch1 = new_fake_channel(), *ch2 = new_fake_channel(); /* setup options so we're sure about what sched we are running */ @@ -783,6 +816,7 @@ test_scheduler_loop_kist(void *arg) MOCK(update_socket_info_impl, update_socket_info_impl_mock); clear_options(); mocked_options.KISTSchedRunInterval = 11; + set_scheduler_options(SCHEDULER_KIST); scheduler_init(); tt_assert(ch1); @@ -844,9 +878,10 @@ static void test_scheduler_channel_states(void *arg) { (void)arg; - perform_channel_state_tests(-1); // vanilla + perform_channel_state_tests(-1, SCHEDULER_VANILLA); + perform_channel_state_tests(11, SCHEDULER_KIST_LITE); #ifdef HAVE_KIST_SUPPORT - perform_channel_state_tests(11); // kist + perform_channel_state_tests(11, SCHEDULER_KIST); #endif } @@ -860,6 +895,10 @@ test_scheduler_initfree(void *arg) mock_event_init(); MOCK(tor_libevent_get_base, tor_libevent_get_base_mock); + MOCK(get_options, mock_get_options); + set_scheduler_options(SCHEDULER_KIST); + set_scheduler_options(SCHEDULER_KIST_LITE); + set_scheduler_options(SCHEDULER_VANILLA); scheduler_init(); @@ -867,11 +906,7 @@ test_scheduler_initfree(void *arg) tt_ptr_op(run_sched_ev, !=, NULL); /* We have specified nothing in the torrc and there's no consensus so the * KIST scheduler is what should be in use */ -#ifdef HAVE_KIST_SUPPORT tt_ptr_op(the_scheduler, ==, get_kist_scheduler()); -#else - tt_ptr_op(the_scheduler, ==, get_vanilla_scheduler()); -#endif tt_int_op(sched_run_interval, ==, 10); scheduler_free_all(); @@ -883,6 +918,8 @@ test_scheduler_initfree(void *arg) tt_ptr_op(run_sched_ev, ==, NULL); done: + UNMOCK(get_options); + cleanup_scheduler_options(); return; } @@ -971,6 +1008,8 @@ test_scheduler_ns_changed(void *arg) MOCK(get_options, mock_get_options); clear_options(); + set_scheduler_options(SCHEDULER_KIST); + set_scheduler_options(SCHEDULER_VANILLA); tt_ptr_op(the_scheduler, ==, NULL); @@ -1012,6 +1051,7 @@ test_scheduler_ns_changed(void *arg) done: UNMOCK(get_options); + cleanup_scheduler_options(); return; }

1 0

[tor/master] sched: Make the outbuf table local to run()
by nickm＠torproject.org 15 Sep '17

15 Sep '17

commit e0984b8a51d2363d9ff30779d339d058887831ee Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Sep 14 14:37:10 2017 -0400 sched: Make the outbuf table local to run() Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/scheduler_kist.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c index 85d0b2a1f..aad29b6a6 100644 --- a/src/or/scheduler_kist.c +++ b/src/or/scheduler_kist.c @@ -85,8 +85,6 @@ outbuf_table_ent_eq(const outbuf_table_ent_t *a, const outbuf_table_ent_t *b) return a->chan->global_identifier == b->chan->global_identifier; } -static outbuf_table_t outbuf_table = HT_INITIALIZER(); - HT_PROTOTYPE(outbuf_table_s, outbuf_table_ent_s, node, outbuf_table_ent_hash, outbuf_table_ent_eq) HT_GENERATE2(outbuf_table_s, outbuf_table_ent_s, node, outbuf_table_ent_hash, @@ -145,13 +143,6 @@ free_outbuf_info_by_ent(outbuf_table_ent_t *ent, void *data) return 1; /* So HT_FOREACH_FN will remove the element */ } -/* Clean up outbuf_table. Probably because the KIST sched impl is going away */ -static void -free_all_outbuf_info(void) -{ - HT_FOREACH_FN(outbuf_table_s, &outbuf_table, free_outbuf_info_by_ent, NULL); -} - /* Free the given socket table entry ent. */ static int free_socket_info_by_ent(socket_table_ent_t *ent, void *data) @@ -425,7 +416,6 @@ have_work(void) static void kist_free_all(void) { - free_all_outbuf_info(); free_all_socket_info(); } @@ -508,6 +498,8 @@ kist_scheduler_run(void) smartlist_t *to_readd = NULL; smartlist_t *cp = get_channels_pending(); + outbuf_table_t outbuf_table = HT_INITIALIZER(); + /* For each pending channel, collect new kernel information */ SMARTLIST_FOREACH_BEGIN(cp, const channel_t *, pchan) { init_socket_info(&socket_table, pchan); @@ -618,7 +610,8 @@ kist_scheduler_run(void) /* Write the outbuf of any channels that still have data */ HT_FOREACH_FN(outbuf_table_s, &outbuf_table, each_channel_write_to_kernel, NULL); - free_all_outbuf_info(); + /* We are done with it. */ + HT_FOREACH_FN(outbuf_table_s, &outbuf_table, free_outbuf_info_by_ent, NULL); HT_CLEAR(outbuf_table_s, &outbuf_table); log_debug(LD_SCHED, "len pending=%d, len to_readd=%d",

1 0

[tor/master] sched: switch to monotonic time; add/fix comments, style, and logs msgs
by nickm＠torproject.org 15 Sep '17

15 Sep '17

commit 61fc9c41ad8241be06587f28d1542c0caba55d1b Author: Matt Traudt <sirmatt(a)ksu.edu> Date: Tue Sep 12 22:12:24 2017 -0400 sched: switch to monotonic time; add/fix comments, style, and logs msgs --- src/or/scheduler.c | 4 +-- src/or/scheduler.h | 8 +++--- src/or/scheduler_kist.c | 63 ++++++++++++++++++++++++++++++++++++---------- src/or/scheduler_vanilla.c | 1 + 4 files changed, 57 insertions(+), 19 deletions(-) diff --git a/src/or/scheduler.c b/src/or/scheduler.c index f02b70793..b4e1aac92 100644 --- a/src/or/scheduler.c +++ b/src/or/scheduler.c @@ -289,8 +289,8 @@ set_scheduler(void) if (old_scheduler && old_scheduler->free_all) { old_scheduler->free_all(); } - /* We don't clean up the old one, we keep any type of scheduler we've - * allocated so we can do an easy switch back. */ + /* We don't clean up the old scheduler_t. We keep any type of scheduler + * we've allocated so we can do an easy switch back. */ /* Initialize the new scheduler. */ if (scheduler->init) { diff --git a/src/or/scheduler.h b/src/or/scheduler.h index ce5163b81..6cf75c585 100644 --- a/src/or/scheduler.h +++ b/src/or/scheduler.h @@ -62,10 +62,10 @@ typedef struct scheduler_s { /* (Optional) To be called whenever Tor finds out about a new consensus. * First the scheduling system as a whole will react to the new consensus - * and change the scheduler if needed. After that, whatever is the (possibly - * new) scheduler will call this so it has the chance to react to the new - * consensus too. If there's a consensus parameter that your scheduler wants - * to keep an eye on, this is where you should check for it. */ + * and change the scheduler if needed. After that, the current scheduler + * (which might be new) will call this so it has the chance to react to the + * new consensus too. If there's a consensus parameter that your scheduler + * wants to keep an eye on, this is where you should check for it. */ void (*on_new_consensus)(const networkstatus_t *old_c, const networkstatus_t *new_c); diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c index eff8ee1cb..d3b19fdd1 100644 --- a/src/or/scheduler_kist.c +++ b/src/or/scheduler_kist.c @@ -86,9 +86,16 @@ HT_GENERATE2(outbuf_table_s, outbuf_table_ent_s, node, outbuf_table_ent_hash, * Other internal data *****************************************************************************/ -static struct timeval scheduler_last_run = {0, 0}; +/* Store the last time the scheduler was run so we can decide when to next run + * the scheduler based on it. */ +static monotime_t scheduler_last_run; +/* This is a factor for the extra_space calculation in kist per-socket limits. + * It is the number of extra congestion windows we want to write to the kernel. + */ static double sock_buf_size_factor = 1.0; +/* How often the scheduler runs. */ STATIC int32_t sched_run_interval = 10; +/* Stores the kist scheduler function pointers. */ static scheduler_t *kist_scheduler = NULL; /***************************************************************************** @@ -170,6 +177,8 @@ free_socket_info_by_chan(socket_table_t *table, const channel_t *chan) free_socket_info_by_ent(ent, NULL); } +/* Perform system calls for the given socket in order to calculate kist's + * per-socket limit as documented in the function body. */ MOCK_IMPL(void, update_socket_info_impl, (socket_table_ent_t *ent)) { @@ -191,8 +200,9 @@ update_socket_info_impl, (socket_table_ent_t *ent)) * with the support previously or if the kernel was updated and lost the * support. */ log_notice(LD_SCHED, "Looks like our kernel doesn't have the support " - "for KIST anymore. Fallback to the naive approach. " - "Set KISTSchedRunInterval=-1 to disable KIST."); + "for KIST anymore. We will fallback to the naive " + "approach. Set KISTSchedRunInterval=-1 to disable " + "KIST."); kist_no_kernel_support = 1; } goto fallback; @@ -200,8 +210,9 @@ update_socket_info_impl, (socket_table_ent_t *ent)) if (ioctl(sock, SIOCOUTQNSD, &(ent->notsent)) < 0) { if (errno == EINVAL) { log_notice(LD_SCHED, "Looks like our kernel doesn't have the support " - "for KIST anymore. Fallback to the naive approach. " - "Set KISTSchedRunInterval=-1 to disable KIST."); + "for KIST anymore. We will fallback to the naive " + "approach. Set KISTSchedRunInterval=-1 to disable " + "KIST."); /* Same reason as the above. */ kist_no_kernel_support = 1; } @@ -211,10 +222,19 @@ update_socket_info_impl, (socket_table_ent_t *ent)) ent->unacked = tcp.tcpi_unacked; ent->mss = tcp.tcpi_snd_mss; + /* TCP space is the number of bytes would could give to the kernel and it + * would be able to immediately push them to the network. */ tcp_space = (ent->cwnd - ent->unacked) * ent->mss; if (tcp_space < 0) { tcp_space = 0; } + /* Imagine we have filled up tcp_space already for a socket and the scheduler + * isn't going to run again for a while. We should write a little extra to the + * kernel so it has some data to send between scheduling runs if it gets ACKs + * back so it doesn't sit idle. With the suggested sock_buf_size_factor of + * 1.0, a socket can have at most 2*cwnd data in the kernel: 1 cwnd on the + * wire waiting for ACKs and 1 cwnd ready and waiting to be sent when those + * ACKs come. */ extra_space = clamp_double_to_int64((ent->cwnd * ent->mss) * sock_buf_size_factor) - ent->notsent; @@ -225,6 +245,12 @@ update_socket_info_impl, (socket_table_ent_t *ent)) return; fallback: + /* If all of a sudden we don't have kist support, we just zero out all the + * variables for this socket since we don't know what they should be. + * We also effectively allow the socket write as much as it wants to the + * kernel, effectively returning it to vanilla scheduler behavior. Writes + * are still limited by the lower layers of Tor: socket blocking, full + * outbuf, etc. */ ent->cwnd = ent->unacked = ent->mss = ent->notsent = 0; ent->limit = INT_MAX; } @@ -391,7 +417,7 @@ kist_scheduler_on_new_consensus(const networkstatus_t *old_c, set_scheduler_run_interval(new_c); } -/* Function of the scheduler interface: run() */ +/* Function of the scheduler interface: on_new_options() */ static void kist_scheduler_on_new_options(void) { @@ -413,15 +439,16 @@ kist_scheduler_init(void) static void kist_scheduler_schedule(void) { - struct timeval now, next_run; + struct monotime_t now; + struct timeval next_run; int32_t diff; struct event *ev = get_run_sched_ev(); tor_assert(ev); if (!have_work()) { return; } - tor_gettimeofday(&now); - diff = (int32_t) tv_mdiff(&scheduler_last_run, &now); + monotime_get(&now); + diff = (int32_t) monotime_diff_msec(&scheduler_last_run, &now); if (diff < sched_run_interval) { next_run.tv_sec = 0; /* 1000 for ms -> us */ @@ -465,7 +492,9 @@ kist_scheduler_run(void) /* if we have switched to a new channel, consider writing the previous * channel's outbuf to the kernel. */ - if (!prev_chan) prev_chan = chan; + if (!prev_chan) { + prev_chan = chan; + } if (prev_chan != chan) { if (channel_should_write_to_kernel(&outbuf_table, prev_chan)) { channel_write_to_kernel(prev_chan); @@ -522,9 +551,17 @@ kist_scheduler_run(void) /* Case 3: cells to send, but cannot write */ + /* + * We want to write, but can't. If we left the channel in + * channels_pending, we would never exit the scheduling loop. We need to + * add it to a temporary list of channels to be added to channels_pending + * after the scheduling loop is over. They can hopefully be taken care of + * in the next scheduling round. + */ chan->scheduler_state = SCHED_CHAN_WAITING_TO_WRITE; - if (!to_readd) + if (!to_readd) { to_readd = smartlist_new(); + } smartlist_add(to_readd, chan); log_debug(LD_SCHED, "chan=%" PRIu64 " now waiting_to_write", chan->global_identifier); @@ -560,7 +597,7 @@ kist_scheduler_run(void) smartlist_free(to_readd); } - tor_gettimeofday(&scheduler_last_run); + monotime_get(&scheduler_last_run); } /***************************************************************************** @@ -593,7 +630,7 @@ get_kist_scheduler(void) /* Maximum interval that KIST runs (in ms). */ #define KIST_SCHED_RUN_INTERVAL_MAX 100 -/* Check the torrc for the configured KIST scheduler run frequency. +/* Check the torrc for the configured KIST scheduler run interval. * - If torrc < 0, then return the negative torrc value (shouldn't even be * using KIST) * - If torrc > 0, then return the positive torrc value (should use KIST, and diff --git a/src/or/scheduler_vanilla.c b/src/or/scheduler_vanilla.c index a5e104e97..d35b48c74 100644 --- a/src/or/scheduler_vanilla.c +++ b/src/or/scheduler_vanilla.c @@ -17,6 +17,7 @@ /* Maximum cells to flush in a single call to channel_flush_some_cells(); */ #define MAX_FLUSH_CELLS 1000 +/* Stores the vanilla scheduler function pointers. */ static scheduler_t *vanilla_scheduler = NULL; /*****************************************************************************

1 0

[tor/master] sched: Define SCHEDULER_KIST_PRIVATE for more encapsulation
by nickm＠torproject.org 15 Sep '17

15 Sep '17

commit 1033e14a693e8d80341428b592779fe145eb3152 Author: David Goulet <dgoulet(a)torproject.org> Date: Fri Sep 15 09:06:11 2017 -0400 sched: Define SCHEDULER_KIST_PRIVATE for more encapsulation Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/scheduler.c | 1 + src/or/scheduler.h | 4 ++++ src/or/scheduler_kist.c | 2 ++ src/test/test_scheduler.c | 1 + 4 files changed, 8 insertions(+) diff --git a/src/or/scheduler.c b/src/or/scheduler.c index 3860c62c4..e2466c01a 100644 --- a/src/or/scheduler.c +++ b/src/or/scheduler.c @@ -6,6 +6,7 @@ #include "compat_libevent.h" #define SCHEDULER_PRIVATE_ +#define SCHEDULER_KIST_PRIVATE #include "scheduler.h" #include <event2/event.h> diff --git a/src/or/scheduler.h b/src/or/scheduler.h index 7766b2142..98c359981 100644 --- a/src/or/scheduler.h +++ b/src/or/scheduler.h @@ -155,6 +155,8 @@ void scheduler_touch_channel(channel_t *chan); * Defined in scheduler_kist.c *********************************/ +#ifdef SCHEDULER_KIST_PRIVATE + /* Socke table entry which holds information of a channel's socket and kernel * TCP information. Only used by KIST. */ typedef struct socket_table_ent_s { @@ -188,6 +190,8 @@ int32_t kist_scheduler_run_interval(const networkstatus_t *ns); extern int32_t sched_run_interval; #endif /* TOR_UNIT_TESTS */ +#endif /* SCHEDULER_KIST_PRIVATE */ + /********************************* * Defined in scheduler_vanilla.c *********************************/ diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c index 262d61816..2dc30306d 100644 --- a/src/or/scheduler_kist.c +++ b/src/or/scheduler_kist.c @@ -1,6 +1,8 @@ /* Copyright (c) 2017, The Tor Project, Inc. */ /* See LICENSE for licensing information */ +#define SCHEDULER_KIST_PRIVATE + #include <event2/event.h> #include <netinet/tcp.h> diff --git a/src/test/test_scheduler.c b/src/test/test_scheduler.c index 963c7927a..51bedb3f9 100644 --- a/src/test/test_scheduler.c +++ b/src/test/test_scheduler.c @@ -6,6 +6,7 @@ #include <math.h> #include <event2/event.h> +#define SCHEDULER_KIST_PRIVATE #define TOR_CHANNEL_INTERNAL_ #define CHANNEL_PRIVATE_ #include "or.h"

1 0

[tor/master] sched: Always call on_channel_free() regardless of state
by nickm＠torproject.org 15 Sep '17

15 Sep '17

commit 84d18f70f3400df3e9c3b3c47288e82cd9160f6e Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Sep 14 13:29:05 2017 -0400 sched: Always call on_channel_free() regardless of state A channel can bounce in the scheduler and bounce out with the IDLE state which means that if it came in the scheduler once, it has socket information that needs to be freed from the global hash table. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/scheduler.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/or/scheduler.c b/src/or/scheduler.c index aaf991407..39427b782 100644 --- a/src/or/scheduler.c +++ b/src/or/scheduler.c @@ -547,11 +547,11 @@ scheduler_release_channel,(channel_t *chan)) offsetof(channel_t, sched_heap_idx), chan); } - if (the_scheduler->on_channel_free) { - the_scheduler->on_channel_free(chan); - } } + if (the_scheduler->on_channel_free) { + the_scheduler->on_channel_free(chan); + } chan->scheduler_state = SCHED_CHAN_IDLE; }

1 0

[tor/master] sched: Make the scheduler object static
by nickm＠torproject.org 15 Sep '17

15 Sep '17

commit 734dbfa590baf560741f0754bd81c07dfb829485 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Sep 14 14:47:59 2017 -0400 sched: Make the scheduler object static Each type of scheduler implements its own static scheduler_t object and returns a reference to it. This commit also makes it a const pointer that is it can only change inside the scheduler type subsystem but not outside for extra protection. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/scheduler.c | 5 ++--- src/or/scheduler.h | 2 +- src/or/scheduler_kist.c | 26 ++++++++++++-------------- src/or/scheduler_vanilla.c | 27 ++++++++++++--------------- src/test/test_scheduler.c | 4 ++-- 5 files changed, 29 insertions(+), 35 deletions(-) diff --git a/src/or/scheduler.c b/src/or/scheduler.c index 8d2dfac08..af4d9e5f6 100644 --- a/src/or/scheduler.c +++ b/src/or/scheduler.c @@ -148,7 +148,7 @@ * outside the scheduling system) *****************************************************************************/ -STATIC scheduler_t *the_scheduler; +STATIC const scheduler_t *the_scheduler; /* * We keep a list of channels that are pending - i.e, have cells to write @@ -317,7 +317,7 @@ select_scheduler(void) static void set_scheduler(void) { - scheduler_t *old_scheduler = the_scheduler; + const scheduler_t *old_scheduler = the_scheduler; /* From the options, select the scheduler type to set. */ select_scheduler(); @@ -395,7 +395,6 @@ scheduler_free_all(void) if (the_scheduler && the_scheduler->free_all) { the_scheduler->free_all(); } - tor_free(the_scheduler); the_scheduler = NULL; } diff --git a/src/or/scheduler.h b/src/or/scheduler.h index c69f763ff..7766b2142 100644 --- a/src/or/scheduler.h +++ b/src/or/scheduler.h @@ -147,7 +147,7 @@ void scheduler_ev_add(const struct timeval *next_run); #ifdef TOR_UNIT_TESTS extern smartlist_t *channels_pending; extern struct event *run_sched_ev; -extern scheduler_t *the_scheduler; +extern const scheduler_t *the_scheduler; void scheduler_touch_channel(channel_t *chan); #endif /* TOR_UNIT_TESTS */ diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c index f35d0fae5..262d61816 100644 --- a/src/or/scheduler_kist.c +++ b/src/or/scheduler_kist.c @@ -103,8 +103,6 @@ static monotime_t scheduler_last_run; static double sock_buf_size_factor = 1.0; /* How often the scheduler runs. */ STATIC int32_t sched_run_interval = 10; -/* Stores the kist scheduler function pointers. */ -static scheduler_t *kist_scheduler = NULL; /***************************************************************************** * Internally called function implementations @@ -637,23 +635,23 @@ kist_scheduler_run(void) * Externally called function implementations not called through scheduler_t *****************************************************************************/ +/* Stores the kist scheduler function pointers. */ +static scheduler_t kist_scheduler = { + .free_all = kist_free_all, + .on_channel_free = kist_on_channel_free, + .init = kist_scheduler_init, + .on_new_consensus = kist_scheduler_on_new_consensus, + .schedule = kist_scheduler_schedule, + .run = kist_scheduler_run, + .on_new_options = kist_scheduler_on_new_options, +}; + /* Return the KIST scheduler object. If it didn't exists, return a newly * allocated one but init() is not called. */ scheduler_t * get_kist_scheduler(void) { - if (!kist_scheduler) { - log_debug(LD_SCHED, "Allocating kist scheduler struct"); - kist_scheduler = tor_malloc_zero(sizeof(*kist_scheduler)); - kist_scheduler->free_all = kist_free_all; - kist_scheduler->on_channel_free = kist_on_channel_free; - kist_scheduler->init = kist_scheduler_init; - kist_scheduler->on_new_consensus = kist_scheduler_on_new_consensus; - kist_scheduler->schedule = kist_scheduler_schedule; - kist_scheduler->run = kist_scheduler_run; - kist_scheduler->on_new_options = kist_scheduler_on_new_options; - } - return kist_scheduler; + return &kist_scheduler; } /* Check the torrc for the configured KIST scheduler run interval. diff --git a/src/or/scheduler_vanilla.c b/src/or/scheduler_vanilla.c index 541ee8006..09653f445 100644 --- a/src/or/scheduler_vanilla.c +++ b/src/or/scheduler_vanilla.c @@ -17,9 +17,6 @@ /* Maximum cells to flush in a single call to channel_flush_some_cells(); */ #define MAX_FLUSH_CELLS 1000 -/* Stores the vanilla scheduler function pointers. */ -static scheduler_t *vanilla_scheduler = NULL; - /***************************************************************************** * Externally called function implementations *****************************************************************************/ @@ -180,20 +177,20 @@ vanilla_scheduler_run(void) n_chans_before - n_chans_after, n_chans_before); } +/* Stores the vanilla scheduler function pointers. */ +static scheduler_t vanilla_scheduler = { + .free_all = NULL, + .on_channel_free = NULL, + .init = NULL, + .on_new_consensus = NULL, + .schedule = vanilla_scheduler_schedule, + .run = vanilla_scheduler_run, + .on_new_options = NULL, +}; + scheduler_t * get_vanilla_scheduler(void) { - if (!vanilla_scheduler) { - log_debug(LD_SCHED, "Initializing vanilla scheduler struct"); - vanilla_scheduler = tor_malloc_zero(sizeof(*vanilla_scheduler)); - vanilla_scheduler->free_all = NULL; - vanilla_scheduler->on_channel_free = NULL; - vanilla_scheduler->init = NULL; - vanilla_scheduler->on_new_consensus = NULL; - vanilla_scheduler->schedule = vanilla_scheduler_schedule; - vanilla_scheduler->run = vanilla_scheduler_run; - vanilla_scheduler->on_new_options = NULL; - } - return vanilla_scheduler; + return &vanilla_scheduler; } diff --git a/src/test/test_scheduler.c b/src/test/test_scheduler.c index 0cc6d88db..633a08922 100644 --- a/src/test/test_scheduler.c +++ b/src/test/test_scheduler.c @@ -408,7 +408,7 @@ perform_channel_state_tests(int KISTSchedRunInterval) * Disable scheduler_run so we can just check the state transitions * without having to make everything it might call work too. */ - the_scheduler->run = scheduler_run_noop_mock; + ((scheduler_t *) the_scheduler)->run = scheduler_run_noop_mock; tt_int_op(smartlist_len(channels_pending), OP_EQ, 0); @@ -617,7 +617,7 @@ test_scheduler_loop_vanilla(void *arg) * without having to make everything it might call work too. */ run_func_ptr = the_scheduler->run; - the_scheduler->run = scheduler_run_noop_mock; + ((scheduler_t *) the_scheduler)->run = scheduler_run_noop_mock; tt_int_op(smartlist_len(channels_pending), OP_EQ, 0);

1 0

[tor/master] sched: Extra careful in channel_outbuf_length()
by nickm＠torproject.org 15 Sep '17

15 Sep '17

commit 3d2d8d6841a7ad7247278e837165e14d2ced43db Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Sep 14 14:27:41 2017 -0400 sched: Extra careful in channel_outbuf_length() In case we don't have a connection object in the channel, just be careful and recover. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/scheduler_kist.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c index b4234a053..2733e78f0 100644 --- a/src/or/scheduler_kist.c +++ b/src/or/scheduler_kist.c @@ -116,6 +116,12 @@ static scheduler_t *kist_scheduler = NULL; static inline size_t channel_outbuf_length(channel_t *chan) { + /* In theory, this can not happen because we can not scheduler a channel + * without a connection that has its outbuf initialized. Just in case, bug + * on this so we can understand a bit more why it happened. */ + if (BUG(BASE_CHAN_TO_TLS(chan)->conn == NULL)) { + return 0; + } return buf_datalen(TO_CONN(BASE_CHAN_TO_TLS(chan)->conn)->outbuf); }

1 0

[tor/master] sched: Don't expose the global scheduler libevent object
by nickm＠torproject.org 15 Sep '17

15 Sep '17

commit 14cd8bdd4ba6f4c5946ecd9877c77c9ba305bbe8 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Sep 14 14:22:22 2017 -0400 sched: Don't expose the global scheduler libevent object Instead, add wrappers to do the needed action the different scheduler needs with the libevent object. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/scheduler.c | 25 ++++++++++++++++++------- src/or/scheduler.h | 3 ++- src/or/scheduler_kist.c | 13 +++---------- src/or/scheduler_vanilla.c | 12 +++--------- 4 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/or/scheduler.c b/src/or/scheduler.c index 39427b782..8d2dfac08 100644 --- a/src/or/scheduler.c +++ b/src/or/scheduler.c @@ -214,13 +214,6 @@ get_channels_pending(void) return channels_pending; } -/* Return our libevent scheduler event. */ -struct event * -get_run_sched_ev(void) -{ - return run_sched_ev; -} - /* Comparison function to use when sorting pending channels */ MOCK_IMPL(int, scheduler_compare_channels, (const void *c1_v, const void *c2_v)) @@ -496,6 +489,24 @@ scheduler_channel_has_waiting_cells,(channel_t *chan)) } } +/* Add the scheduler event to the set of pending events with next_run being + * the time up to libevent should wait before triggering the event. */ +void +scheduler_ev_add(const struct timeval *next_run) +{ + tor_assert(run_sched_ev); + tor_assert(next_run); + event_add(run_sched_ev, next_run); +} + +/* Make the scheduler event active with the given flags. */ +void +scheduler_ev_active(int flags) +{ + tor_assert(run_sched_ev); + event_active(run_sched_ev, flags, 1); +} + /* * Initialize everything scheduling-related from config.c. Note this is only * called when Tor is starting up, while scheduler_t->init() is called both diff --git a/src/or/scheduler.h b/src/or/scheduler.h index b1863b119..c69f763ff 100644 --- a/src/or/scheduler.h +++ b/src/or/scheduler.h @@ -139,9 +139,10 @@ MOCK_DECL(void, scheduler_channel_has_waiting_cells, (channel_t *chan)); * Defined in scheduler.c *********************************/ smartlist_t *get_channels_pending(void); -struct event *get_run_sched_ev(void); MOCK_DECL(int, scheduler_compare_channels, (const void *c1_v, const void *c2_v)); +void scheduler_ev_active(int flags); +void scheduler_ev_add(const struct timeval *next_run); #ifdef TOR_UNIT_TESTS extern smartlist_t *channels_pending; diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c index 8646169ee..b4234a053 100644 --- a/src/or/scheduler_kist.c +++ b/src/or/scheduler_kist.c @@ -471,14 +471,7 @@ kist_scheduler_schedule(void) struct monotime_t now; struct timeval next_run; int32_t diff; - struct event *ev = get_run_sched_ev(); - IF_BUG_ONCE(!ev) { - log_warn(LD_SCHED, "Wow we don't have a scheduler event. That's really " - "weird! We can't really schedule a scheduling run with libevent " - "without it. So we're going to stop trying now and hope we have " - "one next time. If we never get one, we're broken."); - return; - } + if (!have_work()) { return; } @@ -489,9 +482,9 @@ kist_scheduler_schedule(void) /* 1000 for ms -> us */ next_run.tv_usec = (sched_run_interval - diff) * 1000; /* Readding an event reschedules it. It does not duplicate it. */ - event_add(ev, &next_run); + scheduler_ev_add(&next_run); } else { - event_active(ev, EV_TIMEOUT, 1); + scheduler_ev_active(EV_TIMEOUT); } } diff --git a/src/or/scheduler_vanilla.c b/src/or/scheduler_vanilla.c index efc0e98de..541ee8006 100644 --- a/src/or/scheduler_vanilla.c +++ b/src/or/scheduler_vanilla.c @@ -43,15 +43,9 @@ vanilla_scheduler_schedule(void) if (!have_work()) { return; } - struct event *ev = get_run_sched_ev(); - IF_BUG_ONCE(!ev) { - log_warn(LD_SCHED, "Wow we don't have a scheduler event. That's really " - "weird! We can't really schedule a scheduling run with libevent " - "without it. So we're going to stop trying now and hope we have " - "one next time. If we never get one, we're broken."); - return; - } - event_active(ev, EV_TIMEOUT, 1); + + /* Activate our event so it can process channels. */ + scheduler_ev_active(EV_TIMEOUT); } static void

1 0

[tor/master] sched: Compare channel pointer in the socket table
by nickm＠torproject.org 15 Sep '17

15 Sep '17

commit bd34a0d30f754f3dd75199d915febe918d058f30 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Sep 14 14:37:57 2017 -0400 sched: Compare channel pointer in the socket table Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/scheduler_kist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c index aad29b6a6..f35d0fae5 100644 --- a/src/or/scheduler_kist.c +++ b/src/or/scheduler_kist.c @@ -51,7 +51,7 @@ socket_table_ent_hash(const socket_table_ent_t *ent) static unsigned socket_table_ent_eq(const socket_table_ent_t *a, const socket_table_ent_t *b) { - return a->chan->global_identifier == b->chan->global_identifier; + return a->chan == b->chan; } typedef HT_HEAD(socket_table_s, socket_table_ent_s) socket_table_t;

1 0

[tor/master] sched: Improve logging if KIST is disabled
by nickm＠torproject.org 15 Sep '17

15 Sep '17

commit c910610701475d6fcb13284c318fe5c10fc4bdea Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Sep 14 16:04:47 2017 -0400 sched: Improve logging if KIST is disabled Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/scheduler.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/or/scheduler.c b/src/or/scheduler.c index acb6801ab..3860c62c4 100644 --- a/src/or/scheduler.c +++ b/src/or/scheduler.c @@ -293,10 +293,17 @@ select_scheduler(void) goto end; case SCHEDULER_KIST: if (!scheduler_can_use_kist()) { - log_warn(LD_SCHED, "Scheduler KIST can't be used. Consider removing " - "it from Schedulers or if you have a tor built " - "with KIST support, you should make sure " - "KISTSchedRunInterval is a non zero value"); +#ifdef HAVE_KIST_SUPPORT + if (get_options()->KISTSchedRunInterval == -1) { + log_info(LD_SCHED, "Scheduler type KIST can not be used. It is " + "disabled because KISTSchedRunInterval=-1"); + } else { + log_notice(LD_SCHED, "Scheduler type KIST has been disabled by " + "the consensus."); + } +#else /* HAVE_KIST_SUPPORT */ + log_info(LD_SCHED, "Scheduler type KIST not built in"); +#endif /* HAVE_KIST_SUPPORT */ continue; } the_scheduler = get_kist_scheduler();

1 0