April 2018 - tor-commits - lists.torproject.org

[tor/master] Merge branch 'ticket25931'
by nickm＠torproject.org 26 Apr '18

26 Apr '18

commit 857e210b7d4939cf32d7d519b8b4d739d0b23f43 Merge: 96c5ac338 8a81a7087 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Apr 26 13:52:16 2018 -0400 Merge branch 'ticket25931' changes/ticket25931 | 9 +++++++++ src/or/consdiffmgr.c | 33 +++++++++++++++++++++++++++++++-- src/or/main.c | 6 ------ 3 files changed, 40 insertions(+), 8 deletions(-)

1 0

[tor/master] Move close-and-cleanup functions to a postloop event.
by nickm＠torproject.org 26 Apr '18

26 Apr '18

commit 96c5ac338a7df986d2419e14801df42421debd06 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Apr 26 13:15:38 2018 -0400 Move close-and-cleanup functions to a postloop event. Implements ticket 25932. --- changes/ticket25932 | 9 +++++++++ src/or/channel.c | 4 ++++ src/or/circuitlist.c | 1 + src/or/main.c | 43 ++++++++++++++++++++++++++++++------------- src/or/main.h | 1 + 5 files changed, 45 insertions(+), 13 deletions(-) diff --git a/changes/ticket25932 b/changes/ticket25932 new file mode 100644 index 000000000..67f56f7cc --- /dev/null +++ b/changes/ticket25932 @@ -0,0 +1,9 @@ + o Minor features (mainloop): + - Move responsibility for + closing connections, circuits, and channels + from a once-per-second callback to a callback that is only scheduled as + needed. Once enough items are removed from our once-per-second + callback, we can eliminate it entirely to conserve CPU when idle. + Closes ticket + 25932. + diff --git a/src/or/channel.c b/src/or/channel.c index 68245db49..c30e50801 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -69,6 +69,7 @@ #include "circuitmux.h" #include "entrynodes.h" #include "geoip.h" +#include "main.h" #include "nodelist.h" #include "relay.h" #include "rephist.h" @@ -404,6 +405,7 @@ channel_register(channel_t *chan) /* Put it in the finished list, creating it if necessary */ if (!finished_channels) finished_channels = smartlist_new(); smartlist_add(finished_channels, chan); + mainloop_schedule_postloop_cleanup(); } else { /* Put it in the active list, creating it if necessary */ if (!active_channels) active_channels = smartlist_new(); @@ -1548,6 +1550,7 @@ channel_change_state_(channel_t *chan, channel_state_t to_state) if (active_channels) smartlist_remove(active_channels, chan); if (!finished_channels) finished_channels = smartlist_new(); smartlist_add(finished_channels, chan); + mainloop_schedule_postloop_cleanup(); } /* Need to put on active list? */ else if (!was_active && is_active) { @@ -1666,6 +1669,7 @@ channel_listener_change_state(channel_listener_t *chan_l, if (active_listeners) smartlist_remove(active_listeners, chan_l); if (!finished_listeners) finished_listeners = smartlist_new(); smartlist_add(finished_listeners, chan_l); + mainloop_schedule_postloop_cleanup(); } /* Need to put on active list? */ else if (!was_active && is_active) { diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 9a82713cb..ea2f2c15c 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -2067,6 +2067,7 @@ circuit_mark_for_close_, (circuit_t *circ, int reason, int line, circuits_pending_close = smartlist_new(); smartlist_add(circuits_pending_close, circ); + mainloop_schedule_postloop_cleanup(); log_info(LD_GENERAL, "Circuit %u (id: %" PRIu32 ") marked for close at " "%s:%d (orig reason: %d, new reason: %d)", diff --git a/src/or/main.c b/src/or/main.c index ffe407329..18b9b780d 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -446,6 +446,7 @@ add_connection_to_closeable_list(connection_t *conn) tor_assert(conn->marked_for_close); assert_connection_ok(conn, time(NULL)); smartlist_add(closeable_connection_lst, conn); + mainloop_schedule_postloop_cleanup(); } /** Return 1 if conn is on the closeable list, else return 0. */ @@ -1599,6 +1600,30 @@ reschedule_directory_downloads(void) periodic_event_reschedule(launch_descriptor_fetches_event); } +/** Mainloop callback: clean up circuits, channels, and connections + * that are pending close. */ +static void +postloop_cleanup_cb(mainloop_event_t *ev, void *arg) +{ + (void)ev; + (void)arg; + circuit_close_all_marked(); + close_closeable_connections(); + channel_run_cleanup(); + channel_listener_run_cleanup(); +} + +/** Event to run postloop_cleanup_cb */ +static mainloop_event_t *postloop_cleanup_ev=NULL; + +/** Schedule a post-loop event to clean up marked channels, connections, and + * circuits. */ +void +mainloop_schedule_postloop_cleanup(void) +{ + mainloop_event_activate(postloop_cleanup_ev); +} + #define LONGEST_TIMER_PERIOD (30 * 86400) /** Helper: Return the number of seconds between <b>now</b> and <b>next</b>, * clipped to the range [1 second, LONGEST_TIMER_PERIOD]. */ @@ -1710,23 +1735,10 @@ run_scheduled_events(time_t now) run_connection_housekeeping(i, now); } - /* 6. And remove any marked circuits... */ - circuit_close_all_marked(); - - /* 8. and blow away any connections that need to die. have to do this now, - * because if we marked a conn for close and left its socket -1, then - * we'll pass it to poll/select and bad things will happen. - */ - close_closeable_connections(); - /* 8b. And if anything in our state is ready to get flushed to disk, we * flush it. */ or_state_save(now); - /* 8c. Do channel cleanup just like for connections */ - channel_run_cleanup(); - channel_listener_run_cleanup(); - /* 11b. check pending unconfigured managed proxies */ if (!net_is_disabled() && pt_proxies_configuration_pending()) pt_configure_remaining_proxies(); @@ -2628,6 +2640,10 @@ do_main_loop(void) schedule_active_linked_connections_event = mainloop_event_postloop_new(schedule_active_linked_connections_cb, NULL); } + if (!postloop_cleanup_ev) { + postloop_cleanup_ev = + mainloop_event_postloop_new(postloop_cleanup_cb, NULL); + } /* initialize dns resolve map, spawn workers if needed */ if (dns_init() < 0) { @@ -3512,6 +3528,7 @@ tor_free_all(int postfork) tor_event_free(initialize_periodic_events_event); mainloop_event_free(directory_all_unreachable_cb_event); mainloop_event_free(schedule_active_linked_connections_event); + mainloop_event_free(postloop_cleanup_ev); #ifdef HAVE_SYSTEMD_209 periodic_timer_free(systemd_watchdog_timer); diff --git a/src/or/main.h b/src/or/main.h index 0e3de7d4b..4f3b84e30 100644 --- a/src/or/main.h +++ b/src/or/main.h @@ -61,6 +61,7 @@ void dns_servers_relaunch_checks(void); void reset_all_main_loop_timers(void); void reschedule_descriptor_update_check(void); void reschedule_directory_downloads(void); +void mainloop_schedule_postloop_cleanup(void); MOCK_DECL(long,get_uptime,(void)); MOCK_DECL(void,reset_uptime,(void));

1 0

[tor/master] Add a test for geoip_load_file() using geoip6
by dgoulet＠torproject.org 26 Apr '18

26 Apr '18

commit 071236e3e252fd0dcf5102739adecb8e69a76c4c Author: juga0 <juga(a)riseup.net> Date: Mon Apr 2 14:16:04 2018 +0000 Add a test for geoip_load_file() using geoip6 Signed-off-by: Isis Lovecruft <isis(a)torproject.org> --- src/test/test_geoip.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index 20f6114dc..b4719be6c 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -444,6 +444,68 @@ test_geoip_load_file(void *arg) tor_free(dhex); } +static void +test_geoip6_load_file(void *arg) +{ + (void)arg; + struct in6_addr iaddr6; + char *contents = NULL; + char *dhex = NULL; + + /* A nonexistant filename should fail. */ + /* NOTE: this is probably not needed, as the path is the same, just using + AF_INET6 */ + tt_int_op(-1, OP_EQ, + geoip_load_file(AF_INET6, "/you/did/not/put/a/file/here/I/hope")); + + /* Any lookup attempt should say "-1" because we have no info */ + tor_inet_pton(AF_INET6, "2001:4860:4860::8888", &iaddr6); + tt_int_op(-1, OP_EQ, geoip_get_country_by_ipv6(&iaddr6)); + + /* Load geiop6 file */ + const char FNAME6[] = SRCDIR "/src/config/geoip6"; + tt_int_op(0, OP_EQ, geoip_load_file(AF_INET6, FNAME6)); + + /* Check that we loaded some countries; this will fail if there are ever + * fewer than 50 countries in the world. */ + tt_int_op(geoip_get_n_countries(), OP_GE, 50); + + /* Let's see where 2001:4860:4860::8888 (google dns) is. */ + const char *caddr6 = "2001:4860:4860::8888"; + tor_inet_pton(AF_INET6, caddr6, &iaddr6); + int country6 = geoip_get_country_by_ipv6(&iaddr6); + tt_int_op(country6, OP_GE, 1); + + const char *cc6 = geoip_get_country_name(country6); + tt_int_op(strlen(cc6), OP_EQ, 2); + + /* The digest should be set.... */ + tt_str_op("0000000000000000000000000000000000000000", OP_NE, + geoip_db_digest(AF_INET6)); + + /* And it should be set correctly */ + contents = read_file_to_str(FNAME6, RFTS_BIN, NULL); + uint8_t d[DIGEST_LEN]; + crypto_digest((char*)d, contents, strlen(contents)); + dhex = tor_strdup(hex_str((char*)d, DIGEST_LEN)); + tt_str_op(dhex, OP_EQ, geoip_db_digest(AF_INET6)); + + /* Make sure geoip_free_all() works. */ + /* NOTE: is this needed here?, if geoip_free_all, it should not change with + ipv6 */ + geoip_free_all(); + tt_int_op(1, OP_EQ, geoip_get_n_countries()); + tt_str_op("??", OP_EQ, geoip_get_country_name(0)); + tor_inet_pton(AF_INET6, "::1:2:3:4", &iaddr6); + tt_int_op(-1, OP_EQ, geoip_get_country_by_ipv6(&iaddr6)); + tt_str_op("0000000000000000000000000000000000000000", OP_EQ, + geoip_db_digest(AF_INET6)); + + done: + tor_free(contents); + tor_free(dhex); +} + #define ENT(name) \ { #name, test_ ## name , 0, NULL, NULL } #define FORK(name) \ @@ -459,6 +521,7 @@ struct testcase_t geoip_tests[] = { { "geoip", test_geoip, TT_FORK, NULL, NULL }, { "geoip_with_pt", test_geoip_with_pt, TT_FORK, NULL, NULL }, { "load_file", test_geoip_load_file, TT_FORK|SKIP_ON_WINDOWS, NULL, NULL }, + { "load_file6", test_geoip6_load_file, TT_FORK, NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/master] Add test to check that loading a 2nd file replaces the 1st
by dgoulet＠torproject.org 26 Apr '18

26 Apr '18

commit 8be1ac8abe56e605009acc59429bd53e1ab6853f Author: juga0 <juga(a)riseup.net> Date: Mon Apr 9 09:20:12 2018 +0000 Add test to check that loading a 2nd file replaces the 1st Signed-off-by: Isis Lovecruft <isis(a)torproject.org> --- src/test/test_geoip.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index b4719be6c..b35997edc 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -506,6 +506,33 @@ test_geoip6_load_file(void *arg) tor_free(dhex); } +static void +test_geoip_load_2nd_file(void *arg) +{ + (void)arg; + + /* Load 1nd geoip file */ + const char FNAME[] = SRCDIR "/src/config/geoip"; + tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME)); + + int num_countries_geoip = geoip_get_n_countries(); + + /* Load 2st geoip (empty) file */ + const char FNAME2[] = SRCDIR "/src/test/geoip_dummy"; + tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME2)); + + int num_countries_geoip2 = geoip_get_n_countries(); + /* FIXME: should not this be different? */ + /* tt_int_op(num_countries_geoip, OP_NE, num_countries_geoip2); */ + + /* Check that there is no geoip information for 8.8.8.8, */ + /* since loading the empty 2nd file should have delete it. */ + int country = geoip_get_country_by_ipv4(0x08080808); + tt_int_op(country, OP_EQ, 0); + + done: ; +} + #define ENT(name) \ { #name, test_ ## name , 0, NULL, NULL } #define FORK(name) \ @@ -522,6 +549,7 @@ struct testcase_t geoip_tests[] = { { "geoip_with_pt", test_geoip_with_pt, TT_FORK, NULL, NULL }, { "load_file", test_geoip_load_file, TT_FORK|SKIP_ON_WINDOWS, NULL, NULL }, { "load_file6", test_geoip6_load_file, TT_FORK, NULL, NULL }, + { "load_2nd_file", test_geoip_load_2nd_file, TT_FORK, NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/master] tests: Fix a couple typos and remove unnecessary inline comments.
by dgoulet＠torproject.org 26 Apr '18

26 Apr '18

commit 6a28a82998b239ea312289ab19e55330f801e3b9 Author: Isis Lovecruft <isis(a)torproject.org> Date: Mon Apr 16 19:28:03 2018 +0000 tests: Fix a couple typos and remove unnecessary inline comments. --- src/test/test_geoip.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index af3616653..fa5916c05 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -453,8 +453,6 @@ test_geoip6_load_file(void *arg) char *dhex = NULL; /* A nonexistant filename should fail. */ - /* NOTE: this is probably not needed, as the path is the same, just using - AF_INET6 */ tt_int_op(-1, OP_EQ, geoip_load_file(AF_INET6, "/you/did/not/put/a/file/here/I/hope")); @@ -491,8 +489,6 @@ test_geoip6_load_file(void *arg) tt_str_op(dhex, OP_EQ, geoip_db_digest(AF_INET6)); /* Make sure geoip_free_all() works. */ - /* NOTE: is this needed here?, if geoip_free_all, it should not change with - ipv6 */ geoip_free_all(); tt_int_op(1, OP_EQ, geoip_get_n_countries()); tt_str_op("??", OP_EQ, geoip_get_country_name(0)); @@ -511,13 +507,13 @@ test_geoip_load_2nd_file(void *arg) { (void)arg; - /* Load 1nd geoip file */ + /* Load 1st geoip file */ const char FNAME[] = SRCDIR "/src/config/geoip"; tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME)); int num_countries_geoip = geoip_get_n_countries(); - /* Load 2st geoip (empty) file */ + /* Load 2nd geoip (empty) file */ const char FNAME2[] = SRCDIR "/src/test/geoip_dummy"; tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME2));

1 0

[tor/master] tests: Skip two more geoip_load_file tests on Windows.
by dgoulet＠torproject.org 26 Apr '18

26 Apr '18

commit 3f967bfbd13014764b9938c92981b7c3316b5c08 Author: Isis Lovecruft <isis(a)torproject.org> Date: Mon Apr 16 19:06:56 2018 +0000 tests: Skip two more geoip_load_file tests on Windows. * FIXES part of #25515: https://bugs.torproject.org/25515 --- src/test/test_geoip.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index b35997edc..af3616653 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -548,8 +548,10 @@ struct testcase_t geoip_tests[] = { { "geoip", test_geoip, TT_FORK, NULL, NULL }, { "geoip_with_pt", test_geoip_with_pt, TT_FORK, NULL, NULL }, { "load_file", test_geoip_load_file, TT_FORK|SKIP_ON_WINDOWS, NULL, NULL }, - { "load_file6", test_geoip6_load_file, TT_FORK, NULL, NULL }, - { "load_2nd_file", test_geoip_load_2nd_file, TT_FORK, NULL, NULL }, + { "load_file6", test_geoip6_load_file, TT_FORK | SKIP_ON_WINDOWS, + NULL, NULL }, + { "load_2nd_file", test_geoip_load_2nd_file, TT_FORK | SKIP_ON_WINDOWS, + NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/master] Add clarification about type of file expected
by dgoulet＠torproject.org 26 Apr '18

26 Apr '18

commit d0ad74e0f67aa598e50c83df1f6b6832dabbb61a Author: juga0 <juga(a)riseup.net> Date: Tue Apr 17 16:53:18 2018 +0000 Add clarification about type of file expected --- src/test/test_geoip.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index f5949f755..302af8577 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -512,6 +512,7 @@ test_geoip_load_2nd_file(void *arg) tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME)); /* Load 2nd geoip (empty) file */ + /* It has to be the same IP address family */ const char FNAME2[] = SRCDIR "/src/test/geoip_dummy"; tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME2));

1 0

[tor/master] Remove FIXME about comparing num countries,
by dgoulet＠torproject.org 26 Apr '18

26 Apr '18

commit 96469b82f8d27f9c69d863799b624c93cd1ea6f0 Author: juga0 <juga(a)riseup.net> Date: Tue Apr 17 16:38:24 2018 +0000 Remove FIXME about comparing num countries, * remove the fixme since clearing the countries should be other issue * remove unused variables related to it since that cause travis to fail --- src/test/test_geoip.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index fa5916c05..f5949f755 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -511,16 +511,10 @@ test_geoip_load_2nd_file(void *arg) const char FNAME[] = SRCDIR "/src/config/geoip"; tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME)); - int num_countries_geoip = geoip_get_n_countries(); - /* Load 2nd geoip (empty) file */ const char FNAME2[] = SRCDIR "/src/test/geoip_dummy"; tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME2)); - int num_countries_geoip2 = geoip_get_n_countries(); - /* FIXME: should not this be different? */ - /* tt_int_op(num_countries_geoip, OP_NE, num_countries_geoip2); */ - /* Check that there is no geoip information for 8.8.8.8, */ /* since loading the empty 2nd file should have delete it. */ int country = geoip_get_country_by_ipv4(0x08080808); @@ -551,4 +545,3 @@ struct testcase_t geoip_tests[] = { END_OF_TESTCASES }; -

1 0

[tor/master] tests: Add forgotten empty file required for geoip
by dgoulet＠torproject.org 26 Apr '18

26 Apr '18

commit 3d4bbf94c67e4bdd91d5378ca0321237fbe7d082 Author: juga0 <juga(a)riseup.net> Date: Wed Apr 18 08:51:39 2018 +0000 tests: Add forgotten empty file required for geoip --- src/test/geoip_dummy | 0 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/src/test/geoip_dummy b/src/test/geoip_dummy new file mode 100644 index 000000000..e69de29bb

1 0

[tor/master] Merge remote-tracking branch 'dgoulet/ticket25515_034_01-squashed'
by dgoulet＠torproject.org 26 Apr '18

26 Apr '18

commit 7b09282dc723381e68fef79c3474a56315a0edfa Merge: 33cba1195 f4ad30448 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Apr 26 11:38:15 2018 -0400 Merge remote-tracking branch 'dgoulet/ticket25515_034_01-squashed' src/test/geoip_dummy | 0 src/test/test_geoip.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+)

1 0