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

[tor/master] Add configure option to control ZSTD_STATIC_LINKING_ONLY
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit a03488954c15b2fe2643d19c5530d9c927665b6f Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 6 11:58:05 2018 -0500 Add configure option to control ZSTD_STATIC_LINKING_ONLY --- configure.ac | 6 ++++++ src/common/compress_zstd.c | 8 ++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index 3cb187b0e..2063e9a83 100644 --- a/configure.ac +++ b/configure.ac @@ -61,6 +61,8 @@ AC_ARG_ENABLE(cargo-online-mode, AS_HELP_STRING(--enable-cargo-online-mode, [Allow cargo to make network requests to fetch crates. For builds with rust only.])) AC_ARG_ENABLE(restart-debugging, AS_HELP_STRING(--enable-restart-debugging, [Build Tor with support for debugging in-process restart. Developers only.])) +AC_ARG_ENABLE(zstd-advanced-apis, + AS_HELP_STRING(--disable-zstd-advanced-apis, [Build without support for zstd's "static-only" APIs.])) if test "x$enable_coverage" != "xyes" -a "x$enable_asserts_in_tests" = "xno" ; then AC_MSG_ERROR([Can't disable assertions outside of coverage build]) @@ -114,6 +116,10 @@ if test "$enable_restart_debugging" = "yes"; then [Defined if we're building with support for in-process restart debugging.]) fi +if test "$enable_zstd_advanced_apis" != "no"; then + AC_DEFINE(ENABLE_ZSTD_ADVANCED_APIS, 1, + [Defined if we're going to try to use zstd's "static-only" APIs.]) +fi # systemd support if test "x$enable_systemd" = "xno"; then diff --git a/src/common/compress_zstd.c b/src/common/compress_zstd.c index 96906efb9..0c3e7104f 100644 --- a/src/common/compress_zstd.c +++ b/src/common/compress_zstd.c @@ -18,12 +18,12 @@ #include "compress.h" #include "compress_zstd.h" +#ifdef ENABLE_ZSTD_ADVANCED_APIS /* This is a lie, but we make sure it doesn't get us in trouble by wrapping * all invocations of zstd's static-only functions in a check to make sure - * that the compile-time version matches the run-time version. - * - * Note: Make sure that this file still builds with this macro disabled. */ + * that the compile-time version matches the run-time version. */ #define ZSTD_STATIC_LINKING_ONLY +#endif #ifdef HAVE_ZSTD #include <zstd.h> @@ -499,7 +499,7 @@ tor_zstd_init(void) void tor_zstd_warn_if_version_mismatched(void) { -#ifdef HAVE_ZSTD +#if defined(HAVE_ZSTD) && defined(ENABLE_ZSTD_ADVANCED_APIS) if (! tor_zstd_can_use_static_apis()) { char header_version[VERSION_STR_MAX_LEN]; char runtime_version[VERSION_STR_MAX_LEN];

1 0

[tor/master] Warn on zstd header/library version mismatch
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit a77a366b87fb04a878392284241fe72850b4dd88 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 6 11:02:41 2018 -0500 Warn on zstd header/library version mismatch If we're going to potentially degrade performance in this case, we may as well tell people so. --- src/common/compress.c | 10 ++++++++++ src/common/compress.h | 1 + src/common/compress_zstd.c | 46 ++++++++++++++++++++++++++++++++++++++-------- src/common/compress_zstd.h | 1 + src/or/main.c | 2 ++ 5 files changed, 52 insertions(+), 8 deletions(-) diff --git a/src/common/compress.c b/src/common/compress.c index 47c93cf6a..cb1549f1a 100644 --- a/src/common/compress.c +++ b/src/common/compress.c @@ -663,3 +663,13 @@ tor_compress_init(void) tor_zstd_init(); } +/** Warn if we had any problems while setting up our compression libraries. + * + * (This isn't part of tor_compress_init, since the logs aren't set up yet.) + */ +void +tor_compress_log_init_warnings(void) +{ + tor_zstd_warn_if_version_mismatched(); +} + diff --git a/src/common/compress.h b/src/common/compress.h index 952102bf9..65d63a438 100644 --- a/src/common/compress.h +++ b/src/common/compress.h @@ -87,6 +87,7 @@ void tor_compress_free_(tor_compress_state_t *state); size_t tor_compress_state_size(const tor_compress_state_t *state); void tor_compress_init(void); +void tor_compress_log_init_warnings(void); #endif /* !defined(TOR_COMPRESS_H) */ diff --git a/src/common/compress_zstd.c b/src/common/compress_zstd.c index 02469ced9..96906efb9 100644 --- a/src/common/compress_zstd.c +++ b/src/common/compress_zstd.c @@ -58,21 +58,29 @@ tor_zstd_method_supported(void) #endif } +/** Format a zstd version number as a string in buf. */ +static void +tor_zstd_format_version(char *buf, size_t buflen, unsigned version_number) +{ + tor_snprintf(buf, buflen, + "%u.%u.%u", + version_number / 10000 % 100, + version_number / 100 % 100, + version_number % 100); +} + +#define VERSION_STR_MAX_LEN 16 /* more than enough space for 99.99.99 */ + /** Return a string representation of the version of the currently running * version of libzstd. Returns NULL if Zstandard is unsupported. */ const char * tor_zstd_get_version_str(void) { #ifdef HAVE_ZSTD - static char version_str[16]; - size_t version_number; + static char version_str[VERSION_STR_MAX_LEN]; - version_number = ZSTD_versionNumber(); - tor_snprintf(version_str, sizeof(version_str), - "%d.%d.%d", - (int) version_number / 10000 % 100, - (int) version_number / 100 % 100, - (int) version_number % 100); + tor_zstd_format_version(version_str, sizeof(version_str), + ZSTD_versionNumber()); return version_str; #else /* !(defined(HAVE_ZSTD)) */ @@ -487,6 +495,27 @@ tor_zstd_init(void) atomic_counter_init(&total_zstd_allocation); } +/** Warn if the header and library versions don't match. */ +void +tor_zstd_warn_if_version_mismatched(void) +{ +#ifdef HAVE_ZSTD + if (! tor_zstd_can_use_static_apis()) { + char header_version[VERSION_STR_MAX_LEN]; + char runtime_version[VERSION_STR_MAX_LEN]; + tor_zstd_format_version(header_version, sizeof(header_version), + ZSTD_VERSION_NUMBER); + tor_zstd_format_version(runtime_version, sizeof(runtime_version), + ZSTD_versionNumber()); + + log_warn(LD_GENERAL, + "Tor was compiled with zstd %s, but is running with zstd %s. " + "For safety, we'll avoid using advanced zstd functionality.", + header_version, runtime_version); + } +#endif +} + #ifdef TOR_UNIT_TESTS /** Testing only: disable usage of static-only APIs, so we can make sure that * we still work without them. */ @@ -496,3 +525,4 @@ tor_zstd_set_static_apis_disabled_for_testing(int disabled) static_apis_disable_for_testing = disabled; } #endif + diff --git a/src/common/compress_zstd.h b/src/common/compress_zstd.h index 8882617c9..bd42cf65c 100644 --- a/src/common/compress_zstd.h +++ b/src/common/compress_zstd.h @@ -43,6 +43,7 @@ size_t tor_zstd_compress_state_size(const tor_zstd_compress_state_t *state); size_t tor_zstd_get_total_allocation(void); void tor_zstd_init(void); +void tor_zstd_warn_if_version_mismatched(void); #ifdef TOR_UNIT_TESTS void tor_zstd_set_static_apis_disabled_for_testing(int disabled); diff --git a/src/or/main.c b/src/or/main.c index 98566c0c9..894b12cc3 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -3323,6 +3323,8 @@ tor_init(int argc, char *argv[]) if (strstr(version, "alpha") || strstr(version, "beta")) log_notice(LD_GENERAL, "This version is not a stable Tor release. " "Expect more bugs than usual."); + + tor_compress_log_init_warnings(); } #ifdef HAVE_RUST

1 0

[tor/master] Make zstd unit tests try running with static-only fns disabled
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 7cb954209d2c3b12d0d60a46a74f2c31ad6d48c5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 6 10:35:37 2018 -0500 Make zstd unit tests try running with static-only fns disabled Since we're making it so that unstable zstd apis can be disabled, we need to test them. I do this by adding a variant setup/cleanup function for the tests, and teaching it about a fake compression method called "x-zstd:nostatic". --- src/test/test_util.c | 43 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/src/test/test_util.c b/src/test/test_util.c index 1fd41a348..7ecee4c38 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -16,6 +16,7 @@ #include "memarea.h" #include "util_process.h" #include "log_test_helpers.h" +#include "compress_zstd.h" #ifdef HAVE_PWD_H #include <pwd.h> @@ -2396,6 +2397,37 @@ test_util_compress_stream_impl(compress_method_t method, tor_free(buf3); } +/** Setup function for compression tests: handles x-zstd:nostatic + */ +static void * +compression_test_setup(const struct testcase_t *testcase) +{ + tor_assert(testcase->setup_data); + tor_assert(testcase->setup_data != (void*)TT_SKIP); + const char *methodname = testcase->setup_data; + + if (!strcmp(methodname, "x-zstd:nostatic")) { + methodname = "x-zstd"; + tor_zstd_set_static_apis_disabled_for_testing(1); + } + + return (void *)methodname; +} + +/** Cleanup for compression tests: disables nostatic */ +static int +compression_test_cleanup(const struct testcase_t *testcase, void *ptr) +{ + (void)testcase; + (void)ptr; + tor_zstd_set_static_apis_disabled_for_testing(0); + return 1; +} + +static const struct testcase_setup_t compress_setup = { + compression_test_setup, compression_test_cleanup +}; + /** Run unit tests for compression functions */ static void test_util_compress(void *arg) @@ -6095,22 +6127,22 @@ test_util_get_unquoted_path(void *arg) { #name, test_util_ ## name, flags, NULL, NULL } #define COMPRESS(name, identifier) \ - { "compress/" #name, test_util_compress, 0, &passthrough_setup, \ + { "compress/" #name, test_util_compress, 0, &compress_setup, \ (char*)(identifier) } #define COMPRESS_CONCAT(name, identifier) \ { "compress_concat/" #name, test_util_decompress_concatenated, 0, \ - &passthrough_setup, \ + &compress_setup, \ (char*)(identifier) } #define COMPRESS_JUNK(name, identifier) \ { "compress_junk/" #name, test_util_decompress_junk, 0, \ - &passthrough_setup, \ + &compress_setup, \ (char*)(identifier) } #define COMPRESS_DOS(name, identifier) \ { "compress_dos/" #name, test_util_decompress_dos, 0, \ - &passthrough_setup, \ + &compress_setup, \ (char*)(identifier) } #ifdef _WIN32 @@ -6141,11 +6173,13 @@ struct testcase_t util_tests[] = { COMPRESS(gzip, "gzip"), COMPRESS(lzma, "x-tor-lzma"), COMPRESS(zstd, "x-zstd"), + COMPRESS(zstd_nostatic, "x-zstd:nostatic"), COMPRESS(none, "identity"), COMPRESS_CONCAT(zlib, "deflate"), COMPRESS_CONCAT(gzip, "gzip"), COMPRESS_CONCAT(lzma, "x-tor-lzma"), COMPRESS_CONCAT(zstd, "x-zstd"), + COMPRESS_CONCAT(zstd_nostatic, "x-zstd:nostatic"), COMPRESS_CONCAT(none, "identity"), COMPRESS_JUNK(zlib, "deflate"), COMPRESS_JUNK(gzip, "gzip"), @@ -6154,6 +6188,7 @@ struct testcase_t util_tests[] = { COMPRESS_DOS(gzip, "gzip"), COMPRESS_DOS(lzma, "x-tor-lzma"), COMPRESS_DOS(zstd, "x-zstd"), + COMPRESS_DOS(zstd_nostatic, "x-zstd:nostatic"), UTIL_TEST(gzip_compression_bomb, TT_FORK), UTIL_LEGACY(datadir), UTIL_LEGACY(memarea),

1 0

[tor/master] Use "static-only" zstd functions to estimate memory usage.
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit f98cb5d3552666ede73137d998162094d5a31a1a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 6 10:36:59 2018 -0500 Use "static-only" zstd functions to estimate memory usage. These should provide better and more accurate results when we can use them; we fall back to the old approach when we can't. --- src/common/compress_zstd.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/common/compress_zstd.c b/src/common/compress_zstd.c index 799448744..02469ced9 100644 --- a/src/common/compress_zstd.c +++ b/src/common/compress_zstd.c @@ -139,9 +139,11 @@ struct tor_zstd_compress_state_t { #ifdef HAVE_ZSTD /** Return an approximate number of bytes stored in memory to hold the - * Zstandard compression/decompression state. */ + * Zstandard compression/decompression state. This is a fake estimate + * based on inspecting the zstd source: tor_zstd_state_size_precalc() is + * more accurate when it's allowed to use "static-only" functions */ static size_t -tor_zstd_state_size_precalc(int compress, int preset) +tor_zstd_state_size_precalc_fake(int compress, int preset) { tor_assert(preset > 0); @@ -198,6 +200,24 @@ tor_zstd_state_size_precalc(int compress, int preset) return memory_usage; } + +/** Return an approximate number of bytes stored in memory to hold the + * Zstandard compression/decompression state. */ +static size_t +tor_zstd_state_size_precalc(int compress, int preset) +{ +#ifdef ZSTD_STATIC_LINKING_ONLY + if (tor_zstd_can_use_static_apis()) { + if (compress) { + return ZSTD_estimateCStreamSize(preset); + } else { + /* Could use DStream, but that takes a windowSize. */ + return ZSTD_estimateDCtxSize(); + } + } +#endif + return tor_zstd_state_size_precalc_fake(compress, preset); +} #endif /* defined(HAVE_ZSTD) */ /** Construct and return a tor_zstd_compress_state_t object using

1 0

[tor/master] Merge branch '25162_zstd_static'
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit a1dd8afc16ebeacceb5b4093ef01e2ad8f21ad6c Merge: 066a15af6 a03488954 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 15 20:28:07 2018 -0500 Merge branch '25162_zstd_static' changes/ticket25162 | 6 +++ configure.ac | 6 +++ src/common/compress.c | 10 +++++ src/common/compress.h | 1 + src/common/compress_zstd.c | 106 ++++++++++++++++++++++++++++++++++++++++----- src/common/compress_zstd.h | 7 +++ src/or/main.c | 2 + src/test/test_util.c | 43 ++++++++++++++++-- 8 files changed, 167 insertions(+), 14 deletions(-)

1 0

[tor/master] refactor: using get_uptime() (and reset_uptime()) consistently.
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit a4c85312604c1d215f9f46a24ac242baf666ed56 Author: Caio Valente <valentecaio95(a)gmail.com> Date: Wed Jan 31 02:36:38 2018 +0100 refactor: using get_uptime() (and reset_uptime()) consistently. Using get_uptime() and reset_uptime() instead of accessing stats_n_seconds_working directly. stats_n_seconds_working is not extern anymore. Ticket #25081 --- src/or/dirserv.c | 2 +- src/or/hibernate.c | 2 +- src/or/main.c | 19 +++++++++++++------ src/or/main.h | 2 +- src/or/router.c | 2 +- 5 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index d3bae241f..5f778322b 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1417,7 +1417,7 @@ dirserv_thinks_router_is_hs_dir(const routerinfo_t *router, * tests aren't instant. If we haven't been running long enough, * trust the relay. */ - if (stats_n_seconds_working > + if (get_uptime() > get_options()->MinUptimeHidServDirectoryV2 * 1.1) uptime = MIN(rep_hist_get_uptime(router->cache_info.identity_digest, now), real_uptime(router, now)); diff --git a/src/or/hibernate.c b/src/or/hibernate.c index 4dc35f68d..7261cf800 100644 --- a/src/or/hibernate.c +++ b/src/or/hibernate.c @@ -866,7 +866,7 @@ hibernate_end(hibernate_state_t new_state) hibernate_state = new_state; hibernate_end_time = 0; /* no longer hibernating */ - stats_n_seconds_working = 0; /* reset published uptime */ + reset_uptime(); /* reset published uptime */ } /** A wrapper around hibernate_begin, for when we get SIGINT. */ diff --git a/src/or/main.c b/src/or/main.c index 96c7e77c7..7b88686fd 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1094,7 +1094,7 @@ directory_all_unreachable(time_t now) { (void)now; - stats_n_seconds_working=0; /* reset it */ + reset_uptime(); /* reset it */ if (!directory_all_unreachable_cb_event) { directory_all_unreachable_cb_event = @@ -2062,7 +2062,7 @@ check_for_reachability_bw_callback(time_t now, const or_options_t *options) if (server_mode(options) && (have_completed_a_circuit() || !any_predicted_circuits(now)) && !net_is_disabled()) { - if (stats_n_seconds_working < TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT) { + if (get_uptime() < TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT) { consider_testing_reachability(1, dirport_reachability_count==0); if (++dirport_reachability_count > 5) dirport_reachability_count = 0; @@ -2319,8 +2319,8 @@ second_elapsed_callback(periodic_timer_t *timer, void *arg) !net_is_disabled() && seconds_elapsed > 0 && have_completed_a_circuit() && - stats_n_seconds_working / TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT != - (stats_n_seconds_working+seconds_elapsed) / + get_uptime() / TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT != + (get_uptime()+seconds_elapsed) / TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT) { /* every 20 minutes, check and complain if necessary */ const routerinfo_t *me = router_get_my_routerinfo(); @@ -2464,9 +2464,9 @@ ip_address_changed(int at_interface) } } else { if (server) { - if (stats_n_seconds_working > UPTIME_CUTOFF_FOR_NEW_BANDWIDTH_TEST) + if (get_uptime() > UPTIME_CUTOFF_FOR_NEW_BANDWIDTH_TEST) reset_bandwidth_test(); - stats_n_seconds_working = 0; + reset_uptime(); router_reset_reachability(); } } @@ -3004,6 +3004,13 @@ get_uptime,(void)) return stats_n_seconds_working; } +/** Reset Tor's uptime. */ +MOCK_IMPL(void, +reset_uptime,(void)) +{ + stats_n_seconds_working = 0; +} + /** * Write current memory usage information to the log. */ diff --git a/src/or/main.h b/src/or/main.h index c49d216f4..f01506fce 100644 --- a/src/or/main.h +++ b/src/or/main.h @@ -63,6 +63,7 @@ void reschedule_descriptor_update_check(void); void reschedule_directory_downloads(void); MOCK_DECL(long,get_uptime,(void)); +MOCK_DECL(void,reset_uptime,(void)); unsigned get_signewnym_epoch(void); @@ -87,7 +88,6 @@ uint64_t get_main_loop_error_count(void); uint64_t get_main_loop_idle_count(void); extern time_t time_of_process_start; -extern long stats_n_seconds_working; extern int quiet_level; extern int global_read_bucket; extern int global_write_bucket; diff --git a/src/or/router.c b/src/or/router.c index 9c053cad4..ddfc187e9 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2922,7 +2922,7 @@ router_dump_router_to_string(routerinfo_t *router, proto_line, published, fingerprint, - stats_n_seconds_working, + get_uptime(), (int) router->bandwidthrate, (int) router->bandwidthburst, (int) router->bandwidthcapacity,

1 0

[tor/master] Changes file for ticket 25081.
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit ad06e0b406f5b3dfaa28da0f03ede4b0f850d00f Author: Caio Valente <valentecaio95(a)gmail.com> Date: Thu Feb 1 02:25:08 2018 +0100 Changes file for ticket 25081. --- changes/ticket25081 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/changes/ticket25081 b/changes/ticket25081 new file mode 100644 index 000000000..b6e6c2a5a --- /dev/null +++ b/changes/ticket25081 @@ -0,0 +1,5 @@ + o Code simplification and refactoring: + - Remove extern declaration of stats_n_seconds_working variable from main, + protecting its accesses with get_uptime() and reset_uptime() functions. + Closes ticket 25081, patch by “valentecaio”. +

1 0

[tor/master] Merge remote-tracking branch 'valentecaio/t-25081'
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 3ca04aada2916ce6963358007476eccd851e67cc Merge: 3d7bf98d1 ad06e0b40 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 15 20:23:23 2018 -0500 Merge remote-tracking branch 'valentecaio/t-25081' changes/ticket25081 | 5 +++++ src/or/dirserv.c | 2 +- src/or/hibernate.c | 2 +- src/or/main.c | 19 +++++++++++++------ src/or/main.h | 2 +- src/or/router.c | 2 +- 6 files changed, 22 insertions(+), 10 deletions(-)

1 0

[tor/master] This stats_n_seconds_working variable needs to be static now.
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 066a15af635b1e75476834812f6a326eecf9aa98 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 15 20:26:09 2018 -0500 This stats_n_seconds_working variable needs to be static now. (When a variable isn't going to be declared extern in the header, we require that it be static.) --- src/or/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/main.c b/src/or/main.c index 5583f370f..7276917e4 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -179,7 +179,7 @@ static uint64_t stats_n_bytes_written = 0; /** What time did this process start up? */ time_t time_of_process_start = 0; /** How many seconds have we been running? */ -long stats_n_seconds_working = 0; +static long stats_n_seconds_working = 0; /** How many times have we returned from the main loop successfully? */ static uint64_t stats_n_main_loop_successes = 0; /** How many times have we received an error from the main loop? */

1 0

[tor/master] refactor: rename connection_t struct fields.
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 7884ce76e100abc220ddedb76c9b98d22d66d645 Author: Caio Valente <valentecaio95(a)gmail.com> Date: Thu Feb 1 03:12:38 2018 +0100 refactor: rename connection_t struct fields. connection_t.timestamp_lastwritten renamed to connection_t.timestamp_last_write_allowed connection_t.timestamp_lastread renamed to connection_t.timestamp_last_read_allowed Closes ticket 24714. --- changes/ticket24714 | 6 ++++++ src/or/circuituse.c | 2 +- src/or/connection.c | 14 +++++++------- src/or/connection_edge.c | 6 +++--- src/or/directory.c | 2 +- src/or/hs_client.c | 4 ++-- src/or/main.c | 18 ++++++++++-------- src/or/or.h | 8 ++++---- src/or/relay.c | 2 +- src/or/rendclient.c | 4 ++-- 10 files changed, 37 insertions(+), 29 deletions(-) diff --git a/changes/ticket24714 b/changes/ticket24714 new file mode 100644 index 000000000..60353abec --- /dev/null +++ b/changes/ticket24714 @@ -0,0 +1,6 @@ + o Code simplification and refactoring: + - Rename two fields of connection_t struct. + timestamp_lastwritten is renamed to timestamp_last_write_allowed and + timestamp_lastread is renamed to timestamp_last_read_allowed. + Closes ticket 24714, patch by "valentecaio". + diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 3a14a3ccf..1af0b178d 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -2585,7 +2585,7 @@ link_apconn_to_circ(entry_connection_t *apconn, origin_circuit_t *circ, log_debug(LD_APP|LD_CIRC, "attaching new conn to circ. n_circ_id %u.", (unsigned)circ->base_.n_circ_id); /* reset it, so we can measure circ timeouts */ - ENTRY_TO_CONN(apconn)->timestamp_lastread = time(NULL); + ENTRY_TO_CONN(apconn)->timestamp_last_read_allowed = time(NULL); ENTRY_TO_EDGE_CONN(apconn)->next_stream = circ->p_streams; ENTRY_TO_EDGE_CONN(apconn)->on_circuit = TO_CIRCUIT(circ); /* assert_connection_ok(conn, time(NULL)); */ diff --git a/src/or/connection.c b/src/or/connection.c index 5bbb61dfa..93b7847f1 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -462,8 +462,8 @@ connection_init(time_t now, connection_t *conn, int type, int socket_family) } conn->timestamp_created = now; - conn->timestamp_lastread = now; - conn->timestamp_lastwritten = now; + conn->timestamp_last_read_allowed = now; + conn->timestamp_last_write_allowed = now; } /** Create a link between conn_a and conn_b. */ @@ -861,7 +861,7 @@ connection_mark_for_close_internal_, (connection_t *conn, /* in case we're going to be held-open-til-flushed, reset * the number of seconds since last successful write, so * we get our whole 15 seconds */ - conn->timestamp_lastwritten = time(NULL); + conn->timestamp_last_write_allowed = time(NULL); } /** Find each connection that has hold_open_until_flushed set to @@ -883,7 +883,7 @@ connection_expire_held_open(void) */ if (conn->hold_open_until_flushed) { tor_assert(conn->marked_for_close); - if (now - conn->timestamp_lastwritten >= 15) { + if (now - conn->timestamp_last_write_allowed >= 15) { int severity; if (conn->type == CONN_TYPE_EXIT || (conn->type == CONN_TYPE_DIR && @@ -3420,7 +3420,7 @@ connection_handle_read_impl(connection_t *conn) if (conn->marked_for_close) return 0; /* do nothing */ - conn->timestamp_lastread = approx_time(); + conn->timestamp_last_read_allowed = approx_time(); switch (conn->type) { case CONN_TYPE_OR_LISTENER: @@ -3821,7 +3821,7 @@ update_send_buffer_size(tor_socket_t sock) * when libevent tells us that conn wants to write, or below * from connection_buf_add() when an entire TLS record is ready. * - * Update conn-\>timestamp_lastwritten to now, and call flush_buf + * Update conn-\>timestamp_last_write_allowed to now, and call flush_buf * or flush_buf_tls appropriately. If it succeeds and there are no more * more bytes on conn-\>outbuf, then call connection_finished_flushing * on it too. @@ -3854,7 +3854,7 @@ connection_handle_write_impl(connection_t *conn, int force) return 0; } - conn->timestamp_lastwritten = now; + conn->timestamp_last_write_allowed = now; /* Sometimes, "writable" means "connected". */ if (connection_state_is_connecting(conn)) { diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index a47f044e0..948c8722b 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -739,7 +739,7 @@ connection_ap_expire_beginning(void) /* if it's an internal linked connection, don't yell its status. */ severity = (tor_addr_is_null(&base_conn->addr) && !base_conn->port) ? LOG_INFO : LOG_NOTICE; - seconds_idle = (int)( now - base_conn->timestamp_lastread ); + seconds_idle = (int)( now - base_conn->timestamp_last_read_allowed ); seconds_since_born = (int)( now - base_conn->timestamp_created ); if (base_conn->state == AP_CONN_STATE_OPEN) @@ -825,7 +825,7 @@ connection_ap_expire_beginning(void) mark_circuit_unusable_for_new_conns(TO_ORIGIN_CIRCUIT(circ)); /* give our stream another 'cutoff' seconds to try */ - conn->base_.timestamp_lastread += cutoff; + conn->base_.timestamp_last_read_allowed += cutoff; if (entry_conn->num_socks_retries < 250) /* avoid overflow */ entry_conn->num_socks_retries++; /* move it back into 'pending' state, and try to attach. */ @@ -1135,7 +1135,7 @@ connection_ap_detach_retriable(entry_connection_t *conn, int reason) { control_event_stream_status(conn, STREAM_EVENT_FAILED_RETRIABLE, reason); - ENTRY_TO_CONN(conn)->timestamp_lastread = time(NULL); + ENTRY_TO_CONN(conn)->timestamp_last_read_allowed = time(NULL); /* Roll back path bias use state so that we probe the circuit * if nothing else succeeds on it */ diff --git a/src/or/directory.c b/src/or/directory.c index c55f81bc6..f5f9e3e55 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2437,7 +2437,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn) * and the date header. (We used to check now-date_header, but that's * inaccurate if we spend a lot of time downloading.) */ - apparent_skew = conn->base_.timestamp_lastwritten - date_header; + apparent_skew = conn->base_.timestamp_last_write_allowed - date_header; if (labs(apparent_skew)>ALLOW_DIRECTORY_TIME_SKEW) { int trusted = router_digest_is_trusted_dir(conn->identity_digest); clock_skew_warning(TO_CONN(conn), apparent_skew, trusted, LD_HTTP, diff --git a/src/or/hs_client.c b/src/or/hs_client.c index 2999f85d3..931145364 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -1439,8 +1439,8 @@ hs_client_desc_has_arrived(const hs_ident_dir_conn_t *ident) * connection is considered "fresh" and can continue without being closed * too early. */ base_conn->timestamp_created = now; - base_conn->timestamp_lastread = now; - base_conn->timestamp_lastwritten = now; + base_conn->timestamp_last_read_allowed = now; + base_conn->timestamp_last_write_allowed = now; /* Change connection's state into waiting for a circuit. */ base_conn->state = AP_CONN_STATE_CIRCUIT_WAIT; diff --git a/src/or/main.c b/src/or/main.c index 96c7e77c7..b7e223e48 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1008,7 +1008,8 @@ conn_close_if_marked(int i) LOG_FN_CONN(conn, (LOG_INFO,LD_NET, "Holding conn (fd %d) open for more flushing.", (int)conn->s)); - conn->timestamp_lastwritten = now; /* reset so we can flush more */ + conn->timestamp_last_write_allowed = now; /* reset so we can flush + * more */ } else if (sz == 0) { /* Also, retval==0. If we get here, we didn't want to write anything * (because of rate-limiting) and we didn't. */ @@ -1159,7 +1160,7 @@ run_connection_housekeeping(int i, time_t now) channel_t *chan = NULL; int have_any_circuits; int past_keepalive = - now >= conn->timestamp_lastwritten + options->KeepalivePeriod; + now >= conn->timestamp_last_write_allowed + options->KeepalivePeriod; if (conn->outbuf && !connection_get_outbuf_len(conn) && conn->type == CONN_TYPE_OR) @@ -1174,10 +1175,10 @@ run_connection_housekeeping(int i, time_t now) * if a server or received if a client) for 5 min */ if (conn->type == CONN_TYPE_DIR && ((DIR_CONN_IS_SERVER(conn) && - conn->timestamp_lastwritten + conn->timestamp_last_write_allowed + options->TestingDirConnectionMaxStall < now) || (!DIR_CONN_IS_SERVER(conn) && - conn->timestamp_lastread + conn->timestamp_last_read_allowed + options->TestingDirConnectionMaxStall < now))) { log_info(LD_DIR,"Expiring wedged directory conn (fd %d, purpose %d)", (int)conn->s, conn->purpose); @@ -1253,13 +1254,14 @@ run_connection_housekeeping(int i, time_t now) connection_or_close_normally(TO_OR_CONN(conn), 0); } else if ( now >= or_conn->timestamp_lastempty + options->KeepalivePeriod*10 && - now >= conn->timestamp_lastwritten + options->KeepalivePeriod*10) { + now >= + conn->timestamp_last_write_allowed + options->KeepalivePeriod*10) { log_fn(LOG_PROTOCOL_WARN,LD_PROTOCOL, "Expiring stuck OR connection to fd %d (%s:%d). (%d bytes to " "flush; %d seconds since last write)", (int)conn->s, conn->address, conn->port, (int)connection_get_outbuf_len(conn), - (int)(now-conn->timestamp_lastwritten)); + (int)(now-conn->timestamp_last_write_allowed)); connection_or_close_normally(TO_OR_CONN(conn), 0); } else if (past_keepalive && !connection_get_outbuf_len(conn)) { /* send a padding cell */ @@ -3047,13 +3049,13 @@ dumpstats(int severity) i, (int)connection_get_inbuf_len(conn), (int)buf_allocation(conn->inbuf), - (int)(now - conn->timestamp_lastread)); + (int)(now - conn->timestamp_last_read_allowed)); tor_log(severity,LD_GENERAL, "Conn %d: %d bytes waiting on outbuf " "(len %d, last written %d secs ago)",i, (int)connection_get_outbuf_len(conn), (int)buf_allocation(conn->outbuf), - (int)(now - conn->timestamp_lastwritten)); + (int)(now - conn->timestamp_last_write_allowed)); if (conn->type == CONN_TYPE_OR) { or_connection_t *or_conn = TO_OR_CONN(conn); if (or_conn->tls) { diff --git a/src/or/or.h b/src/or/or.h index 0436533a9..7177045e4 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1361,10 +1361,10 @@ typedef struct connection_t { * connection. */ size_t outbuf_flushlen; /**< How much data should we try to flush from the * outbuf? */ - time_t timestamp_lastread; /**< When was the last time libevent said we could - * read? */ - time_t timestamp_lastwritten; /**< When was the last time libevent said we - * could write? */ + time_t timestamp_last_read_allowed; /**< When was the last time libevent said + * we could read? */ + time_t timestamp_last_write_allowed; /**< When was the last time libevent + * said we could write? */ time_t timestamp_created; /**< When was this connection_t created? */ diff --git a/src/or/relay.c b/src/or/relay.c index b1b99526d..caccb1282 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -1449,7 +1449,7 @@ connection_edge_process_relay_cell_not_open( "after %d seconds.", (unsigned)circ->n_circ_id, rh->stream_id, - (int)(time(NULL) - conn->base_.timestamp_lastread)); + (int)(time(NULL) - conn->base_.timestamp_last_read_allowed)); if (connected_cell_parse(rh, cell, &addr, &ttl) < 0) { log_fn(LOG_PROTOCOL_WARN, LD_APP, "Got a badly formatted connected cell. Closing."); diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 07fa33306..29f1d9bd1 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -915,8 +915,8 @@ rend_client_desc_trynow(const char *query) /* restart their timeout values, so they get a fair shake at * connecting to the hidden service. */ base_conn->timestamp_created = now; - base_conn->timestamp_lastread = now; - base_conn->timestamp_lastwritten = now; + base_conn->timestamp_last_read_allowed = now; + base_conn->timestamp_last_write_allowed = now; connection_ap_mark_as_pending_circuit(conn); } else { /* 404, or fetch didn't get that far */

1 0