From 954f263ed5eb451a0742f8888681e10e64dd193a Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Wed, 24 Oct 2012 17:34:18 -0700 Subject: Add the ability to count circuit timeouts for guards. This is purely for informational reasons for debugging. --- src/or/entrynodes.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) (limited to 'src/or/entrynodes.c') diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 8712241f6..d9a06a657 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1021,7 +1021,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) digestmap_set(added_by, d, tor_strdup(line->value+HEX_DIGEST_LEN+1)); } else if (!strcasecmp(line->key, "EntryGuardPathBias")) { const or_options_t *options = get_options(); - unsigned hop_cnt, success_cnt; + unsigned hop_cnt, success_cnt, timeouts; if (!node) { *msg = tor_strdup("Unable to parse entry nodes: " @@ -1029,14 +1029,20 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) break; } - if (tor_sscanf(line->value, "%u %u", &success_cnt, &hop_cnt) != 2) { - log_warn(LD_GENERAL, "Unable to parse guard path bias info: " + /* First try 3 params, then 2. */ + if (tor_sscanf(line->value, "%u %u %u", &success_cnt, &hop_cnt, + &timeouts) != 3) { + timeouts = 0; + if (tor_sscanf(line->value, "%u %u", &success_cnt, &hop_cnt) != 2) { + log_warn(LD_GENERAL, "Unable to parse guard path bias info: " "Misformated EntryGuardPathBias %s", escaped(line->value)); - continue; + continue; + } } node->first_hops = hop_cnt; node->circuit_successes = success_cnt; + node->timeouts = timeouts; log_info(LD_GENERAL, "Read %u/%u path bias for node %s", node->circuit_successes, node->first_hops, node->nickname); /* Note: We rely on the < comparison here to allow us to set a 0 @@ -1173,8 +1179,8 @@ entry_guards_update_state(or_state_t *state) if (e->first_hops) { *next = line = tor_malloc_zero(sizeof(config_line_t)); line->key = tor_strdup("EntryGuardPathBias"); - tor_asprintf(&line->value, "%u %u", - e->circuit_successes, e->first_hops); + tor_asprintf(&line->value, "%u %u %u", + e->circuit_successes, e->first_hops, e->timeouts); next = &(line->next); } -- cgit v1.2.3 From 248fbc3619664e1d9f4b16732ccbdb484939624d Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Wed, 24 Oct 2012 18:03:09 -0700 Subject: Update pathbias parameters to match Proposal 209. Needs manpage update and testing still.. --- src/or/entrynodes.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src/or/entrynodes.c') diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index d9a06a657..faf5269a5 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1048,8 +1048,9 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) /* Note: We rely on the < comparison here to allow us to set a 0 * rate and disable the feature entirely. If refactoring, don't * change to <= */ - if (node->circuit_successes/((double)node->first_hops) - < pathbias_get_disable_rate(options)) { + if ((node->circuit_successes/((double)node->first_hops) + < pathbias_get_crit_rate(options)) && + pathbias_get_dropguards(options)) { node->path_bias_disabled = 1; log_info(LD_GENERAL, "Path bias is too high (%u/%u); disabling node %s", -- cgit v1.2.3 From bb548134cd4fd7b4b330cea15111ff257859bba8 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Wed, 31 Oct 2012 18:49:49 -0700 Subject: Update with code review changes from Nick. --- src/or/entrynodes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/or/entrynodes.c') diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index faf5269a5..317a08847 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1049,7 +1049,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) * rate and disable the feature entirely. If refactoring, don't * change to <= */ if ((node->circuit_successes/((double)node->first_hops) - < pathbias_get_crit_rate(options)) && + < pathbias_get_extreme_rate(options)) && pathbias_get_dropguards(options)) { node->path_bias_disabled = 1; log_info(LD_GENERAL, -- cgit v1.2.3 From 412ae099cb656ab47fc8cbb408aa5f4cee956961 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Sat, 17 Nov 2012 16:30:50 -0800 Subject: Prop 209: Add path bias counts for timeouts and other mechanisms. Turns out there's more than one way to block a tagged circuit. This seems to successfully handle all of the normal exit circuits. Hidden services need additional tweaks, still. --- src/or/entrynodes.c | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) (limited to 'src/or/entrynodes.c') diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 317a08847..1e64aaf98 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1021,7 +1021,8 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) digestmap_set(added_by, d, tor_strdup(line->value+HEX_DIGEST_LEN+1)); } else if (!strcasecmp(line->key, "EntryGuardPathBias")) { const or_options_t *options = get_options(); - unsigned hop_cnt, success_cnt, timeouts; + unsigned hop_cnt, success_cnt, timeouts, collapsed, successful_closed, + unusable; if (!node) { *msg = tor_strdup("Unable to parse entry nodes: " @@ -1030,19 +1031,33 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) } /* First try 3 params, then 2. */ - if (tor_sscanf(line->value, "%u %u %u", &success_cnt, &hop_cnt, - &timeouts) != 3) { - timeouts = 0; + // XXX: We want to convert this to floating point before merge + /* In the long run: circuit_success ~= successful_circuit_close + + * collapsed_circuits + + * unusable_circuits */ + if (tor_sscanf(line->value, "%u %u %u %u %u %u", + &hop_cnt, &success_cnt, &successful_closed, + &collapsed, &unusable, &timeouts) != 6) { if (tor_sscanf(line->value, "%u %u", &success_cnt, &hop_cnt) != 2) { - log_warn(LD_GENERAL, "Unable to parse guard path bias info: " - "Misformated EntryGuardPathBias %s", escaped(line->value)); continue; } + log_info(LD_GENERAL, "Reading old-style EntryGuardPathBias %s", + escaped(line->value)); + + successful_closed = success_cnt; + timeouts = 0; + collapsed = 0; + unusable = 0; } node->first_hops = hop_cnt; node->circuit_successes = success_cnt; + + node->successful_circuits_closed = successful_closed; node->timeouts = timeouts; + node->collapsed_circuits = collapsed; + node->unusable_circuits = unusable; + log_info(LD_GENERAL, "Read %u/%u path bias for node %s", node->circuit_successes, node->first_hops, node->nickname); /* Note: We rely on the < comparison here to allow us to set a 0 @@ -1180,8 +1195,13 @@ entry_guards_update_state(or_state_t *state) if (e->first_hops) { *next = line = tor_malloc_zero(sizeof(config_line_t)); line->key = tor_strdup("EntryGuardPathBias"); - tor_asprintf(&line->value, "%u %u %u", - e->circuit_successes, e->first_hops, e->timeouts); + /* In the long run: circuit_success ~= successful_circuit_close + + * collapsed_circuits + + * unusable_circuits */ + tor_asprintf(&line->value, "%u %u %u %u %u %u", + e->first_hops, e->circuit_successes, + pathbias_get_closed_count(e), e->collapsed_circuits, + e->unusable_circuits, e->timeouts); next = &(line->next); } -- cgit v1.2.3 From a90f165b83bc1603873308d7918e99057afdf72a Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Sun, 9 Dec 2012 20:18:31 -0800 Subject: Rename first_hop to circ_attempt. Since we've generalized what we can count from (first or second hop), we should generalize the variable and constant naming too. --- src/or/entrynodes.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'src/or/entrynodes.c') diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 1e64aaf98..14a1e3c7f 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1050,7 +1050,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) unusable = 0; } - node->first_hops = hop_cnt; + node->circ_attempts = hop_cnt; node->circuit_successes = success_cnt; node->successful_circuits_closed = successful_closed; @@ -1059,17 +1059,17 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) node->unusable_circuits = unusable; log_info(LD_GENERAL, "Read %u/%u path bias for node %s", - node->circuit_successes, node->first_hops, node->nickname); + node->circuit_successes, node->circ_attempts, node->nickname); /* Note: We rely on the < comparison here to allow us to set a 0 * rate and disable the feature entirely. If refactoring, don't * change to <= */ - if ((node->circuit_successes/((double)node->first_hops) + if ((node->circuit_successes/((double)node->circ_attempts) < pathbias_get_extreme_rate(options)) && pathbias_get_dropguards(options)) { node->path_bias_disabled = 1; log_info(LD_GENERAL, "Path bias is too high (%u/%u); disabling node %s", - node->circuit_successes, node->first_hops, node->nickname); + node->circuit_successes, node->circ_attempts, node->nickname); } } else { @@ -1192,14 +1192,14 @@ entry_guards_update_state(or_state_t *state) d, e->chosen_by_version, t); next = &(line->next); } - if (e->first_hops) { + if (e->circ_attempts) { *next = line = tor_malloc_zero(sizeof(config_line_t)); line->key = tor_strdup("EntryGuardPathBias"); /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ tor_asprintf(&line->value, "%u %u %u %u %u %u", - e->first_hops, e->circuit_successes, + e->circ_attempts, e->circuit_successes, pathbias_get_closed_count(e), e->collapsed_circuits, e->unusable_circuits, e->timeouts); next = &(line->next); -- cgit v1.2.3 From ab1fce5c19b64b3f1ba15d6ffa1f0d11d6a959c3 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Sun, 9 Dec 2012 20:20:44 -0800 Subject: Also shorten circuit_successes to circ_successes. For consistency and great justice. Ok, mostly consistency. --- src/or/entrynodes.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'src/or/entrynodes.c') diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 14a1e3c7f..84764d186 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1051,7 +1051,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) } node->circ_attempts = hop_cnt; - node->circuit_successes = success_cnt; + node->circ_successes = success_cnt; node->successful_circuits_closed = successful_closed; node->timeouts = timeouts; @@ -1059,17 +1059,17 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) node->unusable_circuits = unusable; log_info(LD_GENERAL, "Read %u/%u path bias for node %s", - node->circuit_successes, node->circ_attempts, node->nickname); + node->circ_successes, node->circ_attempts, node->nickname); /* Note: We rely on the < comparison here to allow us to set a 0 * rate and disable the feature entirely. If refactoring, don't * change to <= */ - if ((node->circuit_successes/((double)node->circ_attempts) + if ((node->circ_successes/((double)node->circ_attempts) < pathbias_get_extreme_rate(options)) && pathbias_get_dropguards(options)) { node->path_bias_disabled = 1; log_info(LD_GENERAL, "Path bias is too high (%u/%u); disabling node %s", - node->circuit_successes, node->circ_attempts, node->nickname); + node->circ_successes, node->circ_attempts, node->nickname); } } else { @@ -1199,7 +1199,7 @@ entry_guards_update_state(or_state_t *state) * collapsed_circuits + * unusable_circuits */ tor_asprintf(&line->value, "%u %u %u %u %u %u", - e->circ_attempts, e->circuit_successes, + e->circ_attempts, e->circ_successes, pathbias_get_closed_count(e), e->collapsed_circuits, e->unusable_circuits, e->timeouts); next = &(line->next); -- cgit v1.2.3 From 2dbb62f1b571ea57af111f1f660a5149d160c4fb Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Sun, 9 Dec 2012 20:53:22 -0800 Subject: Convert to doubles for all pathbias state. Let's hope this solves the rounding error issue.. --- src/or/entrynodes.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'src/or/entrynodes.c') diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 84764d186..21c09f79c 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1021,7 +1021,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) digestmap_set(added_by, d, tor_strdup(line->value+HEX_DIGEST_LEN+1)); } else if (!strcasecmp(line->key, "EntryGuardPathBias")) { const or_options_t *options = get_options(); - unsigned hop_cnt, success_cnt, timeouts, collapsed, successful_closed, + double hop_cnt, success_cnt, timeouts, collapsed, successful_closed, unusable; if (!node) { @@ -1031,20 +1031,22 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) } /* First try 3 params, then 2. */ - // XXX: We want to convert this to floating point before merge /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ - if (tor_sscanf(line->value, "%u %u %u %u %u %u", + if (tor_sscanf(line->value, "%lf %lf %lf %lf %lf %lf", &hop_cnt, &success_cnt, &successful_closed, &collapsed, &unusable, &timeouts) != 6) { - if (tor_sscanf(line->value, "%u %u", &success_cnt, &hop_cnt) != 2) { + int old_success, old_hops; + if (tor_sscanf(line->value, "%u %u", &old_success, &old_hops) != 2) { continue; } log_info(LD_GENERAL, "Reading old-style EntryGuardPathBias %s", escaped(line->value)); - successful_closed = success_cnt; + success_cnt = old_success; + successful_closed = old_success; + hop_cnt = old_hops; timeouts = 0; collapsed = 0; unusable = 0; @@ -1058,7 +1060,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) node->collapsed_circuits = collapsed; node->unusable_circuits = unusable; - log_info(LD_GENERAL, "Read %u/%u path bias for node %s", + log_info(LD_GENERAL, "Read %lf/%lf path bias for node %s", node->circ_successes, node->circ_attempts, node->nickname); /* Note: We rely on the < comparison here to allow us to set a 0 * rate and disable the feature entirely. If refactoring, don't @@ -1068,7 +1070,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) pathbias_get_dropguards(options)) { node->path_bias_disabled = 1; log_info(LD_GENERAL, - "Path bias is too high (%u/%u); disabling node %s", + "Path bias is too high (%lf/%lf); disabling node %s", node->circ_successes, node->circ_attempts, node->nickname); } @@ -1198,7 +1200,7 @@ entry_guards_update_state(or_state_t *state) /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ - tor_asprintf(&line->value, "%u %u %u %u %u %u", + tor_asprintf(&line->value, "%lf %lf %lf %lf %lf %lf", e->circ_attempts, e->circ_successes, pathbias_get_closed_count(e), e->collapsed_circuits, e->unusable_circuits, e->timeouts); -- cgit v1.2.3 From b75880d7b3d02f5c60bf2e215c6e84da4f3e1938 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Sun, 9 Dec 2012 20:56:48 -0800 Subject: Fix a rather serious use-count state bug. We need to use the success count or the use count depending on the consensus parameter. --- src/or/entrynodes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/or/entrynodes.c') diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 21c09f79c..96b075a35 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1065,8 +1065,8 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) /* Note: We rely on the < comparison here to allow us to set a 0 * rate and disable the feature entirely. If refactoring, don't * change to <= */ - if ((node->circ_successes/((double)node->circ_attempts) - < pathbias_get_extreme_rate(options)) && + if (pathbias_get_success_count(node)/node->circ_attempts + < pathbias_get_extreme_rate(options) && pathbias_get_dropguards(options)) { node->path_bias_disabled = 1; log_info(LD_GENERAL, -- cgit v1.2.3 From 4590993ff3d4393caaa1d9d68d04cf0af95c23c7 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Sun, 9 Dec 2012 23:47:04 -0800 Subject: Space fixes. --- src/or/entrynodes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/or/entrynodes.c') diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 96b075a35..066dbecc2 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1031,7 +1031,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) } /* First try 3 params, then 2. */ - /* In the long run: circuit_success ~= successful_circuit_close + + /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ if (tor_sscanf(line->value, "%lf %lf %lf %lf %lf %lf", @@ -1197,7 +1197,7 @@ entry_guards_update_state(or_state_t *state) if (e->circ_attempts) { *next = line = tor_malloc_zero(sizeof(config_line_t)); line->key = tor_strdup("EntryGuardPathBias"); - /* In the long run: circuit_success ~= successful_circuit_close + + /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ tor_asprintf(&line->value, "%lf %lf %lf %lf %lf %lf", -- cgit v1.2.3 From b0fc18c37e0d30d9941109382df4b2b44f0f0ff8 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Tue, 18 Dec 2012 12:39:03 -0800 Subject: Changes from Nick's code review 'part 1' I think this is actually his third code review of this branch so far. --- src/or/entrynodes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/or/entrynodes.c') diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 066dbecc2..fc7f6ed35 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1194,7 +1194,7 @@ entry_guards_update_state(or_state_t *state) d, e->chosen_by_version, t); next = &(line->next); } - if (e->circ_attempts) { + if (e->circ_attempts > 0) { *next = line = tor_malloc_zero(sizeof(config_line_t)); line->key = tor_strdup("EntryGuardPathBias"); /* In the long run: circuit_success ~= successful_circuit_close + -- cgit v1.2.3 From 406d59a9c93e46bcb5be0e0a5c087f4860522d56 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Tue, 18 Dec 2012 14:16:01 -0800 Subject: Nick's Code review #3 part 2. --- src/or/entrynodes.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/or/entrynodes.c') diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index fc7f6ed35..7aa4d7a73 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1060,7 +1060,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) node->collapsed_circuits = collapsed; node->unusable_circuits = unusable; - log_info(LD_GENERAL, "Read %lf/%lf path bias for node %s", + log_info(LD_GENERAL, "Read %f/%f path bias for node %s", node->circ_successes, node->circ_attempts, node->nickname); /* Note: We rely on the < comparison here to allow us to set a 0 * rate and disable the feature entirely. If refactoring, don't @@ -1070,7 +1070,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) pathbias_get_dropguards(options)) { node->path_bias_disabled = 1; log_info(LD_GENERAL, - "Path bias is too high (%lf/%lf); disabling node %s", + "Path bias is too high (%f/%f); disabling node %s", node->circ_successes, node->circ_attempts, node->nickname); } @@ -1200,7 +1200,7 @@ entry_guards_update_state(or_state_t *state) /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ - tor_asprintf(&line->value, "%lf %lf %lf %lf %lf %lf", + tor_asprintf(&line->value, "%f %f %f %f %f %f", e->circ_attempts, e->circ_successes, pathbias_get_closed_count(e), e->collapsed_circuits, e->unusable_circuits, e->timeouts); -- cgit v1.2.3