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

[tor/master] Track bootstrap phase independently of progress
by nickm＠torproject.org 11 Sep '18

11 Sep '18

commit 687bf3ea645ef8c5ce8c9f02a25274121ca13318 Author: Taylor Yu <catalyst(a)torproject.org> Date: Fri Aug 31 17:54:09 2018 -0500 Track bootstrap phase independently of progress Track bootstrap phase (enumerated by bootstrap_status_t) independently from the bootstrap progress (which can represent intermediate progress). This allows control_event_bootstrap_problem() to avoid doing a linear search through the bootstrap progress space to find the current bootstrap phase. --- src/feature/control/control.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/feature/control/control.c b/src/feature/control/control.c index 64785a758..d68f1e3b0 100644 --- a/src/feature/control/control.c +++ b/src/feature/control/control.c @@ -7073,6 +7073,11 @@ bootstrap_status_to_string(bootstrap_status_t s, const char **tag, * Tor initializes. */ static int bootstrap_percent = BOOTSTRAP_STATUS_UNDEF; +/** Like bootstrap_percent, but only takes on the enumerated values in + * bootstrap_status_t. + */ +static int bootstrap_phase = BOOTSTRAP_STATUS_UNDEF; + /** As bootstrap_percent, but holds the bootstrapping level at which we last * logged a NOTICE-level message. We use this, plus BOOTSTRAP_PCT_INCREMENT, * to avoid flooding the log with a new message every time we get a few more @@ -7158,7 +7163,8 @@ control_event_bootstrap(bootstrap_status_t status, int progress) control_event_bootstrap_core(loglevel, status, progress); if (status > bootstrap_percent) { - bootstrap_percent = status; /* new milestone reached */ + bootstrap_phase = status; /* new milestone reached */ + bootstrap_percent = status; } if (progress > bootstrap_percent) { /* incremental progress within a milestone */ @@ -7206,9 +7212,7 @@ control_event_bootstrap_problem(const char *warn, const char *reason, if (we_are_hibernating()) dowarn = 0; - while (status>=0 && bootstrap_status_to_string(status, &tag, &summary) < 0) - status--; /* find a recognized status string based on current progress */ - status = bootstrap_percent; /* set status back to the actual number */ + tor_assert(bootstrap_status_to_string(bootstrap_phase, &tag, &summary) == 0); severity = dowarn ? LOG_WARN : LOG_INFO;

1 0

[tor/master] Merge remote-tracking branch 'tor-github/pr/315'
by nickm＠torproject.org 11 Sep '18

11 Sep '18

commit 5a2374b0740ce662e4b323f104c8ff58164d996a Merge: 902b34a0d 617160895 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 11 15:55:30 2018 -0400 Merge remote-tracking branch 'tor-github/pr/315' changes/ticket27169 | 6 ++ src/core/or/circuitbuild.c | 12 ++-- src/core/or/connection_or.c | 1 + src/core/or/relay.c | 4 +- src/feature/control/control.c | 139 ++++++++++++++++++++++++++------------ src/feature/control/control.h | 4 +- src/feature/dircache/directory.c | 8 +-- src/feature/nodelist/nodelist.c | 9 ++- src/test/test_controller_events.c | 68 +++++++++++++++++++ 9 files changed, 187 insertions(+), 64 deletions(-)

1 0

[tor/master] Defer reporting directory bootstrap progress
by nickm＠torproject.org 11 Sep '18

11 Sep '18

commit 617160895ce9bb403fe5a864925ffb1894f9086c Author: Taylor Yu <catalyst(a)torproject.org> Date: Fri Aug 31 12:56:23 2018 -0500 Defer reporting directory bootstrap progress Existing cached directory information can cause misleadingly high bootstrap percentages. To improve user experience, defer reporting of directory information progress until at least one connection has succeeded to a relay or bridge. Closes ticket 27169. --- changes/ticket27169 | 6 ++++ src/core/or/connection_or.c | 1 + src/core/or/relay.c | 4 +-- src/feature/control/control.c | 39 ++++++++++++++++++++++ src/feature/control/control.h | 2 ++ src/feature/dircache/directory.c | 8 ++--- src/feature/nodelist/nodelist.c | 4 +-- src/test/test_controller_events.c | 68 +++++++++++++++++++++++++++++++++++++++ 8 files changed, 124 insertions(+), 8 deletions(-) diff --git a/changes/ticket27169 b/changes/ticket27169 new file mode 100644 index 000000000..7854532a6 --- /dev/null +++ b/changes/ticket27169 @@ -0,0 +1,6 @@ + o Minor features (bootstrap): + - Improve user experience by deferring directory progress + reporting until after a connection to a relay or bridge has + succeeded. This avoids reporting 80% progress based on cached + directory information when we can't even connect to a bridge or + relay. Closes ticket 27169. diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index 08371d1ad..e6b235401 100644 --- a/src/core/or/connection_or.c +++ b/src/core/or/connection_or.c @@ -705,6 +705,7 @@ connection_or_finished_connecting(or_connection_t *or_conn) log_debug(LD_HANDSHAKE,"OR connect() to router at %s:%u finished.", conn->address,conn->port); control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0); + control_event_boot_first_orconn(); if (proxy_type != PROXY_NONE) { /* start proxy handshake */ diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 6e1adfaff..1d0be0c89 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -1389,8 +1389,8 @@ connection_edge_process_relay_cell_not_open( case DIR_PURPOSE_FETCH_SERVERDESC: case DIR_PURPOSE_FETCH_MICRODESC: if (TO_DIR_CONN(dirconn)->router_purpose == ROUTER_PURPOSE_GENERAL) - control_event_bootstrap(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS, - count_loading_descriptors_progress()); + control_event_boot_dir(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS, + count_loading_descriptors_progress()); break; } } diff --git a/src/feature/control/control.c b/src/feature/control/control.c index d68f1e3b0..2fac3bd6d 100644 --- a/src/feature/control/control.c +++ b/src/feature/control/control.c @@ -7178,6 +7178,45 @@ control_event_bootstrap(bootstrap_status_t status, int progress) } } +/** Flag whether we've opened an OR_CONN yet */ +static int bootstrap_first_orconn = 0; + +/** Like bootstrap_phase, but for (possibly deferred) directory progress */ +static int bootstrap_dir_phase = BOOTSTRAP_STATUS_UNDEF; + +/** Like bootstrap_problems, but for (possibly deferred) directory progress */ +static int bootstrap_dir_progress = BOOTSTRAP_STATUS_UNDEF; + +/** Defer directory info bootstrap events until we have successfully + * completed our first connection to a router. */ +void +control_event_boot_dir(bootstrap_status_t status, int progress) +{ + if (status > bootstrap_dir_progress) { + bootstrap_dir_progress = status; + bootstrap_dir_phase = status; + } + if (progress && progress >= bootstrap_dir_progress) { + bootstrap_dir_progress = progress; + } + + /* Don't report unless we have successfully opened at least one OR_CONN */ + if (!bootstrap_first_orconn) + return; + + control_event_bootstrap(status, progress); +} + +/** Set a flag to allow reporting of directory bootstrap progress. + * (Code that reports completion of an OR_CONN calls this.) Also, + * report directory progress so far. */ +void +control_event_boot_first_orconn(void) +{ + bootstrap_first_orconn = 1; + control_event_bootstrap(bootstrap_dir_phase, bootstrap_dir_progress); +} + /** Called when Tor has failed to make bootstrapping progress in a way * that indicates a problem. <b>warn</b> gives a human-readable hint * as to why, and <b>reason</b> provides a controller-facing short diff --git a/src/feature/control/control.h b/src/feature/control/control.h index 7f57228e5..3445eb0a9 100644 --- a/src/feature/control/control.h +++ b/src/feature/control/control.h @@ -195,6 +195,8 @@ void control_event_bootstrap(bootstrap_status_t status, int progress); MOCK_DECL(void, control_event_bootstrap_prob_or,(const char *warn, int reason, or_connection_t *or_conn)); +void control_event_boot_dir(bootstrap_status_t status, int progress); +void control_event_boot_first_orconn(void); void control_event_bootstrap_problem(const char *warn, const char *reason, const connection_t *conn, int dowarn); diff --git a/src/feature/dircache/directory.c b/src/feature/dircache/directory.c index de0bcdbfa..b94c5317a 100644 --- a/src/feature/dircache/directory.c +++ b/src/feature/dircache/directory.c @@ -2226,8 +2226,8 @@ load_downloaded_routers(const char *body, smartlist_t *which, added = router_load_routers_from_string(body, NULL, SAVED_NOWHERE, which, descriptor_digests, buf); if (added && general) - control_event_bootstrap(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS, - count_loading_descriptors_progress()); + control_event_boot_dir(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS, + count_loading_descriptors_progress()); return added; } @@ -2949,8 +2949,8 @@ handle_response_fetch_microdesc(dir_connection_t *conn, dir_microdesc_download_failed(which, status_code, conn->identity_digest); } if (mds && smartlist_len(mds)) { - control_event_bootstrap(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS, - count_loading_descriptors_progress()); + control_event_boot_dir(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS, + count_loading_descriptors_progress()); directory_info_has_arrived(now, 0, 1); } SMARTLIST_FOREACH(which, char *, cp, tor_free(cp)); diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c index 6dd501ea3..50dc8f7d3 100644 --- a/src/feature/nodelist/nodelist.c +++ b/src/feature/nodelist/nodelist.c @@ -2528,7 +2528,7 @@ update_router_have_minimum_dir_info(void) (int)(paths*100), status); tor_free(status); res = 0; - control_event_bootstrap(BOOTSTRAP_STATUS_REQUESTING_DESCRIPTORS, 0); + control_event_boot_dir(BOOTSTRAP_STATUS_REQUESTING_DESCRIPTORS, 0); goto done; } @@ -2553,7 +2553,7 @@ update_router_have_minimum_dir_info(void) /* If paths have just become available in this update. */ if (res && !have_min_dir_info) { control_event_client_status(LOG_NOTICE, "ENOUGH_DIR_INFO"); - control_event_bootstrap(BOOTSTRAP_STATUS_CONN_OR, 0); + control_event_boot_dir(BOOTSTRAP_STATUS_CONN_OR, 0); log_info(LD_DIR, "We now have enough directory information to build circuits."); } diff --git a/src/test/test_controller_events.c b/src/test/test_controller_events.c index e935b7042..4c404876b 100644 --- a/src/test/test_controller_events.c +++ b/src/test/test_controller_events.c @@ -322,6 +322,72 @@ test_cntev_event_mask(void *arg) ; } +static char *saved_event_str = NULL; + +static void +mock_queue_control_event_string(uint16_t event, char *msg) +{ + (void)event; + + tor_free(saved_event_str); + saved_event_str = msg; +} + +/* Helper macro for checking bootstrap control event strings */ +#define assert_bootmsg(s) \ + tt_ptr_op(strstr(saved_event_str, "650 STATUS_CLIENT NOTICE " \ + "BOOTSTRAP PROGRESS=" s), OP_EQ, saved_event_str) + +/* Test deferral of directory bootstrap messages (requesting_descriptors) */ +static void +test_cntev_dirboot_defer_desc(void *arg) +{ + (void)arg; + + MOCK(queue_control_event_string, mock_queue_control_event_string); + control_testing_set_global_event_mask(EVENT_MASK_(EVENT_STATUS_CLIENT)); + control_event_bootstrap(BOOTSTRAP_STATUS_STARTING, 0); + assert_bootmsg("0 TAG=starting"); + /* This event should get deferred */ + control_event_boot_dir(BOOTSTRAP_STATUS_REQUESTING_DESCRIPTORS, 0); + assert_bootmsg("0 TAG=starting"); + control_event_bootstrap(BOOTSTRAP_STATUS_CONN_DIR, 0); + assert_bootmsg("5 TAG=conn_dir"); + control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0); + assert_bootmsg("10 TAG=handshake_dir"); + /* The deferred event should appear */ + control_event_boot_first_orconn(); + assert_bootmsg("45 TAG=requesting_descriptors"); + done: + tor_free(saved_event_str); + UNMOCK(queue_control_event_string); +} + +/* Test deferral of directory bootstrap messages (conn_or) */ +static void +test_cntev_dirboot_defer_orconn(void *arg) +{ + (void)arg; + + MOCK(queue_control_event_string, mock_queue_control_event_string); + control_testing_set_global_event_mask(EVENT_MASK_(EVENT_STATUS_CLIENT)); + control_event_bootstrap(BOOTSTRAP_STATUS_STARTING, 0); + assert_bootmsg("0 TAG=starting"); + /* This event should get deferred */ + control_event_boot_dir(BOOTSTRAP_STATUS_CONN_OR, 0); + assert_bootmsg("0 TAG=starting"); + control_event_bootstrap(BOOTSTRAP_STATUS_CONN_DIR, 0); + assert_bootmsg("5 TAG=conn_dir"); + control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0); + assert_bootmsg("10 TAG=handshake_dir"); + /* The deferred event should appear */ + control_event_boot_first_orconn(); + assert_bootmsg("80 TAG=conn_or"); + done: + tor_free(saved_event_str); + UNMOCK(queue_control_event_string); +} + #define TEST(name, flags) \ { #name, test_cntev_ ## name, flags, 0, NULL } @@ -330,5 +396,7 @@ struct testcase_t controller_event_tests[] = { TEST(append_cell_stats, TT_FORK), TEST(format_cell_stats, TT_FORK), TEST(event_mask, TT_FORK), + TEST(dirboot_defer_desc, TT_FORK), + TEST(dirboot_defer_orconn, TT_FORK), END_OF_TESTCASES };

1 0

[tor/release-0.3.4] Bug 25505: Check circuitmux queues before padding.
by nickm＠torproject.org 11 Sep '18

11 Sep '18

commit ae1aea4cc490658e917b0c08584c7115a57da00b Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Tue Mar 13 13:50:35 2018 +0000 Bug 25505: Check circuitmux queues before padding. --- changes/bug25505 | 3 +++ src/or/channelpadding.c | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/changes/bug25505 b/changes/bug25505 new file mode 100644 index 000000000..101c7d524 --- /dev/null +++ b/changes/bug25505 @@ -0,0 +1,3 @@ + o Minor bugfixes (netflow padding): + - Ensure circuitmux queues are empty before scheduling or sending padding. + Fixes bug 25505; bugfix on 0.3.1.1-alpha. diff --git a/src/or/channelpadding.c b/src/or/channelpadding.c index 5da3009e6..ba432e716 100644 --- a/src/or/channelpadding.c +++ b/src/or/channelpadding.c @@ -379,7 +379,8 @@ channelpadding_send_padding_cell_for_callback(channel_t *chan) chan->pending_padding_callback = 0; if (monotime_coarse_is_zero(&chan->next_padding_time) || - chan->has_queued_writes(chan)) { + chan->has_queued_writes(chan) || + (chan->cmux && circuitmux_num_cells(chan->cmux))) { /* We must have been active before the timer fired */ monotime_coarse_zero(&chan->next_padding_time); return; @@ -755,7 +756,11 @@ channelpadding_decide_to_pad_channel(channel_t *chan) return CHANNELPADDING_WONTPAD; } - if (!chan->has_queued_writes(chan)) { + /* There should always be a cmux on the circuit. After that, + * only schedule padding if there are no queued writes and no + * queued cells in circuitmux queues. */ + if (chan->cmux && !chan->has_queued_writes(chan) && + !circuitmux_num_cells(chan->cmux)) { int is_client_channel = 0; if (CHANNEL_IS_CLIENT(chan, options)) {

1 0

[tor/release-0.3.3] Merge branch 'maint-0.3.3' into release-0.3.3
by nickm＠torproject.org 11 Sep '18

11 Sep '18

commit 1239fcfc37aaf6b732e0733ad747a7b3e56534c4 Merge: 95f06d61d ae1aea4cc Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 11 15:53:06 2018 -0400 Merge branch 'maint-0.3.3' into release-0.3.3 changes/bug25505 | 3 +++ src/or/channelpadding.c | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-)

1 0

[tor/release-0.3.4] Merge branch 'maint-0.3.3' into maint-0.3.4
by nickm＠torproject.org 11 Sep '18

11 Sep '18

commit affbe376f9a15c7c839287b8127d3bd9ac2eab02 Merge: 89c5b8d7d ae1aea4cc Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 11 15:53:06 2018 -0400 Merge branch 'maint-0.3.3' into maint-0.3.4 changes/bug25505 | 3 +++ src/or/channelpadding.c | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-)

1 0

[tor/release-0.3.4] Merge branch 'maint-0.3.4' into release-0.3.4
by nickm＠torproject.org 11 Sep '18

11 Sep '18

commit 80b13f7e1e637390e7cde5695800e6a16f4f08fa Merge: 337204915 affbe376f Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 11 15:53:06 2018 -0400 Merge branch 'maint-0.3.4' into release-0.3.4 changes/bug25505 | 3 +++ src/or/channelpadding.c | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-)

1 0

[tor/maint-0.3.3] Bug 25505: Check circuitmux queues before padding.
by nickm＠torproject.org 11 Sep '18

11 Sep '18

commit ae1aea4cc490658e917b0c08584c7115a57da00b Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Tue Mar 13 13:50:35 2018 +0000 Bug 25505: Check circuitmux queues before padding. --- changes/bug25505 | 3 +++ src/or/channelpadding.c | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/changes/bug25505 b/changes/bug25505 new file mode 100644 index 000000000..101c7d524 --- /dev/null +++ b/changes/bug25505 @@ -0,0 +1,3 @@ + o Minor bugfixes (netflow padding): + - Ensure circuitmux queues are empty before scheduling or sending padding. + Fixes bug 25505; bugfix on 0.3.1.1-alpha. diff --git a/src/or/channelpadding.c b/src/or/channelpadding.c index 5da3009e6..ba432e716 100644 --- a/src/or/channelpadding.c +++ b/src/or/channelpadding.c @@ -379,7 +379,8 @@ channelpadding_send_padding_cell_for_callback(channel_t *chan) chan->pending_padding_callback = 0; if (monotime_coarse_is_zero(&chan->next_padding_time) || - chan->has_queued_writes(chan)) { + chan->has_queued_writes(chan) || + (chan->cmux && circuitmux_num_cells(chan->cmux))) { /* We must have been active before the timer fired */ monotime_coarse_zero(&chan->next_padding_time); return; @@ -755,7 +756,11 @@ channelpadding_decide_to_pad_channel(channel_t *chan) return CHANNELPADDING_WONTPAD; } - if (!chan->has_queued_writes(chan)) { + /* There should always be a cmux on the circuit. After that, + * only schedule padding if there are no queued writes and no + * queued cells in circuitmux queues. */ + if (chan->cmux && !chan->has_queued_writes(chan) && + !circuitmux_num_cells(chan->cmux)) { int is_client_channel = 0; if (CHANNEL_IS_CLIENT(chan, options)) {

1 0

[tor/maint-0.3.4] Bug 25505: Check circuitmux queues before padding.
by nickm＠torproject.org 11 Sep '18

11 Sep '18

commit ae1aea4cc490658e917b0c08584c7115a57da00b Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Tue Mar 13 13:50:35 2018 +0000 Bug 25505: Check circuitmux queues before padding. --- changes/bug25505 | 3 +++ src/or/channelpadding.c | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/changes/bug25505 b/changes/bug25505 new file mode 100644 index 000000000..101c7d524 --- /dev/null +++ b/changes/bug25505 @@ -0,0 +1,3 @@ + o Minor bugfixes (netflow padding): + - Ensure circuitmux queues are empty before scheduling or sending padding. + Fixes bug 25505; bugfix on 0.3.1.1-alpha. diff --git a/src/or/channelpadding.c b/src/or/channelpadding.c index 5da3009e6..ba432e716 100644 --- a/src/or/channelpadding.c +++ b/src/or/channelpadding.c @@ -379,7 +379,8 @@ channelpadding_send_padding_cell_for_callback(channel_t *chan) chan->pending_padding_callback = 0; if (monotime_coarse_is_zero(&chan->next_padding_time) || - chan->has_queued_writes(chan)) { + chan->has_queued_writes(chan) || + (chan->cmux && circuitmux_num_cells(chan->cmux))) { /* We must have been active before the timer fired */ monotime_coarse_zero(&chan->next_padding_time); return; @@ -755,7 +756,11 @@ channelpadding_decide_to_pad_channel(channel_t *chan) return CHANNELPADDING_WONTPAD; } - if (!chan->has_queued_writes(chan)) { + /* There should always be a cmux on the circuit. After that, + * only schedule padding if there are no queued writes and no + * queued cells in circuitmux queues. */ + if (chan->cmux && !chan->has_queued_writes(chan) && + !circuitmux_num_cells(chan->cmux)) { int is_client_channel = 0; if (CHANNEL_IS_CLIENT(chan, options)) {

1 0

[tor/master] Bug 25505: Check circuitmux queues before padding.
by nickm＠torproject.org 11 Sep '18

11 Sep '18

commit ae1aea4cc490658e917b0c08584c7115a57da00b Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Tue Mar 13 13:50:35 2018 +0000 Bug 25505: Check circuitmux queues before padding. --- changes/bug25505 | 3 +++ src/or/channelpadding.c | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/changes/bug25505 b/changes/bug25505 new file mode 100644 index 000000000..101c7d524 --- /dev/null +++ b/changes/bug25505 @@ -0,0 +1,3 @@ + o Minor bugfixes (netflow padding): + - Ensure circuitmux queues are empty before scheduling or sending padding. + Fixes bug 25505; bugfix on 0.3.1.1-alpha. diff --git a/src/or/channelpadding.c b/src/or/channelpadding.c index 5da3009e6..ba432e716 100644 --- a/src/or/channelpadding.c +++ b/src/or/channelpadding.c @@ -379,7 +379,8 @@ channelpadding_send_padding_cell_for_callback(channel_t *chan) chan->pending_padding_callback = 0; if (monotime_coarse_is_zero(&chan->next_padding_time) || - chan->has_queued_writes(chan)) { + chan->has_queued_writes(chan) || + (chan->cmux && circuitmux_num_cells(chan->cmux))) { /* We must have been active before the timer fired */ monotime_coarse_zero(&chan->next_padding_time); return; @@ -755,7 +756,11 @@ channelpadding_decide_to_pad_channel(channel_t *chan) return CHANNELPADDING_WONTPAD; } - if (!chan->has_queued_writes(chan)) { + /* There should always be a cmux on the circuit. After that, + * only schedule padding if there are no queued writes and no + * queued cells in circuitmux queues. */ + if (chan->cmux && !chan->has_queued_writes(chan) && + !circuitmux_num_cells(chan->cmux)) { int is_client_channel = 0; if (CHANNEL_IS_CLIENT(chan, options)) {

1 0