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

[tor/master] fix a wide line
by nickm＠torproject.org 19 Feb '13

19 Feb '13

commit 337e32f5b8f5f3b310da20bf0135f17d06efb3ab Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 19 15:08:54 2013 -0500 fix a wide line --- src/or/connection_edge.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index abde2e1..fcbcb95 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -658,8 +658,8 @@ connection_ap_expire_beginning(void) circuit_purpose_to_string(circ->purpose), circuit_state_to_string(circ->state), CIRCUIT_IS_ORIGIN(circ) ? - pathbias_state_to_string(TO_ORIGIN_CIRCUIT(circ)->path_state) : - "none"); + pathbias_state_to_string(TO_ORIGIN_CIRCUIT(circ)->path_state) : + "none"); } log_fn(cutoff < 15 ? LOG_INFO : severity, LD_APP, "We tried for %d seconds to connect to '%s' using exit %s."

1 0

[tor/master] Downgrade an assert to LD_BUG
by nickm＠torproject.org 19 Feb '13

19 Feb '13

commit 3340d3279de0bb411c506a759778ee7a9b45165b Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 19 14:38:04 2013 -0500 Downgrade an assert to LD_BUG This should prevent crashes on further recurrence of 8065, and help diagnose such if they occur --- changes/bug8065 | 6 ++++++ src/or/circuitbuild.c | 2 +- src/or/circuitbuild.h | 1 + src/or/connection_edge.c | 11 ++++++++++- 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/changes/bug8065 b/changes/bug8065 new file mode 100644 index 0000000..06dbae8 --- /dev/null +++ b/changes/bug8065 @@ -0,0 +1,6 @@ + o Minor bugfixes: + - Downgrade an assertion in connection_ap_expire_beginning to + an LD_BUG message. The fix for bug 8024 should prevent this + message from displaying, but just in case a warn that we can + diagnose is better than more assert crashes. Fix for bug 8065; + bugfix on 0.2.4.8-alpha. diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 40751e0..8a3a36a 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1269,7 +1269,7 @@ pathbias_get_scale_use_threshold(const or_options_t *options) /** * Convert a Guard's path state to string. */ -static const char * +const char * pathbias_state_to_string(path_state_t state) { switch (state) { diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index 3ca8d15..a309170 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -66,6 +66,7 @@ int pathbias_check_probe_response(circuit_t *circ, const cell_t *cell); void pathbias_count_use_attempt(origin_circuit_t *circ); void pathbias_mark_use_success(origin_circuit_t *circ); void pathbias_mark_use_rollback(origin_circuit_t *circ); +const char *pathbias_state_to_string(path_state_t state); #endif diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index b4fa3e6..abde2e1 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -651,7 +651,16 @@ connection_ap_expire_beginning(void) } continue; } - tor_assert(circ->purpose == CIRCUIT_PURPOSE_C_GENERAL); + if (circ->purpose != CIRCUIT_PURPOSE_C_GENERAL) { + log_warn(LD_BUG, "circuit->purpose == CIRCUIT_PURPOSE_C_GENERAL failed. " + "The purpose on the circuit was %s; it was in state %s, " + "path_state %s.", + circuit_purpose_to_string(circ->purpose), + circuit_state_to_string(circ->state), + CIRCUIT_IS_ORIGIN(circ) ? + pathbias_state_to_string(TO_ORIGIN_CIRCUIT(circ)->path_state) : + "none"); + } log_fn(cutoff < 15 ? LOG_INFO : severity, LD_APP, "We tried for %d seconds to connect to '%s' using exit %s." " Retrying on a new circuit.",

1 0

[tor/master] Merge branch 'bug8065_v2'
by nickm＠torproject.org 19 Feb '13

19 Feb '13

commit 8ddf4e218e24167d1c595b74af62dd83b60d99a7 Merge: 804be10 3340d32 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 19 15:02:22 2013 -0500 Merge branch 'bug8065_v2' changes/bug8065 | 6 ++++++ src/or/circuitbuild.c | 2 +- src/or/circuitbuild.h | 1 + src/or/connection_edge.c | 11 ++++++++++- 4 files changed, 18 insertions(+), 2 deletions(-)

1 0

[tor/master] Note some annoyinc copy-and-paste code
by nickm＠torproject.org 19 Feb '13

19 Feb '13

commit 869826581d01ae430b15721b0ff25ebe94103d1a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue May 22 09:51:02 2012 -0400 Note some annoyinc copy-and-paste code --- src/or/dirvote.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/or/dirvote.c b/src/or/dirvote.c index d1ac8f8..96fc8a5 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -1590,6 +1590,7 @@ networkstatus_compute_consensus(smartlist_t *votes, if (consensus_method >= MIN_METHOD_TO_CLIP_UNMEASURED_BW) { char *max_unmeasured_param = NULL; + /* XXXX Extract this code into a common function */ if (params) { if (strcmpstart(params, "maxunmeasuredbw=") == 0) max_unmeasured_param = params; @@ -2097,6 +2098,7 @@ networkstatus_compute_consensus(smartlist_t *votes, // Parse params, extract BW_WEIGHT_SCALE if present // DO NOT use consensus_param_bw_weight_scale() in this code! // The consensus is not formed yet! + /* XXXX Extract this code into a common function */ if (params) { if (strcmpstart(params, "bwweightscale=") == 0) bw_weight_param = params;

1 0

[tor/master] New consensus method: clip the maximum votable unmeasured bw
by nickm＠torproject.org 19 Feb '13

19 Feb '13

commit 1cd67443383966687af3b2f9086ceeb7915017bf Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed May 16 17:04:51 2012 -0400 New consensus method: clip the maximum votable unmeasured bw If we're deciding on a node's bandwidth based on "Bandwidth=" declarations, clip it to "20" or to the maxunmeasuredbw parameter, if it's voted on. This adds a new consensus method. This is "part A" of bug 2286 --- src/or/dirvote.c | 31 +++++++++++++++++++++++++++++++ src/or/dirvote.h | 6 +++++- 2 files changed, 36 insertions(+), 1 deletions(-) diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 43b9f5e..08a9eb4 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -1388,6 +1388,8 @@ networkstatus_compute_consensus(smartlist_t *votes, char *client_versions = NULL, *server_versions = NULL; smartlist_t *flags; const char *flavor_name; +#define DEFAULT_MAX_UNMEASURED_BW 20 + uint32_t max_unmeasured_bw = DEFAULT_MAX_UNMEASURED_BW; 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; @@ -1586,6 +1588,30 @@ networkstatus_compute_consensus(smartlist_t *votes, smartlist_free(dir_sources); } + if (consensus_method >= MIN_METHOD_TO_CLIP_UNMEASURED_BW) { + char *max_unmeasured_param = NULL; + if (params) { + if (strcmpstart(params, "maxunmeasuredbw=") == 0) + max_unmeasured_param = params; + else + max_unmeasured_param = strstr(params, " maxunmeasuredbw="); + } + if (max_unmeasured_param) { + int ok = 0; + char *eq = strchr(max_unmeasured_param, '='); + if (eq) { + max_unmeasured_bw = (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; + } + } + } + } + + /* Add the actual router entries. */ { int *index; /* index[j] is the current index into votes[j]. */ @@ -1867,6 +1893,11 @@ networkstatus_compute_consensus(smartlist_t *votes, } else if (consensus_method >= 5 && num_bandwidths > 0) { rs_out.has_bandwidth = 1; rs_out.bandwidth = median_uint32(bandwidths, num_bandwidths); + if (consensus_method >= MIN_METHOD_TO_CLIP_UNMEASURED_BW) { + /* Cap non-measured bandwidths to 20k. */ + if (rs_out.bandwidth > max_unmeasured_bw) + rs_out.bandwidth = max_unmeasured_bw; + } } /* Fix bug 2203: Do not count BadExit nodes as Exits for bw weights */ diff --git a/src/or/dirvote.h b/src/or/dirvote.h index f134454..a739874 100644 --- a/src/or/dirvote.h +++ b/src/or/dirvote.h @@ -20,7 +20,7 @@ #define MIN_VOTE_INTERVAL 300 /** The highest consensus method that we currently support. */ -#define MAX_SUPPORTED_CONSENSUS_METHOD 16 +#define MAX_SUPPORTED_CONSENSUS_METHOD 17 /** Lowest consensus method that contains a 'directory-footer' marker */ #define MIN_METHOD_FOR_FOOTER 9 @@ -52,6 +52,10 @@ * line */ #define MIN_METHOD_FOR_NTOR_KEY 16 +/** Lowest consensus method that ensures that authorities output an + * Unmeasured=1 flag for unmeasured bandwidths */ +#define MIN_METHOD_TO_CLIP_UNMEASURED_BW 17 + void dirvote_free_all(void); /* vote manipulation */

1 0

[tor/master] Add unit test for unmeasured bandwidth clipping in consensus
by nickm＠torproject.org 19 Feb '13

19 Feb '13

commit 4c45b3d8455ecc14daeaacc02330f60603925d95 Author: Andrea Shepard <andrea(a)torproject.org> Date: Tue Feb 19 06:20:29 2013 -0800 Add unit test for unmeasured bandwidth clipping in consensus --- src/or/dirvote.c | 2 - src/or/dirvote.h | 4 + src/test/test_dir.c | 390 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 394 insertions(+), 2 deletions(-) diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 96fc8a5..a3fb03a 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -1388,7 +1388,6 @@ networkstatus_compute_consensus(smartlist_t *votes, char *client_versions = NULL, *server_versions = NULL; smartlist_t *flags; const char *flavor_name; -#define DEFAULT_MAX_UNMEASURED_BW 20 uint32_t max_unmeasured_bw = DEFAULT_MAX_UNMEASURED_BW; int64_t G=0, M=0, E=0, D=0, T=0; /* For bandwidth weights */ const routerstatus_format_type_t rs_format = @@ -1612,7 +1611,6 @@ networkstatus_compute_consensus(smartlist_t *votes, } } - /* Add the actual router entries. */ { int *index; /* index[j] is the current index into votes[j]. */ diff --git a/src/or/dirvote.h b/src/or/dirvote.h index a739874..e395265 100644 --- a/src/or/dirvote.h +++ b/src/or/dirvote.h @@ -56,6 +56,10 @@ * Unmeasured=1 flag for unmeasured bandwidths */ #define MIN_METHOD_TO_CLIP_UNMEASURED_BW 17 +/** Default bandwidth to clip unmeasured bandwidths to using method >= + * MIN_METHOD_TO_CLIP_UNMEASURED_BW */ +#define DEFAULT_MAX_UNMEASURED_BW 20 + void dirvote_free_all(void); /* vote manipulation */ diff --git a/src/test/test_dir.c b/src/test/test_dir.c index ec89606..269da22 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -746,6 +746,17 @@ generate_ri_from_rs(const vote_routerstatus_t *vrs) strlen(r->cache_info.signed_descriptor_body); r->exit_policy = smartlist_new(); r->cache_info.published_on = ++published + time(NULL); + if (rs->has_bandwidth) { + /* + * Multiply by 1000 because the routerinfo_t and the routerstatus_t + * seem to use different units (*sigh*) and because we seem stuck on + * icky and perverse decimal kilobytes (*double sigh*) - see + * 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; + } return r; } @@ -1684,6 +1695,384 @@ test_dir_random_weighted(void *testdata) ; } +/* Function pointers for test_dir_clip_unmeasured_bw() */ + +/** + * Generate a routerstatus for clip_unmeasured_bw test; based on the + * v3_networkstatus ones. + */ +static vote_routerstatus_t * +gen_routerstatus_for_umbw(int idx, time_t now) +{ + vote_routerstatus_t *vrs; + routerstatus_t *rs; + tor_addr_t addr_ipv6; + + switch (idx) { + case 0: + /* Generate the first routerstatus. */ + vrs = tor_malloc_zero(sizeof(vote_routerstatus_t)); + rs = &vrs->status; + vrs->version = tor_strdup("0.1.2.14"); + rs->published_on = now-1500; + strlcpy(rs->nickname, "router2", sizeof(rs->nickname)); + memset(rs->identity_digest, 3, DIGEST_LEN); + memset(rs->descriptor_digest, 78, DIGEST_LEN); + rs->addr = 0x99008801; + rs->or_port = 443; + rs->dir_port = 8000; + /* all flags but running cleared */ + rs->is_flagged_running = 1; + /* + * This one has measured bandwidth below the clip cutoff, and + * so shouldn't be clipped; we'll have to test that it isn't + * later. + */ + rs->has_measured_bw = 1; + rs->has_bandwidth = 1; + rs->measured_bw = rs->bandwidth = DEFAULT_MAX_UNMEASURED_BW / 2; + break; + case 1: + /* Generate the second routerstatus. */ + vrs = tor_malloc_zero(sizeof(vote_routerstatus_t)); + rs = &vrs->status; + vrs->version = tor_strdup("0.2.0.5"); + rs->published_on = now-1000; + strlcpy(rs->nickname, "router1", sizeof(rs->nickname)); + memset(rs->identity_digest, 5, DIGEST_LEN); + memset(rs->descriptor_digest, 77, DIGEST_LEN); + rs->addr = 0x99009901; + rs->or_port = 443; + rs->dir_port = 0; + tor_addr_parse(&addr_ipv6, "[1:2:3::4]"); + tor_addr_copy(&rs->ipv6_addr, &addr_ipv6); + rs->ipv6_orport = 4711; + rs->is_exit = rs->is_stable = rs->is_fast = rs->is_flagged_running = + rs->is_valid = rs->is_v2_dir = rs->is_possible_guard = 1; + /* + * This one has measured bandwidth above the clip cutoff, and + * so shouldn't be clipped; we'll have to test that it isn't + * later. + */ + rs->has_measured_bw = 1; + rs->has_bandwidth = 1; + rs->measured_bw = rs->bandwidth = 2 * DEFAULT_MAX_UNMEASURED_BW; + break; + case 2: + /* Generate the third routerstatus. */ + vrs = tor_malloc_zero(sizeof(vote_routerstatus_t)); + rs = &vrs->status; + vrs->version = tor_strdup("0.1.0.3"); + rs->published_on = now-1000; + strlcpy(rs->nickname, "router3", sizeof(rs->nickname)); + memset(rs->identity_digest, 0x33, DIGEST_LEN); + memset(rs->descriptor_digest, 79, DIGEST_LEN); + rs->addr = 0xAA009901; + rs->or_port = 400; + rs->dir_port = 9999; + rs->is_authority = rs->is_exit = rs->is_stable = rs->is_fast = + rs->is_flagged_running = rs->is_valid = rs->is_v2_dir = + rs->is_possible_guard = 1; + /* + * This one has unmeasured bandwidth above the clip cutoff, and + * so should be clipped; we'll have to test that it isn't + * later. + */ + rs->has_measured_bw = 0; + rs->has_bandwidth = 1; + rs->measured_bw = 0; + rs->bandwidth = 2 * DEFAULT_MAX_UNMEASURED_BW; + break; + case 3: + /* Generate a fourth routerstatus that is not running. */ + vrs = tor_malloc_zero(sizeof(vote_routerstatus_t)); + rs = &vrs->status; + vrs->version = tor_strdup("0.1.6.3"); + rs->published_on = now-1000; + strlcpy(rs->nickname, "router4", sizeof(rs->nickname)); + memset(rs->identity_digest, 0x34, DIGEST_LEN); + memset(rs->descriptor_digest, 47, DIGEST_LEN); + rs->addr = 0xC0000203; + rs->or_port = 500; + rs->dir_port = 1999; + /* all flags but running cleared */ + rs->is_flagged_running = 1; + /* + * This one has unmeasured bandwidth below the clip cutoff, and + * so shouldn't be clipped; we'll have to test that it isn't + * later. + */ + rs->has_measured_bw = 0; + rs->has_bandwidth = 1; + rs->measured_bw = 0; + rs->bandwidth = DEFAULT_MAX_UNMEASURED_BW / 2; + break; + case 4: + /* No more for this test; return NULL */ + vrs = NULL; + break; + default: + /* Shouldn't happen */ + test_assert(0); + } + + done: + return vrs; +} + +/** Apply tweaks to the vote list for each voter; for the umbw test this is + * just adding the right consensus methods to let clipping happen */ +static void +vote_tweaks_for_umbw(networkstatus_t *v, int voter, time_t now) +{ + test_assert(v); + (void)voter; + (void)now; + + test_assert(v->supported_methods); + smartlist_clear(v->supported_methods); + /* Method 17 is MIN_METHOD_TO_CLIP_UNMEASURED_BW */ + 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); + + done: + return; +} + +/** + * Test a parsed vote_routerstatus_t for umbw test. + */ +static void +test_vrs_for_umbw(vote_routerstatus_t *vrs, int voter, time_t now) +{ + routerstatus_t *rs; + tor_addr_t addr_ipv6; + + (void)voter; + test_assert(vrs); + rs = &(vrs->status); + test_assert(rs); + + /* Split out by digests to test */ + if (tor_memeq(rs->identity_digest, + "\x3\x3\x3\x3\x3\x3\x3\x3\x3\x3" + "\x3\x3\x3\x3\x3\x3\x3\x3\x3\x3", + DIGEST_LEN)) { + /* + * Check the first routerstatus - measured bandwidth below the clip + * cutoff. + */ + test_streq(vrs->version, "0.1.2.14"); + test_eq(rs->published_on, now-1500); + test_streq(rs->nickname, "router2"); + test_memeq(rs->identity_digest, + "\x3\x3\x3\x3\x3\x3\x3\x3\x3\x3" + "\x3\x3\x3\x3\x3\x3\x3\x3\x3\x3", + DIGEST_LEN); + test_memeq(rs->descriptor_digest, "NNNNNNNNNNNNNNNNNNNN", DIGEST_LEN); + test_eq(rs->addr, 0x99008801); + test_eq(rs->or_port, 443); + test_eq(rs->dir_port, 8000); + test_assert(rs->has_bandwidth); + test_assert(rs->has_measured_bw); + test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW / 2); + test_eq(rs->measured_bw, DEFAULT_MAX_UNMEASURED_BW / 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", + DIGEST_LEN)) { + + /* + * Check the second routerstatus - measured bandwidth above the clip + * cutoff. + */ + test_streq(vrs->version, "0.2.0.5"); + test_eq(rs->published_on, now-1000); + test_streq(rs->nickname, "router1"); + test_memeq(rs->identity_digest, + "\x5\x5\x5\x5\x5\x5\x5\x5\x5\x5" + "\x5\x5\x5\x5\x5\x5\x5\x5\x5\x5", + DIGEST_LEN); + test_memeq(rs->descriptor_digest, "MMMMMMMMMMMMMMMMMMMM", DIGEST_LEN); + test_eq(rs->addr, 0x99009901); + test_eq(rs->or_port, 443); + test_eq(rs->dir_port, 0); + tor_addr_parse(&addr_ipv6, "[1:2:3::4]"); + test_assert(tor_addr_eq(&rs->ipv6_addr, &addr_ipv6)); + test_eq(rs->ipv6_orport, 4711); + test_assert(rs->has_bandwidth); + test_assert(rs->has_measured_bw); + test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW * 2); + test_eq(rs->measured_bw, DEFAULT_MAX_UNMEASURED_BW * 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", + DIGEST_LEN)) { + /* + * Check the third routerstatus - unmeasured bandwidth above the clip + * cutoff; this one should be clipped later on in the consensus, but + * appears unclipped in the vote. + */ + test_assert(rs->has_bandwidth); + test_assert(!(rs->has_measured_bw)); + test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW * 2); + test_eq(rs->measured_bw, 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", + DIGEST_LEN)) { + /* + * Check the fourth routerstatus - unmeasured bandwidth below the clip + * cutoff; this one should not be clipped. + */ + test_assert(rs->has_bandwidth); + test_assert(!(rs->has_measured_bw)); + test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW / 2); + test_eq(rs->measured_bw, 0); + } else { + test_assert(0); + } + + done: + return; +} + +/** + * Test a consensus for v3_networkstatus_test + */ +static void +test_consensus_for_umbw(networkstatus_t *con, time_t now) +{ + (void)now; + + test_assert(con); + test_assert(!con->cert); + /* test_assert(con->consensus_method >= MIN_METHOD_TO_CLIP_UNMEASURED_BW); */ + 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 */ + + done: + return; +} + +/** + * Test a router list entry for umbw test + */ +static void +test_routerstatus_for_umbw(routerstatus_t *rs, time_t now) +{ + tor_addr_t addr_ipv6; + + test_assert(rs); + + /* There should be four listed routers, as constructed above */ + if (tor_memeq(rs->identity_digest, + "\x3\x3\x3\x3\x3\x3\x3\x3\x3\x3" + "\x3\x3\x3\x3\x3\x3\x3\x3\x3\x3", + DIGEST_LEN)) { + test_memeq(rs->identity_digest, + "\x3\x3\x3\x3\x3\x3\x3\x3\x3\x3" + "\x3\x3\x3\x3\x3\x3\x3\x3\x3\x3", + DIGEST_LEN); + test_memeq(rs->descriptor_digest, "NNNNNNNNNNNNNNNNNNNN", DIGEST_LEN); + test_assert(!rs->is_authority); + test_assert(!rs->is_exit); + test_assert(!rs->is_fast); + test_assert(!rs->is_possible_guard); + test_assert(!rs->is_stable); + /* (If it wasn't running it wouldn't be here) */ + test_assert(rs->is_flagged_running); + test_assert(!rs->is_v2_dir); + test_assert(!rs->is_valid); + test_assert(!rs->is_named); + /* This one should have measured bandwidth below the clip cutoff */ + test_assert(rs->has_bandwidth); + test_assert(rs->has_measured_bw); + test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW / 2); + test_eq(rs->measured_bw, DEFAULT_MAX_UNMEASURED_BW / 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", + DIGEST_LEN)) { + /* This one showed up in 3 digests. Twice with ID 'M', once with 'Z'. */ + test_memeq(rs->identity_digest, + "\x5\x5\x5\x5\x5\x5\x5\x5\x5\x5" + "\x5\x5\x5\x5\x5\x5\x5\x5\x5\x5", + DIGEST_LEN); + test_streq(rs->nickname, "router1"); + test_memeq(rs->descriptor_digest, "MMMMMMMMMMMMMMMMMMMM", DIGEST_LEN); + test_eq(rs->published_on, now-1000); + test_eq(rs->addr, 0x99009901); + test_eq(rs->or_port, 443); + test_eq(rs->dir_port, 0); + tor_addr_parse(&addr_ipv6, "[1:2:3::4]"); + test_assert(tor_addr_eq(&rs->ipv6_addr, &addr_ipv6)); + test_eq(rs->ipv6_orport, 4711); + test_assert(!rs->is_authority); + test_assert(rs->is_exit); + test_assert(rs->is_fast); + test_assert(rs->is_possible_guard); + test_assert(rs->is_stable); + test_assert(rs->is_flagged_running); + test_assert(rs->is_v2_dir); + test_assert(rs->is_valid); + test_assert(!rs->is_named); + /* This one should have measured bandwidth above the clip cutoff */ + test_assert(rs->has_bandwidth); + test_assert(rs->has_measured_bw); + test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW * 2); + test_eq(rs->measured_bw, DEFAULT_MAX_UNMEASURED_BW * 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", + DIGEST_LEN)) { + /* + * This one should have unmeasured bandwidth above the clip cutoff, + * and so should be clipped + */ + test_assert(rs->has_bandwidth); + test_assert(!(rs->has_measured_bw)); + test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW); + test_eq(rs->measured_bw, 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", + DIGEST_LEN)) { + /* + * This one should have unmeasured bandwidth below the clip cutoff, + * and so should not be clipped + */ + test_assert(rs->has_bandwidth); + test_assert(!(rs->has_measured_bw)); + test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW / 2); + test_eq(rs->measured_bw, 0); + } else { + /* Weren't expecting this... */ + test_assert(0); + } + + done: + return; +} + +/** + * Compute a consensus involving clipping unmeasured bandwidth with consensus + * method 17; this uses the same test_a_networkstatus() function that the + * v3_networkstatus test uses. + */ + +static void +test_dir_clip_unmeasured_bw(void) +{ + test_a_networkstatus(gen_routerstatus_for_umbw, + vote_tweaks_for_umbw, + test_vrs_for_umbw, + test_consensus_for_umbw, + test_routerstatus_for_umbw); +} + #define DIR_LEGACY(name) \ { #name, legacy_test_helper, TT_FORK, &legacy_setup, test_dir_ ## name } @@ -1701,6 +2090,7 @@ struct testcase_t dir_tests[] = { DIR_LEGACY(v3_networkstatus), DIR(random_weighted), DIR(scale_bw), + DIR_LEGACY(clip_unmeasured_bw), END_OF_TESTCASES };

1 0

[tor/master] Test alternate values of maxunmeasuredbw too
by nickm＠torproject.org 19 Feb '13

19 Feb '13

commit 699c39dada7f76d5b0cc8dd4b56df1fe4bba7763 Author: Andrea Shepard <andrea(a)torproject.org> Date: Tue Feb 19 08:58:09 2013 -0800 Test alternate values of maxunmeasuredbw too --- src/test/test_dir.c | 104 ++++++++++++++++++++++++++++++++++++++------------ 1 files changed, 79 insertions(+), 25 deletions(-) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index c25d4d9..bef01e1 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -865,7 +865,7 @@ gen_routerstatus_for_v3ns(int idx, time_t now) } /** Apply tweaks to the vote list for each voter */ -static void +static int vote_tweaks_for_v3ns(networkstatus_t *v, int voter, time_t now) { vote_routerstatus_t *vrs; @@ -902,7 +902,7 @@ vote_tweaks_for_v3ns(networkstatus_t *v, int voter, time_t now) } done: - return; + return 0; } /** @@ -1077,7 +1077,7 @@ test_routerstatus_for_v3ns(routerstatus_t *rs, time_t now) static void test_a_networkstatus( vote_routerstatus_t * (*vrs_gen)(int idx, time_t now), - void (*vote_tweaks)(networkstatus_t *v, int voter, time_t now), + int (*vote_tweaks)(networkstatus_t *v, int voter, time_t now), void (*vrs_test)(vote_routerstatus_t *vrs, int voter, time_t now), void (*consensus_test)(networkstatus_t *con, time_t now), void (*rs_test)(routerstatus_t *rs, time_t now)) @@ -1086,6 +1086,12 @@ test_a_networkstatus( crypto_pk_t *sign_skey_1=NULL, *sign_skey_2=NULL, *sign_skey_3=NULL; crypto_pk_t *sign_skey_leg1=NULL; const char *msg=NULL; + /* + * Sum the non-zero returns from vote_tweaks() we've seen; if vote_tweaks() + * returns non-zero, it changed net_params and we should skip the tests for + * that later as they will fail. + */ + int params_tweaked = 0; time_t now = time(NULL); networkstatus_voter_info_t *voter; @@ -1211,7 +1217,7 @@ test_a_networkstatus( tor_free(cp); test_eq(smartlist_len(v1->routerstatus_list), n_vrs); - if (vote_tweaks) vote_tweaks(v1, 1, now); + if (vote_tweaks) params_tweaked += vote_tweaks(v1, 1, now); /* Check the routerstatuses. */ for (idx = 0; idx < n_vrs; ++idx) { @@ -1249,7 +1255,7 @@ test_a_networkstatus( v2 = networkstatus_parse_vote_from_string(v2_text, NULL, NS_TYPE_VOTE); test_assert(v2); - if (vote_tweaks) vote_tweaks(v2, 2, now); + if (vote_tweaks) params_tweaked += vote_tweaks(v2, 2, now); /* Check that flags come out right.*/ cp = smartlist_join_strings(v2->known_flags, ":", 0, NULL); @@ -1293,7 +1299,7 @@ test_a_networkstatus( v3 = networkstatus_parse_vote_from_string(v3_text, NULL, NS_TYPE_VOTE); test_assert(v3); - if (vote_tweaks) vote_tweaks(v3, 3, now); + if (vote_tweaks) params_tweaked += vote_tweaks(v3, 3, now); /* Compute a consensus as voter 3. */ smartlist_add(votes, v3); @@ -1337,9 +1343,12 @@ test_a_networkstatus( test_streq(cp, "Authority:Exit:Fast:Guard:MadeOfCheese:MadeOfTin:" "Running:Stable:V2Dir:Valid"); tor_free(cp); - cp = smartlist_join_strings(con->net_params, ":", 0, NULL); - test_streq(cp, "circuitwindow=80:foo=660"); - tor_free(cp); + if (!params_tweaked) { + /* Skip this one if vote_tweaks() messed with the param lists */ + cp = smartlist_join_strings(con->net_params, ":", 0, NULL); + test_streq(cp, "circuitwindow=80:foo=660"); + tor_free(cp); + } test_eq(4, smartlist_len(con->voters)); /*3 voters, 1 legacy key.*/ /* The voter id digests should be in this order. */ @@ -1697,6 +1706,8 @@ test_dir_random_weighted(void *testdata) /* Function pointers for test_dir_clip_unmeasured_bw() */ +static uint32_t alternate_clip_bw = 0; + /** * Generate a routerstatus for clip_unmeasured_bw test; based on the * v3_networkstatus ones. @@ -1707,6 +1718,8 @@ gen_routerstatus_for_umbw(int idx, time_t now) vote_routerstatus_t *vrs; routerstatus_t *rs; tor_addr_t addr_ipv6; + uint32_t max_unmeasured_bw = (alternate_clip_bw > 0) ? + alternate_clip_bw : DEFAULT_MAX_UNMEASURED_BW; switch (idx) { case 0: @@ -1730,7 +1743,7 @@ gen_routerstatus_for_umbw(int idx, time_t now) */ vrs->has_measured_bw = 1; rs->has_bandwidth = 1; - vrs->measured_bw = rs->bandwidth = DEFAULT_MAX_UNMEASURED_BW / 2; + vrs->measured_bw = rs->bandwidth = max_unmeasured_bw / 2; break; case 1: /* Generate the second routerstatus. */ @@ -1756,7 +1769,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 * DEFAULT_MAX_UNMEASURED_BW; + vrs->measured_bw = rs->bandwidth = 2 * max_unmeasured_bw; break; case 2: /* Generate the third routerstatus. */ @@ -1781,7 +1794,7 @@ 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 * DEFAULT_MAX_UNMEASURED_BW; + rs->bandwidth = 2 * max_unmeasured_bw; break; case 3: /* Generate a fourth routerstatus that is not running. */ @@ -1805,7 +1818,7 @@ gen_routerstatus_for_umbw(int idx, time_t now) vrs->has_measured_bw = 0; rs->has_bandwidth = 1; vrs->measured_bw = 0; - rs->bandwidth = DEFAULT_MAX_UNMEASURED_BW / 2; + rs->bandwidth = max_unmeasured_bw / 2; break; case 4: /* No more for this test; return NULL */ @@ -1822,9 +1835,12 @@ gen_routerstatus_for_umbw(int idx, time_t now) /** Apply tweaks to the vote list for each voter; for the umbw test this is * just adding the right consensus methods to let clipping happen */ -static void +static int vote_tweaks_for_umbw(networkstatus_t *v, int voter, time_t now) { + char *maxbw_param = NULL; + int rv = 0; + test_assert(v); (void)voter; (void)now; @@ -1835,9 +1851,18 @@ vote_tweaks_for_umbw(networkstatus_t *v, int voter, time_t now) 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); + /* If we're using a non-default clip bandwidth, add it to net_params */ + if (alternate_clip_bw > 0) { + tor_asprintf(&maxbw_param, "maxunmeasuredbw=%u", alternate_clip_bw); + test_assert(maxbw_param); + if (maxbw_param) { + smartlist_add(v->net_params, maxbw_param); + rv = 1; + } + } done: - return; + return rv; } /** @@ -1848,6 +1873,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; (void)voter; test_assert(vrs); @@ -1876,8 +1903,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, DEFAULT_MAX_UNMEASURED_BW / 2); - test_eq(vrs->measured_bw, DEFAULT_MAX_UNMEASURED_BW / 2); + test_eq(rs->bandwidth, max_unmeasured_bw / 2); + test_eq(vrs->measured_bw, max_unmeasured_bw / 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", @@ -1903,8 +1930,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, DEFAULT_MAX_UNMEASURED_BW * 2); - test_eq(vrs->measured_bw, DEFAULT_MAX_UNMEASURED_BW * 2); + test_eq(rs->bandwidth, max_unmeasured_bw * 2); + test_eq(vrs->measured_bw, max_unmeasured_bw * 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", @@ -1916,7 +1943,7 @@ 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, DEFAULT_MAX_UNMEASURED_BW * 2); + test_eq(rs->bandwidth, max_unmeasured_bw * 2); test_eq(vrs->measured_bw, 0); } else if (tor_memeq(rs->identity_digest, "\x34\x34\x34\x34\x34\x34\x34\x34\x34\x34" @@ -1928,7 +1955,7 @@ 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, DEFAULT_MAX_UNMEASURED_BW / 2); + test_eq(rs->bandwidth, max_unmeasured_bw / 2); test_eq(vrs->measured_bw, 0); } else { test_assert(0); @@ -1964,6 +1991,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; test_assert(rs); @@ -1989,7 +2018,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, DEFAULT_MAX_UNMEASURED_BW / 2); + test_eq(rs->bandwidth, max_unmeasured_bw / 2); test_assert(!(rs->bw_is_unmeasured)); } else if (tor_memeq(rs->identity_digest, "\x5\x5\x5\x5\x5\x5\x5\x5\x5\x5" @@ -2020,7 +2049,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, DEFAULT_MAX_UNMEASURED_BW * 2); + test_eq(rs->bandwidth, max_unmeasured_bw * 2); test_assert(!(rs->bw_is_unmeasured)); } else if (tor_memeq(rs->identity_digest, "\x33\x33\x33\x33\x33\x33\x33\x33\x33\x33" @@ -2031,7 +2060,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, DEFAULT_MAX_UNMEASURED_BW); + test_eq(rs->bandwidth, max_unmeasured_bw); test_assert(rs->bw_is_unmeasured); } else if (tor_memeq(rs->identity_digest, "\x34\x34\x34\x34\x34\x34\x34\x34\x34\x34" @@ -2042,7 +2071,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, DEFAULT_MAX_UNMEASURED_BW / 2); + test_eq(rs->bandwidth, max_unmeasured_bw / 2); test_assert(rs->bw_is_unmeasured); } else { /* Weren't expecting this... */ @@ -2062,6 +2091,30 @@ test_routerstatus_for_umbw(routerstatus_t *rs, time_t now) static void test_dir_clip_unmeasured_bw(void) { + /* Run the test with the default clip bandwidth */ + alternate_clip_bw = 0; + test_a_networkstatus(gen_routerstatus_for_umbw, + vote_tweaks_for_umbw, + test_vrs_for_umbw, + test_consensus_for_umbw, + test_routerstatus_for_umbw); +} + +/** + * This version of test_dir_clip_unmeasured_bw() uses a non-default choice of + * clip bandwidth. + */ + +static void +test_dir_clip_unmeasured_bw_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 + * cutoff it will fail the test. + */ + alternate_clip_bw = 3 * DEFAULT_MAX_UNMEASURED_BW; test_a_networkstatus(gen_routerstatus_for_umbw, vote_tweaks_for_umbw, test_vrs_for_umbw, @@ -2087,6 +2140,7 @@ struct testcase_t dir_tests[] = { DIR(random_weighted), DIR(scale_bw), DIR_LEGACY(clip_unmeasured_bw), + DIR_LEGACY(clip_unmeasured_bw_alt), END_OF_TESTCASES };

1 0

[tor/master] Merge branch 'bug2286_unit_test_squashed'
by nickm＠torproject.org 19 Feb '13

19 Feb '13

commit b0b0d6af6369f53caed2222837149dc4a90c739c Merge: f219976 699c39d Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 19 14:54:05 2013 -0500 Merge branch 'bug2286_unit_test_squashed' changes/bug2286 | 5 + src/or/dirserv.c | 21 +- src/or/dirserv.h | 3 +- src/or/dirvote.c | 57 +++- src/or/dirvote.h | 10 +- src/or/networkstatus.c | 13 +- src/or/networkstatus.h | 2 + src/or/or.h | 10 +- src/or/routerparse.c | 17 +- src/or/routerparse.h | 1 + src/test/test_dir.c | 982 +++++++++++++++++++++++++++++++++++++++--------- 11 files changed, 923 insertions(+), 198 deletions(-)

1 0

[tor/master] Update unmeasured bandwidth clipping unit test per NickM's bugfix for the Unmeasured=1 issue
by nickm＠torproject.org 19 Feb '13

19 Feb '13

commit f445aae2e30b0a926111b9ca08d19b6847fcbcb8 Author: Andrea Shepard <andrea(a)torproject.org> Date: Tue Feb 19 07:57:49 2013 -0800 Update unmeasured bandwidth clipping unit test per NickM's bugfix for the Unmeasured=1 issue --- src/test/test_dir.c | 48 ++++++++++++++++++++++-------------------------- 1 files changed, 22 insertions(+), 26 deletions(-) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 269da22..c25d4d9 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -972,8 +972,8 @@ test_vrs_for_v3ns(vote_routerstatus_t *vrs, int voter, time_t now) DIGEST_LEN) && (voter == 1 || voter == 2)) { /* Check the measured bandwidth bits */ - test_assert(vrs->status.has_measured_bw && - vrs->status.measured_bw == 1024); + test_assert(vrs->has_measured_bw && + vrs->measured_bw == 1024); } else { /* * Didn't expect this, but the old unit test only checked some of them, @@ -1728,9 +1728,9 @@ gen_routerstatus_for_umbw(int idx, time_t now) * so shouldn't be clipped; we'll have to test that it isn't * later. */ - rs->has_measured_bw = 1; + vrs->has_measured_bw = 1; rs->has_bandwidth = 1; - rs->measured_bw = rs->bandwidth = DEFAULT_MAX_UNMEASURED_BW / 2; + vrs->measured_bw = rs->bandwidth = DEFAULT_MAX_UNMEASURED_BW / 2; break; case 1: /* Generate the second routerstatus. */ @@ -1754,9 +1754,9 @@ gen_routerstatus_for_umbw(int idx, time_t now) * so shouldn't be clipped; we'll have to test that it isn't * later. */ - rs->has_measured_bw = 1; + vrs->has_measured_bw = 1; rs->has_bandwidth = 1; - rs->measured_bw = rs->bandwidth = 2 * DEFAULT_MAX_UNMEASURED_BW; + vrs->measured_bw = rs->bandwidth = 2 * DEFAULT_MAX_UNMEASURED_BW; break; case 2: /* Generate the third routerstatus. */ @@ -1778,9 +1778,9 @@ gen_routerstatus_for_umbw(int idx, time_t now) * so should be clipped; we'll have to test that it isn't * later. */ - rs->has_measured_bw = 0; + vrs->has_measured_bw = 0; rs->has_bandwidth = 1; - rs->measured_bw = 0; + vrs->measured_bw = 0; rs->bandwidth = 2 * DEFAULT_MAX_UNMEASURED_BW; break; case 3: @@ -1802,9 +1802,9 @@ gen_routerstatus_for_umbw(int idx, time_t now) * so shouldn't be clipped; we'll have to test that it isn't * later. */ - rs->has_measured_bw = 0; + vrs->has_measured_bw = 0; rs->has_bandwidth = 1; - rs->measured_bw = 0; + vrs->measured_bw = 0; rs->bandwidth = DEFAULT_MAX_UNMEASURED_BW / 2; break; case 4: @@ -1875,9 +1875,9 @@ test_vrs_for_umbw(vote_routerstatus_t *vrs, int voter, time_t now) test_eq(rs->or_port, 443); test_eq(rs->dir_port, 8000); test_assert(rs->has_bandwidth); - test_assert(rs->has_measured_bw); + test_assert(vrs->has_measured_bw); test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW / 2); - test_eq(rs->measured_bw, DEFAULT_MAX_UNMEASURED_BW / 2); + test_eq(vrs->measured_bw, DEFAULT_MAX_UNMEASURED_BW / 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", @@ -1902,9 +1902,9 @@ test_vrs_for_umbw(vote_routerstatus_t *vrs, int voter, time_t now) test_assert(tor_addr_eq(&rs->ipv6_addr, &addr_ipv6)); test_eq(rs->ipv6_orport, 4711); test_assert(rs->has_bandwidth); - test_assert(rs->has_measured_bw); + test_assert(vrs->has_measured_bw); test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW * 2); - test_eq(rs->measured_bw, DEFAULT_MAX_UNMEASURED_BW * 2); + test_eq(vrs->measured_bw, DEFAULT_MAX_UNMEASURED_BW * 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", @@ -1915,9 +1915,9 @@ test_vrs_for_umbw(vote_routerstatus_t *vrs, int voter, time_t now) * appears unclipped in the vote. */ test_assert(rs->has_bandwidth); - test_assert(!(rs->has_measured_bw)); + test_assert(!(vrs->has_measured_bw)); test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW * 2); - test_eq(rs->measured_bw, 0); + test_eq(vrs->measured_bw, 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", @@ -1927,9 +1927,9 @@ test_vrs_for_umbw(vote_routerstatus_t *vrs, int voter, time_t now) * cutoff; this one should not be clipped. */ test_assert(rs->has_bandwidth); - test_assert(!(rs->has_measured_bw)); + test_assert(!(vrs->has_measured_bw)); test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW / 2); - test_eq(rs->measured_bw, 0); + test_eq(vrs->measured_bw, 0); } else { test_assert(0); } @@ -1989,9 +1989,8 @@ 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_assert(rs->has_measured_bw); test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW / 2); - test_eq(rs->measured_bw, DEFAULT_MAX_UNMEASURED_BW / 2); + test_assert(!(rs->bw_is_unmeasured)); } 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", @@ -2021,9 +2020,8 @@ 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_assert(rs->has_measured_bw); test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW * 2); - test_eq(rs->measured_bw, DEFAULT_MAX_UNMEASURED_BW * 2); + test_assert(!(rs->bw_is_unmeasured)); } 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", @@ -2033,9 +2031,8 @@ test_routerstatus_for_umbw(routerstatus_t *rs, time_t now) * and so should be clipped */ test_assert(rs->has_bandwidth); - test_assert(!(rs->has_measured_bw)); test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW); - test_eq(rs->measured_bw, 0); + test_assert(rs->bw_is_unmeasured); } 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", @@ -2045,9 +2042,8 @@ test_routerstatus_for_umbw(routerstatus_t *rs, time_t now) * and so should not be clipped */ test_assert(rs->has_bandwidth); - test_assert(!(rs->has_measured_bw)); test_eq(rs->bandwidth, DEFAULT_MAX_UNMEASURED_BW / 2); - test_eq(rs->measured_bw, 0); + test_assert(rs->bw_is_unmeasured); } else { /* Weren't expecting this... */ test_assert(0);

1 0

[tor/master] Refactor storing of measured_bw versus Unmeasured=1.
by nickm＠torproject.org 19 Feb '13

19 Feb '13

commit 6170bc5a9378bda9b300241fa53785b5cbe44b55 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 19 10:39:27 2013 -0500 Refactor storing of measured_bw versus Unmeasured=1. This patch moves the measured_bw field and the has_measured_bw field into vote_routerstatus_t, since only votes have 'Measured=XX' set on their weight line. I also added a new bw_is_unmeasured flag to routerstatus_t to represent the Unmeasured=1 flag on a w line. Previously, I was using has_measured_bw for this, which was quite incorrect: has_measured_bw means that the measured_bw field is set, and it's probably a mistake to have it serve double duty as meaning that 'baandwidth' represents a measured value. While making this change,I also found a harmless but stupid bug in dirserv_read_measured_bandwidths: It assumes that it's getting a smartlist of routerstatus_t, when really it's getting a smartlist of vote_routerstatus_t. C's struct layout rules mean that we could never actually get an error because of that, but it's still quite incorrect. I fixed that, and in the process needed to add two more sorting and searching helpers. Finally, I made the Unmeasured=1 flag get parsed. We don't use it for anything yet, but someday we might. This isn't complete yet -- the new 2286 unit test doesn't build. --- src/or/dirserv.c | 21 ++++++++++++--------- src/or/dirserv.h | 3 ++- src/or/dirvote.c | 13 +++++++------ src/or/networkstatus.c | 13 ++++++++++++- src/or/networkstatus.h | 2 ++ src/or/or.h | 7 ++++--- src/or/routerparse.c | 16 +++++++++++++--- src/or/routerparse.h | 1 + 8 files changed, 53 insertions(+), 23 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 6209842..42d44ac 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -2114,13 +2114,15 @@ version_from_platform(const char *platform) * NS_V3_CONSENSUS - Output the first portion of a V3 NS consensus entry * NS_V3_CONSENSUS_MICRODESC - Output the first portion of a V3 microdesc * consensus entry. - * NS_V3_VOTE - Output a complete V3 NS vote + * NS_V3_VOTE - Output a complete V3 NS vote. If <b>vrs</b> is present, + * it contains additional information for the vote. * NS_CONTROL_PORT - Output a NS document for the control port */ int routerstatus_format_entry(char *buf, size_t buf_len, const routerstatus_t *rs, const char *version, - routerstatus_format_type_t format) + routerstatus_format_type_t format, + const vote_routerstatus_t *vrs) { int r; char *cp; @@ -2266,10 +2268,10 @@ routerstatus_format_entry(char *buf, size_t buf_len, return -1; } cp += strlen(cp); - if (format == NS_V3_VOTE && rs->has_measured_bw) { + 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", rs->measured_bw); + " Measured=%d\n", vrs->measured_bw); if (r<0) { log_warn(LD_BUG, "Not enough space in buffer for weight line."); return -1; @@ -2652,12 +2654,12 @@ int measured_bw_line_apply(measured_bw_line_t *parsed_line, smartlist_t *routerstatuses) { - routerstatus_t *rs = NULL; + vote_routerstatus_t *rs = NULL; if (!routerstatuses) return 0; rs = smartlist_bsearch(routerstatuses, parsed_line->node_id, - compare_digest_to_routerstatus_entry); + compare_digest_to_vote_routerstatus_entry); if (rs) { rs->has_measured_bw = 1; @@ -2672,7 +2674,7 @@ measured_bw_line_apply(measured_bw_line_t *parsed_line, /** * Read the measured bandwidth file and apply it to the list of - * routerstatuses. Returns -1 on error, 0 otherwise. + * vote_routerstatus_t. Returns -1 on error, 0 otherwise. */ int dirserv_read_measured_bandwidths(const char *from_file, @@ -2714,7 +2716,7 @@ dirserv_read_measured_bandwidths(const char *from_file, } if (routerstatuses) - smartlist_sort(routerstatuses, compare_routerstatus_entries); + smartlist_sort(routerstatuses, compare_vote_routerstatus_entries); while (!feof(fp)) { measured_bw_line_t parsed_line; @@ -3080,7 +3082,8 @@ generate_v2_networkstatus_opinion(void) if (digestmap_get(omit_as_sybil, ri->cache_info.identity_digest)) clear_status_flags_on_sybil(&rs); - if (routerstatus_format_entry(outp, endp-outp, &rs, version, NS_V2)) { + if (routerstatus_format_entry(outp, endp-outp, &rs, version, NS_V2, + NULL)) { log_warn(LD_BUG, "Unable to print router status."); tor_free(version); goto done; diff --git a/src/or/dirserv.h b/src/or/dirserv.h index add09f4..0caf55f 100644 --- a/src/or/dirserv.h +++ b/src/or/dirserv.h @@ -131,7 +131,8 @@ size_t dirserv_estimate_microdesc_size(const smartlist_t *fps, int compressed); int routerstatus_format_entry(char *buf, size_t buf_len, const routerstatus_t *rs, const char *platform, - routerstatus_format_type_t format); + routerstatus_format_type_t format, + const vote_routerstatus_t *vrs); void dirserv_free_all(void); void cached_dir_decref(cached_dir_t *d); cached_dir_t *new_cached_dir(char *s, time_t published); diff --git a/src/or/dirvote.c b/src/or/dirvote.c index a3fb03a..0087909 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -212,7 +212,7 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key, vrs) { vote_microdesc_hash_t *h; if (routerstatus_format_entry(outp, endp-outp, &vrs->status, - vrs->version, NS_V3_VOTE) < 0) { + vrs->version, NS_V3_VOTE, vrs) < 0) { log_warn(LD_BUG, "Unable to print router status."); goto err; } @@ -1803,8 +1803,8 @@ networkstatus_compute_consensus(smartlist_t *votes, } /* count bandwidths */ - if (rs->status.has_measured_bw) - measured_bws[num_mbws++] = rs->status.measured_bw; + if (rs->has_measured_bw) + measured_bws[num_mbws++] = rs->measured_bw; if (rs->status.has_bandwidth) bandwidths[num_bandwidths++] = rs->status.bandwidth; @@ -1897,10 +1897,11 @@ networkstatus_compute_consensus(smartlist_t *votes, /* Pick a bandwidth */ if (consensus_method >= 6 && num_mbws > 2) { rs_out.has_bandwidth = 1; - rs_out.has_measured_bw = 1; + rs_out.bw_is_unmeasured = 0; rs_out.bandwidth = median_uint32(measured_bws, 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); if (consensus_method >= MIN_METHOD_TO_CLIP_UNMEASURED_BW && n_authorities_measuring_bandwidth > 2) { @@ -2029,7 +2030,7 @@ networkstatus_compute_consensus(smartlist_t *votes, /* Okay!! Now we can write the descriptor... */ /* First line goes into "buf". */ routerstatus_format_entry(buf, sizeof(buf), &rs_out, NULL, - rs_format); + rs_format, NULL); smartlist_add(chunks, tor_strdup(buf)); } /* Now an m line, if applicable. */ @@ -2050,7 +2051,7 @@ networkstatus_compute_consensus(smartlist_t *votes, smartlist_add(chunks, tor_strdup("\n")); /* Now the weight line. */ if (rs_out.has_bandwidth) { - int unmeasured = ! rs_out.has_measured_bw && + 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, unmeasured?" Unmeasured=1":""); diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 71ac054..c63c76f 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -937,6 +937,17 @@ compare_digest_to_routerstatus_entry(const void *_key, const void **_member) return tor_memcmp(key, rs->identity_digest, DIGEST_LEN); } +/** Helper for bsearching a list of routerstatus_t pointers: compare a + * digest in the key to the identity digest of a routerstatus_t. */ +int +compare_digest_to_vote_routerstatus_entry(const void *_key, + const void **_member) +{ + const char *key = _key; + const vote_routerstatus_t *vrs = *_member; + return tor_memcmp(key, vrs->status.identity_digest, DIGEST_LEN); +} + /** As networkstatus_v2_find_entry, but do not return a const pointer */ routerstatus_t * networkstatus_v2_find_mutable_entry(networkstatus_v2_t *ns, const char *digest) @@ -2117,7 +2128,7 @@ char * networkstatus_getinfo_helper_single(const routerstatus_t *rs) { char buf[RS_ENTRY_LEN+1]; - routerstatus_format_entry(buf, sizeof(buf), rs, NULL, NS_CONTROL_PORT); + routerstatus_format_entry(buf, sizeof(buf), rs, NULL, NS_CONTROL_PORT, NULL); return tor_strdup(buf); } diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h index b437c5e..b64e4b8 100644 --- a/src/or/networkstatus.h +++ b/src/or/networkstatus.h @@ -38,6 +38,8 @@ int router_set_networkstatus_v2(const char *s, time_t arrived_at, void networkstatus_v2_list_clean(time_t now); int compare_digest_to_routerstatus_entry(const void *_key, const void **_member); +int compare_digest_to_vote_routerstatus_entry(const void *_key, + const void **_member); const routerstatus_t *networkstatus_v2_find_entry(networkstatus_v2_t *ns, const char *digest); const routerstatus_t *networkstatus_vote_find_entry(networkstatus_t *ns, diff --git a/src/or/or.h b/src/or/or.h index d53d9ae..620b0ba 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2072,9 +2072,8 @@ typedef struct routerstatus_t { unsigned int has_bandwidth:1; /**< The vote/consensus had bw info */ unsigned int has_exitsummary:1; /**< The vote/consensus had exit summaries */ - unsigned int has_measured_bw:1; /**< The vote/consensus had a measured bw */ - - uint32_t measured_bw; /**< Measured bandwidth (capacity) of the router */ + 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 * the vote/consensus, in kilobytes/sec. */ @@ -2318,6 +2317,8 @@ typedef struct vote_routerstatus_t { * networkstatus_t.known_flags. */ 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 */ /** The hash or hashes that the authority claims this microdesc has. */ vote_microdesc_hash_t *microdesc; } vote_routerstatus_t; diff --git a/src/or/routerparse.c b/src/or/routerparse.c index c5a584c..ce2cd5c 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -1974,9 +1974,9 @@ routerstatus_parse_entry_from_string(memarea_t *area, goto err; } rs->has_bandwidth = 1; - } else if (!strcmpstart(tok->args[i], "Measured=")) { + } else if (!strcmpstart(tok->args[i], "Measured=") && vote_rs) { int ok; - rs->measured_bw = + vote_rs->measured_bw = (uint32_t)tor_parse_ulong(strchr(tok->args[i], '=')+1, 10, 0, UINT32_MAX, &ok, NULL); if (!ok) { @@ -1984,8 +1984,10 @@ routerstatus_parse_entry_from_string(memarea_t *area, escaped(tok->args[i])); goto err; } - rs->has_measured_bw = 1; + vote_rs->has_measured_bw = 1; vote->has_measured_bws = 1; + } else if (!strcmpstart(tok->args[i], "Unmeasured=1")) { + rs->bw_is_unmeasured = 1; } } } @@ -2063,6 +2065,14 @@ compare_routerstatus_entries(const void **_a, const void **_b) return fast_memcmp(a->identity_digest, b->identity_digest, DIGEST_LEN); } +int +compare_vote_routerstatus_entries(const void **_a, const void **_b) +{ + const vote_routerstatus_t *a = *_a, *b = *_b; + return fast_memcmp(a->status.identity_digest, b->status.identity_digest, + DIGEST_LEN); +} + /** Helper: used in call to _smartlist_uniq to clear out duplicate entries. */ static void free_duplicate_routerstatus_entry_(void *e) diff --git a/src/or/routerparse.h b/src/or/routerparse.h index 4cc9bfd..859a691 100644 --- a/src/or/routerparse.h +++ b/src/or/routerparse.h @@ -52,6 +52,7 @@ void assert_addr_policy_ok(smartlist_t *t); void dump_distinct_digest_count(int severity); int compare_routerstatus_entries(const void **_a, const void **_b); +int compare_vote_routerstatus_entries(const void **_a, const void **_b); networkstatus_v2_t *networkstatus_v2_parse_from_string(const char *s); int networkstatus_verify_bw_weights(networkstatus_t *ns); networkstatus_t *networkstatus_parse_vote_from_string(const char *s,

1 0