diff --git a/changes/bug6538 b/changes/bug6538
new file mode 100644
index 0000000000000000000000000000000000000000..1e882eb1cce11aa49e4ff1b9ec9ed58df6686b77
--- /dev/null
+++ b/changes/bug6538
@@ -0,0 +1,4 @@
+ o Minor bugfixes:
+ - Switch weighted node selection rule from using a list of doubles
+ to using a list of int64_t. This should make the process slightly
+ easier to debug and maintain. Needed for fix for bug 6538.
diff --git a/configure.in b/configure.in
index 1db7d08e20b13d47eb183fd29f7a39f0f0fd53bb..f5f8eacb7e605d31601391d8c5258c0f86907653 100644
--- a/configure.in
+++ b/configure.in
@@ -300,6 +300,7 @@ AC_CHECK_FUNCS(
gmtime_r \
inet_aton \
ioctl \
+ llround \
localtime_r \
lround \
memmem \
diff --git a/src/common/util.c b/src/common/util.c
index a0dff2e35b698eba4dc0ef0836bb9ec807ef497e..60222f40839cbf04c6c8069acc1ea60cd4011262 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -378,6 +378,21 @@ tor_lround(double d)
#endif
}
+/** Return the 64-bit integer closest to d. We define this wrapper here so
+ * that not all users of math.h need to use the right incancations to get the
+ * c99 functions. */
+int64_t
+tor_llround(double d)
+{
+#if defined(HAVE_LLROUND)
+ return (int64_t)llround(d);
+#elif defined(HAVE_RINT)
+ return (int64_t)rint(d);
+#else
+ return (int64_t)(d > 0 ? d + 0.5 : ceil(d - 0.5));
+#endif
+}
+
/** Returns floor(log2(u64)). If u64 is 0, (incorrectly) returns 0. */
int
tor_log2(uint64_t u64)
diff --git a/src/common/util.h b/src/common/util.h
index a2ab0ccac873d8c6a616580455fad729be3441aa..f700e9f4cbd6905f4826b93203f9ae2f2f367594 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -161,6 +161,7 @@ void tor_log_mallinfo(int severity);
/* Math functions */
double tor_mathlog(double d) ATTR_CONST;
long tor_lround(double d) ATTR_CONST;
+int64_t tor_llround(double d) ATTR_CONST;
int tor_log2(uint64_t u64) ATTR_CONST;
uint64_t round_to_power_of_2(uint64_t u64);
unsigned round_to_next_multiple_of(unsigned number, unsigned divisor);
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 4979b933ade100dd55a2acc6e0ace6a125cd4c2c..382d45746c0b4b3a122bba371855b43129a13ae0 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1702,12 +1702,12 @@ smartlist_choose_node_by_bandwidth_weights(smartlist_t *sl,
bandwidth_weight_rule_t rule)
{
int64_t weight_scale;
- int64_t rand_bw;
+ uint64_t rand_bw;
double Wg = -1, Wm = -1, We = -1, Wd = -1;
double Wgb = -1, Wmb = -1, Web = -1, Wdb = -1;
- double weighted_bw = 0, unweighted_bw = 0;
- double *bandwidths;
- double tmp = 0;
+ uint64_t weighted_bw = 0, unweighted_bw = 0;
+ uint64_t *bandwidths;
+ uint64_t tmp;
unsigned int i;
unsigned int i_chosen;
unsigned int i_has_been_chosen;
@@ -1792,7 +1792,7 @@ smartlist_choose_node_by_bandwidth_weights(smartlist_t *sl,
Web /= weight_scale;
Wdb /= weight_scale;
- bandwidths = tor_malloc_zero(sizeof(double)*smartlist_len(sl));
+ bandwidths = tor_malloc_zero(sizeof(uint64_t)*smartlist_len(sl));
// Cycle through smartlist and total the bandwidth.
SMARTLIST_FOREACH_BEGIN(sl, const node_t *, node) {
@@ -1831,24 +1831,30 @@ smartlist_choose_node_by_bandwidth_weights(smartlist_t *sl,
} else { // middle
weight = (is_dir ? Wmb*Wm : Wm);
}
-
- bandwidths[node_sl_idx] = weight*this_bw;
- weighted_bw += weight*this_bw;
+ /* These should be impossible; but overflows here would be bad, so let's
+ * make sure. */
+ if (this_bw < 0)
+ this_bw = 0;
+ if (weight < 0.0)
+ weight = 0.0;
+
+ bandwidths[node_sl_idx] = tor_llround(weight*this_bw + 0.5);
+ weighted_bw += bandwidths[node_sl_idx];
unweighted_bw += this_bw;
if (is_me)
- sl_last_weighted_bw_of_me = weight*this_bw;
+ sl_last_weighted_bw_of_me = bandwidths[node_sl_idx];
} SMARTLIST_FOREACH_END(node);
/* XXXX this is a kludge to expose these values. */
sl_last_total_weighted_bw = weighted_bw;
log_debug(LD_CIRC, "Choosing node for rule %s based on weights "
- "Wg=%f Wm=%f We=%f Wd=%f with total bw %f",
+ "Wg=%f Wm=%f We=%f Wd=%f with total bw "U64_FORMAT,
bandwidth_weight_rule_to_string(rule),
- Wg, Wm, We, Wd, weighted_bw);
+ Wg, Wm, We, Wd, U64_PRINTF_ARG(weighted_bw));
/* If there is no bandwidth, choose at random */
- if (DBL_TO_U64(weighted_bw) == 0) {
+ if (weighted_bw == 0) {
/* Don't warn when using bridges/relays not in the consensus */
if (!have_unknown) {
#define ZERO_BANDWIDTH_WARNING_INTERVAL (15)
@@ -1858,24 +1864,25 @@ smartlist_choose_node_by_bandwidth_weights(smartlist_t *sl,
if ((msg = rate_limit_log(&zero_bandwidth_warning_limit,
approx_time()))) {
log_warn(LD_CIRC,
- "Weighted bandwidth is %f in node selection for rule %s "
- "(unweighted was %f) %s",
- weighted_bw, bandwidth_weight_rule_to_string(rule),
- unweighted_bw, msg);
+ "Weighted bandwidth is "U64_FORMAT" in node selection for "
+ "rule %s (unweighted was "U64_FORMAT") %s",
+ U64_PRINTF_ARG(weighted_bw),
+ bandwidth_weight_rule_to_string(rule),
+ U64_PRINTF_ARG(unweighted_bw), msg);
}
}
tor_free(bandwidths);
return smartlist_choose(sl);
}
- rand_bw = crypto_rand_uint64(DBL_TO_U64(weighted_bw));
+ rand_bw = crypto_rand_uint64(weighted_bw);
rand_bw++; /* crypto_rand_uint64() counts from 0, and we need to count
* from 1 below. See bug 1203 for details. */
/* Last, count through sl until we get to the element we picked */
i_chosen = (unsigned)smartlist_len(sl);
i_has_been_chosen = 0;
- tmp = 0.0;
+ tmp = 0;
for (i=0; i < (unsigned)smartlist_len(sl); i++) {
tmp += bandwidths[i];
if (tmp >= rand_bw && !i_has_been_chosen) {
@@ -1892,8 +1899,9 @@ smartlist_choose_node_by_bandwidth_weights(smartlist_t *sl,
--i;
log_warn(LD_BUG, "Round-off error in computing bandwidth had an effect on "
" which router we chose. Please tell the developers. "
- "%f " U64_FORMAT " %f", tmp, U64_PRINTF_ARG(rand_bw),
- weighted_bw);
+ U64_FORMAT" "U64_FORMAT" "U64_FORMAT,
+ U64_PRINTF_ARG(tmp), U64_PRINTF_ARG(rand_bw),
+ U64_PRINTF_ARG(weighted_bw));
}
tor_free(bandwidths);
return smartlist_get(sl, i);