From 4ab2e9a5990bd6f1fd65f2600dc7487686c801ff Mon Sep 17 00:00:00 2001 From: juga0 Date: Tue, 19 Mar 2019 11:48:42 +0000 Subject: [PATCH 1/3] bwauth: Ignore bandwidth file lines with "vote=0" so that the relays that would be "excluded" from the bandwidth file because of something failed can be included to diagnose what failed, without still including these relays in the bandwidth authorities vote. Closes #29806. --- changes/ticket29806 | 7 ++++++ src/or/dirserv.c | 8 ++++++- src/test/test_dir.c | 57 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 changes/ticket29806 diff --git a/changes/ticket29806 b/changes/ticket29806 new file mode 100644 index 0000000000..6afefd4c04 --- /dev/null +++ b/changes/ticket29806 @@ -0,0 +1,7 @@ + o Minor features (bandwidth authority): + - Make bandwidth authorities to ignore relays that are reported in the + bandwidth file with the key-value "vote=0". + This change allows to report the relays that were not measured due + some failure and diagnose the reasons without the bandwidth being included in the + bandwidth authorities vote. + Closes ticket 29806. diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 4e09c1c65b..ae1975f166 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -2497,7 +2497,13 @@ measured_bw_line_parse(measured_bw_line_t *out, const char *orig_line, } do { - if (strcmpstart(cp, "bw=") == 0) { + // If the line contains vote=0, ignore it. + if (strcmpstart(cp, "vote=0") == 0) { + log_debug(LD_DIRSERV, "Ignoring bandwidth file line that contains " + "vote=0: %s",escaped(orig_line)); + tor_free(line); + return -1; + } else if (strcmpstart(cp, "bw=") == 0) { int parse_ok = 0; char *endptr; if (got_bw) { diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 37b015b72d..52d3ef159c 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -1508,6 +1508,19 @@ test_dir_measured_bw_kb(void *arg) /* check whether node_id can be at the end and something in the * in the middle of bw and node_id */ "bw=1024 foo=bar node_id=$557365204145532d32353620696e73746561642e\n", + + /* Test that a line with vote=1 will pass. */ + "node_id=$557365204145532d32353620696e73746561642e bw=1024 vote=1\n", + /* Test that a line with unmeasured=1 will pass. */ + "node_id=$557365204145532d32353620696e73746561642e bw=1024 unmeasured=1\n", + /* Test that a line with vote=1 and unmeasured=1 will pass. */ + "node_id=$557365204145532d32353620696e73746561642e bw=1024 vote=1" + "unmeasured=1\n", + /* Test that a line with unmeasured=0 will pass. */ + "node_id=$557365204145532d32353620696e73746561642e bw=1024 unmeasured=0\n", + /* Test that a line with vote=1 and unmeasured=0 will pass. */ + "node_id=$557365204145532d32353620696e73746561642e bw=1024 vote=1" + "unmeasured=0\n", "end" }; const char *lines_fail[] = { @@ -1541,6 +1554,12 @@ test_dir_measured_bw_kb(void *arg) "node_id=$55736520414552d32353620696e73746561642e bw=1024\n", "node_id=557365204145532d32353620696e73746561642e bw=1024\n", "node_id= $557365204145532d32353620696e73746561642e bw=0.23\n", + + /* Test that a line with vote=0 will fail too, so that it is ignored. */ + "node_id=$557365204145532d32353620696e73746561642e bw=1024 vote=0\n", + /* Test that a line with vote=0 will fail even if unmeasured=0. */ + "node_id=$557365204145532d32353620696e73746561642e bw=1024 vote=0 " + "unmeasured=0\n", "end" }; @@ -1706,6 +1725,44 @@ test_dir_dirserv_read_measured_bandwidths(void *arg) tor_free(content); tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL)); + /* Test v1.x.x bandwidth line with vote=0. + * It will be ignored it and logged it at debug level. */ + const char *relay_lines_ignore = + "node_id=$68A483E05A2ABDCA6DA5A3EF8DB5177638A27F80 bw=1024 vote=0\n" + "node_id=$68A483E05A2ABDCA6DA5A3EF8DB5177638A27F80 bw=1024 vote=0" + "unmeasured=1\n" + "node_id=$68A483E05A2ABDCA6DA5A3EF8DB5177638A27F80 bw=1024 vote=0" + "unmeasured=0\n"; + + /* Create the bandwidth file */ + tor_asprintf(&content, "%ld\n%s", (long)timestamp, relay_lines_ignore); + write_str_to_file(fname, content, 0); + tor_free(content); + + /* Read the bandwidth file */ + setup_full_capture_of_logs(LOG_DEBUG); + tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL)); + expect_log_msg_containing("Ignoring bandwidth file line"); + teardown_capture_of_logs(); + + /* Test v1.x.x bandwidth line with "vote=1" or "unmeasured=1" or + * "unmeasured=0". + * They will not be ignored. */ + /* Create the bandwidth file */ + const char *relay_lines_vote = + "node_id=$68A483E05A2ABDCA6DA5A3EF8DB5177638A27F80 bw=1024 vote=1\n" + "node_id=$68A483E05A2ABDCA6DA5A3EF8DB5177638A27F80 bw=1024 unmeasured=0\n" + "node_id=$68A483E05A2ABDCA6DA5A3EF8DB5177638A27F80 bw=1024 unmeasured=1\n"; + tor_asprintf(&content, "%ld\n%s", (long)timestamp, relay_lines_vote); + write_str_to_file(fname, content, 0); + tor_free(content); + + /* Read the bandwidth file */ + setup_full_capture_of_logs(LOG_DEBUG); + tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL)); + expect_log_msg_not_containing("Ignoring bandwidth file line"); + teardown_capture_of_logs(); + done: tor_free(fname); } From 091f8688b8ee15b57ed5bc24bac12a7a0b7f5725 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 21 Mar 2019 12:56:28 +1000 Subject: [PATCH 2/3] test/dir: add an extra argument to dirserv_read_measured_bandwidths() Part of 29806. --- src/test/test_dir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index cbb414ed62..0e44c47f3f 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -2014,7 +2014,7 @@ test_dir_dirserv_read_measured_bandwidths(void *arg) /* Read the bandwidth file */ setup_full_capture_of_logs(LOG_DEBUG); - tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL)); + tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL, NULL)); expect_log_msg_containing("Ignoring bandwidth file line"); teardown_capture_of_logs(); @@ -2032,7 +2032,7 @@ test_dir_dirserv_read_measured_bandwidths(void *arg) /* Read the bandwidth file */ setup_full_capture_of_logs(LOG_DEBUG); - tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL)); + tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL, NULL)); expect_log_msg_not_containing("Ignoring bandwidth file line"); teardown_capture_of_logs(); From 3af9a51118aadde538580f6d4f8043a0a9bba512 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 21 Mar 2019 23:35:56 +1000 Subject: [PATCH 3/3] test/dir: add a 4th argument to dirserv_read_measured_bandwidths() Part of 29806. --- src/test/test_dir.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index b9a48c6506..07a2641c9f 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -2064,7 +2064,8 @@ test_dir_dirserv_read_measured_bandwidths(void *arg) /* Read the bandwidth file */ setup_full_capture_of_logs(LOG_DEBUG); - tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL, NULL)); + tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL, NULL, + NULL)); expect_log_msg_containing("Ignoring bandwidth file line"); teardown_capture_of_logs(); @@ -2082,7 +2083,8 @@ test_dir_dirserv_read_measured_bandwidths(void *arg) /* Read the bandwidth file */ setup_full_capture_of_logs(LOG_DEBUG); - tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL, NULL)); + tt_int_op(0, OP_EQ, dirserv_read_measured_bandwidths(fname, NULL, NULL, + NULL)); expect_log_msg_not_containing("Ignoring bandwidth file line"); teardown_capture_of_logs();