From 34f07ec8629a94b87fd19f93bb62d4f91286fbc6 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 11 Mar 2013 20:50:02 -0400 Subject: [PATCH 01/16] When hibernating, don't heartbeat about problems. Fixes part of 7302. --- changes/bug7302 | 5 +++++ src/or/status.c | 11 +++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) create mode 100644 changes/bug7302 diff --git a/changes/bug7302 b/changes/bug7302 new file mode 100644 index 000000000..2949e4094 --- /dev/null +++ b/changes/bug7302 @@ -0,0 +1,5 @@ + o Minor bugfixes: + - Don't log inappropriate heartbeat messages when hibernating: a + hibernating node is _expected_ to drop out of the consensus, + decide it isn't bootstrapped, and so forth. Fixes part of bug + #7302; bugfix on 0.2.3.1-alpha. diff --git a/src/or/status.c b/src/or/status.c index 126167dcb..6a43fc4d8 100644 --- a/src/or/status.c +++ b/src/or/status.c @@ -14,6 +14,7 @@ #include "router.h" #include "circuitlist.h" #include "main.h" +#include "hibernate.h" /** Return the total number of circuits. */ static int @@ -85,11 +86,12 @@ log_heartbeat(time_t now) char *bw_rcvd = NULL; char *uptime = NULL; const routerinfo_t *me; + const int hibernating = we_are_hibernating(); const or_options_t *options = get_options(); (void)now; - if (public_server_mode(options)) { + if (public_server_mode(options) && !hibernating) { /* Let's check if we are in the current cached consensus. */ if (!(me = router_get_my_routerinfo())) return -1; /* Something stinks, we won't even attempt this. */ @@ -104,10 +106,11 @@ log_heartbeat(time_t now) bw_sent = bytes_to_usage(get_bytes_written()); log_fn(LOG_NOTICE, LD_HEARTBEAT, "Heartbeat: Tor's uptime is %s, with %d " - "circuits open. I've sent %s and received %s.", - uptime, count_circuits(),bw_sent,bw_rcvd); + "circuits open. I've sent %s and received %s.%s", + uptime, count_circuits(),bw_sent,bw_rcvd, + hibernating?" We are currently hibernating.":""); - if (stats_n_data_cells_packaged) + if (stats_n_data_cells_packaged && !hibernating) log_notice(LD_HEARTBEAT, "Average packaged cell fullness: %2.3f%%", 100*(U64_TO_DBL(stats_n_data_bytes_packaged) / U64_TO_DBL(stats_n_data_cells_packaged*RELAY_PAYLOAD_SIZE)) ); From 805ecb8719e5e66d708f040027fecc6de56b3a5b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 11 Mar 2013 20:52:20 -0400 Subject: [PATCH 02/16] Make control_event_bootstrap_problem always INFO when hibernating When we're hibernating, the main reqason we can't bootstrap will always be that we're hibernating: reporting anything else at severity WARN is pointless. Fixes part of 7302. --- changes/bug7302 | 8 +++++++- src/or/control.c | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/changes/bug7302 b/changes/bug7302 index 2949e4094..fec615ff9 100644 --- a/changes/bug7302 +++ b/changes/bug7302 @@ -2,4 +2,10 @@ - Don't log inappropriate heartbeat messages when hibernating: a hibernating node is _expected_ to drop out of the consensus, decide it isn't bootstrapped, and so forth. Fixes part of bug - #7302; bugfix on 0.2.3.1-alpha. + 7302; bugfix on 0.2.3.1-alpha. + + - Don't complain about bootstrapping problems while hibernating. + These complaints reflect a general code problems, but not one + with any problematic effects. (No connections are actually + opened.) Fixes part of bug 7302; bugfix on 0.2.3.2-alpha. + diff --git a/src/or/control.c b/src/or/control.c index 03e5d79c8..5e61cd787 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -4717,6 +4717,9 @@ control_event_bootstrap_problem(const char *warn, int reason) !any_pending_bridge_descriptor_fetches()) recommendation = "warn"; + if (we_are_hibernating()) + recommendation = "ignore"; + 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 */ From 7d1ade251bad76c82b3f1288097587e0fbd1c4ae Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 19 Mar 2013 17:00:40 -0400 Subject: [PATCH 03/16] Debugging log for bug 8185 If the bug recurs, log the filename and line number that triggered it --- changes/bug8185_diagnostic | 3 +++ src/or/relay.c | 22 +++++++++++++++------- src/or/relay.h | 10 ++++++++-- 3 files changed, 26 insertions(+), 9 deletions(-) create mode 100644 changes/bug8185_diagnostic diff --git a/changes/bug8185_diagnostic b/changes/bug8185_diagnostic new file mode 100644 index 000000000..b0f888475 --- /dev/null +++ b/changes/bug8185_diagnostic @@ -0,0 +1,3 @@ + o Minor features: + - Improve debugging output to attempt to diagnose the underlying + cause of bug 8185. diff --git a/src/or/relay.c b/src/or/relay.c index 1da993269..c71fe2a6d 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -379,15 +379,22 @@ relay_crypt(circuit_t *circ, cell_t *cell, cell_direction_t cell_direction, static int circuit_package_relay_cell(cell_t *cell, circuit_t *circ, cell_direction_t cell_direction, - crypt_path_t *layer_hint, streamid_t on_stream) + crypt_path_t *layer_hint, streamid_t on_stream, + const char *filename, int lineno) { channel_t *chan; /* where to send the cell */ if (cell_direction == CELL_DIRECTION_OUT) { crypt_path_t *thishop; /* counter for repeated crypts */ chan = circ->n_chan; - if (!CIRCUIT_IS_ORIGIN(circ) || !chan) { - log_warn(LD_BUG,"outgoing relay cell has n_chan==NULL. Dropping."); + if (!chan) { + log_warn(LD_BUG,"outgoing relay cell sent from %s:%d has n_chan==NULL." + " Dropping.", filename, lineno); + return 0; /* just drop it */ + } + if (!CIRCUIT_IS_ORIGIN(circ)) { + log_warn(LD_BUG,"outgoing relay cell sent from %s:%d on non-origin " + "circ. Dropping.", filename, lineno); return 0; /* just drop it */ } @@ -548,9 +555,10 @@ relay_command_to_string(uint8_t command) * return 0. */ int -relay_send_command_from_edge(streamid_t stream_id, circuit_t *circ, - uint8_t relay_command, const char *payload, - size_t payload_len, crypt_path_t *cpath_layer) +relay_send_command_from_edge_(streamid_t stream_id, circuit_t *circ, + uint8_t relay_command, const char *payload, + size_t payload_len, crypt_path_t *cpath_layer, + const char *filename, int lineno) { cell_t cell; relay_header_t rh; @@ -633,7 +641,7 @@ relay_send_command_from_edge(streamid_t stream_id, circuit_t *circ, } if (circuit_package_relay_cell(&cell, circ, cell_direction, cpath_layer, - stream_id) < 0) { + stream_id, filename, lineno) < 0) { log_warn(LD_BUG,"circuit_package_relay_cell failed. Closing."); circuit_mark_for_close(circ, END_CIRC_REASON_INTERNAL); return -1; diff --git a/src/or/relay.h b/src/or/relay.h index 7e59838f9..229fb4f73 100644 --- a/src/or/relay.h +++ b/src/or/relay.h @@ -20,9 +20,15 @@ int circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, void relay_header_pack(uint8_t *dest, const relay_header_t *src); void relay_header_unpack(relay_header_t *dest, const uint8_t *src); -int relay_send_command_from_edge(streamid_t stream_id, circuit_t *circ, +int relay_send_command_from_edge_(streamid_t stream_id, circuit_t *circ, uint8_t relay_command, const char *payload, - size_t payload_len, crypt_path_t *cpath_layer); + size_t payload_len, crypt_path_t *cpath_layer, + const char *filename, int lineno); +#define relay_send_command_from_edge(stream_id, circ, relay_command, payload, \ + payload_len, cpath_layer) \ + relay_send_command_from_edge_((stream_id), (circ), (relay_command), \ + (payload), (payload_len), (cpath_layer), \ + __FILE__, __LINE__) int connection_edge_send_command(edge_connection_t *fromconn, uint8_t relay_command, const char *payload, size_t payload_len); From a264c4fedab87ce7c8cbb94632657a90e95e7a4e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 20 Mar 2013 15:37:47 -0400 Subject: [PATCH 04/16] Prefer SOCKS_USER_PASS over SOCKS_NO_AUTH --- src/or/buffers.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index ad5ab83e4..4554a02d6 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -1783,19 +1783,19 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, return -1; req->replylen = 2; /* 2 bytes of response */ req->reply[0] = 5; /* socks5 reply */ - if (memchr(data+2, SOCKS_NO_AUTH, nummethods)) { - req->reply[1] = SOCKS_NO_AUTH; /* tell client to use "none" auth - method */ - req->socks_version = 5; /* remember we've already negotiated auth */ - log_debug(LD_APP,"socks5: accepted method 0 (no authentication)"); - r=0; - } else if (memchr(data+2, SOCKS_USER_PASS, nummethods)) { + if (memchr(data+2, SOCKS_USER_PASS, nummethods)) { req->auth_type = SOCKS_USER_PASS; req->reply[1] = SOCKS_USER_PASS; /* tell client to use "user/pass" auth method */ req->socks_version = 5; /* remember we've already negotiated auth */ log_debug(LD_APP,"socks5: accepted method 2 (username/password)"); r=0; + } else if (memchr(data+2, SOCKS_NO_AUTH, nummethods)) { + req->reply[1] = SOCKS_NO_AUTH; /* tell client to use "none" auth + method */ + req->socks_version = 5; /* remember we've already negotiated auth */ + log_debug(LD_APP,"socks5: accepted method 0 (no authentication)"); + r=0; } else { log_warn(LD_APP, "socks5: offered methods don't include 'no auth' or " From fa3c23773944788125db2f67bcb048376c17fec9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 20 Mar 2013 16:17:06 -0400 Subject: [PATCH 05/16] Per-SOCKSPort configuration for bug 8117 fix. This might be necessary if the bug8117 fix confuses any applications. Also add a changes file. --- changes/bug8117 | 13 +++++++++++++ doc/tor.1.txt | 14 +++++++++++++- src/or/buffers.c | 7 +++++-- src/or/config.c | 9 +++++++++ src/or/connection.c | 6 ++++++ src/or/or.h | 12 ++++++++++++ 6 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 changes/bug8117 diff --git a/changes/bug8117 b/changes/bug8117 new file mode 100644 index 000000000..910e8056f --- /dev/null +++ b/changes/bug8117 @@ -0,0 +1,13 @@ + o Major bugfixes: + + - Many SOCKS5 clients, when configured to offer a username/password, + offer both username/password authentication and "no authentication". + Tor had previously preferred no authentication, but this was + problematic when trying to make applications get proper stream + isolation with IsolateSOCKSAuth. Now, on any SOCKS port with + IsolateSOCKSAuth turned on (which is the default), Tor selects + username/password authentication if it's offered. If this confuses your + application, you can disable it on a per-SOCKSPort basis via + PreferSOCKSNoAuth. Fixes bug 8117; bugfix on 0.2.3.3-alpha. + + diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 773fccf53..85f0835cb 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -860,7 +860,7 @@ The following options are useful only for clients (that is, if the same circuit. Currently, two addresses are "too close" if they lie in the same /16 range. (Default: 1) -**SOCKSPort** \['address':]__port__|**auto** [_isolation flags_]:: +**SOCKSPort** \['address':]__port__|**auto** [_flags_] [_isolation flags_]:: Open this port to listen for connections from SOCKS-speaking applications. Set this to 0 if you don't want to allow application connections via SOCKS. Set it to "auto" to have Tor pick a port for @@ -894,6 +894,18 @@ The following options are useful only for clients (that is, if port with the same session group. (By default, streams received on different SOCKSPorts, TransPorts, etc are always isolated from one another. This option overrides that behavior.) ++ + Other recognized _flags_ for a SOCKSPort are: + **PreferSOCKSNoAuth**;; + Ordinarily, when an application offers both "username/password + authentication" and "no authentication" to Tor via SOCKS5, Tor + selects username/password authentication so that IsolateSOCKSAuth can + work. This can confuse some applications, if they offer a + username/password combination then get confused when asked for + one. You can disable this behavior, so that Tor will select "No + authentication" when IsolateSOCKSAuth is disabled, or when this + option is set. + **SOCKSListenAddress** __IP__[:__PORT__]:: Bind to this address to listen for connections from Socks-speaking diff --git a/src/or/buffers.c b/src/or/buffers.c index 4554a02d6..c0868f479 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -1773,6 +1773,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, if (req->socks_version != 5) { /* we need to negotiate a method */ unsigned char nummethods = (unsigned char)*(data+1); + int have_user_pass, have_no_auth; int r=0; tor_assert(!req->socks_version); if (datalen < 2u+nummethods) { @@ -1783,14 +1784,16 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, return -1; req->replylen = 2; /* 2 bytes of response */ req->reply[0] = 5; /* socks5 reply */ - if (memchr(data+2, SOCKS_USER_PASS, nummethods)) { + have_user_pass = (memchr(data+2, SOCKS_USER_PASS, nummethods) !=NULL); + have_no_auth = (memchr(data+2, SOCKS_NO_AUTH, nummethods) !=NULL); + if (have_user_pass && !(have_no_auth && req->socks_prefer_no_auth)) { req->auth_type = SOCKS_USER_PASS; req->reply[1] = SOCKS_USER_PASS; /* tell client to use "user/pass" auth method */ req->socks_version = 5; /* remember we've already negotiated auth */ log_debug(LD_APP,"socks5: accepted method 2 (username/password)"); r=0; - } else if (memchr(data+2, SOCKS_NO_AUTH, nummethods)) { + } else if (have_no_auth) { req->reply[1] = SOCKS_NO_AUTH; /* tell client to use "none" auth method */ req->socks_version = 5; /* remember we've already negotiated auth */ diff --git a/src/or/config.c b/src/or/config.c index 90a5dfbda..a80576e20 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -5787,6 +5787,7 @@ parse_port_config(smartlist_t *out, int port; int sessiongroup = SESSION_GROUP_UNSET; unsigned isolation = ISO_DEFAULT; + int prefer_no_auth = 0; char *addrport; uint16_t ptmp=0; @@ -5916,6 +5917,11 @@ parse_port_config(smartlist_t *out, no = 1; elt += 2; } + if (!strcasecmp(elt, "PreferSOCKSNoAuth")) { + prefer_no_auth = ! no; + continue; + } + if (!strcasecmpend(elt, "s")) elt[strlen(elt)-1] = '\0'; /* kill plurals. */ @@ -5959,6 +5965,9 @@ parse_port_config(smartlist_t *out, cfg->all_addrs = all_addrs; cfg->ipv4_only = ipv4_only; cfg->ipv6_only = ipv6_only; + cfg->socks_prefer_no_auth = prefer_no_auth; + if (! (isolation & ISO_SOCKSAUTH)) + cfg->socks_prefer_no_auth = 1; smartlist_add(out, cfg); } diff --git a/src/or/connection.c b/src/or/connection.c index eac9c4f32..aeb4949e0 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -1056,6 +1056,7 @@ connection_listener_new(const struct sockaddr *listensockaddr, lis_conn->session_group = global_next_session_group--; } } + lis_conn->socks_prefer_no_auth = port_cfg->socks_prefer_no_auth; if (connection_add(conn) < 0) { /* no space, forget it */ log_warn(LD_NET,"connection_add for listener failed. Giving up."); @@ -1238,6 +1239,11 @@ connection_handle_listener_read(connection_t *conn, int new_type) newconn->port = port; newconn->address = tor_dup_addr(&addr); + if (new_type == CONN_TYPE_AP) { + TO_ENTRY_CONN(newconn)->socks_request->socks_prefer_no_auth = + TO_LISTENER_CONN(conn)->socks_prefer_no_auth; + } + } else if (conn->socket_family == AF_UNIX) { /* For now only control ports can be Unix domain sockets * and listeners at the same time */ diff --git a/src/or/or.h b/src/or/or.h index 51c23d305..ca28c0e7b 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1085,6 +1085,10 @@ typedef struct listener_connection_t { /** One or more ISO_ flags to describe how to isolate streams. */ uint8_t isolation_flags; /**@}*/ + /** For SOCKS connections only: If this is set, we will choose "no + * authentication" instead of "username/password" authentication if both + * are offered. Used as input to parse_socks. */ + unsigned int socks_prefer_no_auth : 1; } listener_connection_t; @@ -2910,6 +2914,10 @@ typedef struct port_cfg_t { uint8_t isolation_flags; /**< Zero or more isolation flags */ int session_group; /**< A session group, or -1 if this port is not in a * session group. */ + /* Socks only: */ + /** When both no-auth and user/pass are advertised by a SOCKS client, select + * no-auth. */ + unsigned int socks_prefer_no_auth : 1; /* Server port types (or, dir) only: */ unsigned int no_advertise : 1; @@ -3729,6 +3737,10 @@ struct socks_request_t { * make sure we send back a socks reply for * every connection. */ unsigned int got_auth : 1; /**< Have we received any authentication data? */ + /** If this is set, we will choose "no authentication" instead of + * "username/password" authentication if both are offered. Used as input to + * parse_socks. */ + unsigned int socks_prefer_no_auth : 1; /** Number of bytes in username; 0 if username is NULL */ size_t usernamelen; From e9e430403cb70e18ae3c22e47d24c189be8e492c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 25 Mar 2013 10:07:41 -0400 Subject: [PATCH 06/16] Fix two dump bugs in "whether we can use curve25519-donna-c64" test Dumb bug 1: == has higher precedence than &. Dumb bug 2: the main() function in an AC_RUN_IFELSE test is expected to return 0 on success, not 1. --- changes/bug8587 | 5 +++++ configure.ac | 18 ++++++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) create mode 100644 changes/bug8587 diff --git a/changes/bug8587 b/changes/bug8587 new file mode 100644 index 000000000..84d2f1ec0 --- /dev/null +++ b/changes/bug8587 @@ -0,0 +1,5 @@ + o Minor bugfixes (build): + - Build Tor correctly on 32-bit platforms where the compiler can build + but not run code using the "uint128_t" construction. Fixes bug 8587; + bugfix on 0.2.4.8-alpha. + diff --git a/configure.ac b/configure.ac index ed452cb54..8a79653e0 100644 --- a/configure.ac +++ b/configure.ac @@ -667,10 +667,11 @@ if test x$enable_curve25519 != xno; then uint64_t a = ((uint64_t)2000000000) * 1000000000; uint64_t b = ((uint64_t)1234567890) << 24; uint128_t c = ((uint128_t)a) * b; - return ((uint64_t)(c>>96)) == 522859 && - ((uint64_t)(c>>64))&0xffffffffL == 3604448702L && - ((uint64_t)(c>>32))&0xffffffffL == 2351960064L && - ((uint64_t)(c))&0xffffffffL == 0; + int ok = ((uint64_t)(c>>96)) == 522859 && + (((uint64_t)(c>>64))&0xffffffffL) == 3604448702L && + (((uint64_t)(c>>32))&0xffffffffL) == 2351960064L && + (((uint64_t)(c))&0xffffffffL) == 0; + return !ok; ])], [tor_cv_can_use_curve25519_donna_c64=yes], [tor_cv_can_use_curve25519_donna_c64=no], @@ -682,10 +683,11 @@ if test x$enable_curve25519 != xno; then uint64_t a = ((uint64_t)2000000000) * 1000000000; uint64_t b = ((uint64_t)1234567890) << 24; uint128_t c = ((uint128_t)a) * b; - return ((uint64_t)(c>>96)) == 522859 && - ((uint64_t)(c>>64))&0xffffffffL == 3604448702L && - ((uint64_t)(c>>32))&0xffffffffL == 2351960064L && - ((uint64_t)(c))&0xffffffffL == 0; + int ok = ((uint64_t)(c>>96)) == 522859 && + (((uint64_t)(c>>64))&0xffffffffL) == 3604448702L && + (((uint64_t)(c>>32))&0xffffffffL) == 2351960064L && + (((uint64_t)(c))&0xffffffffL) == 0; + return !ok; ])], [tor_cv_can_use_curve25519_donna_c64=cross], [tor_cv_can_use_curve25519_donna_c64=no])])]) From ec4ee3197fb0891a094a5fc4860c0b94eefee3c7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Apr 2013 10:24:11 -0400 Subject: [PATCH 07/16] Use correct units for dirserv_get_{credible_bandwidth,bandwidth_for_router} We were mixing bandwidth file entries (which are in kilobytes) with router_get_advertised_bw() entries, which were in bytes. Also, use router_get_advertised_bandwidth_capped() for credible_bandwidth. --- src/or/dirserv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index e837e4bed..ed19406ba 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -2214,7 +2214,7 @@ dirserv_get_bandwidth_for_router(const routerinfo_t *ri) bw = (uint32_t)mbw; } else { /* If not, fall back to advertised */ - bw = router_get_advertised_bandwidth(ri); + bw = router_get_advertised_bandwidth(ri) / 1000; } } @@ -2263,7 +2263,7 @@ dirserv_get_credible_bandwidth(const routerinfo_t *ri) bw = 0; } else { /* Return an advertised bandwidth otherwise */ - bw = router_get_advertised_bandwidth(ri); + bw = router_get_advertised_bandwidth_capped(ri) / 1000; } } else { /* We have the measured bandwidth in mbw */ From 265a7ebca6128ef5002d70e9b52b67800e4ed77b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Apr 2013 10:26:31 -0400 Subject: [PATCH 08/16] Use credible_bandwidth uniformly in setting/using fast_bandwidth We were using credible_bandwidth to build the fast_bandwidth threshold, but comparing it to bandwidth_for_router. --- src/or/dirserv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index ed19406ba..11a24235c 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1830,7 +1830,7 @@ dirserv_thinks_router_is_unreliable(time_t now, } } if (need_capacity) { - uint32_t bw = dirserv_get_bandwidth_for_router(router); + uint32_t bw = dirserv_get_credible_bandwidth(router); if (bw < fast_bandwidth) return 1; } From d028c005da8e61f705aff751b5236adf983b310a Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Thu, 11 Apr 2013 02:53:26 -0400 Subject: [PATCH 09/16] socks5 will ask for username/password if we offer it Commit a264c4fe made the socks5 server prefer auth 2 if it's offered, but it didn't update the unit test to expect it. --- src/test/test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/test.c b/src/test/test.c index 0b55f7bb7..42aab4cbc 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -362,7 +362,7 @@ test_socks_5_unsupported_commands(void *ptr) test_eq(5, socks->socks_version); test_eq(2, socks->replylen); test_eq(5, socks->reply[0]); - test_eq(0, socks->reply[1]); + test_eq(2, socks->reply[1]); ADD_DATA(buf, "\x05\x03\x00\x01\x02\x02\x02\x01\x01\x01"); test_eq(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks), -1); From 85e7de68bcfe6b137e0d4bb5fbfdc5f4a3db6a2d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Apr 2013 17:52:26 -0400 Subject: [PATCH 10/16] Better test program for 128-bit math support Clang 3.2 does constant-folding and variable substitution to determine that the program is equivalent to "return 1". Splitting the 128-bit math into a new function seems sufficient to fix this. --- configure.ac | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/configure.ac b/configure.ac index 8a79653e0..6f40ac4ad 100644 --- a/configure.ac +++ b/configure.ac @@ -663,32 +663,38 @@ if test x$enable_curve25519 != xno; then [AC_LANG_PROGRAM([dnl #include typedef unsigned uint128_t __attribute__((mode(TI))); - ], [dnl - uint64_t a = ((uint64_t)2000000000) * 1000000000; - uint64_t b = ((uint64_t)1234567890) << 24; - uint128_t c = ((uint128_t)a) * b; - int ok = ((uint64_t)(c>>96)) == 522859 && + int func(uint64_t a, uint64_t b) { + uint128_t c = ((uint128_t)a) * b; + int ok = ((uint64_t)(c>>96)) == 522859 && (((uint64_t)(c>>64))&0xffffffffL) == 3604448702L && (((uint64_t)(c>>32))&0xffffffffL) == 2351960064L && (((uint64_t)(c))&0xffffffffL) == 0; + return ok; + } + ], [dnl + int ok = func( ((uint64_t)2000000000) * 1000000000, + ((uint64_t)1234567890) << 24); return !ok; ])], [tor_cv_can_use_curve25519_donna_c64=yes], [tor_cv_can_use_curve25519_donna_c64=no], [AC_COMPILE_IFELSE( [AC_LANG_PROGRAM([dnl - #include - typedef unsigned uint128_t __attribute__((mode(TI))); - ], [dnl - uint64_t a = ((uint64_t)2000000000) * 1000000000; - uint64_t b = ((uint64_t)1234567890) << 24; - uint128_t c = ((uint128_t)a) * b; - int ok = ((uint64_t)(c>>96)) == 522859 && - (((uint64_t)(c>>64))&0xffffffffL) == 3604448702L && - (((uint64_t)(c>>32))&0xffffffffL) == 2351960064L && - (((uint64_t)(c))&0xffffffffL) == 0; - return !ok; - ])], + #include + typedef unsigned uint128_t __attribute__((mode(TI))); + int func(uint64_t a, uint64_t b) { + uint128_t c = ((uint128_t)a) * b; + int ok = ((uint64_t)(c>>96)) == 522859 && + (((uint64_t)(c>>64))&0xffffffffL) == 3604448702L && + (((uint64_t)(c>>32))&0xffffffffL) == 2351960064L && + (((uint64_t)(c))&0xffffffffL) == 0; + return ok; + } + ], [dnl + int ok = func( ((uint64_t)2000000000) * 1000000000, + ((uint64_t)1234567890) << 24); + return !ok; + ])], [tor_cv_can_use_curve25519_donna_c64=cross], [tor_cv_can_use_curve25519_donna_c64=no])])]) From 39ac1db60e8b920e1e6b07e08f7f3343960ece79 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 10 Apr 2013 19:30:41 -0400 Subject: [PATCH 11/16] Avoid busy-looping on WANTREAD within connection_handle_write Fix for bug 5650. Also, if we get a WANTREAD while reading while writing, make sure we're reading. --- changes/bug5650 | 5 +++++ src/or/connection.c | 17 ++++++++++++++++- src/or/or.h | 3 +++ 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 changes/bug5650 diff --git a/changes/bug5650 b/changes/bug5650 new file mode 100644 index 000000000..401e31707 --- /dev/null +++ b/changes/bug5650 @@ -0,0 +1,5 @@ + o Major bugfixes: + - Avoid a bug where our response to TLS renegotation under certain + network conditions could lead to a busy-loop, with 100% CPU + consumption. Fixes bug 5650; bugfix on 0.2.0.16-alpha. + diff --git a/src/or/connection.c b/src/or/connection.c index eac9c4f32..e366e0e77 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -2825,7 +2825,20 @@ connection_read_to_buf(connection_t *conn, ssize_t *max_to_read, case TOR_TLS_WANTWRITE: connection_start_writing(conn); return 0; - case TOR_TLS_WANTREAD: /* we're already reading */ + case TOR_TLS_WANTREAD: + if (conn->in_connection_handle_write) { + /* We've been invoked from connection_handle_write, because we're + * waiting for a TLS renegotiation, the renegotiation started, and + * SSL_read returned WANTWRITE. But now SSL_read is saying WANTREAD + * again. Stop waiting for write events now, or else we'll + * busy-loop until data arrives for us to read. */ + connection_stop_writing(conn); + if (!connection_is_reading(conn)) + connection_start_reading(conn); + } + /* we're already reading, one hopes */ + result = 0; + break; case TOR_TLS_DONE: /* no data read, so nothing to process */ result = 0; break; /* so we call bucket_decrement below */ @@ -3337,7 +3350,9 @@ connection_handle_write(connection_t *conn, int force) { int res; tor_gettimeofday_cache_clear(); + conn->in_connection_handle_write = 1; res = connection_handle_write_impl(conn, force); + conn->in_connection_handle_write = 0; return res; } diff --git a/src/or/or.h b/src/or/or.h index 51c23d305..6728b862b 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -998,6 +998,9 @@ typedef struct connection_t { /** Set to 1 when we're inside connection_flushed_some to keep us from * calling connection_handle_write() recursively. */ unsigned int in_flushed_some:1; + /** True if connection_handle_write is currently running on this connection. + */ + unsigned int in_connection_handle_write:1; /* For linked connections: */ From 8aded5b07c14824b6fcbd7583af9ed4f4d8466d3 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 13 Apr 2013 18:27:08 -0400 Subject: [PATCH 12/16] Manpage: refer to ExcludeExitNodes, not the nonexistent ExcludeEntryNodes Spotted on tor-talk by "hamahangi". --- changes/fix-geoipexclude-doc | 4 ++++ doc/tor.1.txt | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 changes/fix-geoipexclude-doc diff --git a/changes/fix-geoipexclude-doc b/changes/fix-geoipexclude-doc new file mode 100644 index 000000000..63b544ef2 --- /dev/null +++ b/changes/fix-geoipexclude-doc @@ -0,0 +1,4 @@ + o Documentation fixes: + - Fix the GeoIPExcludeUnknown documentation to refer to ExcludeExitNodes + rather than the currently nonexistent ExcludeEntryNodes. Spotted by + "hamahangi" on tor-talk. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index f35d63958..adcf6b718 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -695,10 +695,10 @@ The following options are useful only for clients (that is, if **GeoIPExcludeUnknown** **0**|**1**|**auto**:: If this option is set to 'auto', then whenever any country code is set in - ExcludeNodes or ExcludeEntryNodes, all nodes with unknown country (\{??} and + ExcludeNodes or ExcludeExitNodes, all nodes with unknown country (\{??} and possibly \{A1}) are treated as excluded as well. If this option is set to '1', then all unknown countries are treated as excluded in ExcludeNodes - and ExcludeEntryNodes. This option has no effect when a GeoIP file isn't + and ExcludeExitNodes. This option has no effect when a GeoIP file isn't configured or can't be found. (Default: auto) **ExitNodes** __node__,__node__,__...__:: From 49696786fb1ffee28ea17f624591311b075f2842 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Apr 2013 10:36:53 -0400 Subject: [PATCH 13/16] Fix some KB/B confusion in flag threshold minima. --- src/or/dirserv.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 11a24235c..80dc15bf5 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1879,7 +1879,7 @@ dirserv_thinks_router_is_hs_dir(const routerinfo_t *router, /** Don't consider routers with less bandwidth than this when computing * thresholds. */ -#define ABSOLUTE_MIN_BW_VALUE_TO_CONSIDER 4096 +#define ABSOLUTE_MIN_BW_VALUE_TO_CONSIDER 4 /** Helper for dirserv_compute_performance_thresholds(): Decide whether to * include a router in our calculations, and return true iff we should; the @@ -1994,7 +1994,7 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, /* The 12.5th percentile bandwidth is fast. */ fast_bandwidth = find_nth_uint32(bandwidths, n_active, n_active/8); /* (Now bandwidths is sorted.) */ - if (fast_bandwidth < ROUTER_REQUIRED_MIN_BANDWIDTH/2) + if (fast_bandwidth < ROUTER_REQUIRED_MIN_BANDWIDTH/(2 * 1000)) fast_bandwidth = bandwidths[n_active/4]; guard_bandwidth_including_exits = bandwidths[(n_active-1)/2]; guard_tk = find_nth_long(tks, n_active, n_active/8); @@ -2005,14 +2005,14 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, { /* We can vote on a parameter for the minimum and maximum. */ -#define ABSOLUTE_MIN_VALUE_FOR_FAST_FLAG 4096 +#define ABSOLUTE_MIN_VALUE_FOR_FAST_FLAG 4 int32_t min_fast, max_fast; min_fast = networkstatus_get_param(NULL, "FastFlagMinThreshold", ABSOLUTE_MIN_VALUE_FOR_FAST_FLAG, ABSOLUTE_MIN_VALUE_FOR_FAST_FLAG, INT32_MAX); if (options->TestingTorNetwork) { - min_fast = (int32_t)options->TestingMinFastFlagThreshold; + min_fast = (int32_t)options->TestingMinFastFlagThreshold/1000; } max_fast = networkstatus_get_param(NULL, "FastFlagMaxThreshold", INT32_MAX, min_fast, INT32_MAX); @@ -2024,8 +2024,8 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, /* Protect sufficiently fast nodes from being pushed out of the set * of Fast nodes. */ if (options->AuthDirFastGuarantee && - fast_bandwidth > options->AuthDirFastGuarantee) - fast_bandwidth = (uint32_t)options->AuthDirFastGuarantee; + fast_bandwidth > options->AuthDirFastGuarantee/1000) + fast_bandwidth = (uint32_t)options->AuthDirFastGuarantee/1000; /* Now that we have a time-known that 7/8 routers are known longer than, * fill wfus with the wfu of every such "familiar" router. */ @@ -2056,9 +2056,10 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, log_info(LD_DIRSERV, "Cutoffs: For Stable, %lu sec uptime, %lu sec MTBF. " - "For Fast: %lu bytes/sec. " + "For Fast: %lu kilobytes/sec. " "For Guard: WFU %.03f%%, time-known %lu sec, " - "and bandwidth %lu or %lu bytes/sec. We%s have enough stability data.", + "and bandwidth %lu or %lu kilobytes/sec. " + "We%s have enough stability data.", (unsigned long)stable_uptime, (unsigned long)stable_mtbf, (unsigned long)fast_bandwidth, @@ -2723,7 +2724,7 @@ set_routerstatus_from_routerinfo(routerstatus_t *rs, if (node->is_fast && ((options->AuthDirGuardBWGuarantee && - routerbw >= options->AuthDirGuardBWGuarantee) || + routerbw >= options->AuthDirGuardBWGuarantee/1000) || routerbw >= MIN(guard_bandwidth_including_exits, guard_bandwidth_excluding_exits)) && is_router_version_good_for_possible_guard(ri->platform)) { From 52cadff0d60e393d7b75001aee304cdc1ab79c6b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Apr 2013 11:43:40 -0400 Subject: [PATCH 14/16] Rename all fields which measure bw in kb to end with _kb --- src/or/dirserv.c | 179 ++++++++++++++++++++++--------------------- src/or/dirserv.h | 4 +- src/or/dirvote.c | 42 +++++----- src/or/dirvote.h | 2 +- src/or/or.h | 6 +- src/or/routerlist.c | 4 +- src/or/routerparse.c | 35 +++++---- src/test/test_dir.c | 104 ++++++++++++------------- 8 files changed, 191 insertions(+), 185 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 80dc15bf5..bf4750645 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -92,8 +92,8 @@ static const signed_descriptor_t *get_signed_descriptor_by_fp( time_t publish_cutoff); static was_router_added_t dirserv_add_extrainfo(extrainfo_t *ei, const char **msg); -static uint32_t dirserv_get_bandwidth_for_router(const routerinfo_t *ri); -static uint32_t dirserv_get_credible_bandwidth(const routerinfo_t *ri); +static uint32_t dirserv_get_bandwidth_for_router_kb(const routerinfo_t *ri); +static uint32_t dirserv_get_credible_bandwidth_kb(const routerinfo_t *ri); /************** Fingerprint handling code ************/ @@ -1776,17 +1776,17 @@ static double guard_wfu = 0.0; * many seconds. */ static long guard_tk = 0; /** Any router with a bandwidth at least this high is "Fast" */ -static uint32_t fast_bandwidth = 0; +static uint32_t fast_bandwidth_kb = 0; /** If exits can be guards, then all guards must have a bandwidth this * high. */ -static uint32_t guard_bandwidth_including_exits = 0; +static uint32_t guard_bandwidth_including_exits_kb = 0; /** If exits can't be guards, then all guards must have a bandwidth this * high. */ -static uint32_t guard_bandwidth_excluding_exits = 0; +static uint32_t guard_bandwidth_excluding_exits_kb = 0; /** Total bandwidth of all the routers we're considering. */ -static uint64_t total_bandwidth = 0; +static uint64_t total_bandwidth_kb = 0; /** Total bandwidth of all the exit routers we're considering. */ -static uint64_t total_exit_bandwidth = 0; +static uint64_t total_exit_bandwidth_kb = 0; /** Helper: estimate the uptime of a router given its stated uptime and the * amount of time since it last stated its stated uptime. */ @@ -1830,8 +1830,8 @@ dirserv_thinks_router_is_unreliable(time_t now, } } if (need_capacity) { - uint32_t bw = dirserv_get_credible_bandwidth(router); - if (bw < fast_bandwidth) + uint32_t bw_kb = dirserv_get_credible_bandwidth_kb(router); + if (bw_kb < fast_bandwidth_kb) return 1; } return 0; @@ -1879,7 +1879,7 @@ dirserv_thinks_router_is_hs_dir(const routerinfo_t *router, /** Don't consider routers with less bandwidth than this when computing * thresholds. */ -#define ABSOLUTE_MIN_BW_VALUE_TO_CONSIDER 4 +#define ABSOLUTE_MIN_BW_VALUE_TO_CONSIDER_KB 4 /** Helper for dirserv_compute_performance_thresholds(): Decide whether to * include a router in our calculations, and return true iff we should; the @@ -1894,16 +1894,16 @@ router_counts_toward_thresholds(const node_t *node, time_t now, /* Have measured bw? */ int have_mbw = dirserv_has_measured_bw(node->ri->cache_info.identity_digest); - uint64_t min_bw = ABSOLUTE_MIN_BW_VALUE_TO_CONSIDER; + uint64_t min_bw_kb = ABSOLUTE_MIN_BW_VALUE_TO_CONSIDER_KB; const or_options_t *options = get_options(); if (options->TestingTorNetwork) { - min_bw = (int64_t)options->TestingMinExitFlagThreshold; + min_bw_kb = (int64_t)options->TestingMinExitFlagThreshold / 1000; } return node->ri && router_is_active(node->ri, node, now) && !digestmap_get(omit_as_sybil, node->ri->cache_info.identity_digest) && - (dirserv_get_credible_bandwidth(node->ri) >= min_bw) && + (dirserv_get_credible_bandwidth_kb(node->ri) >= min_bw_kb) && (have_mbw || !require_mbw); } @@ -1920,7 +1920,7 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, digestmap_t *omit_as_sybil) { int n_active, n_active_nonexit, n_familiar; - uint32_t *uptimes, *bandwidths, *bandwidths_excluding_exits; + uint32_t *uptimes, *bandwidths_kb, *bandwidths_excluding_exits_kb; long *tks; double *mtbfs, *wfus; time_t now = time(NULL); @@ -1934,13 +1934,13 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, /* initialize these all here, in case there are no routers */ stable_uptime = 0; stable_mtbf = 0; - fast_bandwidth = 0; - guard_bandwidth_including_exits = 0; - guard_bandwidth_excluding_exits = 0; + fast_bandwidth_kb = 0; + guard_bandwidth_including_exits_kb = 0; + guard_bandwidth_excluding_exits_kb = 0; guard_tk = 0; guard_wfu = 0; - total_bandwidth = 0; - total_exit_bandwidth = 0; + total_bandwidth_kb = 0; + total_exit_bandwidth_kb = 0; /* Initialize arrays that will hold values for each router. We'll * sort them and use that to compute thresholds. */ @@ -1948,9 +1948,9 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, /* Uptime for every active router. */ uptimes = tor_malloc(sizeof(uint32_t)*smartlist_len(rl->routers)); /* Bandwidth for every active router. */ - bandwidths = tor_malloc(sizeof(uint32_t)*smartlist_len(rl->routers)); + bandwidths_kb = tor_malloc(sizeof(uint32_t)*smartlist_len(rl->routers)); /* Bandwidth for every active non-exit router. */ - bandwidths_excluding_exits = + bandwidths_excluding_exits_kb = tor_malloc(sizeof(uint32_t)*smartlist_len(rl->routers)); /* Weighted mean time between failure for each active router. */ mtbfs = tor_malloc(sizeof(double)*smartlist_len(rl->routers)); @@ -1967,18 +1967,18 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, require_mbw)) { routerinfo_t *ri = node->ri; const char *id = ri->cache_info.identity_digest; - uint32_t bw; + uint32_t bw_kb; node->is_exit = (!router_exit_policy_rejects_all(ri) && exit_policy_is_general_exit(ri->exit_policy)); uptimes[n_active] = (uint32_t)real_uptime(ri, now); mtbfs[n_active] = rep_hist_get_stability(id, now); tks [n_active] = rep_hist_get_weighted_time_known(id, now); - bandwidths[n_active] = bw = dirserv_get_credible_bandwidth(ri); - total_bandwidth += bw; + bandwidths_kb[n_active] = bw_kb = dirserv_get_credible_bandwidth_kb(ri); + total_bandwidth_kb += bw_kb; if (node->is_exit && !node->is_bad_exit) { - total_exit_bandwidth += bw; + total_exit_bandwidth_kb += bw_kb; } else { - bandwidths_excluding_exits[n_active_nonexit] = bw; + bandwidths_excluding_exits_kb[n_active_nonexit] = bw_kb; ++n_active_nonexit; } ++n_active; @@ -1992,11 +1992,11 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, /* The median mtbf is stable, if we have enough mtbf info */ stable_mtbf = median_double(mtbfs, n_active); /* The 12.5th percentile bandwidth is fast. */ - fast_bandwidth = find_nth_uint32(bandwidths, n_active, n_active/8); + fast_bandwidth_kb = find_nth_uint32(bandwidths_kb, n_active, n_active/8); /* (Now bandwidths is sorted.) */ - if (fast_bandwidth < ROUTER_REQUIRED_MIN_BANDWIDTH/(2 * 1000)) - fast_bandwidth = bandwidths[n_active/4]; - guard_bandwidth_including_exits = bandwidths[(n_active-1)/2]; + if (fast_bandwidth_kb < ROUTER_REQUIRED_MIN_BANDWIDTH/(2 * 1000)) + fast_bandwidth_kb = bandwidths_kb[n_active/4]; + guard_bandwidth_including_exits_kb = bandwidths_kb[(n_active-1)/2]; guard_tk = find_nth_long(tks, n_active, n_active/8); } @@ -2006,26 +2006,29 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, { /* We can vote on a parameter for the minimum and maximum. */ #define ABSOLUTE_MIN_VALUE_FOR_FAST_FLAG 4 - int32_t min_fast, max_fast; + int32_t min_fast_kb, max_fast_kb, min_fast, max_fast; min_fast = networkstatus_get_param(NULL, "FastFlagMinThreshold", ABSOLUTE_MIN_VALUE_FOR_FAST_FLAG, ABSOLUTE_MIN_VALUE_FOR_FAST_FLAG, INT32_MAX); if (options->TestingTorNetwork) { - min_fast = (int32_t)options->TestingMinFastFlagThreshold/1000; + min_fast = (int32_t)options->TestingMinFastFlagThreshold; } max_fast = networkstatus_get_param(NULL, "FastFlagMaxThreshold", INT32_MAX, min_fast, INT32_MAX); - if (fast_bandwidth < (uint32_t)min_fast) - fast_bandwidth = min_fast; - if (fast_bandwidth > (uint32_t)max_fast) - fast_bandwidth = max_fast; + min_fast_kb = min_fast / 1000; + max_fast_kb = max_fast / 1000; + + if (fast_bandwidth_kb < (uint32_t)min_fast_kb) + fast_bandwidth_kb = min_fast_kb; + if (fast_bandwidth_kb > (uint32_t)max_fast_kb) + fast_bandwidth_kb = max_fast_kb; } /* Protect sufficiently fast nodes from being pushed out of the set * of Fast nodes. */ if (options->AuthDirFastGuarantee && - fast_bandwidth > options->AuthDirFastGuarantee/1000) - fast_bandwidth = (uint32_t)options->AuthDirFastGuarantee/1000; + fast_bandwidth_kb > options->AuthDirFastGuarantee/1000) + fast_bandwidth_kb = (uint32_t)options->AuthDirFastGuarantee/1000; /* Now that we have a time-known that 7/8 routers are known longer than, * fill wfus with the wfu of every such "familiar" router. */ @@ -2050,8 +2053,8 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, enough_mtbf_info = rep_hist_have_measured_enough_stability(); if (n_active_nonexit) { - guard_bandwidth_excluding_exits = - median_uint32(bandwidths_excluding_exits, n_active_nonexit); + guard_bandwidth_excluding_exits_kb = + median_uint32(bandwidths_excluding_exits_kb, n_active_nonexit); } log_info(LD_DIRSERV, @@ -2062,24 +2065,24 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, "We%s have enough stability data.", (unsigned long)stable_uptime, (unsigned long)stable_mtbf, - (unsigned long)fast_bandwidth, + (unsigned long)fast_bandwidth_kb, guard_wfu*100, (unsigned long)guard_tk, - (unsigned long)guard_bandwidth_including_exits, - (unsigned long)guard_bandwidth_excluding_exits, + (unsigned long)guard_bandwidth_including_exits_kb, + (unsigned long)guard_bandwidth_excluding_exits_kb, enough_mtbf_info ? "" : " don't "); tor_free(uptimes); tor_free(mtbfs); - tor_free(bandwidths); - tor_free(bandwidths_excluding_exits); + tor_free(bandwidths_kb); + tor_free(bandwidths_excluding_exits_kb); tor_free(tks); tor_free(wfus); } /** Measured bandwidth cache entry */ typedef struct mbw_cache_entry_s { - long mbw; + long mbw_kb; time_t as_of; } mbw_cache_entry_t; @@ -2106,13 +2109,13 @@ dirserv_cache_measured_bw(const measured_bw_line_t *parsed_line, if (e) { /* Check that we really are newer, and update */ if (as_of > e->as_of) { - e->mbw = parsed_line->bw; + e->mbw_kb = parsed_line->bw_kb; e->as_of = as_of; } } else { /* We'll have to insert a new entry */ e = tor_malloc(sizeof(*e)); - e->mbw = parsed_line->bw; + e->mbw_kb = parsed_line->bw_kb; e->as_of = as_of; digestmap_set(mbw_cache, parsed_line->node_id, e); } @@ -2163,8 +2166,8 @@ dirserv_get_measured_bw_cache_size(void) * we found it. The bw_out and as_of_out pointers receive the cached * bandwidth value and the time it was cached if not NULL. */ int -dirserv_query_measured_bw_cache(const char *node_id, long *bw_out, - time_t *as_of_out) +dirserv_query_measured_bw_cache_kb(const char *node_id, long *bw_kb_out, + time_t *as_of_out) { mbw_cache_entry_t *v = NULL; int rv = 0; @@ -2174,7 +2177,7 @@ dirserv_query_measured_bw_cache(const char *node_id, long *bw_out, if (v) { /* Found something */ rv = 1; - if (bw_out) *bw_out = v->mbw; + if (bw_kb_out) *bw_kb_out = v->mbw_kb; if (as_of_out) *as_of_out = v->as_of; } } @@ -2186,22 +2189,22 @@ dirserv_query_measured_bw_cache(const char *node_id, long *bw_out, int dirserv_has_measured_bw(const char *node_id) { - return dirserv_query_measured_bw_cache(node_id, NULL, NULL); + return dirserv_query_measured_bw_cache_kb(node_id, NULL, NULL); } /** Get the best estimate of a router's bandwidth for dirauth purposes, * preferring measured to advertised values if available. */ static uint32_t -dirserv_get_bandwidth_for_router(const routerinfo_t *ri) +dirserv_get_bandwidth_for_router_kb(const routerinfo_t *ri) { - uint32_t bw = 0; + uint32_t bw_kb = 0; /* * Yeah, measured bandwidths in measured_bw_line_t are (implicitly * signed) longs and the ones router_get_advertised_bandwidth() returns * are uint32_t. */ - long mbw = 0; + long mbw_kb = 0; if (ri) { /* @@ -2209,17 +2212,17 @@ dirserv_get_bandwidth_for_router(const routerinfo_t *ri) * as_of_out here, on the theory that a stale measured bandwidth is still * better to trust than an advertised one. */ - if (dirserv_query_measured_bw_cache(ri->cache_info.identity_digest, - &mbw, NULL)) { + if (dirserv_query_measured_bw_cache_kb(ri->cache_info.identity_digest, + &mbw_kb, NULL)) { /* Got one! */ - bw = (uint32_t)mbw; + bw_kb = (uint32_t)mbw_kb; } else { /* If not, fall back to advertised */ - bw = router_get_advertised_bandwidth(ri) / 1000; + bw_kb = router_get_advertised_bandwidth(ri) / 1000; } } - return bw; + return bw_kb; } /** Look through the routerlist, and using the measured bandwidth cache count @@ -2248,30 +2251,30 @@ dirserv_count_measured_bws(routerlist_t *rl) * bandwidths, we don't want to ever give flags to unmeasured routers, so * return 0. */ static uint32_t -dirserv_get_credible_bandwidth(const routerinfo_t *ri) +dirserv_get_credible_bandwidth_kb(const routerinfo_t *ri) { int threshold; - uint32_t bw = 0; - long mbw; + uint32_t bw_kb = 0; + long mbw_kb; tor_assert(ri); /* Check if we have a measured bandwidth, and check the threshold if not */ - if (!(dirserv_query_measured_bw_cache(ri->cache_info.identity_digest, - &mbw, NULL))) { + if (!(dirserv_query_measured_bw_cache_kb(ri->cache_info.identity_digest, + &mbw_kb, NULL))) { threshold = get_options()->MinMeasuredBWsForAuthToIgnoreAdvertised; if (routers_with_measured_bw > threshold) { /* Return zero for unmeasured bandwidth if we are above threshold */ - bw = 0; + bw_kb = 0; } else { /* Return an advertised bandwidth otherwise */ - bw = router_get_advertised_bandwidth_capped(ri) / 1000; + bw_kb = router_get_advertised_bandwidth_capped(ri) / 1000; } } else { /* We have the measured bandwidth in mbw */ - bw = (uint32_t)mbw; + bw_kb = (uint32_t)mbw_kb; } - return bw; + return bw_kb; } /** Give a statement of our current performance thresholds for inclusion @@ -2288,11 +2291,11 @@ dirserv_get_flag_thresholds_line(void) "enough-mtbf=%d", (unsigned long)stable_uptime, (unsigned long)stable_mtbf, - (unsigned long)fast_bandwidth, + (unsigned long)fast_bandwidth_kb*1000, guard_wfu*100, (unsigned long)guard_tk, - (unsigned long)guard_bandwidth_including_exits, - (unsigned long)guard_bandwidth_excluding_exits, + (unsigned long)guard_bandwidth_including_exits_kb*1000, + (unsigned long)guard_bandwidth_excluding_exits_kb*1000, enough_mtbf_info ? 1 : 0); return result; @@ -2426,7 +2429,7 @@ routerstatus_format_entry(char *buf, size_t buf_len, if (format != NS_V2) { const routerinfo_t* desc = router_get_by_id_digest(rs->identity_digest); - uint32_t bw; + uint32_t bw_kb; if (format != NS_CONTROL_PORT) { /* Blow up more or less nicely if we didn't get anything or not the @@ -2471,13 +2474,13 @@ routerstatus_format_entry(char *buf, size_t buf_len, } if (format == NS_CONTROL_PORT && rs->has_bandwidth) { - bw = rs->bandwidth; + bw_kb = rs->bandwidth_kb; } else { tor_assert(desc); - bw = router_get_advertised_bandwidth_capped(desc) / 1000; + bw_kb = router_get_advertised_bandwidth_capped(desc) / 1000; } r = tor_snprintf(cp, buf_len - (cp-buf), - "w Bandwidth=%d\n", bw); + "w Bandwidth=%d\n", bw_kb); if (r<0) { log_warn(LD_BUG, "Not enough space in buffer."); @@ -2487,7 +2490,7 @@ routerstatus_format_entry(char *buf, size_t buf_len, if (format == NS_V3_VOTE && vrs && vrs->has_measured_bw) { *--cp = '\0'; /* Kill "\n" */ r = tor_snprintf(cp, buf_len - (cp-buf), - " Measured=%d\n", vrs->measured_bw); + " Measured=%d\n", vrs->measured_bw_kb); if (r<0) { log_warn(LD_BUG, "Not enough space in buffer for weight line."); return -1; @@ -2521,7 +2524,7 @@ compare_routerinfo_by_ip_and_bw_(const void **a, const void **b) { routerinfo_t *first = *(routerinfo_t **)a, *second = *(routerinfo_t **)b; int first_is_auth, second_is_auth; - uint32_t bw_first, bw_second; + uint32_t bw_kb_first, bw_kb_second; const node_t *node_first, *node_second; int first_is_running, second_is_running; @@ -2556,12 +2559,12 @@ compare_routerinfo_by_ip_and_bw_(const void **a, const void **b) else if (!first_is_running && second_is_running) return 1; - bw_first = dirserv_get_bandwidth_for_router(first); - bw_second = dirserv_get_bandwidth_for_router(second); + bw_kb_first = dirserv_get_bandwidth_for_router_kb(first); + bw_kb_second = dirserv_get_bandwidth_for_router_kb(second); - if (bw_first > bw_second) + if (bw_kb_first > bw_kb_second) return -1; - else if (bw_first < bw_second) + else if (bw_kb_first < bw_kb_second) return 1; /* They're equal! Compare by identity digest, so there's a @@ -2697,7 +2700,7 @@ set_routerstatus_from_routerinfo(routerstatus_t *rs, int listbaddirs, int vote_on_hsdirs) { const or_options_t *options = get_options(); - uint32_t routerbw = dirserv_get_credible_bandwidth(ri); + uint32_t routerbw_kb = dirserv_get_credible_bandwidth_kb(ri); memset(rs, 0, sizeof(routerstatus_t)); @@ -2724,9 +2727,9 @@ set_routerstatus_from_routerinfo(routerstatus_t *rs, if (node->is_fast && ((options->AuthDirGuardBWGuarantee && - routerbw >= options->AuthDirGuardBWGuarantee/1000) || - routerbw >= MIN(guard_bandwidth_including_exits, - guard_bandwidth_excluding_exits)) && + routerbw_kb >= options->AuthDirGuardBWGuarantee/1000) || + routerbw_kb >= MIN(guard_bandwidth_including_exits_kb, + guard_bandwidth_excluding_exits_kb)) && is_router_version_good_for_possible_guard(ri->platform)) { long tk = rep_hist_get_weighted_time_known( node->identity, now); @@ -2821,7 +2824,7 @@ measured_bw_line_parse(measured_bw_line_t *out, const char *orig_line) } cp+=strlen("bw="); - out->bw = tor_parse_long(cp, 0, 0, LONG_MAX, &parse_ok, &endptr); + out->bw_kb = tor_parse_long(cp, 0, 0, LONG_MAX, &parse_ok, &endptr); if (!parse_ok || (*endptr && !TOR_ISSPACE(*endptr))) { log_warn(LD_DIRSERV, "Invalid bandwidth in bandwidth file line: %s", escaped(orig_line)); @@ -2879,7 +2882,7 @@ measured_bw_line_apply(measured_bw_line_t *parsed_line, if (rs) { rs->has_measured_bw = 1; - rs->measured_bw = (uint32_t)parsed_line->bw; + rs->measured_bw_kb = (uint32_t)parsed_line->bw_kb; } else { log_info(LD_DIRSERV, "Node ID %s not found in routerstatus list", parsed_line->node_hex); diff --git a/src/or/dirserv.h b/src/or/dirserv.h index a84ae964c..d6eb4ab77 100644 --- a/src/or/dirserv.h +++ b/src/or/dirserv.h @@ -151,8 +151,8 @@ void dirserv_cache_measured_bw(const measured_bw_line_t *parsed_line, void dirserv_clear_measured_bw_cache(void); void dirserv_expire_measured_bw_cache(time_t now); int dirserv_get_measured_bw_cache_size(void); -int dirserv_query_measured_bw_cache(const char *node_id, long *bw_out, - time_t *as_of_out); +int dirserv_query_measured_bw_cache_kb(const char *node_id, long *bw_out, + time_t *as_of_out); int dirserv_has_measured_bw(const char *node_id); #endif diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 7043cef24..417721dc3 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -1388,7 +1388,7 @@ networkstatus_compute_consensus(smartlist_t *votes, char *client_versions = NULL, *server_versions = NULL; smartlist_t *flags; const char *flavor_name; - uint32_t max_unmeasured_bw = DEFAULT_MAX_UNMEASURED_BW; + uint32_t max_unmeasured_bw_kb = DEFAULT_MAX_UNMEASURED_BW_KB; int64_t G=0, M=0, E=0, D=0, T=0; /* For bandwidth weights */ const routerstatus_format_type_t rs_format = flavor == FLAV_NS ? NS_V3_CONSENSUS : NS_V3_CONSENSUS_MICRODESC; @@ -1600,12 +1600,12 @@ networkstatus_compute_consensus(smartlist_t *votes, int ok = 0; char *eq = strchr(max_unmeasured_param, '='); if (eq) { - max_unmeasured_bw = (uint32_t) + max_unmeasured_bw_kb = (uint32_t) tor_parse_ulong(eq+1, 10, 1, UINT32_MAX, &ok, NULL); if (!ok) { log_warn(LD_DIR, "Bad element '%s' in max unmeasured bw param", escaped(max_unmeasured_param)); - max_unmeasured_bw = DEFAULT_MAX_UNMEASURED_BW; + max_unmeasured_bw_kb = DEFAULT_MAX_UNMEASURED_BW_KB; } } } @@ -1622,9 +1622,10 @@ networkstatus_compute_consensus(smartlist_t *votes, smartlist_t *chosen_flags = smartlist_new(); smartlist_t *versions = smartlist_new(); smartlist_t *exitsummaries = smartlist_new(); - uint32_t *bandwidths = tor_malloc(sizeof(uint32_t) * smartlist_len(votes)); - uint32_t *measured_bws = tor_malloc(sizeof(uint32_t) * - smartlist_len(votes)); + uint32_t *bandwidths_kb = tor_malloc(sizeof(uint32_t) * + smartlist_len(votes)); + uint32_t *measured_bws_kb = tor_malloc(sizeof(uint32_t) * + smartlist_len(votes)); int num_bandwidths; int num_mbws; @@ -1804,10 +1805,10 @@ networkstatus_compute_consensus(smartlist_t *votes, /* count bandwidths */ if (rs->has_measured_bw) - measured_bws[num_mbws++] = rs->measured_bw; + measured_bws_kb[num_mbws++] = rs->measured_bw_kb; if (rs->status.has_bandwidth) - bandwidths[num_bandwidths++] = rs->status.bandwidth; + bandwidths_kb[num_bandwidths++] = rs->status.bandwidth_kb; } SMARTLIST_FOREACH_END(v); /* We don't include this router at all unless more than half of @@ -1898,16 +1899,16 @@ networkstatus_compute_consensus(smartlist_t *votes, if (consensus_method >= 6 && num_mbws > 2) { rs_out.has_bandwidth = 1; rs_out.bw_is_unmeasured = 0; - rs_out.bandwidth = median_uint32(measured_bws, num_mbws); + rs_out.bandwidth_kb = median_uint32(measured_bws_kb, num_mbws); } else if (consensus_method >= 5 && num_bandwidths > 0) { rs_out.has_bandwidth = 1; rs_out.bw_is_unmeasured = 1; - rs_out.bandwidth = median_uint32(bandwidths, num_bandwidths); + rs_out.bandwidth_kb = median_uint32(bandwidths_kb, num_bandwidths); if (consensus_method >= MIN_METHOD_TO_CLIP_UNMEASURED_BW && n_authorities_measuring_bandwidth > 2) { /* Cap non-measured bandwidths. */ - if (rs_out.bandwidth > max_unmeasured_bw) { - rs_out.bandwidth = max_unmeasured_bw; + if (rs_out.bandwidth_kb > max_unmeasured_bw_kb) { + rs_out.bandwidth_kb = max_unmeasured_bw_kb; } } } @@ -1919,15 +1920,15 @@ networkstatus_compute_consensus(smartlist_t *votes, if (consensus_method >= MIN_METHOD_FOR_BW_WEIGHTS) { if (rs_out.has_bandwidth) { - T += rs_out.bandwidth; + T += rs_out.bandwidth_kb; if (is_exit && is_guard) - D += rs_out.bandwidth; + D += rs_out.bandwidth_kb; else if (is_exit) - E += rs_out.bandwidth; + E += rs_out.bandwidth_kb; else if (is_guard) - G += rs_out.bandwidth; + G += rs_out.bandwidth_kb; else - M += rs_out.bandwidth; + M += rs_out.bandwidth_kb; } else { log_warn(LD_BUG, "Missing consensus bandwidth for router %s", rs_out.nickname); @@ -2053,7 +2054,8 @@ networkstatus_compute_consensus(smartlist_t *votes, if (rs_out.has_bandwidth) { int unmeasured = rs_out.bw_is_unmeasured && consensus_method >= MIN_METHOD_TO_CLIP_UNMEASURED_BW; - smartlist_add_asprintf(chunks, "w Bandwidth=%d%s\n", rs_out.bandwidth, + smartlist_add_asprintf(chunks, "w Bandwidth=%d%s\n", + rs_out.bandwidth_kb, unmeasured?" Unmeasured=1":""); } @@ -2080,8 +2082,8 @@ networkstatus_compute_consensus(smartlist_t *votes, smartlist_free(chosen_flags); smartlist_free(versions); smartlist_free(exitsummaries); - tor_free(bandwidths); - tor_free(measured_bws); + tor_free(bandwidths_kb); + tor_free(measured_bws_kb); } if (consensus_method >= MIN_METHOD_FOR_FOOTER) { diff --git a/src/or/dirvote.h b/src/or/dirvote.h index 8d036d6c1..b23645212 100644 --- a/src/or/dirvote.h +++ b/src/or/dirvote.h @@ -61,7 +61,7 @@ /** Default bandwidth to clip unmeasured bandwidths to using method >= * MIN_METHOD_TO_CLIP_UNMEASURED_BW */ -#define DEFAULT_MAX_UNMEASURED_BW 20 +#define DEFAULT_MAX_UNMEASURED_BW_KB 20 void dirvote_free_all(void); diff --git a/src/or/or.h b/src/or/or.h index ece2bc7b1..4a9534727 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2096,7 +2096,7 @@ typedef struct routerstatus_t { unsigned int bw_is_unmeasured:1; /**< This is a consensus entry, with * the Unmeasured flag set. */ - uint32_t bandwidth; /**< Bandwidth (capacity) of the router as reported in + uint32_t bandwidth_kb; /**< Bandwidth (capacity) of the router as reported in * the vote/consensus, in kilobytes/sec. */ char *exitsummary; /**< exit policy summary - * XXX weasel: this probably should not stay a string. */ @@ -2342,7 +2342,7 @@ typedef struct vote_routerstatus_t { char *version; /**< The version that the authority says this router is * running. */ unsigned int has_measured_bw:1; /**< The vote had a measured bw */ - uint32_t measured_bw; /**< Measured bandwidth (capacity) of the router */ + uint32_t measured_bw_kb; /**< Measured bandwidth (capacity) of the router */ /** The hash or hashes that the authority claims this microdesc has. */ vote_microdesc_hash_t *microdesc; } vote_routerstatus_t; @@ -4478,7 +4478,7 @@ typedef enum { typedef struct measured_bw_line_t { char node_id[DIGEST_LEN]; char node_hex[MAX_HEX_NICKNAME_LEN+1]; - long int bw; + long int bw_kb; } measured_bw_line_t; #endif diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 0c978e9d0..6ed168e55 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1793,7 +1793,7 @@ compute_weighted_bandwidths(const smartlist_t *sl, "old router selection algorithm."); return -1; } - this_bw = kb_to_bytes(node->rs->bandwidth); + this_bw = kb_to_bytes(node->rs->bandwidth_kb); } else if (node->ri) { /* bridge or other descriptor not in our consensus */ this_bw = bridge_get_advertised_bandwidth_bounded(node->ri); @@ -1944,7 +1944,7 @@ smartlist_choose_node_by_bandwidth(const smartlist_t *sl, is_guard = node->is_possible_guard; if (node->rs) { if (node->rs->has_bandwidth) { - this_bw = kb_to_bytes(node->rs->bandwidth); + this_bw = kb_to_bytes(node->rs->bandwidth_kb); } else { /* guess */ is_known = 0; } diff --git a/src/or/routerparse.c b/src/or/routerparse.c index f8edb8407..0eadcc90f 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -1966,9 +1966,10 @@ routerstatus_parse_entry_from_string(memarea_t *area, for (i=0; i < tok->n_args; ++i) { if (!strcmpstart(tok->args[i], "Bandwidth=")) { int ok; - rs->bandwidth = (uint32_t)tor_parse_ulong(strchr(tok->args[i], '=')+1, - 10, 0, UINT32_MAX, - &ok, NULL); + rs->bandwidth_kb = + (uint32_t)tor_parse_ulong(strchr(tok->args[i], '=')+1, + 10, 0, UINT32_MAX, + &ok, NULL); if (!ok) { log_warn(LD_DIR, "Invalid Bandwidth %s", escaped(tok->args[i])); goto err; @@ -1976,7 +1977,7 @@ routerstatus_parse_entry_from_string(memarea_t *area, rs->has_bandwidth = 1; } else if (!strcmpstart(tok->args[i], "Measured=") && vote_rs) { int ok; - vote_rs->measured_bw = + vote_rs->measured_bw_kb = (uint32_t)tor_parse_ulong(strchr(tok->args[i], '=')+1, 10, 0, UINT32_MAX, &ok, NULL); if (!ok) { @@ -2351,23 +2352,23 @@ networkstatus_verify_bw_weights(networkstatus_t *ns, int consensus_method) is_exit = rs->is_exit; } if (rs->has_bandwidth) { - T += rs->bandwidth; + T += rs->bandwidth_kb; if (is_exit && rs->is_possible_guard) { - D += rs->bandwidth; - Gtotal += Wgd*rs->bandwidth; - Mtotal += Wmd*rs->bandwidth; - Etotal += Wed*rs->bandwidth; + D += rs->bandwidth_kb; + Gtotal += Wgd*rs->bandwidth_kb; + Mtotal += Wmd*rs->bandwidth_kb; + Etotal += Wed*rs->bandwidth_kb; } else if (is_exit) { - E += rs->bandwidth; - Mtotal += Wme*rs->bandwidth; - Etotal += Wee*rs->bandwidth; + E += rs->bandwidth_kb; + Mtotal += Wme*rs->bandwidth_kb; + Etotal += Wee*rs->bandwidth_kb; } else if (rs->is_possible_guard) { - G += rs->bandwidth; - Gtotal += Wgg*rs->bandwidth; - Mtotal += Wmg*rs->bandwidth; + G += rs->bandwidth_kb; + Gtotal += Wgg*rs->bandwidth_kb; + Mtotal += Wmg*rs->bandwidth_kb; } else { - M += rs->bandwidth; - Mtotal += Wmm*rs->bandwidth; + M += rs->bandwidth_kb; + Mtotal += Wmm*rs->bandwidth_kb; } } else { log_warn(LD_BUG, "Missing consensus bandwidth for router %s", diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 606dfe51a..849bb9e71 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -508,7 +508,7 @@ test_dir_split_fps(void *testdata) } static void -test_dir_measured_bw(void) +test_dir_measured_bw_kb(void) { measured_bw_line_t mbwl; int i; @@ -564,7 +564,7 @@ test_dir_measured_bw(void) for (i = 0; strcmp(lines_pass[i], "end"); i++) { //fprintf(stderr, "Testing: %s %d\n", lines_pass[i], TOR_ISSPACE('\n')); test_assert(measured_bw_line_parse(&mbwl, lines_pass[i]) == 0); - test_assert(mbwl.bw == 1024); + test_assert(mbwl.bw_kb == 1024); test_assert(strcmp(mbwl.node_hex, "557365204145532d32353620696e73746561642e") == 0); } @@ -577,7 +577,7 @@ test_dir_measured_bw(void) /** Do the measured bandwidth cache unit test */ static void -test_dir_measured_bw_cache(void) +test_dir_measured_bw_kb_cache(void) { /* Initial fake time_t for testing */ time_t curr = MBWC_INIT_TIME; @@ -595,23 +595,23 @@ test_dir_measured_bw_cache(void) * the node_hex field. */ memset(mbwl[0].node_id, 0x01, DIGEST_LEN); - mbwl[0].bw = 20; + mbwl[0].bw_kb = 20; memset(mbwl[1].node_id, 0x02, DIGEST_LEN); - mbwl[1].bw = 40; + mbwl[1].bw_kb = 40; memset(mbwl[2].node_id, 0x03, DIGEST_LEN); - mbwl[2].bw = 80; + mbwl[2].bw_kb = 80; /* Try caching something */ dirserv_cache_measured_bw(&(mbwl[0]), curr); test_eq(dirserv_get_measured_bw_cache_size(), 1); /* Okay, let's see if we can retrieve it */ - test_assert(dirserv_query_measured_bw_cache(mbwl[0].node_id, &bw, &as_of)); + test_assert(dirserv_query_measured_bw_cache_kb(mbwl[0].node_id,&bw, &as_of)); test_eq(bw, 20); test_eq(as_of, MBWC_INIT_TIME); /* Try retrieving it without some outputs */ - test_assert(dirserv_query_measured_bw_cache(mbwl[0].node_id, NULL, NULL)); - test_assert(dirserv_query_measured_bw_cache(mbwl[0].node_id, &bw, NULL)); + test_assert(dirserv_query_measured_bw_cache_kb(mbwl[0].node_id,NULL, NULL)); + test_assert(dirserv_query_measured_bw_cache_kb(mbwl[0].node_id,&bw, NULL)); test_eq(bw, 20); - test_assert(dirserv_query_measured_bw_cache(mbwl[0].node_id, NULL, &as_of)); + test_assert(dirserv_query_measured_bw_cache_kb(mbwl[0].node_id,NULL, &as_of)); test_eq(as_of, MBWC_INIT_TIME); /* Now expire it */ curr += MAX_MEASUREMENT_AGE + 1; @@ -619,7 +619,7 @@ test_dir_measured_bw_cache(void) /* Check that the cache is empty */ test_eq(dirserv_get_measured_bw_cache_size(), 0); /* Check that we can't retrieve it */ - test_assert(!dirserv_query_measured_bw_cache(mbwl[0].node_id, NULL, NULL)); + test_assert(!dirserv_query_measured_bw_cache_kb(mbwl[0].node_id, NULL, NULL)); /* Try caching a few things now */ dirserv_cache_measured_bw(&(mbwl[0]), curr); test_eq(dirserv_get_measured_bw_cache_size(), 1); @@ -829,8 +829,8 @@ generate_ri_from_rs(const vote_routerstatus_t *vrs) * router_get_advertised_bandwidth_capped() of routerlist.c and * routerstatus_format_entry() of dirserv.c. */ - r->bandwidthrate = rs->bandwidth * 1000; - r->bandwidthcapacity = rs->bandwidth * 1000; + r->bandwidthrate = rs->bandwidth_kb * 1000; + r->bandwidthcapacity = rs->bandwidth_kb * 1000; } return r; } @@ -952,7 +952,7 @@ vote_tweaks_for_v3ns(networkstatus_t *v, int voter, time_t now) if (voter == 1) { measured_bw_line_t mbw; memset(mbw.node_id, 33, sizeof(mbw.node_id)); - mbw.bw = 1024; + mbw.bw_kb = 1024; test_assert(measured_bw_line_apply(&mbw, v->routerstatus_list) == 1); } else if (voter == 2 || voter == 3) { @@ -1048,7 +1048,7 @@ test_vrs_for_v3ns(vote_routerstatus_t *vrs, int voter, time_t now) (voter == 1 || voter == 2)) { /* Check the measured bandwidth bits */ test_assert(vrs->has_measured_bw && - vrs->measured_bw == 1024); + vrs->measured_bw_kb == 1024); } else { /* * Didn't expect this, but the old unit test only checked some of them, @@ -1779,12 +1779,12 @@ test_dir_random_weighted(void *testdata) ; } -/* Function pointers for test_dir_clip_unmeasured_bw() */ +/* Function pointers for test_dir_clip_unmeasured_bw_kb() */ static uint32_t alternate_clip_bw = 0; /** - * Generate a routerstatus for clip_unmeasured_bw test; based on the + * Generate a routerstatus for clip_unmeasured_bw_kb test; based on the * v3_networkstatus ones. */ static vote_routerstatus_t * @@ -1793,8 +1793,8 @@ gen_routerstatus_for_umbw(int idx, time_t now) vote_routerstatus_t *vrs = NULL; routerstatus_t *rs; tor_addr_t addr_ipv6; - uint32_t max_unmeasured_bw = (alternate_clip_bw > 0) ? - alternate_clip_bw : DEFAULT_MAX_UNMEASURED_BW; + uint32_t max_unmeasured_bw_kb = (alternate_clip_bw > 0) ? + alternate_clip_bw : DEFAULT_MAX_UNMEASURED_BW_KB; switch (idx) { case 0: @@ -1818,7 +1818,7 @@ gen_routerstatus_for_umbw(int idx, time_t now) */ vrs->has_measured_bw = 1; rs->has_bandwidth = 1; - vrs->measured_bw = rs->bandwidth = max_unmeasured_bw / 2; + vrs->measured_bw_kb = rs->bandwidth_kb = max_unmeasured_bw_kb / 2; break; case 1: /* Generate the second routerstatus. */ @@ -1844,7 +1844,7 @@ gen_routerstatus_for_umbw(int idx, time_t now) */ vrs->has_measured_bw = 1; rs->has_bandwidth = 1; - vrs->measured_bw = rs->bandwidth = 2 * max_unmeasured_bw; + vrs->measured_bw_kb = rs->bandwidth_kb = 2 * max_unmeasured_bw_kb; break; case 2: /* Generate the third routerstatus. */ @@ -1868,8 +1868,8 @@ gen_routerstatus_for_umbw(int idx, time_t now) */ vrs->has_measured_bw = 0; rs->has_bandwidth = 1; - vrs->measured_bw = 0; - rs->bandwidth = 2 * max_unmeasured_bw; + vrs->measured_bw_kb = 0; + rs->bandwidth_kb = 2 * max_unmeasured_bw_kb; break; case 3: /* Generate a fourth routerstatus that is not running. */ @@ -1892,8 +1892,8 @@ gen_routerstatus_for_umbw(int idx, time_t now) */ vrs->has_measured_bw = 0; rs->has_bandwidth = 1; - vrs->measured_bw = 0; - rs->bandwidth = max_unmeasured_bw / 2; + vrs->measured_bw_kb = 0; + rs->bandwidth_kb = max_unmeasured_bw_kb / 2; break; case 4: /* No more for this test; return NULL */ @@ -1922,7 +1922,7 @@ vote_tweaks_for_umbw(networkstatus_t *v, int voter, time_t now) test_assert(v->supported_methods); smartlist_clear(v->supported_methods); - /* Method 17 is MIN_METHOD_TO_CLIP_UNMEASURED_BW */ + /* Method 17 is MIN_METHOD_TO_CLIP_UNMEASURED_BW_KB */ smartlist_split_string(v->supported_methods, "1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17", NULL, 0, -1); @@ -1948,8 +1948,8 @@ test_vrs_for_umbw(vote_routerstatus_t *vrs, int voter, time_t now) { routerstatus_t *rs; tor_addr_t addr_ipv6; - uint32_t max_unmeasured_bw = (alternate_clip_bw > 0) ? - alternate_clip_bw : DEFAULT_MAX_UNMEASURED_BW; + uint32_t max_unmeasured_bw_kb = (alternate_clip_bw > 0) ? + alternate_clip_bw : DEFAULT_MAX_UNMEASURED_BW_KB; (void)voter; test_assert(vrs); @@ -1978,8 +1978,8 @@ test_vrs_for_umbw(vote_routerstatus_t *vrs, int voter, time_t now) test_eq(rs->dir_port, 8000); test_assert(rs->has_bandwidth); test_assert(vrs->has_measured_bw); - test_eq(rs->bandwidth, max_unmeasured_bw / 2); - test_eq(vrs->measured_bw, max_unmeasured_bw / 2); + test_eq(rs->bandwidth_kb, max_unmeasured_bw_kb / 2); + test_eq(vrs->measured_bw_kb, max_unmeasured_bw_kb / 2); } else if (tor_memeq(rs->identity_digest, "\x5\x5\x5\x5\x5\x5\x5\x5\x5\x5" "\x5\x5\x5\x5\x5\x5\x5\x5\x5\x5", @@ -2005,8 +2005,8 @@ test_vrs_for_umbw(vote_routerstatus_t *vrs, int voter, time_t now) test_eq(rs->ipv6_orport, 4711); test_assert(rs->has_bandwidth); test_assert(vrs->has_measured_bw); - test_eq(rs->bandwidth, max_unmeasured_bw * 2); - test_eq(vrs->measured_bw, max_unmeasured_bw * 2); + test_eq(rs->bandwidth_kb, max_unmeasured_bw_kb * 2); + test_eq(vrs->measured_bw_kb, max_unmeasured_bw_kb * 2); } else if (tor_memeq(rs->identity_digest, "\x33\x33\x33\x33\x33\x33\x33\x33\x33\x33" "\x33\x33\x33\x33\x33\x33\x33\x33\x33\x33", @@ -2018,8 +2018,8 @@ test_vrs_for_umbw(vote_routerstatus_t *vrs, int voter, time_t now) */ test_assert(rs->has_bandwidth); test_assert(!(vrs->has_measured_bw)); - test_eq(rs->bandwidth, max_unmeasured_bw * 2); - test_eq(vrs->measured_bw, 0); + test_eq(rs->bandwidth_kb, max_unmeasured_bw_kb * 2); + test_eq(vrs->measured_bw_kb, 0); } else if (tor_memeq(rs->identity_digest, "\x34\x34\x34\x34\x34\x34\x34\x34\x34\x34" "\x34\x34\x34\x34\x34\x34\x34\x34\x34\x34", @@ -2030,8 +2030,8 @@ test_vrs_for_umbw(vote_routerstatus_t *vrs, int voter, time_t now) */ test_assert(rs->has_bandwidth); test_assert(!(vrs->has_measured_bw)); - test_eq(rs->bandwidth, max_unmeasured_bw / 2); - test_eq(vrs->measured_bw, 0); + test_eq(rs->bandwidth_kb, max_unmeasured_bw_kb / 2); + test_eq(vrs->measured_bw_kb, 0); } else { test_assert(0); } @@ -2050,7 +2050,7 @@ test_consensus_for_umbw(networkstatus_t *con, time_t now) test_assert(con); test_assert(!con->cert); - /* test_assert(con->consensus_method >= MIN_METHOD_TO_CLIP_UNMEASURED_BW); */ + /* test_assert(con->consensus_method >= MIN_METHOD_TO_CLIP_UNMEASURED_BW_KB); */ test_assert(con->consensus_method >= 16); test_eq(4, smartlist_len(con->routerstatus_list)); /* There should be four listed routers; all voters saw the same in this */ @@ -2066,8 +2066,8 @@ static void test_routerstatus_for_umbw(routerstatus_t *rs, time_t now) { tor_addr_t addr_ipv6; - uint32_t max_unmeasured_bw = (alternate_clip_bw > 0) ? - alternate_clip_bw : DEFAULT_MAX_UNMEASURED_BW; + uint32_t max_unmeasured_bw_kb = (alternate_clip_bw > 0) ? + alternate_clip_bw : DEFAULT_MAX_UNMEASURED_BW_KB; test_assert(rs); @@ -2093,7 +2093,7 @@ test_routerstatus_for_umbw(routerstatus_t *rs, time_t now) test_assert(!rs->is_named); /* This one should have measured bandwidth below the clip cutoff */ test_assert(rs->has_bandwidth); - test_eq(rs->bandwidth, max_unmeasured_bw / 2); + test_eq(rs->bandwidth_kb, max_unmeasured_bw_kb / 2); test_assert(!(rs->bw_is_unmeasured)); } else if (tor_memeq(rs->identity_digest, "\x5\x5\x5\x5\x5\x5\x5\x5\x5\x5" @@ -2124,7 +2124,7 @@ test_routerstatus_for_umbw(routerstatus_t *rs, time_t now) test_assert(!rs->is_named); /* This one should have measured bandwidth above the clip cutoff */ test_assert(rs->has_bandwidth); - test_eq(rs->bandwidth, max_unmeasured_bw * 2); + test_eq(rs->bandwidth_kb, max_unmeasured_bw_kb * 2); test_assert(!(rs->bw_is_unmeasured)); } else if (tor_memeq(rs->identity_digest, "\x33\x33\x33\x33\x33\x33\x33\x33\x33\x33" @@ -2135,7 +2135,7 @@ test_routerstatus_for_umbw(routerstatus_t *rs, time_t now) * and so should be clipped */ test_assert(rs->has_bandwidth); - test_eq(rs->bandwidth, max_unmeasured_bw); + test_eq(rs->bandwidth_kb, max_unmeasured_bw_kb); test_assert(rs->bw_is_unmeasured); } else if (tor_memeq(rs->identity_digest, "\x34\x34\x34\x34\x34\x34\x34\x34\x34\x34" @@ -2146,7 +2146,7 @@ test_routerstatus_for_umbw(routerstatus_t *rs, time_t now) * and so should not be clipped */ test_assert(rs->has_bandwidth); - test_eq(rs->bandwidth, max_unmeasured_bw / 2); + test_eq(rs->bandwidth_kb, max_unmeasured_bw_kb / 2); test_assert(rs->bw_is_unmeasured); } else { /* Weren't expecting this... */ @@ -2164,7 +2164,7 @@ test_routerstatus_for_umbw(routerstatus_t *rs, time_t now) */ static void -test_dir_clip_unmeasured_bw(void) +test_dir_clip_unmeasured_bw_kb(void) { /* Run the test with the default clip bandwidth */ alternate_clip_bw = 0; @@ -2176,20 +2176,20 @@ test_dir_clip_unmeasured_bw(void) } /** - * This version of test_dir_clip_unmeasured_bw() uses a non-default choice of + * This version of test_dir_clip_unmeasured_bw_kb() uses a non-default choice of * clip bandwidth. */ static void -test_dir_clip_unmeasured_bw_alt(void) +test_dir_clip_unmeasured_bw_kb_alt(void) { /* * Try a different one; this value is chosen so that the below-the-cutoff * unmeasured nodes the test uses, at alternate_clip_bw / 2, will be above - * DEFAULT_MAX_UNMEASURED_BW and if the consensus incorrectly uses that + * DEFAULT_MAX_UNMEASURED_BW_KB and if the consensus incorrectly uses that * cutoff it will fail the test. */ - alternate_clip_bw = 3 * DEFAULT_MAX_UNMEASURED_BW; + alternate_clip_bw = 3 * DEFAULT_MAX_UNMEASURED_BW_KB; test_a_networkstatus(gen_routerstatus_for_umbw, vote_tweaks_for_umbw, test_vrs_for_umbw, @@ -2209,14 +2209,14 @@ struct testcase_t dir_tests[] = { DIR_LEGACY(versions), DIR_LEGACY(fp_pairs), DIR(split_fps), - DIR_LEGACY(measured_bw), + DIR_LEGACY(measured_bw_kb), DIR_LEGACY(param_voting), DIR_LEGACY(v3_networkstatus), DIR(random_weighted), DIR(scale_bw), - DIR_LEGACY(clip_unmeasured_bw), - DIR_LEGACY(clip_unmeasured_bw_alt), - DIR_LEGACY(measured_bw_cache), + DIR_LEGACY(clip_unmeasured_bw_kb), + DIR_LEGACY(clip_unmeasured_bw_kb_alt), + DIR_LEGACY(measured_bw_kb_cache), END_OF_TESTCASES }; From c4de828923b7983003fa32732ee68581f3c59a89 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Apr 2013 15:10:35 -0400 Subject: [PATCH 15/16] Remvoe total_bandwidth and total_exit_bandwidth as unused. --- src/or/dirserv.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index bf4750645..142f80d35 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1783,10 +1783,6 @@ static uint32_t guard_bandwidth_including_exits_kb = 0; /** If exits can't be guards, then all guards must have a bandwidth this * high. */ static uint32_t guard_bandwidth_excluding_exits_kb = 0; -/** Total bandwidth of all the routers we're considering. */ -static uint64_t total_bandwidth_kb = 0; -/** Total bandwidth of all the exit routers we're considering. */ -static uint64_t total_exit_bandwidth_kb = 0; /** Helper: estimate the uptime of a router given its stated uptime and the * amount of time since it last stated its stated uptime. */ @@ -1912,7 +1908,6 @@ router_counts_toward_thresholds(const node_t *node, time_t now, * the Stable, Fast, and Guard flags. Update the fields stable_uptime, * stable_mtbf, enough_mtbf_info, guard_wfu, guard_tk, fast_bandwidth, * guard_bandwidh_including_exits, guard_bandwidth_excluding_exits, - * total_bandwidth, and total_exit_bandwidth. * * Also, set the is_exit flag of each router appropriately. */ static void @@ -1939,8 +1934,6 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, guard_bandwidth_excluding_exits_kb = 0; guard_tk = 0; guard_wfu = 0; - total_bandwidth_kb = 0; - total_exit_bandwidth_kb = 0; /* Initialize arrays that will hold values for each router. We'll * sort them and use that to compute thresholds. */ @@ -1974,9 +1967,8 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, mtbfs[n_active] = rep_hist_get_stability(id, now); tks [n_active] = rep_hist_get_weighted_time_known(id, now); bandwidths_kb[n_active] = bw_kb = dirserv_get_credible_bandwidth_kb(ri); - total_bandwidth_kb += bw_kb; if (node->is_exit && !node->is_bad_exit) { - total_exit_bandwidth_kb += bw_kb; + ; } else { bandwidths_excluding_exits_kb[n_active_nonexit] = bw_kb; ++n_active_nonexit; From f3ae628517c311c4339f91bbeb8230d13a172d53 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Apr 2013 15:11:46 -0400 Subject: [PATCH 16/16] Remove a now-empty if body; invert the sense of its condition --- src/or/dirserv.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 142f80d35..e4533b773 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1967,9 +1967,7 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, mtbfs[n_active] = rep_hist_get_stability(id, now); tks [n_active] = rep_hist_get_weighted_time_known(id, now); bandwidths_kb[n_active] = bw_kb = dirserv_get_credible_bandwidth_kb(ri); - if (node->is_exit && !node->is_bad_exit) { - ; - } else { + if (!node->is_exit || node->is_bad_exit) { bandwidths_excluding_exits_kb[n_active_nonexit] = bw_kb; ++n_active_nonexit; }