commit 3f7f976d4812a92bdbf5f14e25db0d276f123cef Author: teor <teor@riseup.net> Date: Mon May 11 17:58:28 2020 +1000 nodelist: Stop recursing in router_choose_random_node() Instead, call out to a helper function, repeating the call if needed. Avoids duplicating exclusions for: * the current relay's family, and * any exclusions specified by the caller. Part of 34200. --- src/feature/nodelist/node_select.c | 69 ++++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 22 deletions(-) diff --git a/src/feature/nodelist/node_select.c b/src/feature/nodelist/node_select.c index 11abc84d4..85ef5ad56 100644 --- a/src/feature/nodelist/node_select.c +++ b/src/feature/nodelist/node_select.c @@ -930,6 +930,42 @@ nodelist_subtract(smartlist_t *sl, const smartlist_t *excluded) bitarray_free(excluded_idx); } +/* Node selection helper for router_choose_random_node(). + * + * Populates a node list based on <b>flags</b>, ignoring nodes in + * <b>excludednodes</b> and <b>excludedset</b>. Chooses the node based on + * <b>rule</b>. */ +static const node_t * +router_choose_random_node_helper(smartlist_t *excludednodes, + routerset_t *excludedset, + router_crn_flags_t flags, + bandwidth_weight_rule_t rule) +{ + smartlist_t *sl=smartlist_new(); + const node_t *choice = NULL; + + router_add_running_nodes_to_smartlist(sl, flags); + log_debug(LD_CIRC, + "We found %d running nodes.", + smartlist_len(sl)); + + nodelist_subtract(sl, excludednodes); + + if (excludedset) { + routerset_subtract_nodes(sl,excludedset); + log_debug(LD_CIRC, + "We removed excludedset, leaving %d nodes.", + smartlist_len(sl)); + } + + // Always weight by bandwidth + choice = node_sl_choose_by_bandwidth(sl, rule); + + smartlist_free(sl); + + return choice; +} + /** Return a random running node from the nodelist. Never pick a node that is * in <b>excludedsmartlist</b>, or which matches <b>excludedset</b>, even if * they are the only nodes available. @@ -942,15 +978,14 @@ router_choose_random_node(smartlist_t *excludedsmartlist, routerset_t *excludedset, router_crn_flags_t flags) { - /* A limited set of flags, used for weighting and fallback node selection. + /* A limited set of flags, used for fallback node selection. */ const int need_uptime = (flags & CRN_NEED_UPTIME) != 0; const int need_capacity = (flags & CRN_NEED_CAPACITY) != 0; const int need_guard = (flags & CRN_NEED_GUARD) != 0; const int pref_addr = (flags & CRN_PREF_ADDR) != 0; - smartlist_t *sl=smartlist_new(), - *excludednodes=smartlist_new(); + smartlist_t *excludednodes=smartlist_new(); const node_t *choice = NULL; const routerinfo_t *r; bandwidth_weight_rule_t rule; @@ -963,29 +998,17 @@ router_choose_random_node(smartlist_t *excludedsmartlist, if ((r = router_get_my_routerinfo())) routerlist_add_node_and_family(excludednodes, r); - router_add_running_nodes_to_smartlist(sl, flags); - log_debug(LD_CIRC, - "We found %d running nodes.", - smartlist_len(sl)); - if (excludedsmartlist) { smartlist_add_all(excludednodes, excludedsmartlist); } - nodelist_subtract(sl, excludednodes); - if (excludedset) { - routerset_subtract_nodes(sl,excludedset); - log_debug(LD_CIRC, - "We removed excludedset, leaving %d nodes.", - smartlist_len(sl)); - } + choice = router_choose_random_node_helper(excludednodes, + excludedset, + flags, + rule); - // Always weight by bandwidth - choice = node_sl_choose_by_bandwidth(sl, rule); - - smartlist_free(sl); if (!choice && (need_uptime || need_capacity || need_guard || pref_addr)) { - /* try once more -- recurse but with fewer restrictions. */ + /* try once more, with fewer restrictions. */ log_info(LD_CIRC, "We couldn't find any live%s%s%s%s routers; falling back " "to list of all routers.", @@ -995,8 +1018,10 @@ router_choose_random_node(smartlist_t *excludedsmartlist, pref_addr?", preferred address":""); flags &= ~ (CRN_NEED_UPTIME|CRN_NEED_CAPACITY|CRN_NEED_GUARD| CRN_PREF_ADDR); - choice = router_choose_random_node( - excludedsmartlist, excludedset, flags); + choice = router_choose_random_node_helper(excludednodes, + excludedset, + flags, + rule); } smartlist_free(excludednodes); if (!choice) {