commit 6170bc5a9378bda9b300241fa53785b5cbe44b55 Author: Nick Mathewson <nickm@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,