#18929: Fix selection of directories with non-preferred address families -------------------------------------------------+------------------------- Reporter: teor | Owner: Type: defect | Status: Priority: Medium | needs_review Component: Core Tor/Tor | Milestone: Tor: Severity: Normal | 0.2.8.x-final Keywords: must-fix-before-028-rc, | Version: Tor: TorCoreTeam201605 | 0.2.8.1-alpha Parent ID: #18483 | Resolution: Reviewer: isis | Actual Points: 2 hours | Points: small | Sponsor: -------------------------------------------------+-------------------------
Comment (by nickm): okay, trying this again! First, I'm reviewing the first two commits (aaa6ac0a1a94f2cf058c3bb45879c1919889f6df and cbb834587b8eed1148125a382891c7f6003ebf60) as one, since they're supposed to get squashed eventually. * This is cosmetic, but I don't understand why `router_has_non_preferred_address` takes prefer_ipv6_or as an argument, but derives perfer_ipv6_dir for iteslf. * For router_has_non_preferred_address_node -- an unlisted bridge has a node_t, but does not have a routerstatus. Can this function ever get called on a bridge? Do we care? * Maybe you now handle this in #18483 or someplace else -- I haven't reviewed that yet -- but I'm a little confused about how router_has_non_preferred_address* looks at both the ORPort and the DirPort, but it doesn't actually take into account whether we're picking a directory for a connection where we would absolutely not use the ORPort or absolutely not use the DirPort. All other patches on this branch look fine to me. One last question: * Would it be even simpler to just omit most of this patch, remove `n_not_preferred`, and just do this? {{{ #define RETRY_ALTERNATE_IP_VERSION(retry_label) \ STMT_BEGIN \ if (result == NULL && try_ip_pref && options->ClientUseIPv4 \ && fascist_firewall_use_ipv6(options) && !server_mode(options) \ - && n_not_preferred && !n_busy) { \ + && !n_busy) { \ n_excluded = 0; \ n_busy = 0; \ try_ip_pref = 0; \ - n_not_preferred = 0; \ goto retry_label; \ } \ STMT_END \ }}} Sure, we would sometimes make two passes needlessly, but not in a common case. What do you think? Now I'm going to get more tea and read #18483; we'll see how much of this I ~~strikethrough~~ once I'm done. :) -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18929#comment:9> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online _______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs