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.

diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index 6209842881f283148fd35ba27ac55a0d74b9a39e..42d44acf3294c0aa6f28a5959d685f353467382d 100644 (file)
--- 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 add09f44a3d8844cd8b61075ba33a7ecf84de599..0caf55f8308a3a9d07ca7a1baf4ee97fe3024ce4 100644 (file)
--- 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 a3fb03a47877f4c2c44439c1763e5eaaccc45e2e..0087909cf95cf93eea9b23708d2a82bdaa4d03cf 100644 (file)
--- 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 71ac054f881b94c594a62dc51b992d9ea817a677..c63c76fccd2738bd11d5799608be7609f4add96e 100644 (file)
--- 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 b437c5ec2fe8d4d47d6a3383329efbd24b93054c..b64e4b8e1a62b41a9b1a26aff97eb0cba9609582 100644 (file)
--- 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 d53d9ae6c826f60196de3cba7549d6d0d90b7e77..620b0baf468d7288ed9338bad0850cfb4a7e12b0 100644 (file)
--- 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 c5a584c53ffb9aa0fb4856b973a4b14d2fd24dc5..ce2cd5c51388a4eeab12cc66a15366fda3f029b1 100644 (file)
--- 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 4cc9bfd3ae8cec48276585603dbd50877c9ee3d3..859a691e2ab53cb7fdcb6cdaa65f4fbf7491f75b 100644 (file)
--- 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,